GNOME Bugzilla – Bug 646369
Support compilation with standalone mozjs185 release
Last modified: 2011-05-04 19:42:59 UTC
First, adjust the detection logic so that we look for mozjs-185.pc first. If we have this, we can skip all kinds of insanity. See https://bugzilla.mozilla.org/show_bug.cgi?id=628723 for the discussion about creating this release. Also, the JSScript API changed, see: See https://bugzilla.mozilla.org/show_bug.cgi?id=630209
Created attachment 184810 [details] [review] Support compilation with standalone mozjs185 release
Review of attachment 184810 [details] [review]: Fedora's standalone js package doesn't provide js-config but instead ships a libjs.pc, which this wouldn't catch.
Review of attachment 184810 [details] [review]: Sort of torn here, I understand what you are trying to do here - add another option without disturbing the existing logic which is all magical and about weird old bugs. But I hate the state that this patch leaves the configure.ac: - We check for mozjs185 via pkg-config - We check for js-config, which we don't know where it comes from - We check for mozilla-js via pkg-config - We check for xulrunner-js, which is documented in comments to be too old to work via pkg-config - We check for firefox-js which we don't even know who uses via pkg-config I guess it's OK like this - probably we should rewrite when removing support for xulrunner-1.9 ::: configure.ac @@ +96,3 @@ +PKG_CHECK_EXISTS([mozjs185], JS_PACKAGE=mozjs185,) +if test x$JS_PACKAGE != x; then + FIREFOX_JS_LIBDIR=`$PKG_CONFIG --variable=libdir $JS_PACKAGE` Does the standalone mozjs really install with a weird libdir instead of just being a system library? And shouldn't this patch remove the rpath hacks if it is? @@ +108,2 @@ # Look for Spidermonkey. If js-config exists, use that; # otherwise we try some pkgconfig files from various distributions. this comment is now broken by your changees @@ +117,1 @@ if test -n "$JS_CONFIG"; then I think you need to byte the bullet and reindent (M-4 M-x indent-rigidly) - shell code is hard enough to understand without misplaced indentation. @@ +177,3 @@ +if test x$MOZJS_IS_STANDALONE != xyes; then + AC_MSG_CHECKING([for mozilla-js >= 1.9.2 ]) Somewhere you need a comment explaining the confusing versioning discrepancy between mozjs and mozilla-js. @@ +189,3 @@ +AC_CHECK_LIB([$MOZJS_LIB], [JS_GetStringChars], AC_DEFINE([HAVE_JS_GETSTRINGCHARS], [1], [Define if we still have JS_GetStringChars]),, [$JS_LIBS]) +AC_CHECK_LIB([$MOZJS_LIB], [JS_StrictPropertyStub], AC_DEFINE([HAVE_JS_STRICTPROPERTYSTUB], [1], [Define if we have JS_StrictPropertyStub]),, [$JS_LIBS]) +AC_CHECK_LIB([$MOZJS_LIB], [JS_GetGlobalForScopeChain], AC_DEFINE([HAVE_JS_GETGLOBALFORSCOPECHAIN], [1], [Define if we have JS_GetGlobalForScopeChain]),, [$JS_LIBS]) For future reference, the preferrable way to do this is shown in the gnome-shell configure.ac: saved_CFLAGS=$CFLAGS saved_LIBS=$LIBS CFLAGS=$GNOME_SHELL_CFLAGS LIBS=$GNOME_SHELL_LIBS AC_CHECK_FUNCS(JS_NewGlobalObject sn_startup_sequence_get_application_id XFixesCreatePointerBarrier) CFLAGS=$saved_CFLAGS LIBS=$saved_LIBS @@ +216,2 @@ AC_MSG_CHECKING([if SpiderMonkey needs extra compiler flags]) save_CFLAGS="$CFLAGS" Again, have to reindent - no choice. *But* I don't see why you are if'ing this out. It looks like it will work fine, and the extra few seconds of compilation time aren't worth creating untested code paths in the configure.
(In reply to comment #3) > Review of attachment 184810 [details] [review]: > I guess it's OK like this - probably we should rewrite when removing support > for xulrunner-1.9 It will be a lot cleaner when I drop support for compiling against Firefox/XULRunner =) > Does the standalone mozjs really install with a weird libdir instead of just > being a system library? And shouldn't this patch remove the rpath hacks if it > is? Well...setting the rpath doesn't hurt; build systems can strip it out. I agree it'd be nice to avoid but it would make a bigger mess of the build system for little gain. > I think you need to byte the bullet and reindent (M-4 M-x indent-rigidly) - > shell code is hard enough to understand without misplaced indentation. Done. > Somewhere you need a comment explaining the confusing versioning discrepancy > between mozjs and mozilla-js. So what I ended up doing with all of this is reordering things so that all the gunk for firefox/xulrunner ends up under one single if, rather than having it scattered. This works better.
Did you mean to attach a new version?
Created attachment 185984 [details] [review] Adapt to change in JSScript API from tracemonkey tree See https://bugzilla.mozilla.org/show_bug.cgi?id=630209
Created attachment 185985 [details] [review] Support compilation with standalone mozjs185 release Adjust the detection logic so that we look for mozjs-185.pc first. If we have this, we can skip all kinds of insanity. See https://bugzilla.mozilla.org/show_bug.cgi?id=628723 for the discussion about creating this release.
Created attachment 187223 [details] [review] Support compilation with standalone mozjs185 release Rebased on git master
Attachment 187223 [details] pushed as c3f95ea - Support compilation with standalone mozjs185 release