-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Add support for SpiderMonkey v140 #5677
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go with the cat principle "If I fits, I sits". It compiles without issues and passes the test suites (including Windows) then +1 to go ahead with it.
|
The “problem” is, it’s not active in any CI run yet, so the tests are misleading 😉. I tried it with macOS, but got some errors (don’t know if it has todo with SpiderMonkey) … |
|
Oops, @big-r81, good point. Maybe we can kick the macos runner again and see it can run with the latest 140. If there are compilation errors, then I retract my +1 |
|
Compilation is fine, there were two Elixir errors, one UTF-8 test and the other I don’t remember. Will paste it here. But maybe it’s good to test this by others… |
|
These Elixir test fail: Failed EUnit tests (idk if it has something todo with SM140): |
|
Hey, did a test with QuickJS and I got 3 of the 4 errors too! Failing Elixir tests: Failing EUnit tests: This three failing tests have nothing to do with the SM140 it seems. The test |
Tried to add SM v140. There are again some internal SpiderMonkey API changes.
Tried to compile against SpiderMonkey v140 and got following errors:
I changed the csp_allow method signature to fit the
typedefinjs/public/Principals.h. I don't know if there is a better solution.Before:
After:
Here are some additional infos what changed in SpiderMonkey from 128 -> 140.