Override ResolveReadyToRunCompilers for TestReadyToRun=true test builds #123734
Conversation
github-actions
bot
added
the
needs-area-label
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #123499 where runtime-coreclr crossgen2 TestReadyToRun_Libraries pipelines fail with error NETSDK1095 about ReadyToRun compilation not being supported for the selected target platform or architecture.
Changes:
- Creates a new
tests.readytorun.targetsfile that overrides the SDK'sResolveReadyToRunCompilerstarget to use the in-tree crossgen2 compiler instead of relying on SDK-provided crossgen2 packs - Integrates this new targets file into the test build process by importing it via
AfterMicrosoftNETSdkTargetswhenTestReadyToRunis enabled
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eng/testing/tests.singlefile.targets | Adds import of tests.readytorun.targets after SDK targets when TestReadyToRun is enabled |
| eng/testing/tests.readytorun.targets | New file that overrides ResolveReadyToRunCompilers target to point to in-tree crossgen2 compiler, with special handling for Android targets |
| @@ -0,0 +1,17 @@ | |||
| <Project> | |||
| <!-- Override the path to the in-tree crossgen2. --> | |||
| <Target Name="ResolveReadyToRunCompilers"> | |||
There was a problem hiding this comment.
The ResolveReadyToRunCompilers target is missing a Condition attribute. Similar overrides in the codebase (see src/mono/msbuild/android/build/AndroidBuild.InTree.targets:28 and src/mono/msbuild/apple/build/AppleBuild.ReadyToRun.targets:5) include Condition="'$(PublishReadyToRun)' == true" for defensive programming and consistency. While this file is only imported when TestReadyToRun is true (which sets PublishReadyToRun to true), adding the condition would align with established patterns and provide an extra safeguard.
| <Target Name="ResolveReadyToRunCompilers"> | |
| <Target Name="ResolveReadyToRunCompilers" Condition="'$(PublishReadyToRun)' == true"> |
| @@ -0,0 +1,17 @@ | |||
| <Project> | |||
| <!-- Override the path to the in-tree crossgen2. --> | |||
| <Target Name="ResolveReadyToRunCompilers"> | |||
There was a problem hiding this comment.
The ResolveReadyToRunCompilers target is missing a DependsOnTargets attribute. Similar overrides in the codebase (see src/mono/msbuild/android/build/AndroidBuild.InTree.targets:28 and src/mono/msbuild/apple/build/AppleBuild.ReadyToRun.targets:5) include DependsOnTargets="ResolveRuntimeFilesFromLocalBuild". While Crossgen2InBuildDir is statically defined and doesn't strictly require this dependency, adding it would ensure proper initialization order and align with established patterns in the codebase for consistency.
| <Target Name="ResolveReadyToRunCompilers"> | |
| <Target Name="ResolveReadyToRunCompilers" DependsOnTargets="ResolveRuntimeFilesFromLocalBuild"> |
|
/azp run runtime-coreclr crossgen2 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 participant
Fixes #123499