wabt icon indicating copy to clipboard operation
wabt copied to clipboard

WASI Integration

Open syrusakbary opened this issue 6 years ago β€’ 18 comments

This PR brings WASI to wabt using wasienv.

Old description - wapm integration

We just released WAPM: a Package Manager for WebAssembly (announcement here).

We adapted wabt to emit .wasm files (using Emscripten) and then published to wapm so they can be used very easily (just one install command) across any OS/platform.

You can try it with:

# First, let's install wapm
curl https://get.wasmer.io -sSfL | sh

# Then, install wabt
wapm install wabt

# And then, you can use wasm2wat, ...
wapm run wasm2wat ...

It would be awesome if it could be integrated (maybe in the CI?) so any new releases are published automatically.

Let me know your thoughts!! :)


In order to be able to transfer you the ownership of the wabt package on wapm, I would need an username. You can register here: https://wapm.io/signup

syrusakbary avatar Apr 23 '19 17:04 syrusakbary

Thanks, looks pretty good!

I'm not sure this needs to be upstream (see for example, https://www.npmjs.com/package/wabt and https://github.com/AssemblyScript/wabt.js). One downside is that it will increase the size of the source repo, since the files are checked-in. That said, the binaries aren't too big, and it might be convenient for others to have these files readily available.

I also noticed that the .js scripts don't seem to work with node (I just get stat failed: No such file or directory). Not sure why, but we may just want to include the .wasm files.

binji avatar Apr 23 '19 23:04 binji

This is indeed pretty exciting.

I kind of agree with ben that package manager metadata tends not to live in upstream repos, but I can also see an argument for it in this case since wabt is such a core part of wasm.

Is there a reason why the binaries need to be checked in? This change would be pretty tiny without them, right?

sbc100 avatar Apr 24 '19 00:04 sbc100

@binji @sbc100 I have completely re-worked on the PR.

This PR is now much cleaner. Builds the executables directly with WASI and doesn't check the binaries in the repo. You can build the WASI executables just by doing:

./scripts/build_wasi.sh

Looking forward hearing your thoughts!

syrusakbary avatar Sep 26 '19 00:09 syrusakbary

@sbc100 feedback addressed!

I think I can address Travis automation in another PR πŸ™‚ (and even automatic deployment to wapm! πŸŽ‰)

Note: if we can review wapm in this PR it would make my life a bit easier! I'm super happy to iterate on it though if you have any feedback :)

syrusakbary avatar Sep 26 '19 01:09 syrusakbary

The reason I suggest the split is that part of this change (adding a wasi build) is completely uncontroversial.

Its the adding of distro-specific files in the top level directory that I think is a somewhat controversial. Just like we would probably wouldn't want debian or redhat packaging rules here at the top level. How about putting them under dist/wapm or at least under wapm subdirectory? I could be overthinking it.. what does @binji think?

sbc100 avatar Sep 26 '19 01:09 sbc100

Its the adding of distro-specific files in the top level directory that I think is a somewhat controversial. Just like we would probably wouldn't want debian or redhat packaging rules here at the top level. How about putting them under dist/wapm or at least under wapm subdirectory?

wapm.toml is generally positioned in main dir for convenience, similarly on how CI tools (travis or appveyor), or other package managers position the manifests in the root dir (package.json, requirements.txt, Cargo.toml, ...).

However I'm happy to change dirs if you think that would make more sense

syrusakbary avatar Sep 26 '19 02:09 syrusakbary

Its the adding of distro-specific files in the top level directory that I think is a somewhat controversial. Just like we would probably wouldn't want debian or redhat packaging rules here at the top level. How about putting them under dist/wapm or at least under wapm subdirectory?

wapm.toml is generally positioned in main dir for convenience, similarly on how CI tools (travis or appveyor), or other package managers position the manifests in the root dir (package.json, requirements.txt, Cargo.toml, ...).

However I'm happy to change dirs if you think that would make more sense

I think the difference with requirements.txt, Cargo.toml or package.json, is that they are linked to the language the project is written in. I would see that as more akin the CMakeLists.txt file here in wabt. I see wapm.toml as a description of how to distribute the program on one particular platform, more like packaging for debian/ubuntu/redhat etc or a windows nsis installer script. Perhaps I'm nitpiking too much and such files should be welcome at the top level? I'm curious what others think? I'm also curious about other projects? Have other C/C++ projects been ok with this stuff in the root?

I'm also curious if being in the root dir is required? Perhaps we can allow wapm to accept metadata outside the root dir?

sbc100 avatar Sep 26 '19 08:09 sbc100

Perhaps I'm nitpiking too much and such files should be welcome at the top level? I'm curious what others think?

Yeah, I agree that it's not trivial to see what's the ideal scenario. I would love to get other people thoughts πŸ™‚

I'm also curious about other projects? Have other C/C++ projects been ok with this stuff in the root?

In general projects have been happy positioning the manifest config in the root. However, they have been mostly Rust/AssemblyScript projects. Most C/C++ projects we forked them and we haven't created a PR to them yet (this is the first one!).

I'm also curious if being in the root dir is required?

The manifest have to be in a parent dir of the WebAssembly files exposed. Doesn't need to be the root per-se though

Perhaps we can allow wapm to accept metadata outside the root dir?

It will require a bit of engineering effort on the wapm cli side, but it's completely feasible. Perhaps the first thing that we need to figure out is where to position the wapm.toml config, and then adapt the ergonomics :)

syrusakbary avatar Sep 26 '19 16:09 syrusakbary

This dicussion is why I suggested to split the wasi-building part of the PR out, so that we could discuss that separably from the wapm metadata.

I would rather see wapm.toml continue to live downstream and be maintained by package maintainers as it would be in debian/redhat/etc. Pushing this stuff upstream to C/C++ projects seems like the difficult direction. They/We don't want to give any particular distro special status, and we can't possible support all distros upstream. I obviously can't speak to other projects but I imagine you will meet similar pushback there.

One approach (that I believe freebsd ports takes) is to put the metadata for all packages into a single repo that package maintainers then commit to. Have you considered that approach. Then the entire wapm universe could be described in a single place and you can do thinks like "make world" to build every single package.

For example here is the freebsd vim package data: https://github.com/freebsd/freebsd-ports/tree/master/editors/vim

Interstingly waby doesn't seem to be in freebsd ports .. somebody should add it :)

sbc100 avatar Sep 26 '19 16:09 sbc100

I don’t have an opinion about whether the packaging metadata should be upstream or elsewhere, but if we do decide to have it in-tree, it seems reasonable to me to put it and the binary artifacts together in some separate subdirectory.

tlively avatar Sep 26 '19 17:09 tlively

I agree with @sbc100, I think it'll be nicer to have the materials required for packaging kept in a separate repo. This gives us a lot of flexibility too -- so the package can change without having to land a change in the upstream repo, and so changes to wabt don't break the packaging scripts.

binji avatar Sep 26 '19 18:09 binji

Would you mind splitting this PR so we can land the wasi builder script on its own?

sbc100 avatar Oct 11 '19 19:10 sbc100

Would you mind splitting this PR so we can land the wasi builder script on its own?

Yes! Will do that soon :)

syrusakbary avatar Oct 11 '19 19:10 syrusakbary

And... done! βœ…

I removed all the references to WAPM in this PR, focusing only on compiling to WASI.

But... how? This PR now uses wasienv to compile easily to WebAssembly WASI (article coming up soon!)

@sbc100 @binji the PR should be ready to re-review :)

syrusakbary avatar Oct 22 '19 17:10 syrusakbary

Here's the article analyzing wasienv! https://medium.com/wasmer/wasienv-wasi-development-workflow-for-humans-1811d9a50345

syrusakbary avatar Oct 22 '19 21:10 syrusakbary

Since WABT is a very general set of tools, it does not make sense to me to make it depend upon wasienv. It be better to add this script to wasienv (make it depend on WABT instead).

aardappel avatar Oct 24 '19 19:10 aardappel

Since WABT is a very general set of tools, it does not make sense to me to make it depend upon wasienv.

I'm not sure if I understand correctly. I think wasienv falls in the same category as Emscripten? (Emscripten is already used in this project)

Do wabt wants WASI compilation target?

I think the main question that we have to answer is whether wabt wants to have first-class support for WASI in the main repo, and if so, what would be the path forward for that.

In the case that WASI compilation is desired in wabt, there will always be some kind of dependency that will need to be fulfilled (either downloading the SDK for the specific system/platform and having the CMake configuration apart, using wasienv which will make the process much easier, ...).

Summary

I thought using wasienv could be very useful for this project to enable WASI compilation in a very simple and easy way... but feel free to close this PR if you think that's not the case!

I'm here to help, but only if the help is desired and useful for the project (it's fine if it's not, no hard feelings!) πŸ™‚

syrusakbary avatar Oct 24 '19 20:10 syrusakbary

Sorry for the delay responding here. I'm afraid I haven't had a change to take a deep look at wasienv. I think such tool could be a useful part of the ecosystem. However, I'm wary of taking a dependency on it at this point in time.

I suggest the following approach, to mirror the current support for emscripten:

  1. Rename this script to scripts/travis-wasi.sh and have it just to the building .. no downloading of anything (see travis-emcc.sh).
  2. Add test-wasi to the build matrix that downloads the wasi-sdk (just the linux version) (as in a previous of the PR) and similar to the existing test-emcc (but without the need for docker) that runs scripts/travis-wasi.sh with $WASI_SDK_ROOT set.

Developers who want to build locally can then call scripts/travis-wasi.sh with $WASI_SDK_ROOT rather than having scripts/travis-wasi.sh do any downloading.

I'm sorry for so much back and forth on this PR. I do think useful things have already come out of it and I hope we can still land it still. I do hope to see wabt as part of wapm and hopefully this change will mean that we are doing CI to ensure that the wasi build doesn't regress.

I plan on keeping an eye wasienv as it evolves and maybe it will make sense to transition to using it at later date.

sbc100 avatar Oct 29 '19 17:10 sbc100

Closing as stale. (wasienv also appears to have gone dormant, or maybe just stable.)

keithw avatar Aug 16 '22 20:08 keithw