Skip to content
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.

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

[Wasi] switch to preview 2 #104683

Merged
merged 15 commits into from
Jul 12, 2024
Merged

[Wasi] switch to preview 2 #104683

merged 15 commits into from
Jul 12, 2024

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jul 10, 2024

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
Copy link
Contributor

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

# 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

cc @dicej, feedback welcome

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.

eng/native/gen-buildsys.cmd Outdated Show resolved Hide resolved
eng/native/gen-buildsys.sh Outdated Show resolved Hide resolved
src/mono/mono.proj Show resolved Hide resolved
src/mono/wasi/build/WasiApp.InTree.targets Outdated Show resolved Hide resolved
src/mono/wasi/build/WasiApp.targets Show resolved Hide resolved
- _BuildNativeEnvironmentVariables for build-native.sh and build-native.cmd
@pavelsavara
Copy link
Member Author

pavelsavara commented Jul 12, 2024

There is #104786 and #103520 on CI

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

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 ?

@pavelsavara
Copy link
Member Author

/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
151 of 159 checks passed
@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

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).

@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.
Labels
arch-wasm WebAssembly architecture area-Build-mono os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants