WebAssembly / binaryen

Optimizer and compiler/toolchain library for WebAssembly
Apache License 2.0
7.29k stars 717 forks source link

Keep bundling binaryen.js/wasm.js? #1571

Open kripken opened 6 years ago

kripken commented 6 years ago

We currently bundle them in this repo, under bin/. I worry though that over time it increases the size of the git repo significantly. Am I overreacting?

Another option would be a separate location for the builds, perhaps.

Another benefit of no longer bundling them is that PRs no longer need to bother with rebuilding&bundling. But on the other side, not bundling them means if a user just wants those .js files then they must go someplace else to get them.

sbc100 commented 6 years ago

I'm generally against keeping generated files under source control. But I suppose in this case these files are not wholly reconstructible based on binaryen and its submodules? i.e They are not that easy to rebuild exactly because their exact contents depends on the whole emscripten SDK, right?

Can travis/circle build and publish these to github releases? Maybe they already do?

kripken commented 6 years ago

The builds depend on the compiler too, the emsdk, yeah.

Currently we build a debug version on travis and run tests on that, to prevent regressions. A release build takes quite some time but should also be doable in the 50 minutes travis allows us, so perhaps we could set it to publish to github releases. I'm not really sure how github releasees work, but if new tags (for new versions) could trigger such builds being added, that sounds good.

xtuc commented 6 years ago

Yes a tag can trigger a build and then a simple curl (plus a token) can create the release with the files.

kripken commented 6 years ago

Thanks @xtuc , this sounds doable then.

kripken commented 6 years ago

@dcodeIO how do you handle builds for binaryen.js? In this issue we're considering stopping the bundling in the binaryen repo.

dcodeIO commented 6 years ago

My buildbot basically

  1. Installs and compiles Emscripten incoming from source (using ccache so it's relatively fast)
  2. Updates Binaryen, which is a submodule, to latest (for nightlies) or a specific tag if there is a new one (for releases)
  3. Runs a build script very similar to build-js.sh and outputs the result to index.js
  4. Runs a sanity test to make sure that the binary is working
  5. Creates a Git tag, pushes the changes back to GH and publishes to npm

While this doesn't create any GH releases (and index.js is a asm.js binary), this essentially looks, feels and behaves like a simple, regular node module repo otherwise. I am ok with moving the bot somewhere else if that sounds good to you. Can of course also make one for wasm.js.

xtuc commented 6 years ago

That's a good point, I would prefer publishing on npm instead of a GH release. It's more convenient.

kripken commented 6 years ago

Oh cool! So maybe we can do something like this:

dcodeIO commented 6 years ago

The one tricky bit is the rest of the tests, that is, that tested the bundled optimized build.

Maybe the JS specific tests could then be run by the buildbot (the sanity tests it currently has are relatively basic so any improvement there would be great anyway). One thing to note afaik is that the JS tests cover some parts of the C-API that the C tests do not cover yet.

kripken commented 6 years ago

@dcodeIO makes sense, yeah, improving the buildbots testing could offset removing tests here.

Another thing I realized meanwhile is that wasm.js can go away once we finish wasm2js - that is meant to be a drop-in replacement for wasm after all. So we'll need to figure out testing for that anyhow, and can remove wasm.js in the meanwhile.

kripken commented 6 years ago

Is there a way to link directly to binaryen.js builds on npm? Maybe there's an obvious place but I can't seem to find it. I think that would be useful for people that just want the binaryen.js build, without npm (might not be many people, but still).

xtuc commented 6 years ago

You can use Unpkg or bundle.run (if you need to bundle it)

dcodeIO commented 6 years ago

It's basically the index.js file.

GitHub:

rawgit.com as a CDN:

npm:

kripken commented 6 years ago

Thanks for the links @dcodeIO !

I opened #1609 to stop bundling binaryen.js, and point to those links.

Btw, do people rely on rawgit and unpkg? At first glance they don't seem to be official products of github or npm, whose content they serve.

kripken commented 6 years ago

And I guess the same question for @xtuc about bundle.run? Just curious how mainstream all these services are...

xtuc commented 6 years ago

Yes, millions of peoples rely on these services even if none of them are official products.

Could we fix the latest builds of binaryen.js? For example https://travis-ci.org/AssemblyScript/binaryen.js/builds/395478938 (I have trouble understanding the log on mobile sorry)

dcodeIO commented 6 years ago

From what I can tell unpkg.com is relatively popular among node.js developers because it pretty much behaves like npm with its urls. It's open source and according to its README sponsored by CloudFlare and Heroku.

Not sure how common RawGit is, but I like its sheer simplicitly (just something simple serving GH repos via CloudFlareStackPath afaik) and that one can pick exact commits as well.

kripken commented 6 years ago

I see, thanks @xtuc & @dcodeIO

That binaryen.js CI error looks like

error: failure to process js library "library.js": Missing C define TIME_UTC! If you just added it to struct_info.json, you need to ./emcc --clear-cache

Should be fixed by clearing the cache. I assume the bot uses latest incoming? If it does, might be worth clearing the cache on each run (but will add some time to building libc etc.). Or, getting the latest tagged version would avoid that issue too.

dcodeIO commented 6 years ago

Commit above should fix that, I hope.

dcodeIO commented 6 years ago

Or, getting the latest tagged version would avoid that issue too.

Being able to just emsdk install latest would be ideal, but last time I checked precompiled binaries didn't work on Travis CI because of a glibc mismatch. I think there are a couple issues I opened or replied to at emsdk / emscripten somewhere. Do you know if there has been any progress on making proper static binaries?

kripken commented 6 years ago

The binaries work on 16.04, this is what we use for emscripten CI on travis: https://github.com/kripken/emscripten/blob/incoming/Dockerfile Also works on circle (also using 16.04).

dcodeIO commented 6 years ago

I see, thanks. goes learn docker