Skip to content

Enable wasi-common by default#177

Merged
tschneidereit merged 1 commit intomasterfrom
wasi-c
Jun 25, 2019
Merged

Enable wasi-common by default#177
tschneidereit merged 1 commit intomasterfrom
wasi-c

Conversation

@sunfishcode
Copy link
Member

This removes the --wasi-common, as it's now on by default, and adds a
--wasi-c option to enable the wasi-c implementation.

This removes the --wasi-common, as it's now on by default, and adds a
--wasi-c option to enable the wasi-c implementation.
@sunfishcode sunfishcode requested a review from kubkon June 24, 2019 23:50
Copy link
Member

@tschneidereit tschneidereit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I'm going to merge this PR, as there are people blocked on this because of build system issues.

@tschneidereit tschneidereit merged commit c0ba475 into master Jun 25, 2019
@kubkon
Copy link
Member

kubkon commented Jun 25, 2019

This is great! I'm going to merge this PR, as there are people blocked on this because of build system issues.

It gets even better as now you can actually build wasmtime with WASI support on Windows! Mind you, the range of supported WASI syscalls on Windows is minimal, nonetheless it's steadily progressing forward CraneStation/wasi-common#22.

On a related note, I guess it might be a good idea to re-enable AppVeyor builds for this repo. @sunfishcode @tschneidereit what do you guys reckon?

@tschneidereit
Copy link
Member

On a related note, I guess it might be a good idea to re-enable AppVeyor builds for this repo. @sunfishcode @tschneidereit what do you guys reckon?

I have a WIP Azure Pipelines definition here. Pipelines is a much nicer environment and allows us to unify CI for Linux, Mac, and Windows, so I think instead of re-enabling AppVeyor we should probably finish that. Now that the Windows version actually does something, I'll spend some time today really setting things up.

@kubkon
Copy link
Member

kubkon commented Jun 25, 2019

@tschneidereit the link seems to be incorrectly pointing to CraneStation/wasi-common#22 PR instead of WIP Azure Pipelines def. Anyhow, that sounds really cool! I'm looking forward to it then, and I'll be happy to "steal" it as well as your expertise with it to set up something similar for CraneStation/wasi-common 😈

@kubkon kubkon deleted the wasi-c branch June 25, 2019 13:56
@tschneidereit
Copy link
Member

@kubkon oops, apologies! :) This is what I meant ...

The setup actually fully works, except the tests fail on Windows, which is probably to be expected right now.

@kubkon
Copy link
Member

kubkon commented Jun 25, 2019

@tschneidereit Nice! That looks really good! I'll be curious to hear your opinion on Azure Pipelines versus Travis CI which now features Win support :-)

Anyhow, I don't see the tests failing, but rather building wasmtime-runtime trips. As far as I can remember, that is to be expected on Win without LLVM 8 installed, and AFAIK is a direct consequence of compiling the only remaining C code that we still have left in wasmtime, namely the wasmtime-runtime/signalhandlers which are used to handle traps :-)

@tschneidereit
Copy link
Member

Anyhow, I don't see the tests failing, but rather building wasmtime-runtime trips. As far as I can remember, that is to be expected on Win without LLVM 8 installed, and AFAIK is a direct consequence of compiling the only remaining C code that we still have left in wasmtime, namely the wasmtime-runtime/signalhandlers which are used to handle traps :-)

Ah, sorry—this time I almost had the right link ;) I experimented with not installing LLVM to speed up the build a little bit, but that indeed fails because of the signalhandlers stuff. See the next build for the actual error.

@tschneidereit
Copy link
Member

@tschneidereit Nice! That looks really good! I'll be curious to hear your opinion on Azure Pipelines versus Travis CI which now features Win support :-)

I experimented with Travis, but in particular their Windows support has many more issues than Azure Pipelines. The Pipelines team is also very responsive, and actively engaged with the Rust community to resolve any issues that pop up. And finally, their Windows support is quite a bit faster to begin with, and also has significantly higher time outs than Travis—up to 8 hour, IIRC.

@kubkon
Copy link
Member

kubkon commented Jun 25, 2019

Anyhow, I don't see the tests failing, but rather building wasmtime-runtime trips. As far as I can remember, that is to be expected on Win without LLVM 8 installed, and AFAIK is a direct consequence of compiling the only remaining C code that we still have left in wasmtime, namely the wasmtime-runtime/signalhandlers which are used to handle traps :-)

Ah, sorry—this time I almost had the right link ;) I experimented with not installing LLVM to speed up the build a little bit, but that indeed fails because of the signalhandlers stuff. See the next build for the actual error.

No probs! :-) Ah, I see. Anyhow, with regards to the actual error, it seems that it's something to do with "Access violation writing location ..." and it immediately trips at misc_testsuite/stack_overflow.wast. If you remove the test, the next couple pass fine on Win, although it seems nondeterministic at best. I'll need to investigate further, but I'm quite surprised that the spec fails on Win. Would you expect it to? @sunfishcode any thoughts on this?

@sunfishcode
Copy link
Member Author

Yeah, the stack_overflow.wast (and infinitely recursive tests in spec_testsuite/call.wast and others) which test stack overflow don't fully work on Windows yet, because detecting stack overflow requires platform-specific code that we don't have yet.

dhil added a commit to dhil/wasmtime that referenced this pull request May 23, 2024
This patch removes the "interpret continuation tables as function
references tables" hack in favour of a dedicated handling of
continuation tables.

Note this patch does not implement the embedder-side of things.

Resolves bytecodealliance#133.
avanhatt pushed a commit to wellesley-prog-sys/wasmtime that referenced this pull request Oct 23, 2024
Expand attribute syntax to allow rule tagging.

Updates avanhatt#47
avanhatt pushed a commit to wellesley-prog-sys/wasmtime that referenced this pull request Oct 23, 2024
Expand attribute syntax to allow rule tagging.

Updates avanhatt#47
dicej pushed a commit to dicej/wasmtime that referenced this pull request Jun 3, 2025
…oncurrent-expanded-tests

Re-enable concurrent expansion tests
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.

3 participants