Provision wasm build/test tools into a shared, version-keyed cache - #131351
Provision wasm build/test tools into a shared, version-keyed cache#131351lewing wants to merge 10 commits into
Conversation
The externally-acquired WebAssembly tools (Emscripten SDK, WASI SDK, wasmtime, Chrome, chromedriver, Firefox, geckodriver, V8) were each provisioned by a bespoke target into one of three locations, with three different version schemes and three different staleness strategies: * emsdk -> src/mono/browser/emsdk (gated on !Exists(dir)) * wasi-sdk -> artifacts/wasi-sdk (version hardcoded in the target) * wasmtime -> artifacts/obj/wasmtime (gated on !Exists(dir)) * browsers -> artifacts/bin/<tool> This had 'mono' in the emsdk path even though CoreCLR-wasm consumes it, re-downloaded ~2 GB per worktree, was wiped by cleaning artifacts, and -- because emsdk and wasmtime were gated only on directory existence -- kept silently reusing a stale tool after a version bump. Consolidate all eight onto a single cache laid out as <root>/<tool>/<version>-<host rid>/ the tool <root>/<tool>/<version>-<host rid>.complete sentinel written last Because the version and host RID are part of the path, a version bump is a cache miss by construction: there is no staleness comparison to get wrong, switching branches reuses both versions, and x64/arm64 payloads coexist. The default cache root is the repository's main checkout, <main checkout>/.dotnet/wasm-tools, derived by reading the worktree .git file (no 'git' process): all worktrees of one clone share a single copy, yet the cache is deleted with the repo. DOTNET_WASM_TOOL_CACHE_DIR overrides it (e.g. a user-global cache or a CI agent cache), and EMSDK_PATH / WASI_SDK_PATH / WASMTIME_PATH still take precedence per tool. A generic eng/wasm/AcquireWasmTool.proj (invoked once per tool) replaces the duplicated download/extract/stamp logic: it extracts to a unique temp directory and only then moves into the final name, so an interrupted or concurrent acquisition can never leave a half-populated entry. wasi-sdk and wasmtime now download native arm64 builds instead of x86_64. The native build scripts (gen-buildsys.sh/.cmd) and the two wasm Makefiles resolve the same cache through eng/wasm/wasm-tool-cache.sh/.cmd, and the out-of-tree WasmApp.LocalBuild.props mirrors the resolution. Add a 'clean.wasmtools' subset to prune unreferenced entries. Move the wasi-sdk version into eng/wasm/wasi-sdk-version.txt and remove the now-unused eng/download-wasi-sdk.ps1. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
…perty function PruneWasmToolCache.proj used System.IO.Directory.GetDirectories / GetFileSystemEntries, which MSBuild rejects as property functions (MSB4185), so `./build.sh -s clean.wasmtools` always failed. Enumerate the cache instead via an item glob over the '.complete' sentinel files, and derive each entry directory by stripping the sentinel's '.complete' extension. Verified: a stale versioned entry is pruned while the current one is kept, the no-op case reports nothing to prune, and /p:PruneAllWasmTools=true removes the whole cache -- including cache paths containing spaces and non-ASCII characters. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
There was a problem hiding this comment.
Pull request overview
This PR refactors WebAssembly tool acquisition (emscripten, wasi-sdk, wasmtime, browsers/engines) to use a shared, version-keyed cache and updates build/test infrastructure to resolve tools from that cache instead of per-worktree / artifacts/ locations.
Changes:
- Introduces
eng/wasm/WasmToolCache.propsand shared cache helpers/projects to key tool installs by<tool>/<version>-<hostrid>with.completesentinels. - Updates MSBuild targets, native build scripts, and Makefiles to resolve tools from the shared cache and stage copies for Helix payloads where needed.
- Moves wasi-sdk version to
eng/wasm/wasi-sdk-version.txt, adds a cache-prune subset (clean.wasmtools), and updates documentation/pipeline path triggers accordingly.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/FunctionalTests/WebAssembly/Directory.Build.props | Switch functional tests to use cached emsdk path when provisioned. |
| src/tests/Common/helixpublishwitharcade.proj | Stage wasmtime for Helix using the new stamp/sentinel scheme. |
| src/mono/wasm/Makefile | Resolve EMSDK_PATH via the shared wasm tool cache helper. |
| src/mono/wasm/build/WasmApp.LocalBuild.props | Mirror cache resolution logic for out-of-tree consumers. |
| src/mono/wasi/build/WasiApp.targets | Allow expected wasi-sdk version to be overridden by in-tree targets. |
| src/mono/wasi/build/WasiApp.InTree.targets | Feed in-tree wasi-sdk version into the shared version check. |
| src/mono/sample/wasi/wasi.mk | Resolve wasmtime provisioning directory via the shared cache helper. |
| src/mono/browser/README.md | Update docs to describe shared wasm tool cache and version pins. |
| src/mono/browser/build/WasmApp.InTree.props | Resolve EMSDK_PATH from shared cache when provisioned. |
| src/mono/browser/browser.proj | Remove now-unneeded version-line clearing tied to old provisioning flow. |
| src/mono/.gitignore | Stop ignoring wasi-sdk under src/mono (no longer provisioned there). |
| src/libraries/sendtohelix-browser.targets | Stage browser/engine tools from cache into Helix-safe staging paths. |
| eng/wasm/WasmToolCache.props | Define shared cache root, host RID keying, and core tool cache paths/stamps. |
| eng/wasm/wasm-tool-cache.sh | Provide POSIX helper to resolve provisioned tool directories from cache. |
| eng/wasm/wasm-tool-cache.cmd | Provide Windows helper to resolve provisioned tool directories from cache. |
| eng/wasm/wasi-sdk-version.txt | New single source of truth for pinned wasi-sdk version. |
| eng/wasm/PruneWasmToolCache.proj | Add pruning project for removing unreferenced cache entries. |
| eng/wasm/AcquireWasmTool.proj | Add generic download/extract/publish project for cached tools. |
| eng/testing/wasm-provisioning.targets | Provision browsers/V8 into cache via generic acquisition project. |
| eng/testing/wasi-provisioning.targets | Provision wasmtime into cache via generic acquisition project. |
| eng/testing/tests.browser.targets | Update CI emsdk path resolution to shared cache when provisioned. |
| eng/Subsets.props | Add clean.wasmtools subset wiring to the prune project. |
| eng/pipelines/common/templates/browser-wasm-coreclr-build-tests.yml | Ensure pipeline provisions emsdk into the shared cache (comment update). |
| eng/pipelines/common/evaluate-default-paths.yml | Update “wasmbuildtests” path triggers to include new wasm cache infrastructure. |
| eng/native/gen-buildsys.sh | Resolve EMSDK/WASI_SDK paths via the shared cache helper on wasm builds. |
| eng/native/gen-buildsys.cmd | Resolve EMSDK via shared cache helper for wasm builds. |
| eng/download-wasi-sdk.ps1 | Remove old Windows wasi-sdk download script (no longer used). |
| eng/AcquireWasiSdk.targets | Acquire wasi-sdk into the shared cache keyed by version/host RID. |
| eng/AcquireEmscriptenSdk.targets | Provision emscripten SDK into shared cache with version-keyed sentinel. |
| docs/workflow/debugging/mono/wasm-debugging.md | Update deterministic patching guidance to match cached provisioning. |
| docs/workflow/building/libraries/webassembly-instructions.md | Document shared wasm tool provisioning and prune commands. |
| docs/workflow/building/coreclr/wasm.md | Note emsdk is installed into a shared cache under the main checkout. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
eng/wasm/WasmToolCache.props:14
- Comment is inconsistent with the actual default cache location. The default cache is under the repo's main checkout (e.g. /.dotnet/wasm-tools), not "outside the repository"; it is only outside
artifacts/. This can mislead readers about lifetime/cleanup expectations.
The cache deliberately lives outside the repository so that it survives `clean`, is shared
by every clone and git worktree, and isn't tied to any one runtime flavor's source directory.
eng/wasm/AcquireWasmTool.proj:58
- AcquireWasmTool downloads into a shared $(WasmToolDownloadDir) and later deletes the downloaded archive. If multiple builds acquire the same tool/version concurrently, one invocation can delete the archive while another is still extracting it, causing intermittent acquisition failures. Downloading into the per-invocation temp directory avoids cross-process races.
<DownloadFile SourceUrl="$(WasmToolUrl)"
DestinationFolder="$(WasmToolDownloadDir)"
Retries="3"
SkipUnchangedFiles="true">
<Output TaskParameter="DownloadedFile" PropertyName="_Archive" />
eng/native/gen-buildsys.cmd:20
- This update resolves EMSDK_PATH via the shared wasm tool cache, but later in this same script the WASI branch still falls back to %__repoRoot%\artifacts\wasi-sdk (the old location). That makes Windows WASI builds inconsistent with the new cache layout and can fail after
artifactscleanup even if the shared cache is populated. Please update the WASI branch to resolve wasi-sdk through eng/wasm/wasm-tool-cache.cmd as well.
:: Set up the EMSDK environment before setlocal so that it propagates to the caller.
:: Written without a parenthesized block so that %WASM_TOOL_CACHE_RESULT% expands without
:: delayed expansion, which cannot be enabled here without discarding emsdk_env's variables.
if /i not "%__Os%" == "browser" goto :AfterEmsdkEnv
if not "%EMSDK_PATH%" == "" goto :CallEmsdkEnv
…ths, download race
Fixes from the automated PR review:
* gen-buildsys.cmd still resolved the WASI SDK from the removed
artifacts/wasi-sdk directory; resolve it via eng/wasm/wasm-tool-cache.cmd
against eng/wasm/wasi-sdk-version.txt, matching the .sh script and the
EMSDK branch. (Windows WASI native builds would otherwise fail.)
* WasmApp.LocalBuild.props set EMSDK_PATH/WASI_SDK_PATH to a cache path
without checking it was provisioned, so an unprovisioned cache pointed
the variables at a non-existent directory and defeated on-demand
detection. Gate both on the entry's '.complete' sentinel, mirroring the
in-tree props.
* AcquireWasmTool.proj downloaded each archive into the shared
$(WasmToolDownloadDir) under a fixed name, so concurrent acquisitions of
the same tool could delete the archive out from under one another.
Download into a per-invocation GUID subdirectory and remove it with the
rest of the temp state (dropping the now-redundant explicit archive
delete).
* Correct the WasmToolCache.props header (the cache is anchored under the
main checkout's gitignored .dotnet/, not "outside the repository") and
the wasm-tool-cache.sh comment (it mirrors the props only when a repo
root is supplied, which every caller does).
The prior clean.wasmtools property-function fix already removed the
unsupported ->TrimEnd item transform the review also flagged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
…, sample run guard
Second round of automated PR review feedback:
* wasm-provisioning.targets: the Chrome/chromedriver/Firefox/geckodriver/V8
download URLs are x86_64-only on Linux, but the cache entry is keyed by
$(WasmToolHostRid). Error out when browser/engine provisioning is
requested on Linux for a non-x64 architecture instead of silently caching
an x64 payload under a linux-arm64 key. (macOS selects arm64/x64 URLs;
Windows is x64.)
* helixpublishwitharcade.proj: StageWasmtimeForHelixPayload used a fixed
Outputs stamp name, so switching to an already-provisioned older wasmtime
(whose .complete predates the last staging) could skip re-staging and ship
a stale copy. Name the Outputs stamp after the version-keyed source
sentinel.
* wasi.mk: run-console prepended "${WASMTIME_PROV_DIR}:" to PATH
unconditionally; when wasmtime isn't provisioned that injected the current
directory. Fail fast with a clear message when WASMTIME_PROV_DIR is empty.
* wasm-tool-cache.sh: drop the dead $HOME fallback in wasm_tool_cache_root
(every caller passes a repo root) so it can't diverge from
WasmToolCache.props, and fix the stale comment.
* src/mono/browser/README.md: note that `make provision-wasm` installs a
separate hackable emsdk under src/mono/browser/emsdk and is independent of
the shared cache.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
…V8 download dir
Third round of automated PR review feedback:
* AcquireWasmTool.proj / AcquireEmscriptenSdk.targets: the publish step
deleted $(_FinalDir) before moving the staged tool in, so a concurrent
build that had already renamed its copy into place (but not yet written
the sentinel) could have its live directory removed. The staging ->
final move is a rename within the cache root, i.e. atomic, so a final
directory that already exists is a complete tool. Never delete it: move
ours in only when the slot is free, and always (re-)Touch the sentinel,
which self-heals a complete-but-unstamped directory left by a build
killed between the move and the Touch. Verified the self-heal path leaves
the directory intact and restores the stamp.
* wasm-provisioning.targets: the V8 launcher was written into
$(WasmToolDownloadDir) before anything created that directory, which
fails on a fresh checkout. Create it first.
The reviewer's concern about the browser Helix staging ItemGroups (that they
evaluate empty before the wasm-provisioning.targets import, and that a
version-keyed Include breaks StageDependenciesForHelix) does not apply:
project-scope items are evaluated after all properties/imports (verified
empirically), and StageDependenciesForHelix overwrites Name with the Include
file name, so the version-keyed directory stays consistent across MakeDir,
the staged copy, and the correlation payload.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
The recovery path (final directory present but '.complete' missing, e.g. a build killed between the move and the Touch) still ran DownloadFile and the extract, re-fetching a large payload only to end up touching the sentinel. Compute _NeedsFetch from the existence of the final directory up front and gate the download/extract/launcher-copy on it; the post-download re-check that guards against a concurrent publisher is retained so the move still only happens when the slot is free. Verified on osx-arm64 for both wasmtime and V8: a fresh cache downloads, a stamp-less-but-complete entry reports "already present" without downloading and restores the sentinel, a fully cached entry is a no-op, and `./build.sh mono+libs -os wasi` succeeds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
…ging names
Two failures from the first CI run, both introduced by this PR:
* wasi-wasm windows _BuildOnly: $(_ExtractDir) ends with a directory
separator, so on Windows the generated `tar ... -C "...\"` had its
closing quote escaped by the trailing backslash and tar reported
`could not chdir to '...tmp-<guid>"'`. Trim the separator before
quoting, matching what the firefox-installer branch already did.
* browser-wasm test legs: StageDependenciesForHelix sets the staging
directory name from %(FileName), which strips everything after the last
'.'. The version-keyed payload directories added earlier in this PR
contain dots, so files were copied to e.g. 'chrome-150.0.7871' while
MakeDir created (and the correlation payload later referenced) the full
'chrome-150.0.7871.129-1639810', leaving it empty. Helix then threw
`InvalidOperationException: Sequence contains no elements` from
DirectoryPayload.IsUpToDate. Use %(Filename)%(Extension) so the staged
directory always matches %(Identity); %(Extension) is empty for names
without a dot, so existing callers are unaffected.
Verified on osx-arm64: wasi-sdk re-provisions and extracts correctly with the
trimmed -C argument (VERSION and share/cmake/wasi-sdk-p2.cmake land at the
entry root), the staging name transform preserves dotted names while leaving
'emsdk' and 'WasmTestOnChrome-CLR-ST-wasmtime' unchanged, and
`./build.sh mono+libs -os wasi` succeeds.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
Fourth round of automated PR review feedback:
* AcquireWasmTool.proj / AcquireEmscriptenSdk.targets: both 'mv' and 'move'
move the source *into* an existing destination directory rather than
failing, so a concurrent build that published between the _MoveIntoPlace
check and the rename would get the staging directory nested inside a
complete cache entry. Move the existence test into the command itself so
the check and the rename are a single shell statement, and have the losing
build discard its copy. Verified that plain 'mv' does nest in that case
and the guarded form does not.
* AcquireWasmTool.proj: chmod only ran when this invocation won the publish
race, but any invocation may Touch the sentinel, so a loser could stamp a
cache entry before the winner made the payload executable. Run chmod
unconditionally (it is idempotent) before the verify/stamp. Verified that
an entry whose exec bit was cleared is repaired without re-downloading.
* wasm-tool-cache.cmd: only a drive-letter path was treated as rooted, so a
UNC-hosted worktree would anchor differently than MSBuild's IsPathRooted.
Accept a '\\' prefix as well.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
…nores Overriding wasmtime with a local install is disabled in Directory.Build.props (#101528), and neither the cache helpers nor the provisioning targets consult WASMTIME_PATH. Two places added by this PR still pointed users at it: * webassembly-instructions.md claimed WASMTIME_PATH takes precedence over the cache. Limit that statement to EMSDK_PATH/WASI_SDK_PATH and note that WASMTIME_PATH is not honored, with a link to the tracking issue. * the wasi sample's run-console guard suggested setting WASMTIME_PATH. Point at the supported knobs instead: build a wasi target to provision wasmtime, or set DOTNET_WASM_TOOL_CACHE_DIR to a cache that has it. Verified the guard still fires with the new message when the cache entry is absent and passes once it is provisioned. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
… mapping
* The generated Windows V8 launcher never closed the quote it opened, so cmd
treated "d8.exe --snapshot_blob= as part of the executable path:
"%~dp0\d8.exe --snapshot_blob="%~dp0\snapshot_blob.bin" %*
Quote the executable and each path separately. %~dp0 already ends with a
separator, so the extra backslash is dropped as well. This quoting bug
predates this PR, but the V8 launcher is now published as part of the cache
entry by this change, so fix it here rather than caching a broken script.
* wasm_tool_host_rid only normalized arm64/aarch64 and x86_64/amd64, so an
i686 host produced 'linux-i686' while MSBuild's $(BuildArchitecture) says
'x86', breaking the guarantee that the shell helper and MSBuild resolve the
same cache entry. Normalize i[3456]86/x86 to x86 and armv*/arm to arm, and
teach the cmd helper the x86 case (including the PROCESSOR_ARCHITEW6432
32-bit-process-on-64-bit-Windows caveat).
Verified the normalization table matches $(BuildArchitecture) naming and that
the sh helper and MSBuild agree on this host, and re-provisioned V8 to confirm
the launcher is still written and executable.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a67b5483-297c-4a86-ba28-ed4333676751
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Hmm. I thought we migrated away from the "LocalBuild". I see it's imported in https://github.com/dotnet/runtime/blob/main/src/mono/wasm/data/aot-tests/Directory.Build.props#L26. Did you see it fail because of these?
There was a problem hiding this comment.
Could we potentially do the same with .dotnet folder?
Summary
The externally-acquired WebAssembly tools were each provisioned by a bespoke target into one of three locations, with three different version schemes and three different staleness strategies:
src/mono/browser/emsdk/eng/Versions.props+emscripten-version.txt!Exists(dir))artifacts/wasi-sdk/artifacts/obj/wasmtime/wasmtime-version.txt!Exists(dir))artifacts/bin/<tool>/BrowserVersions.propsThis put
monoin the emsdk path even though CoreCLR-wasm consumes it (src/coreclr/runtime.proj), re-downloaded ~2 GB per worktree, was wiped by cleaningartifacts/, and — because emsdk and wasmtime were gated only on directory existence — kept silently reusing a stale tool after a version bump.Approach
Consolidate all eight tools onto a single cache:
Because the version and host RID are part of the path, a version bump is a cache miss by construction — no staleness comparison to get wrong, switching branches reuses both versions, and x64/arm64 payloads coexist.
Default root: the repository's main checkout (
<main checkout>/.dotnet/wasm-tools), derived by reading the worktree.gitfile (nogitprocess spawn). All worktrees of one clone share a single copy, yet the cache is deleted with the repo.DOTNET_WASM_TOOL_CACHE_DIRoverrides it (e.g. a user-global cache, or a CI agent cache);EMSDK_PATH/WASI_SDK_PATH/WASMTIME_PATHstill take precedence per tool.A generic
eng/wasm/AcquireWasmTool.proj(invoked once per tool) replaces the duplicated download/extract/stamp logic: it extracts to a unique temp directory and only then moves into the final name, so an interrupted or concurrent acquisition can never leave a half-populated entry. wasi-sdk and wasmtime now download native arm64 builds instead of x86_64.The native build scripts (
gen-buildsys.sh/.cmd) and the two wasm Makefiles resolve the same cache througheng/wasm/wasm-tool-cache.sh/.cmd; the out-of-treeWasmApp.LocalBuild.propsmirrors the resolution. Aclean.wasmtoolssubset prunes unreferenced entries. The wasi-sdk version moves intoeng/wasm/wasi-sdk-version.txt, and the now-unusedeng/download-wasi-sdk.ps1is removed.Validation (osx-arm64)
./build.sh mono+libs -os browser./build.sh mono+libs -os wasi./build.sh clr+libs -os browser(CoreCLR-wasm — themono-in-path motivator)Still needs validation on Linux and Windows agents in CI.
Note
This pull request was authored with the assistance of GitHub Copilot.