Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]: Shared object #432

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[WIP]: Shared object #432

wants to merge 5 commits into from

Conversation

atbrakhi
Copy link
Member

@atbrakhi atbrakhi commented Nov 28, 2023

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

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

@atbrakhi atbrakhi mentioned this pull request Nov 28, 2023
39 tasks
@atbrakhi
Copy link
Member Author

atbrakhi commented Nov 28, 2023

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! :)

@Redfire75369
Copy link
Contributor

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

@atbrakhi
Copy link
Member Author

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 :)

@jschwe
Copy link
Contributor

jschwe commented Sep 26, 2024

@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).

@wusyong
Copy link
Member

wusyong commented Sep 27, 2024

@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.

@atbrakhi
Copy link
Member Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants