mozjs icon indicating copy to clipboard operation
mozjs copied to clipboard

[WIP]: Shared object

Open atbrakhi opened this issue 1 year ago • 3 comments

Right now we have 3 static library in mozjs: libmozjs.a, libjsglue.a for mozjs/mozjs-sys glue code and libjsglue.a for mozjs/mozjs glue code. we are trying to have shared object instead. See servo/30593

  • [x] shared object Spidermonkey libmozjs-115.dylib (.so/.dll for linux/windows)
  • [x] shared object glue code in mozjs-sys libjsglue.dylib
  • [x] shared object glue code in mozjs libjsglue.dylib
  • [ ] feature flag for static/dynamic(?)

atbrakhi avatar Nov 28 '23 13:11 atbrakhi

cargo build is fine but cargo test is currently failing with this error:

dyld[49541]: Library not loaded: @rpath/libmozjs-115.dylib
  Referenced from: <2C561018-77FB-38DD-82BE-154FFEE4BB16> /Users/atbrakhi/Documents/codespace/mozjs/target/debug/deps/callback-edcd041304273f3b
  Reason: tried: '/System/Volumes/Preboot/Cryptexes/OS@rpath/libmozjs-115.dylib' (no such file), '/usr/local/lib/libmozjs-115.dylib' (no such file), '/usr/lib/libmozjs-115.dylib' (no such file, not in dyld cache)
Process 49541 stopped
* thread #1, stop reason = signal SIGABRT

So far, the investigation has indicated that libmozjs-115.dylib itself is not the problem(I changed the @rpath locally that made me came to this conclusion), but when we do cargo test it is unable to find some symbols such as install_rust_hooks, encoding_mem_is_ascii etc. The problem might lie in the interaction between the SpiderMonkey dylib and mozjs-sys Rust symbols. Specially the extern crate encoding_c (all missing symbols are pointing to this)

I am investigating this further. Ideas and suggestions are most welcome! :)

atbrakhi avatar Nov 28 '23 14:11 atbrakhi

I would like to suggest having a feature flag or similar to enable/disable static/dynamic library-ness.

Redfire75369 avatar Nov 29 '23 13:11 Redfire75369

I would like to suggest having a feature flag or similar to enable/disable static/dynamic library-ness.

I think it's a fair point. I have added it in todo list :)

atbrakhi avatar Nov 29 '23 14:11 atbrakhi

@atbrakhi I'm curious: Is this PR still on your todo list, or did you encounter some major blocking issue?

Would it be an option to build spidermonkey (the C/C++ code) as a normal shared library object with C-ABI, and keep compiling the (small) Rust wrapper library from servo? If we choose to compile the whole mozjs as a rust dylib then we will need to be careful about keeping the Rust compiler versions used in servo and to pre-compile the .so in sync (I'm assuming we would want to ship the .so in a similar form as we currently prebuild mozjs as a static library).

jschwe avatar Sep 26 '24 06:09 jschwe

@jschwe I think this was my suggestion when we need easier mozjs compilation. I chose to publish pre-built archive instead. I still want mozjs has smaller rust wrapper in the future. Right now the sys crate still tries to build two different bindings iirc.

wusyong avatar Sep 27 '24 01:09 wusyong

@atbrakhi I'm curious: Is this PR still on your todo list, or did you encounter some major blocking issue?

Would it be an option to build spidermonkey (the C/C++ code) as a normal shared library object with C-ABI, and keep compiling the (small) Rust wrapper library from servo? If we choose to compile the whole mozjs as a rust dylib then we will need to be careful about keeping the Rust compiler versions used in servo and to pre-compile the .so in sync (I'm assuming we would want to ship the .so in a similar form as we currently prebuild mozjs as a static library).

@jschwe it simply got de-prioritized after @wusyong got static library working. I haven't been able to prioritize this work since then. Maybe I should pick it up again soon.

We had a similar discussion in #30593. When i was working on building mozjs as shared lib I was having some issues with missing symbol that was coming from external crates such as encoding_c, I wrote about it in this comment

atbrakhi avatar Sep 30 '24 13:09 atbrakhi