Allow ReadyToRun-only references in BuildValidator - #85877
Draft
mwiemer-microsoft wants to merge 2 commits into
Draft
mwiemer-microsoft wants to merge 2 commits into
mwiemer-microsoft wants to merge 2 commits into
Conversation
Keep IL candidates preferred for each MVID, then cache ReadyToRun fallbacks. Cover selection under the existing xUnit v2 framework without the xUnit v3 migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
mwiemer-microsoft
marked this pull request as draft
October 1, 2026 19:47
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused implementation matches the resolver contract and has comprehensive regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Enables BuildValidator to resolve ReadyToRun-only references while retaining IL preference for matching MVIDs.
Changes:
- Uses R2R images only when no matching IL candidate exists.
- Adds six focused regression cases.
- Integrates and documents the new test project.
| File | Description |
|---|---|
LocalReferenceResolver.cs |
Adds IL-first, R2R-fallback caching. |
BuildValidator.csproj |
Exposes internals to tests. |
BuildValidator.sln |
Includes the test project. |
LocalReferenceResolverTests.cs |
Tests candidate resolution and ordering. |
BuildValidator.UnitTests.csproj |
Defines the test project. |
Roslyn.slnx |
Adds tests to the main solution. |
.github/memory/testing/compiler.md |
Documents behavior and test command. |
Member
Author
|
I will self-review before sending to others |
Allow an IL image found under a different filename to replace a previously cached ReadyToRun fallback for the same MVID. Cover both lookup orders. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI-generated:
Summary
Isolates the BuildValidator reference-resolution change discussed in #85776 from the xUnit v3 migration. This PR is based on
mainand retains xUnit 2.9.2; no test-framework packages or migration commits are included.mainskips every ReadyToRun (R2R) candidate. Consequently, BuildValidator cannot resolve a reference when its only available candidate is an R2R image, even when its MVID matches. Cache IL candidates first, then consider R2R candidates only for MVIDs still missing. If an IL image is discovered later under a different filename, replace the cached R2R fallback for its MVID. This preserves IL preference across filename lookup order while allowing R2R-only references.An IL assembly and its R2R image can share an MVID but differ in PE header values recorded in the PDB. This change retains the existing IL preference; it does not introduce timestamp/image-size matching or otherwise redesign the MVID cache.
Regression coverage
Adds a focused
BuildValidator.UnitTestsproject to the main and BuildValidator solutions. Cases cover IL-only, R2R-only, both candidate orders, different-MVID IL/R2R candidates, an unknown MVID, and both cross-filename lookup orders. The fixture emits a tiny assembly and marks a copy with synthetic R2R header fields, preserving its MVID; it does not require crossgen2 or execute native code.Before the behavior change, the R2R-only case failed; before the cross-filename cache follow-up, its IL-preference ordering case failed. After both fixes, all eight focused cases pass under xUnit v2.
Validation
dotnet test src\Tools\BuildValidator.UnitTests\BuildValidator.UnitTests.csproj --no-restore -p:RunAnalyzersDuringBuild=true -v:q— 8 passed.dotnet build src\Tools\BuildValidator\BuildValidator.csproj --no-restore -p:RunAnalyzersDuringBuild=true -v:q— 0 warnings/errors.dotnet format whitespace --verify-no-changesandgit diff --check— passed.Full crossgen2 publish/rebuild pipeline validation was not run. #85776 and its review-thread resolution states were not modified.
Microsoft Reviewers: Open in CodeFlow