Use WASM CoreLib when compiling crossgen2 WASM unit tests#131024
Use WASM CoreLib when compiling crossgen2 WASM unit tests#131024jtschuster wants to merge 6 commits into
Conversation
- Use CopyAllJitLibrariesToAotCompilerOutput=true instead of copying ourselves. - Set crossgenDir based on crossgen2_inbuild.csproj output items - Reorder properties and items to keep related items close to each other
…ter-wasm-r2r-crossgen-test
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 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.
Pull request overview
This PR updates the ReadyToRun crossgen2 test runner and CI wiring so WASM-targeted R2R compilations can reference the browser-wasm runtime pack (including WASM System.Private.CoreLib) when it’s available, instead of always using the host runtime pack.
Changes:
- Add browser-wasm runtime-pack path plumbing (and an opt-in “require wasm references” switch) to the ILCompiler.ReadyToRun.Tests test infrastructure.
- Add a per-compilation target selector (
TargetRid) so tests can request browser-wasm (or host-arm) crossgen2 args and reference sets. - Update pipeline to build + publish browser-wasm managed runtime-pack artifacts and download them into the CLR tools test job.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj | Makes IncludeStaticLibrariesInPack overridable so CI can disable static libs for browser/wasi runtime-pack builds. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/TestPaths.cs | Simplifies path config and adds optional wasm runtime-pack locations + RequireWasmReferences switch. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestRunner.cs | Chooses host vs browser-wasm reference paths per compilation and passes reference paths into the resolver. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestCaseCompiler.cs | Changes Roslyn compilation reference setup to consume an explicit reference list. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs | Updates SimpleAssemblyResolver to resolve framework assemblies from a provided reference set rather than TestPaths. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RDriver.cs | Removes the dedicated “target arch arm” option in favor of TargetRid-driven args. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs | Updates wasm and arm test cases to use TargetRid and tightens arm test gating. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/ILCompiler.ReadyToRun.Tests.csproj | Adds wasm runtime-pack discovery/config propagation and CI-oriented validation/warnings. |
| eng/pipelines/runtime.yml | Builds browser-wasm libs for CLR tools tests, publishes them as artifacts, and downloads them into the tools test job. |
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/TestPaths.cs:8
- System.Text.RegularExpressions is no longer used in this file after removing the config-probing helpers; leaving it will either produce an unnecessary compiler warning (CS8019) or fail builds that treat warnings as errors.
using System;
using System.IO;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
using Xunit.Abstractions;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RTestCaseCompiler.cs:77
R2RTestCaseCompilernow compiles test assemblies withOptimizationLevel.Debug, which changes the IL shape (and potentially crossgen2 output/validations) and seems unrelated to the PR’s wasm-reference selection. Unless there’s a specific reason, keepingReleasehere avoids altering baseline test behavior.
new CSharpCompilationOptions(outputKind)
.WithOptimizationLevel(OptimizationLevel.Debug)
.WithAllowUnsafe(true)
.WithNullableContextOptions(NullableContextOptions.Enable));
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/ILCompiler.ReadyToRun.Tests.csproj:67
- MSBuild
Exists(...)checks should avoid hardcoding\path separators. On non-Windows builds this can cause false negatives (and in this case, the error is raised very early). Using/(orNormalizeDirectory) keeps the check portable.
<Error Condition="!Exists('$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2')"
Text="Crossgen2 inbuild publish directory '$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2' does not exist. This is a build authoring error and should be generated in the test project build." />
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 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.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/ILCompiler.ReadyToRun.Tests.csproj:67
- The ValidatePathsExist target builds a path using backslashes (e.g. '$(CoreCLRArtifactsPath)$(BuildArchitecture)\crossgen2'). On non-Windows agents this can become a literal backslash in the path and cause a false failure even when the directory exists. Use forward slashes or NormalizeDirectory/NormalizePath to keep the Exists check cross-platform.
<Error Condition="!Exists('$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2')"
Text="Crossgen2 inbuild publish directory '$(CoreCLRArtifactsPath)\$(BuildArchitecture)\crossgen2' does not exist. This is a build authoring error and should be generated in the test project build." />
| _output.WriteLine($"Using browser-wasm runtime pack references from '{wasmRuntimePackDir}'"); | ||
| paths = | ||
| [ | ||
| ..Directory.GetFiles(wasmRuntimePackDir, "*.dll"), | ||
| wasmCoreLibPath | ||
| ]; | ||
| return paths; |
| paths = [ | ||
| ..Directory.GetFiles(_paths.RuntimePackDir, "*.dll"), | ||
| coreLibPath | ||
| ]; |
ILCompiler.ReadyToRun.Tests uses the host corelib to compile for wasm. If crossgen ever takes a dependency on a wasm-only member being present in corelib, it could cause failures. Since browser-wasm corelib can be built cross-platform, enforce a wasm runtime pack as a prerequisite for running the tests in CI. For local development, a host-targeting corelib can be used as a fallback with a warning.
Adds a pipeline job to build the wasm runtime pack for running in CI.
We do also have tests that compile for the host OS on arm32 (if we support arm on that OS -- linux only now), so we also would run into this issue if there are any arm32-specific members that crossgen2 relies on for compilation. I think this is less likely to cause problems, but it's called out in comments in case tests start failing for that reason.