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] bump WASI SDK to v25.0 #110654

Merged
merged 6 commits into from
Dec 16, 2024
Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Dec 12, 2024

  • bump WASI SDK to v25.0
  • use also minor version
  • use WASI-SDK-VERSION-25.0 as version detection sanity file inside runtime repo
  • read file $(WASI_SDK_PATH)/VERSION in workload outside of runtime repo, which is part of the release .tar
  • set D_WASI_EMULATED_PTHREAD for wasi-sdk, which is an empty implementation
  • remove wasm-opt detection

Fixes #104773
Related dotnet/dotnet-buildtools-prereqs-docker#1299

- use `WASI-SDK-VERSION-25.0` as version detection sanity file inside runtime repo
- read file $(WASI_SDK_PATH)/VERSION in workload outside of runtime repo
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-Build-mono os-wasi Related to WASI variant of arch-wasm labels Dec 12, 2024
@pavelsavara pavelsavara added this to the 10.0.0 milestone Dec 12, 2024
@pavelsavara pavelsavara self-assigned this Dec 12, 2024
@pavelsavara
Copy link
Member Author

cc @SingleAccretion @yowl

@@ -1,6 +1,6 @@
<Project>
<PropertyGroup>
<WASI_SDK_PATH Condition="'$(WASI_SDK_PATH)' == '' or !Exists('$(WASI_SDK_PATH)/VERSION24')">$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'wasi-sdk'))</WASI_SDK_PATH>
<WASI_SDK_PATH Condition="'$(WASI_SDK_PATH)' == '' or !Exists('$(WASI_SDK_PATH)/VERSION')">$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'wasi-sdk'))</WASI_SDK_PATH>
Copy link
Member

Choose a reason for hiding this comment

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

Should that be WASI-SDK-VERSION-25.0 instead of VERSION?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is the VERSION file that wasi-sdk ships with. I don't know why we also write WASI-SDK-VERSION-25.0 as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, VERSION is in the sdk .tar file.

We touch WASI-SDK-VERSION-25.0 only do that inside of the runtime repo.
We do it because it's easy way how to sanity-check version of the WASI-SDK in various places of MSBuild/CI pipeline.
Because it's one-liner Condition, instead of <ReadLinesFromFile ... in a Target

Does that make sense ?

Obviously it could be refactored, into Target in some common place. I don't know where to ...

Copy link
Member

@maraf maraf Dec 13, 2024

Choose a reason for hiding this comment

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

We should do something better for user machines.
There isn't a shared MSBuild file for runtime & user machine build I know of.

Copy link
Member Author

Choose a reason for hiding this comment

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

The users machine check is the actual ReadLinesFromFile, no need to create dummy file there.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you mean better than this, let's improve that in next PR.

Copy link
Member

@radekdoulik radekdoulik left a comment

Choose a reason for hiding this comment

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

Beside the comment it LGTM

@@ -1,6 +1,6 @@
<Project>
<PropertyGroup>
<WASI_SDK_PATH Condition="'$(WASI_SDK_PATH)' == '' or !Exists('$(WASI_SDK_PATH)/VERSION24')">$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'wasi-sdk'))</WASI_SDK_PATH>
<WASI_SDK_PATH Condition="'$(WASI_SDK_PATH)' == '' or !Exists('$(WASI_SDK_PATH)/VERSION')">$([MSBuild]::NormalizeDirectory($(MSBuildThisFileDirectory), '..', 'wasi-sdk'))</WASI_SDK_PATH>
Copy link
Member

@maraf maraf Dec 13, 2024

Choose a reason for hiding this comment

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

We should do something better for user machines.
There isn't a shared MSBuild file for runtime & user machine build I know of.

Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

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

LGTM

@pavelsavara pavelsavara merged commit c969265 into dotnet:main Dec 16, 2024
112 of 116 checks passed
@pavelsavara pavelsavara deleted the wasi_sdk_v25 branch December 16, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

[wasi] upgrade to WASI SDK with LLVM 19
4 participants