Exclude Microsoft.Extensions.FileProviders.Embedded from pruning
#63898
Conversation
github-actions
bot
added
the
area-infrastructure
|
Validated the fix manually. Marking ready for review. |
There was a problem hiding this comment.
Pull Request Overview
Adds exclusion logic so Microsoft.Extensions.FileProviders.Embedded is not pruned from PackageOverrides because its build targets are required at build time. Updates the targeting pack test to account for excluded packages and updates the ref project to remove the package from pruning.
- Introduce exclusion list in test and adjust assertions
- Remove Microsoft.Extensions.FileProviders.Embedded from _AspNetCoreAppPackageOverrides in project file
- Add test assertion ensuring excluded packages are not emitted
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Framework/test/TargetingPackTests.cs | Adds exclusion handling and validation for packages that must not appear in PackageOverrides. |
| src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.sfxproj | Removes Microsoft.Extensions.FileProviders.Embedded from pruning via an ItemGroup adjustment with explanatory comment. |
|
This looks like it will update the targeting pack which will remove this for pruning for 10.0, but for prior target frameworks you will need to update the hard-coded lists in the SDK. Here's the 9.0 version for example: https://github.com/dotnet/sdk/blob/50f42240f56f2a15c1102100911a0d7f4ca4b6ff/src/Tasks/Microsoft.NET.Build.Tasks/FrameworkPackages/FrameworkPackages.net9.0.cs#L153C16-L153C59 |
What is the behavior WRT conflict resolution. Does the SDK still remove the file provided by the package? It should but it would be good to double check this. The behavior might differ after GA as well, but I'd expect any package that's older than ref-pack being built to be dropped as a reference/runtime and any newer package to be included. We have a servicing build (test build of 10.0.101) that you could patch and experiment with. Note that this does mean that users might still get false-positive update/audit/cg alerts should there ever be a CVE in this package, or "out of support" messages if their reference is old enough. That's the tradeoff you're taking here in 10.0 in order to get a low risk fix in place.
Let's not close that issue -- or if you do make sure to open another to track adding the support to the SDK and removing this hack. |
| required to generate the embedded files manifest. The package's build targets are not in the SDK, | ||
| so they must be available from the package itself. See https://github.com/dotnet/aspnetcore/issues/63719 | ||
| --> | ||
| <_AspNetCoreAppPackageOverrides Remove="Microsoft.Extensions.FileProviders.Embedded|$(ReferencePackSharedFxVersion)" /> |
There was a problem hiding this comment.
Wouldn't this exclude it from both conflict resolution & pruning?
Should we have a more targeted fix, one that targets pruning only?
There was a problem hiding this comment.
My understanding (and I could easily be wrong) was that PlatformManifest.txt would still allow the conflict resolution logic to work: https://github.com/dotnet/sdk/blob/50f42240f56f2a15c1102100911a0d7f4ca4b6ff/src/Tasks/Common/ConflictResolution/ResolvePackageFileConflicts.cs#L131
I patched a nightly build of the .NET 10 SDK with the changes in this PR and added a package reference to The build output doesn't contain Please let me know if you think there's anything else I need to check. |
Workaround for #63719