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
spidermonkey_78: 78.4.0 -> 78.8.0 #114030
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.
The patch for 1644600 seems fine to me. I think this could go to the latest release though: 78.7.1?
I think staging is right based on the path for spidermonkey/llvm_11 pinging @SuperSandro2000
/rebase staging |
Rebased, please reopen the pull request to restart CI |
# https://bugzilla.mozilla.org/show_bug.cgi?id=1644600 | ||
# Once that bug is fixed, this can be removed. | ||
# This is needed in, for example, `zeroad`. | ||
sed -i "s/class SharedArrayRawBufferRefs {/class JS_PUBLIC_API SharedArrayRawBufferRefs {/g" \ |
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.
Can we use here substituteInPlace to make this simpler?
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.
updated
This will be needed to fix build after rustc PR #112792 gets merged, I suggest version bump directly to |
We can also bump it in another PR to get this merged faster. The complex sed is a bigger concern for me. |
Sorry for the noise, I did not mean to ping all these reviewers. I force pushed my local branch while it was still based on master and then it included all the staging commits. Then it auto-requested reviews. I have replaced the sed call with |
I have bumped the version to |
--replace "class SharedArrayRawBufferRefs {" \ | ||
"class JS_PUBLIC_API SharedArrayRawBufferRefs {" | ||
''; | ||
|
||
meta = with lib; { | ||
description = "Mozilla's JavaScript engine written in C/C++"; | ||
homepage = "https://developer.mozilla.org/en/SpiderMonkey"; |
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.
The homepage seems to be broken as well
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.
I may change it to https://spidermonkey.dev/
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.
updated
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.
Changes look fine. Builds and runs (checked usage).
Motivation for this change
This update is needed for an upcoming PR for
zeroad
. The attached patch will be needed for that. It also allows for other "external embedders" of spidermonkey as described in the linked bugreport.Should this rather go to staging?
Notify Maintainers
Cc: @abbradar @lostnet
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)