Now that we are in the 59 cycle, we should review any remaining obstacles to having an official SpiderMonkey ESR release for embedders. A few things that come to mind: - Upload tarball to https://archive.mozilla.org/pub/spidermonkey/releases/ - Make sure MDN docs have links and a brief changelog - Have a plan for ESR updates and point releases We should also gather feedback from embedders here and track issues that block 59 embedding.

A few more things after some IRC discussions... - MDN docs are really old and should at a minimum have warnings at the top of them - Doing some cleanup in jsapi.h of docs and structure might be helpful to embedders who must turn to code for answers.

(In reply to Ted Campbell [:tcampbell] from comment #0) > A few things that come to mind: > - Upload tarball to https://archive.mozilla.org/pub/spidermonkey/releases/ See also Bug 1422937.

As a first step, I have selected a release from mozilla-central to be a first "alpha" of SM59. https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&selectedJob=149799451 The packaged tarball for that job is: https://queue.taskcluster.net/v1/task/YhbEOycATOiblYHIKGAoUQ/runs/0/artifacts/public/build/mozjs-59.0a1.0.tar.bz2 Philip, give this a try and let us know any feedback or problems with it.

Just to make it easier to remember what's what, I've uploaded the above tarball to https://ftp.mozilla.org/pub/spidermonkey/prereleases/59/pre1/mozjs-59.0a1.0.tar.bz2

I gave this a try. The good news is that I didn't have to port much of my codebase at all, apart from trivial things. The one exception was the disappearance of getProperty and setProperty from JSClass which will be tedious, but probably not too difficult, to rewrite to use resolve. The bad news is still mozglue. My programs will compile but not link. It turns out libmozglue is built as a separate shared library which 'make install' does not install, so libmozjs refers to it but it can't be found. I am pretty sure that mozglue is not supposed to be a shared library for embedded SpiderMonkey, but instead compiled into the main library. I am testing so far on macOS, and I believe that is why we get the shared library, so I made this change: --- a/mozglue/build/moz.build +++ b/mozglue/build/moz.build @@ -4,9 +4,12 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -# Build mozglue as a shared lib on Windows, OSX and Android. +# Build mozglue as a shared lib on Windows, OSX and Android. But not for +# embedders! # If this is ever changed, update MOZ_SHARED_MOZGLUE in browser/installer/Makefile.in -if CONFIG['OS_TARGET'] in ('WINNT', 'Darwin', 'Android'): +if CONFIG['JS_STANDALONE']: + Library('mozglue') +elif CONFIG['OS_TARGET'] in ('WINNT', 'Darwin', 'Android'): SharedLibrary('mozglue') else: Library('mozglue') This should bring the sharedness of mozglue for embedded SpiderMonkey on any platform in line with what it would be on Linux x64. However this makes SpiderMonkey not build. libmozglue.a rm -f libmozglue.a libmozglue.a.desc make[3]: Nothing to be done for `target'. /Users/ptomato/.cache/jhbuild/build/mozjs/_virtualenv/bin/python /Users/ptomato/jhbuild/source/mozjs/config/expandlibs_exec.py --extract -- ar crs libmozglue.a ../../memory/build/libmemory.a ../../memory/mozalloc/libmemory_mozalloc.a ../../mozglue/misc/libmozglue_misc.a ../../mfbt/libmfbt.a make[3]: Nothing to be done for `target'. Executing: ar crs libmozglue.dylib ../../memory/build/Unified_cpp_memory_build0.o ../../memory/mozalloc/mozalloc_abort.o ../../memory/mozalloc/Unified_cpp_memory_mozalloc0.o ../misc/AutoProfilerLabel.o ../misc/ConditionVariable_posix.o ../misc/Mutex_posix.o ../misc/Printf.o ../misc/StackWalk.o ../misc/TimeStamp.o ../misc/TimeStamp_darwin.o ../../mfbt/lz4.o ../../mfbt/Compression.o ../../mfbt/Decimal.o ../../mfbt/Unified_cpp_mfbt0.o ../../mfbt/Unified_cpp_mfbt1.o ar: libmozglue.dylib: Inappropriate file type or format make[3]: *** [libmozglue.a] Error 1 make[2]: *** [mozglue/build/target] Error 2 make[1]: *** [compile] Error 2 make: *** [default] Error 2 Again I've only tested this on macOS so far, I won't be back to my Linux machine until after the winter break.

(In reply to Philip Chimento [:ptomato] from comment #5) > The one exception was the > disappearance of getProperty and setProperty from JSClass which will be > tedious, but probably not too difficult, to rewrite to use resolve. It depends on your use case: resolve hooks work fine for some of them, proxies might be better for other objects. Bug 1389949 has a patch converting NPAPI NPObjWrapper from getProperty/setProperty Class hooks to a proxy object. Hope this helps.

The next ESR will be 60 rather than 59.

(In reply to Jan de Mooij [:jandem] from comment #6) > (In reply to Philip Chimento [:ptomato] from comment #5) > > The one exception was the > > disappearance of getProperty and setProperty from JSClass which will be > > tedious, but probably not too difficult, to rewrite to use resolve. > > It depends on your use case: resolve hooks work fine for some of them, > proxies might be better for other objects. Bug 1389949 has a patch > converting NPAPI NPObjWrapper from getProperty/setProperty Class hooks to a > proxy object. Hope this helps. Looking at my use case it seems that resolve would probably make sense, but out of curiosity do you have a criterion for deciding which approach to take?

(In reply to Philip Chimento [:ptomato] from comment #8) > Looking at my use case it seems that resolve would probably make sense, but > out of curiosity do you have a criterion for deciding which approach to take? I'd use resolve hooks when there's a fixed number of lazy properties, properties you want to define when they're accessed. Proxies are useful when you want to intercept a large number of properties (say integer indexes) or when the values of these properties are dynamic (with a resolve hook you will have to define a getter/setter, then figure out which property was accessed, etc).

To follow up on this, I've set up Travis CI on my personal mozjs GitHub repo [1] and pushed my work to the mozjs59 branch. I have it building, testing the JS shell, and linking a test embedder application with Clang, GCC 6, GCC 7 on Linux, and Clang on macOS [2]. This should make it easier to test future alphas of ESR60. I updated my patch that fixes building and linking on macOS and will attach it on bug 1176787. I'll continue to port my code base and see what else comes up. Note that bug 1430438 will be semi-required for me to move my code base to ESR60 and get rid of getProperty/setProperty hooks. I can do without it, but in that case I'll have performance regressions due to copying byte arrays where previously that wasn't necessary. [1] https://github.com/ptomato/mozjs/tree/mozjs59 [2] https://travis-ci.org/ptomato/mozjs/builds/328959639

If you have an alpha of ESR60, I'll give it another try.

Steve, are you able to cut an "alpha" of ESR60 onto the ftp to look at?

I used a tarball marked 60.0.0 from Treeherder. Here's the branch in my personal mozjs repo: [1] Relative to the 59a1 that I tested before, there's one regression: the header files are installed into the wrong directory. I will attach a patch to fix this, but since the code that I change in my patch wasn't changed recently, I'm not sure what caused the regression, and if it's necessary to fix this problem somewhere else than I did in the patch. I'm guessing Mike would be the most appropriate reviewer for this? BTW, I wonder if the latest commit on [1] might be useful to you, to prevent these kinds of regressions - it uses Travis CI to compile, link, and run a minimal embedder program with the just-built SpiderMonkey library. [1] https://github.com/ptomato/mozjs/tree/mozjs60

Somehow the header files were being installed directly into $prefix/include, rather than $prefix/include/mozjs-60. Something else changed somewhere that affected this, since this code was the same in older mozjs versions, but this seems the most logical place to fix it.

Hm, I bet the https://archive.mozilla.org/pub/spidermonkey/prereleases/60/pre1/mozjs-60.0.1.tar.bz2 that I uploaded at around the same time probably has the same problem, then.

Comment on attachment 8975487 [details] [diff] [review] Fix SpiderMonkey includedir installs Review of attachment 8975487 [details] [diff] [review]: ----------------------------------------------------------------- FWIW, bug 1427455 is what changed.

Thanks. I suppose I could add back that variable, but it would be easy to remove accidentally again, and this keeps the tarball-install-specific stuff in the same place.

Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/77041935decb Fix SpiderMonkey includedir installs. r=glandium

Comment on attachment 8975487 [details] [diff] [review] Fix SpiderMonkey includedir installs [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Generated SpiderMonkey tarballs don't work out of the box without this. User impact if declined: Embedders can't use SpiderMonkey without patching. Fix Landed on Version: 60 Risk to taking this patch (and alternatives if risky): Shouldn't affect Firefox, as this build code is only used when running "make install" on the SpiderMonkey tarball. String or UUID changes made by this patch: None See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment on attachment 8975487 [details] [diff] [review] Fix SpiderMonkey includedir installs spidermonkey makefile fix, approved for 60.1esr

Note that this is included in http://ftp.mozilla.org/pub/spidermonkey/prereleases/60/pre2/mozjs-60.0.2pre2.tar.bz2

Comment on attachment 8975487 [details] [diff] [review] Fix SpiderMonkey includedir installs https://hg.mozilla.org/releases/mozilla-esr60/rev/8e016d7b01bb Clearing the esr60 approval flag so it doesn't show up on the needs-uplift radar anymore.

Would it be possible to upload a stable release, for example https://queue.taskcluster.net/v1/task/WQHIXhTpRI-pE-2Snadfqw/runs/0/artifacts/public/build/mozjs-60.1.0.tar.bz2 (from job https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&selectedJob=183691323) to the FTP and call that an official release of standalone SpiderMonkey? Other than that, here are some things that were mentioned earlier in this comment thread that might be considered conditions for closing this bug: - Have a plan for ESR updates and point releases - Put warnings at the top of MDN docs - Doing some cleanup in jsapi.h of docs and structure might be helpful to embedders who must turn to code for answers - Fail the standalone build if configured without --disable-jemalloc (bug 1465038) How does that sound?

I have been testing this on Ubuntu, the 64-bit builds are working out ok, but 32-bit builds have test failures relating to double precision numbers see bug 1486328