You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/02/17 21:28:51 UTC

[GitHub] [couchdb] frapa opened a new pull request #3932: Autodetect spidermonkey version in ./configure

frapa opened a new pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932


   I have struggled with this point when building
   couchDB, because it's written nowhere in the documentation
   that you need to configure the version properly.
   
   It seems that other people had the problem as well:
   https://couchdb.slack.com/archives/C49LEE7NW/p1632249155263400
   
   The updated configure script automatically finds
   out the installed library version. It also introduces
   a dependency on `sed`, `ldconfig` and `head`,
   and remove the default to version 1.8.5 which is old
   and not in most distributions anymore.
   
   The argument was also removed from the CI scripts
   because it does not exist anymore.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810323107



##########
File path: configure
##########
@@ -29,9 +29,14 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
 ARCH="$(uname -m)"
 
+# Try to detect spidermonkey (libmozjs) version automatically
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+

Review comment:
       We need this to fail gracefully if it can't find SpiderMonkey. Currently I'm finding that `./configure` will fail with a line like
   
   > Unsupported SpiderMonkey version: 
   
   which comes from `src/couch/rebar.config.script`. I don't have a specific suggestion of where to put the improvement but I think it would be good for the system to tell the user that it could not find a SpiderMonkey library  and bail a little more clearly




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810503413



##########
File path: configure
##########
@@ -29,9 +29,14 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
 ARCH="$(uname -m)"
 
+# Try to detect spidermonkey (libmozjs) version automatically
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+

Review comment:
       If the version cannot be detected, the variable `SM_VSN` will be empty. I added an `if` case exiting the configure script and warning the user. I hope that's good enough.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r814222353



##########
File path: configure
##########
@@ -29,11 +29,22 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
+OS="$(uname)"
 ARCH="$(uname -m)"
 
+SM_VSN=${SM_VSN:-$([ "$OS" = "Linux" ] || echo "1.8.5")}
+# Try to detect SpiderMonkey (libmozjs) version automatically on linux
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ -z "$SM_VSN" ]; then
+  SM_VSN="1.8.5"
+fi
+
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+
 . ${rootdir}/version.mk
-COUCHDB_VERSION=${vsn_major}.${vsn_minor}.${vsn_patch}
+COUCHDB_VERSION=${vsn_major}.${vsn_minor}.${vsn_patch}src/couch/rebar.config.script

Review comment:
       Just noticed this, it's not supposed to be there is it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810309899



##########
File path: build-aux/Jenkinsfile.full
##########
@@ -278,7 +267,7 @@ pipeline {
       steps {
         timeout(time: 15, unit: "MINUTES") {
           sh (script: 'rm -rf apache-couchdb-*', label: 'Clean workspace of any previous release artifacts' )
-          sh "./configure --spidermonkey-version ${spidermonkey}"
+          sh "./configure"

Review comment:
       I think you can also remove the `environment` block above this; it was only there to supply `${spidermonkey}` for this command.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1045141616


   CI is upgraded and back online courtesy of @nickva . I just re-submitted the job for this PR, but in order to test changes to `Jenkinsfile.full` I'll need to publish your changes in a specially-named branch in our repo. I can do that once we're confident about the changes here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1045047015


   @kocolosk If you prefer keeping the options and using this as default, it's fine for me. In that case, should the CI script still be updated? I'll wait for more instructions and update the PR.
   
   > Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :)
   Cool! I come from very close to Trento, it's a really nice place. Looking forward to move back one day :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810503413



##########
File path: configure
##########
@@ -29,9 +29,14 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
 ARCH="$(uname -m)"
 
+# Try to detect spidermonkey (libmozjs) version automatically
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+

Review comment:
       If the version cannot be detected, the variable `SM_VSN` will be empty. I added an `if` case below exiting the configure script and warning the user. I hope that's good enough.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r812513063



##########
File path: configure
##########
@@ -29,9 +29,14 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
 ARCH="$(uname -m)"
 
+# Try to detect spidermonkey (libmozjs) version automatically
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+

Review comment:
       Yep, looks good except for the typos ("SpiderMonkey", "explicitly")




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1049216566


   Thanks for running the CI. I made it Linux-only because I have no access to freeBSD or macOS to test alternatives. Could you update the branch so we see if the CI passes everywhere?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1057340312


   I think this MR should now be finished.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1045146430


   > In that case, should the CI script still be updated?
   
   Yes, I like what you did there.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810503413



##########
File path: configure
##########
@@ -29,9 +29,14 @@ ERLANG_MD5="false"
 SKIP_DEPS=0
 
 COUCHDB_USER="$(whoami 2>/dev/null || echo couchdb)"
-SM_VSN=${SM_VSN:-"1.8.5"}
 ARCH="$(uname -m)"
 
+# Try to detect spidermonkey (libmozjs) version automatically
+SM_VSN=${SM_VSN:-$(/sbin/ldconfig -p | sed -n 's/.libmozjs-*\([0-9]*\).*/\1/p' | head -n 1)}
+if [ "$SM_VSN" = "185" ]; then
+  SM_VSN="1.8.5"
+fi
+

Review comment:
       If the version cannot be detected, the variable `SM_VSN` will be empty. I added an `elif` case exiting the configure script and warning the user. I hope that's good enough.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1048141179


   @kocolosk Any news on this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1048424706


   Looks like this fails on macOS and FreeBSD. FreeBSD doesn't have the `-p` option to `ldconfig`, while macOS doesn't use it at all.
   
   I'd be OK to make the `ldconfig` dance conditional on discovering Linux as the underlying platform if you want to go that route, otherwise maybe you can figure out a way to support this discovery for the other Unix-like systems?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1048396413


   @frapa I think this is looking good modulo a small typo or two in the error message. I pushed it to a specially-named `jenkins-`branch so we can test your changes to `Jenkinsfile.full` in CI:
   
   https://ci-couchdb.apache.org/blue/organizations/jenkins/jenkins-cm1%2FFullPlatformMatrix/detail/jenkins-frapa-autodetect_spidermonkey_version/1/pipeline
   
   Assuming all goes well there we can fix the typo and merge.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa edited a comment on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa edited a comment on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1057340312


   I think this PR should now be finished.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa edited a comment on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa edited a comment on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1045047015


   @kocolosk If you prefer keeping the options and using this as default, it's fine for me. In that case, should the CI script still be updated? I'll wait for more instructions and update the PR.
   
   > Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :)
   
   Cool! I come from very close to Trento, it's a really nice place. Looking forward to move back one day :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1046040380


   @kocolosk I reintroduced the `--spidermonkey-version`, added a check for empty `SM_VSN` and removed the environment block. Let me know if can do anything else.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1044518745


   Thanks! I've been wanting to do something like this ever since I refactored those CI config files and found myself littering them with `--spidermonkey-version` flags everywhere. How do you feel about still allowing the use of `--spidermonkey-version`, and just using the autodetected version as a default if the user doesn't specify one?
   
   It seems we have an unrelated problem with Jenkins today, we'll investigate that separately.
   
   Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa edited a comment on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa edited a comment on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1045047015


   @kocolosk If you prefer keeping the options and using this as default, it's fine for me. In that case, should the CI script still be updated? I'll wait for more instructions and update the PR.
   
   How can I find out if the CI works again?
   
   > Also, Trento? That takes me back. I spent a few weeks at ECT when I was a graduate student years ago. Lovely place :)
   
   Cool! I come from very close to Trento, it's a really nice place. Looking forward to move back one day :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] frapa commented on a change in pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
frapa commented on a change in pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#discussion_r810501609



##########
File path: build-aux/Jenkinsfile.full
##########
@@ -278,7 +267,7 @@ pipeline {
       steps {
         timeout(time: 15, unit: "MINUTES") {
           sh (script: 'rm -rf apache-couchdb-*', label: 'Clean workspace of any previous release artifacts' )
-          sh "./configure --spidermonkey-version ${spidermonkey}"
+          sh "./configure"

Review comment:
       Great catch, removed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on pull request #3932: Autodetect spidermonkey version in ./configure

Posted by GitBox <gi...@apache.org>.
kocolosk commented on pull request #3932:
URL: https://github.com/apache/couchdb/pull/3932#issuecomment-1049988626


   OK that's fine. Can you add the `spidermonkey_vsn` back into the Jenkinsfile for the macOS and FreeBSD builds and add the flag back in `generateNativeStage`? Otherwise macOS is going to fail because the 1.8.5 default is incorrect.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org