Add FrameworkReference to Microsoft.Aspnetcore.App for non-SharedFx projects with SharedFx-only references#65478
Add FrameworkReference to Microsoft.Aspnetcore.App for non-SharedFx projects with SharedFx-only references#65478
Conversation
bbcecb9 to
d4b02b5
Compare
There was a problem hiding this comment.
Pull request overview
This PR adjusts the repo’s MSBuild reference-resolution/pack behavior so that non-SharedFx, packable projects that reference SharedFx-only assemblies can surface a Microsoft.AspNetCore.App framework dependency via restore assets (so packing can pick it up).
Changes:
- Capture the
Microsoft.AspNetCore.AppKnownFrameworkReferencebefore it’s removed globally, so it can be reintroduced conditionally. - Conditionally add a
FrameworkReferencetoMicrosoft.AspNetCore.Appfor packable, non-SharedFx projects with SharedFx-only project references, and add a target to remove the framework reference beforeResolveFrameworkReferences.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eng/targets/ResolveReferences.targets | Adds conditional Microsoft.AspNetCore.App framework reference (and re-adds the known framework reference) plus a target that removes the framework reference before ResolveFrameworkReferences. |
| Directory.Build.targets | Captures the KnownFrameworkReference for Microsoft.AspNetCore.App into an internal item before it gets removed. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| Condition="'@(TransitiveFrameworkReference->WithMetadataValue('Identity', 'Microsoft.AspNetCore.App')->Count())' != '0' AND | ||
| '$(UseAspNetCoreSharedRuntime)' != 'true'" > |
There was a problem hiding this comment.
| Condition="'@(TransitiveFrameworkReference->WithMetadataValue('Identity', 'Microsoft.AspNetCore.App')->Count())' != '0' AND | |
| '$(UseAspNetCoreSharedRuntime)' != 'true'" > | |
| Condition="'$(UseAspNetCoreSharedRuntime)' != 'true'" > |
nit: this condition is redundant, I believe the remove is a noop if the item does not exist.
| BeforeTargets="AddTransitiveFrameworkReferences" | ||
| AfterTargets="ResolvePackageAssets" |
There was a problem hiding this comment.
Before and after here will not give this wiggle room, what it actually does it attempt to run it both before AddTransitiveFrameworkReferences and after ResolvePackageAssets. Whichever is first will win and the other will skip since it sees it already ran. IOW you can remove the BeforeTargets="AddTransitiveFrameworkReferences" as it does nothing right now.
| '@(_FrameworkProjectReference)' != '' AND | ||
| '@(FrameworkReference->WithMetadataValue('Identity', 'Microsoft.AspNetCore.App')->Count())' == '0'"> | ||
| <KnownFrameworkReference Include="@(_RemovedAspNetKnownFrameworkReference)" /> | ||
| <!-- Mark as IsTransitiveFrameworkReference to exclude the pruning data --> |
There was a problem hiding this comment.
nit:
| <!-- Mark as IsTransitiveFrameworkReference to exclude the pruning data --> | |
| <!-- Mark as IsTransitiveFrameworkReference to exclude the pruning data https://github.com/dotnet/sdk/issues/53106 --> |
| '@(_FrameworkProjectReference)' != '' AND | ||
| '@(FrameworkReference->WithMetadataValue('Identity', 'Microsoft.AspNetCore.App')->Count())' != '0'" > |
There was a problem hiding this comment.
Small suggestion - instead of duplicating the condition, just leave a marker on the one you add, then remove that here.
So above do :
<FrameworkReference Include="Microsoft.AspNetCore.App" IsTransitiveFrameworkReference="true" DoNotResolve="true" />
Then here you can just do
<FrameworkReference Remove="@(FrameworkReference->WithMetadataValue('DoNotResolve', 'true')'" />
And remove the entire condition on the target.
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <_RemovedAspNetKnownFrameworkReference Include="@(KnownFrameworkReference->WithMetadataValue('Identity', 'Microsoft.AspNetCore.App'))" /> |
There was a problem hiding this comment.
Rather than removing this and adding it back did you consolidating the logic?
Fixes #65410
Projects that are are not in the shared framework, ship as packages, and have references to libraries that only ship in the shared framework, should wind up with a FrameworkReference to Microsoft.AspNetCore.App in their .nuspec. We used to have a custom post-build task that injected that FrameworkReference, but that went away in #64863.
This is a bit of a hack - we add a FrameworkReference to the affected projects, so that Restore can put that FrameworkRef into the generated assets file. Then we remove that FrameworkReference prior to
ResolveFrameworkReferencesso that the ProjectReferences get used instead. We also have to add aIsTransitiveFrameworkReference=truemetadata to the FrameworkReference so that pruning warnings don't kick in: https://github.com/dotnet/sdk/blob/e90c4413024970eca97d05122243737aa207af11/src/Tasks/Microsoft.NET.Build.Tasks/GetPackagesToPrune.cs#L103-L104. It would be great if there was metadata we could set on the FrameworkReference (something like "NuspecOnly"), so that none of the references from the FrameworkReference would actually be raised, and pruning warnings wouldn't fire, but the FrameworkRef would still wind up in project.assets.json & the .nuspec.