This repository has been archived by the owner on Aug 29, 2023. It is now read-only.
forked from bytecodealliance/spidermonkey-wasi-embedding
-
Notifications
You must be signed in to change notification settings - Fork 6
Pull in weval branch of SpiderMonkey and add build. #17
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
aef99c3
Pull in weval branch of SpiderMonkey and add build.
cfallin ee11412
ignore all the new obj/debug/release output folders
JakeChampion 61549b2
Update to merged head of fixed gecko-dev
cfallin 7c2ddd0
Merge rebuild.sh and build-engine.sh, and use fake wasm-opt
cfallin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
https://github.com/bytecodealliance/gecko-dev | ||
https://github.com/bytecodealliance/gecko-dev |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
69d3daaa3aa4589ac8e60a94c7c7e0592882d272 | ||
f0dc4ca253349a4b8bd3bd253fa8421e76888326 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
ac_add_options --enable-project=js | ||
ac_add_options --enable-application=js | ||
ac_add_options --target=wasm32-unknown-wasi | ||
ac_add_options --without-system-zlib | ||
ac_add_options --without-intl-api | ||
ac_add_options --disable-jit | ||
ac_add_options --disable-shared-js | ||
ac_add_options --disable-shared-memory | ||
ac_add_options --disable-tests | ||
ac_add_options --disable-clang-plugin | ||
ac_add_options --enable-jitspew | ||
ac_add_options --enable-optimize | ||
ac_add_options --enable-js-streams |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
ac_add_options --enable-js-interp-native-callstack | ||
ac_add_options --enable-js-interp-specialization | ||
ac_add_options --enable-js-interp-weval |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Our js-compute-runtime build still assumes
wasm-opt
in path, so I think this may cause conflicts?@elliottt maybe we should take an approach in js-compute-runtime of using different pathing finally here?
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.
This check actually fails if you don't have
wasm-opt
in PATH, so it seems like it must be set to/bin/true
for this to pass?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.
Ah, this is the confusing nature of shell scripting: it's meant to fail (and does fail) if we do have
wasm-opt
in $PATH. The subshell (in parens) returns exit code 1 (failure) ifwasm-opt
doesn't exist or grep fails to find the text (and returns its own failure code). Theif
in the parent shell takes0
as truthy, so this is saying "if success (wasm-opt exists and is real wasm-opt), then fail with message".To the broader point of wasm-opt existing for the rest of the build... this indicates to me that perhaps the better approach is to actively mask wasm-opt with a wrapper and put that at the front of PATH for this build. I'll make that change; it's more general anyway!
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.
How about this instead?
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 js-compute-runtime does usewasm-opt
to strip builds, this might make it difficult to run both builds in the same machine. We worked around the use ofwasm-opt
by llvm by temporarily putting a shell script that exits immediately in the path earlier than the realwasm-opt
. Would that be an option here?You suggested this in an earlier comment, and I missed it completely :)
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've got some edits to this branch, in a different branch https://github.com/fastly/spidermonkey-wasi-embedding/tree/jake/weval-edits which has a similar change but only for CI and not the build.sh script itself
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.
Cool, I pulled in your updates to the gitignore list @JakeChampion and I've pushed an update that shadows
wasm-opt
with its own fake-bin directory in PATH. This avoids the need to move it out of the way during the CI build. I also went ahead and merged rebuild.sh with build-engine.sh (former calls later with an arg) because it was getting a little too unwieldy to duplicate logic.