Skip to content

[Wasi] switch to preview 2 #104683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
pavelsavara merged 15 commits into from
Jul 12, 2024
Merged

[Wasi] switch to preview 2 #104683

pavelsavara merged 15 commits into from
Jul 12, 2024

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jul 10, 2024 •
edited
Loading

After this PR dotnet will produce WASM component (preview 2) instead of WASM module

  • it has different binary header
  • component consumes WASI imports
  • it's not compatible with wasm-opt and old versions of wasm engines

Details

  • switch to WASIp2 sysroot
  • temporary wasi-sdk-p2.cmake file
  • new environment variable DOTNET_WASI_PRINT_EXIT_CODE
    • will print WASM EXIT 42 into stderr when it's not zero
    • because WASI components don't return numeric exit code
    • and because our unit tests usually need to know more than 0/1
  • new dotnet run option --forward-exit-code which sets it
    • the dotnet host will consume the line from stderr and return exit code to caller shell
  • minor wasi cleanup
  • fail fast if there is wasm-opt on PATH

WASI SDK 22 + patches

  • force WASI_SDK_PATH to src\mono\wasi\wasi-sdk if the env WASI_SDK_PATH points to unpatched location
  • new VERSION22PATCHED file to recognize patched installation
  • new _BuildNativeEnvironmentVariables for building native libs to propagate correct WASI_SDK_PATH to shell
    Most of it could go away after update to WASI SDK 23. see [wasi] upgrade to WASI SDK with LLVM 19 #104773

Contributes to #96419
Split from #103752

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-Build-mono os-wasi Related to WASI variant of arch-wasm labels Jul 10, 2024
@pavelsavara pavelsavara added this to the 9.0.0 milestone Jul 10, 2024
@pavelsavara pavelsavara self-assigned this Jul 10, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 10, 2024

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@pavelsavara pavelsavara mentioned this pull request Jul 10, 2024
Merged
pavelsavara
pavelsavara commented Jul 10, 2024
View reviewed changes
This was referenced Jul 11, 2024
Open
Closed
Closed
Closed
Closed
@pavelsavara pavelsavara mentioned this pull request Jul 11, 2024
Closed
@pavelsavara
Merge branch 'main' into wasip2
5db3e44 
# Conflicts:
#	eng/native/gen-buildsys.cmd
#	eng/native/gen-buildsys.sh
#	src/mono/mono.proj
#	src/mono/wasi/build/WasiApp.targets
#	src/mono/wasi/wasi.proj
#	src/native/libs/build-native.sh
@pavelsavara pavelsavara marked this pull request as ready for review July 11, 2024 16:47
@pavelsavara
Copy link
Member Author

pavelsavara commented Jul 11, 2024

cc @dicej, feedback welcome

dicej
dicej reviewed Jul 11, 2024
View reviewed changes
Copy link
Contributor

@dicej dicej left a comment

Choose a reason for hiding this comment

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

LGTM overall. Given that some of the Bash files have changed, is one of the goals of this PR to support Linux and/or MacOS? If so, I can try building and running the smoke tests locally. Otherwise, I can fix any problems with the Linux build that pop up downstream when rebasing and testing dotnet/runtimelab#2614.

SingleAccretion
SingleAccretion reviewed Jul 11, 2024
View reviewed changes
This was referenced Jul 11, 2024
Closed
Closed
pavelsavara
pavelsavara commented Jul 12, 2024
View reviewed changes
pavelsavara
pavelsavara commented Jul 12, 2024
View reviewed changes
@pavelsavara
- stamp file VERSION22PATCHED
bb9ae44 
- _BuildNativeEnvironmentVariables for build-native.sh and build-native.cmd
@pavelsavara
fix touch
9c4ffba
@pavelsavara
Copy link
Member Author

pavelsavara commented Jul 12, 2024 •
edited
Loading

There is #104786 and #103520 on CI

maraf
maraf approved these changes Jul 12, 2024
View reviewed changes
Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

Changes looks good to me 👍

@pavelsavara
Copy link
Member Author

pavelsavara commented Jul 12, 2024

LGTM overall. Given that some of the Bash files have changed, is one of the goals of this PR to support Linux and/or MacOS? If so, I can try building and running the smoke tests locally. Otherwise, I can fix any problems with the Linux build that pop up downstream when rebasing and testing dotnet/runtimelab#2614.

@dicej since I'm out next week, I will try to merge as much as possible today. I will be happy to help to upstream any of your fixes later.

Overall I think that we should consider broader set of changes from the fork. In order to make future code flow downstream easier. @SingleAccretion are there such candidates ?

This was referenced Jul 12, 2024
Open
Closed
@pavelsavara
Copy link
Member Author

pavelsavara commented Jul 12, 2024

/ba-g last still running test is not relevant for this change, other CI issues are known and matched

@pavelsavara pavelsavara merged commit a955d59 into dotnet:main Jul 12, 2024
@pavelsavara pavelsavara deleted the wasip2 branch July 12, 2024 13:42
@dicej
Copy link
Contributor

dicej commented Jul 12, 2024

@dicej since I'm out next week, I will try to merge as much as possible today. I will be happy to help to upstream any of your fixes later.

Sounds good. I'll be out the following week, but we can sync up after that. Enjoy your time off!

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Jul 14, 2024 •
edited
Loading

Overall I think that we should consider broader set of changes from the fork. In order to make future code flow downstream easier. @SingleAccretion are there such candidates ?

I don't think there is a lot that can be upstream (and we try to keep it that way). As you've seen in the merge, the vast majority of actual NAOT-LLVM code is concentrated in the Jit/ILC/NAOT runtime.

One thing that I would love to unify is the EMSDK layout between dotnet/emsdk and upstream EMSDK. But that's orthogonal to this WASI effort (we are, fortunately, aligned on the build system with WASI).

@JanProvaznik JanProvaznik mentioned this pull request Jul 29, 2024
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@maraf maraf maraf approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@akoeplinger akoeplinger Awaiting requested review from akoeplinger akoeplinger is a code owner

@lambdageek lambdageek Awaiting requested review from lambdageek

@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner

@marek-safar marek-safar Awaiting requested review from marek-safar

+2 more reviewers

@dicej dicej dicej left review comments

@SingleAccretion SingleAccretion SingleAccretion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@pavelsavara pavelsavara

Labels

arch-wasm WebAssembly architecture area-Build-mono os-wasi Related to WASI variant of arch-wasm

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@pavelsavara @dicej @SingleAccretion @maraf