GNOME Bugzilla – Bug 711046
mozjs24 port
Last modified: 2013-12-05 19:10:31 UTC
This is the patchset for gjs on spidermonkey 24 libpeas and gnome-shell also need patches to make sure there is no direct access to jsapi from C code.
Created attachment 258393 [details] [review] build: switch to mozjs-24
Created attachment 258394 [details] [review] update for changes in options JSOPTION_STRICT rename JSOPTION_EXTRA_WARNINGS JSOPTION_METHODJIT removed since JaegerMonkey is gone, use JSOPTION_BASELINE instead
Created attachment 258395 [details] [review] update delete property in JSClass definitions to use JS_DeletePropertyStub
Created attachment 258396 [details] [review] Add a wrapper for JS_GetGlobalObject Context global object is only available through a private api now, create a wrapper that emulates the old api. Context globals will be going away in the future.
Created attachment 258397 [details] [review] Misc js24 API changes JS_EncodeStringToBuffer takes add context as first argument, JS_NewRuntime adds a JS_NO_HELPER_THREADS flag Update to new JS_Call*Tracer API
Created attachment 258398 [details] [review] update for JS_GetPrototype api change
Created attachment 258399 [details] [review] profiler: convert to new JSAbstractFramePtr api and update callbacks for api change
Created attachment 258400 [details] [review] Adapt to utf8 changes JS_SetCStringsAreUTF8 has been removed and internally spidermonkey no longer handles utf8 strings. There is a compiletime option setUTF8 that needs to be set, this change requires using JS::Evaluate. Additionally we need to handle utf8 encoding in the gjs_string functions. Finally JS_BufferIsCompilableUnit no longer has a utf8 argument
Created attachment 258401 [details] [review] context: handlify locale callbacks
Created attachment 258402 [details] [review] Compartments are compulsory now We need to add AutoCompartment calls at all entry points
Created attachment 258403 [details] [review] JS_SetVersion has been removed, JS Version is now set on the compartment instead of context.
Created attachment 258404 [details] [review] disable bogus tests note mozjs24 must be built with --disable-intl-api right now for the locale callbacks to work. In theory the locale callbacks are no longer needed since this is handled by ICU, however it looks like js24 will ship with an in-tree ICU, which many distros might disable
Review of attachment 258397 [details] [review]: One comment. ::: gjs/context.cpp @@ +567,3 @@ js_context = GJS_CONTEXT(object); + js_context->runtime = JS_NewRuntime(32*1024*1024 /* max bytes */, JS_NO_HELPER_THREADS); Why don't we want helper threads? The default "js" shell appears to use them... Or is this part of avoiding having e.g. destroy notifications run out of the GC thread which *is* important for gjs?
Review of attachment 258400 [details] [review]: Looks good.
Review of attachment 258393 [details] [review]: Right.
Er, to be clear I'm marking these patches accepted-commit-now but let's only push them all once we're ready?
Review of attachment 258394 [details] [review]: ::: gjs/context.cpp @@ +592,3 @@ if (!g_getenv("GJS_DISABLE_JIT")) { gjs_debug(GJS_DEBUG_CONTEXT, "Enabling JIT"); + options_flags |= JSOPTION_BASELINE | JSOPTION_TYPE_INFERENCE; By default, I'd prefer to do exactly what the upstream "js" shell does. Which in this case, is: JSOPTION_TYPE_INFERENCE | JSOPTION_ION | JSOPTION_BASELINE | JSOPTION_ASMJS
Review of attachment 258395 [details] [review]: Right, just a mechanical change.
Review of attachment 258396 [details] [review]: Ok.
Review of attachment 258398 [details] [review]: Yes, but JS_GetPrototype also now returns JSBool, maybe it can theoretically throw an exception now? Dunno, gjs code is not the best when it comes to handling errors...but if you're feeling charitable, maybe instead do: if (!JS_GetPrototype(...)) goto out; ? But you can also ignore this.
Review of attachment 258394 [details] [review]: We don't actually have any use for asm.js do we?
I don't see any reason to not add it.
Review of attachment 258397 [details] [review]: right, actually we probably do want to use helper threads as that will match the previous js17 behaviour, seems the option was added to avoid having multiple gc threads when you have multiple run-times (which we don't).
Created attachment 258688 [details] [review] update for changes in options JSOPTION_STRICT rename JSOPTION_EXTRA_WARNINGS JSOPTION_METHODJIT removed since JaegerMonkey is gone, use JSOPTION_BASELINE instead use the same options as upstream js shell I will re-attch all patches in order, once the other patches are reviewed
Created attachment 258689 [details] [review] Misc js24 API changes JS_EncodeStringToBuffer takes add context as first argument, JS_NewRuntime adds a JS_USE_HELPER_THREADS flag Update to new JS_Call*Tracer API use helper threads
Review of attachment 258404 [details] [review]: Ok, yeah we don't really need to test the core JS API here.
Tim: Can you push these patches to a wip/mozjs24 git branch of gjs? I think we're at the point where we should just try to land this and iterate on improving it from there. What's the status of the upstream release?
Created attachment 259300 [details] [review] build: switch to mozjs-24 upstream version was bumped to 24.1 for RC tarball
updated wip/js24 branch. Release should happen in the next couple of weeks.
(In reply to comment #29) > updated wip/js24 branch. > > Release should happen in the next couple of weeks. Thanks. Can you link here in this bug to the current mozjs24 snapshot tarballs you're using for this?
https://people.mozilla.org/~sstangl/mozjs-24.1.0.rc1.tar.bz2
(In reply to comment #31) > https://people.mozilla.org/~sstangl/mozjs-24.1.0.rc1.tar.bz2 Hmm...this falls over for me messily inside configure with some virtualenv/"mach" stuff. Can you link me to the Debian packaging you're using?
Ubuntu packaging can be found here: https://launchpad.net/~ricotz/+archive/staging/+packages I'm also running it in jhbuild without any issues.
I'm running into issues with the embedded virtualenv, failure looks like this: Creating Python environment ImportError: Bad magic number in /ostbuild/source/js24/_build/js/src/_virtualenv/lib/python2.7/site.pyo New python executable in /ostbuild/source/js24/_build/js/src/_virtualenv/bin/python2.7 Also creating executable in /ostbuild/source/js24/_build/js/src/_virtualenv/bin/python ERROR: The executable /ostbuild/source/js24/_build/js/src/_virtualenv/bin/python2.7 is not functioning ERROR: It thinks sys.prefix is u'/ostbuild/source/js24/_build/js/src' (should be u'/ostbuild/source/js24/_build/js/src/_virtualenv') ERROR: virtualenv is not compatible with this system or executable Traceback (most recent call last):
+ Trace 232755
manager.ensure()
return self.build()
self.create()
raise Exception('Error creating virtualenv.') Exception: Error creating virtualenv.
Do you have a system package of virtualenv in your buildroot?
Have you seen this issue before? Do you have a system copy of virtualenv maybe in your buildroot, i.e. is "python-virtualenv" pulled in for you?
Created attachment 263031 [details] [review] 0001-virtualenv-Support-cases-where-host-Python-only-ship.patch
Ok, sorry it took me a while to debug this. I'll do a bit more testing now that js24 builds for me, and let's try to land this soon!
Review of attachment 263031 [details] [review]: ::: js/src/python/virtualenv/virtualenv.py @@ +915,3 @@ env['VIRTUALENV_INTERPRETER_RUNNING'] = 'true' file = __file__ + if file.endswith('.pyc') or site_filename.endswith('.pyo'): file.endswith(('.pyc', '.pyo'))
https://git.gnome.org/browse/jhbuild/commit/?id=c4475d8534c5303424fb0b86d2b0c9091dfe582e
Now pushed to master. Everyone: please send any issues/patches as new bugs.