Skip to content

[WASI] bump WASI SDK to v25.0 - #110654

Merged
pavelsavara merged 6 commits into
dotnet:mainfrom
pavelsavara:wasi_sdk_v25
Dec 16, 2024
Merged

[WASI] bump WASI SDK to v25.0#110654
pavelsavara merged 6 commits into
dotnet:mainfrom
pavelsavara:wasi_sdk_v25

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Dec 12, 2024

Copy link
Copy Markdown
Member
  • 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
@pavelsavarapavelsavara added arch-wasm WebAssembly architecture area-Build-mono os-wasi Related to WASI variant of arch-wasm labels Dec 12, 2024
@pavelsavarapavelsavara added this to the 10.0.0 milestone Dec 12, 2024
@pavelsavarapavelsavara self-assigned this Dec 12, 2024
@pavelsavara

Copy link
Copy Markdown
MemberAuthor

cc @SingleAccretion@yowl

<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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
MemberAuthor

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

@marafmarafDec 13, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
MemberAuthor

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
Copy Markdown
MemberAuthor

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.

Comment threadsrc/mono/wasm/build/WasmApp.LocalBuild.props

@radekdoulikradekdoulik left a comment

Copy link
Copy Markdown
Member

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

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

@marafmarafDec 13, 2024

Copy link
Copy Markdown
Member

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.

@jsturtevantjsturtevant left a comment

Copy link
Copy Markdown
Contributor

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:mainDec 16, 2024
@pavelsavara
pavelsavara deleted the wasi_sdk_v25 branch December 16, 2024 10:26
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Build-monoos-wasiRelated 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

@pavelsavara@jsturtevant@radekdoulik@maraf