Skip to content

Refactor build-js.sh into a Makefile#2448

Closed
kripken wants to merge 24 commits intomasterfrom
make
Closed

Refactor build-js.sh into a Makefile#2448
kripken wants to merge 24 commits intomasterfrom
make

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Nov 18, 2019

This cleans up and refactors the binaryen.js build script to be
a proper Makefile. Aside from cleanliness, the main benefits
are:

  • Allows parallel compilation of source files with make -jX.
  • Switches the build to wasm, and not js, as it turns out
    the wasm backend is broken on js builds, so this works around
    that until we solve it. However, maybe we just want the build
    to be wasm anyhow? Binaryen.js builds with wasm2js fail #2446

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 18, 2019

cc @dcodeIO

@dcodeIO
Copy link
Copy Markdown
Contributor

dcodeIO commented Nov 18, 2019

Compiling to Wasm exclusively sounds good to me. Two questions:

  • I remember that browser VMs, unlike node, have relatively tight size constrains for synchronous compilation, so I wonder if disabling async compilation might break binaryen.js in browsers?
  • Providing (only) a Makefile makes me worry about Windows support. Or does emmake take care of this?

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 18, 2019

  • True, running in browsers will be an issue without async compilation. We should provide an async API for starting up binaryen.js, I guess, like Binaryen.then() perhaps? I don't remember what the best practices are for such things. But yeah, given that maybe we should not land this just now.
  • I think the old script was bash-only, so it also probably didn't work on windows?

@dcodeIO
Copy link
Copy Markdown
Contributor

dcodeIO commented Nov 18, 2019

I guess, like Binaryen.then() perhaps? I don't remember what the best practices are for such things. But yeah, given that maybe we should not land this just now

I remember the Binaryen.ready promise idea in #1381, similar to Wabt's ready promise. One could then

if (!Binaryen.isReady) await Binaryen.ready;
...

I think the old script was bash-only, so it also probably didn't work on windows?

Git for Windows includes Git Bash, which makes a bash file quite convenient. Is there maybe a way to make this a cmake task, conditionally creating a VS project? Would be similar since one needs cmake anyway to build parts of Binaryen. I don't know much about this, unfortunately

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Nov 19, 2019

I see, thanks. Ok, sounds like this PR isn't the right way to go - I'll close this, and open issues for those ideas, which I agree are the right way to go. Not sure I'll have time for them myself, though.

@kripken kripken closed this Nov 19, 2019
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.

2 participants