Only download runtimes for referenced frameworks#53518
Open
baronfel wants to merge 1 commit intodotnet:release/10.0.2xxfrom
Open
Only download runtimes for referenced frameworks#53518baronfel wants to merge 1 commit intodotnet:release/10.0.2xxfrom
baronfel wants to merge 1 commit intodotnet:release/10.0.2xxfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a regression related to unwanted runtime pack downloads (notably Microsoft.AspNetCore.App.Runtime.*) by scoping runtime-pack resolution/download to framework references that are actually present in the task’s FrameworkReferences input, and adds unit coverage to lock the behavior in place.
Changes:
- Scope runtime pack resolution/download to frameworks present in
@(FrameworkReference)(instead of iterating allKnownFrameworkReferenceentries). - Add a unit test reproducing a single-file console app scenario to ensure only referenced frameworks’ runtime packs are queued for download.
- Update unit-test
MockTaskItemto support nullable metadata and requiredItemSpec.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs | Limits runtime pack processing to referenced frameworks and adds/updates internal documentation. |
| src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/ProcessFrameworkReferencesTests.cs | Adds/updates tests and introduces a new test pinning the runtime-pack download behavior. |
| src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Mocks/MockTaskItem.cs | Modernizes nullability/required-member handling to support updated test patterns. |
You can also share your feedback on Copilot code review. Take the survey.
| var frameworkReferenceMap = FrameworkReferences.ToDictionary(fr => fr.ItemSpec, StringComparer.OrdinalIgnoreCase); | ||
| Log.LogMessage(MessageImportance.Low, $"Found {frameworkReferenceMap.Count} known framework references for target framework {TargetFrameworkIdentifier}"); | ||
| var frameworkReferencesForThisProject = FrameworkReferences.ToDictionary(fr => fr.ItemSpec, StringComparer.OrdinalIgnoreCase); | ||
| Log.LogMessage(MessageImportance.Low, $"Found {frameworkReferencesForThisProject.Count} known framework references for target framework {TargetFrameworkIdentifier}"); |
Comment on lines
+396
to
+399
| // Only resolve and download runtime packs for framework references that are part of this | ||
| // task's input. Iterating all KnownFrameworkReferences for the TFM is required to build | ||
| // targeting packs and RuntimeFramework items, but runtime pack resolution must be scoped | ||
| // to frameworks the project actually references. |
Comment on lines
+173
to
+178
| private readonly MockTaskItem NetCoreAppFrameworkReference = new MockTaskItem("Microsoft.NETCore.App", null); | ||
|
|
||
| /// <summary> | ||
| /// Helper to have a project use the ASP.NET Core Runtime. | ||
| /// </summary> | ||
| private readonly MockTaskItem AspNetCoreFrameworkReference = new MockTaskItem("Microsoft.AspNetCore.App", null); |
baronfel
changed the title
|
Does this also fixes #53508? |
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.
Potential fix for #53386
We should only add runtime packs to the PackagesToDownload if they are required for the FrameworkReferences that the user's project is actually referencing. We have this idea of 'transitive' framework references currently, but that doesn't seem to distinguish between just 'known' framework references and those that are truly transitive.