Extra package dependencies appearing for current framework.

[ericstj]

Description

We try to avoid dependencies on packages when that same assembly is inbox in a framework.

Take System.Text.Json for example:
https://www.nuget.org/packages/System.Text.Json/5.0.2

https://www.nuget.org/packages/System.Text.Json/4.7.0

https://www.nuget.org/packages/Microsoft.Extensions.Primitives/5.0.1

In PKGProj this is accomplished with https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Build.Tasks.Packaging/src/CreateTrimDependencyGroups.cs

Right now in 6.0 this is broken due to the packageIndex.json missing inboxOn entries for 6.0.

We should also decide how we handle this once we move to csproj. It's really only interesting any more for the current .NET version as that's the only place where we have package/framework duality with exact match with an inbox library. Coincidentally, this will happen automatically as a side-effect of our$(NetCoreAppCurrent) configurations in the projects, even when excluded from the package, since we cannot filter the project.assets.json that nuget's Pack task sees. So long as we ensure not to include ProjectReferences for inbox assemblies, we'll see an empty dependency group. Take Microsoft.Extensions.Primitives as an example (built with changes from #52084): https://gist.github.com/ericstj/624ab958327a1eb5ca2cc76c10c4a836#file-microsoft-extensions-primitives-nuspec-L29

So we should fix the index to include 6.0 versions so that anything that's still built using pkgproj has the expected behavior as in past releases.

We should also be more intentional about preserving this behavior in libraries which move to CSProj. I believe we've discussed having validation for this @Anipik, here's a good use case.

cc @eerhardt

[ghost]

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

We try to avoid dependencies on packages when that same assembly is inbox in a framework.

Take System.Text.Json for example:
https://www.nuget.org/packages/System.Text.Json/5.0.2

https://www.nuget.org/packages/System.Text.Json/4.7.0

https://www.nuget.org/packages/Microsoft.Extensions.Primitives/5.0.1

In PKGProj this is accomplished with https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Build.Tasks.Packaging/src/CreateTrimDependencyGroups.cs

Right now in 6.0 this is broken due to the packageIndex.json missing inboxOn entries for 6.0.

We should also decide how we handle this once we move to csproj. It's really only interesting any more for the current .NET version as that's the only place where we have package/framework duality with exact match with an inbox library. Coincidentally, this will happen automatically as a side-effect of our$(NetCoreAppCurrent) configurations in the projects, even when excluded from the package, since we cannot filter the project.assets.json that nuget's Pack task sees. So long as we ensure not to include ProjectReferences for inbox assemblies, we'll see an empty dependency group. Take Microsoft.Extensions.Primitives as an example (built with changes from #52084): https://gist.github.com/ericstj/624ab958327a1eb5ca2cc76c10c4a836#file-microsoft-extensions-primitives-nuspec-L29

So we should fix the index to include 6.0 versions so that anything that's still built using pkgproj has the expected behavior as in past releases.

We should also be more intentional about preserving this behavior in libraries which move to CSProj. I believe we've discussed having validation for this @Anipik, here's a good use case.

cc @eerhardt

Author:	ericstj
Assignees:	-
Labels:	area-Infrastructure-libraries, untriaged
Milestone:	-

[ericstj]

Related: NuGet/Home#7344

[ericstj]

Note: here we benefit from the fact that project.assets.json isn't trimmed to exclude the dependency groups for frameworks which are omitted from the package. It's not clear to me if that's always desired. @Anipik are you aware of any other case where we'd want to omit both the asset and the dependency group?

[ViktorHofer]

So long as we ensure not to include ProjectReferences for inbox assemblies, we'll see an empty dependency group.

I would hope that NuGet/Home#7344 gets traction as I would really want to switch to P2Ps for inbox references at some point.

IMHO aside from security implications of dependencies I'm not too concerned about these added dependencies as they are trimmed out by package conflict resolution later anyway. Of course it would be great if we could avoid their download entirely but I see this more as an inconvenience and less as a blocker for the pkgproj migration.

[ericstj]

really want to switch to P2Ps for inbox references at some point.

We'll need to solve hiding those P2Ps from NuGet anyway, can't have a package reference to System.Runtime, so I imagine we can choose to hide them on specific frameworks as well to get the right behavior without that NuGet feature. I'm with you on really wanting that NuGet feature though. I think it's a better and simpler overall story and make it so our customers don't need to have this complexity and still get similar benefits.

less as a blocker for the pkgproj migration.

Actually the projects that are migrated to CSProj coincidentally get the right behavior, so I wasn't thinking of it as a blocker. More that we'd want to be more intentional about that, and think about if it would ever be problematic.

[ViktorHofer]

So we should fix the index to include 6.0 versions so that anything that's still built using pkgproj has the expected behavior as in past releases.

@ericstj given that we are past 6.0 and and pkgprojs and the packageindex don't exist anymore, should we close the issue?

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[ericstj]

We should also be more intentional about preserving this behavior in libraries which move to CSProj.

Do we need anything to make sure that we don't unnecessarily rely on a PackageReference when an assembly is inbox?

If I look at our 7.0 packages, I see they have this problem.
https://www.nuget.org/packages/System.Net.Http.Json/7.0.0-preview.4.22229.4
https://www.nuget.org/packages/Microsoft.Extensions.Logging.Console/7.0.0-preview.4.22229.4

[ViktorHofer]

Do we need anything to make sure that we don't unnecessarily rely on a PackageReference when an assembly is inbox?

That behavior is intentional: #67196 (comment). We explicitly want to reference packages that also ship as part of the framework so that in case of a servicing event, the patched library reaches customers, even without updating the unpatched shared framework.

In your example above, we explicitly want to reference the System.Text.Json package.

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[ghost]

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.