Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

NU1511 warning that isn't actionable is reported for a transitive P2P #14103

Open
ViktorHofer opened this issue Feb 13, 2025 · 10 comments
Open
Assignees
Labels
Functionality:Restore Partner:DotNet Priority:1 High priority issues that must be resolved in the current sprint. Type:Bug

Comments

@ViktorHofer
Copy link

ViktorHofer commented Feb 13, 2025

Image

TestUtils.csproj (net10.0) -> Microsoft.NET.HostModel.csproj (netstandard2.0) -> System.Text.Json.csproj (netstandard2.0)

https://github.com/dotnet/runtime/blob/a5f99c6f990ae7bcbde018232bafa93335afe1eb/src/installer/tests/TestUtils/TestUtils.csproj#L11

This warning isn't actionable as TestUtils.csproj receives the STJ P2P transitively.

Repro: dotnet restore src\installer\tests\TestUtils with a .NET 10 P1 or main SDK.

cc @nkolev92 @ericstj

@ViktorHofer ViktorHofer changed the title NU1511 is reported for a transitive P2P that can't be avoided NU1511 warning that isn't actionable is reported for a transitive P2P Feb 13, 2025
@ericstj
Copy link

ericstj commented Feb 13, 2025

Yeah, that's not a direct reference. Let's see about creating a small isolated repro for this.

@ericstj
Copy link

ericstj commented Feb 13, 2025

repro14103.zip

Here's a repro. This doesn't happen with a transitive PackageReference through projects, but it does with a transitive ProjectReference.

I wonder if we really want this to be a warning or not - it's really only for us framework folks. Somewhat related to dotnet/sdk#2674 @dsplaisted

@ViktorHofer
Copy link
Author

I wonder if we really want this to be a warning or not - it's really only for us framework folks

FWIW this warning is helpful for the shared framework repos as it helps us remove P2Ps that aren't necessary. With that, we can minimize our dependencies.

Issue is missing Type label, remember to add a Type label

@microsoft-github-policy-service microsoft-github-policy-service bot added the missing-required-type The required type label is missing. label Feb 14, 2025
@nkolev92 nkolev92 added Type:Bug Priority:1 High priority issues that must be resolved in the current sprint. Functionality:Restore and removed missing-required-type The required type label is missing. labels Feb 18, 2025
@nkolev92 nkolev92 self-assigned this Feb 18, 2025
@nkolev92
Copy link
Member

I think it makes sense to only raise the warning for direct project refs.

I'll go ahead and change the warning. Let me know if you have any concerns or you have different ideas you'd like to discuss.

@ViktorHofer
Copy link
Author

ViktorHofer commented Feb 21, 2025

With what you are suggesting, will the transitive P2P get pruned from the restore graph? I think it makes sense to raise NU1510 and NU1511 for any assets that don't get pruned. Without that warning, nothing guarantees that all expected inbox assets got pruned out, without looking at the project.assets.json file.

Here, I was wondering why the transitive P2P doesn't get pruned.

@nkolev92
Copy link
Member

nkolev92 commented Feb 21, 2025

Project references do not get pruned at all.
PackageReference are the only ones that get pruned.

The idea is similar to why direct package references do not get pruned. We won't prune things specified in a project file. You should remove them yourself.

@ericstj
Copy link

ericstj commented Feb 21, 2025

I wonder if NuGet should really be treating ProjectReferences this way at all. Especially those that don't set IsPackable. Seems to me a confusion about what is a package and not.

@ViktorHofer
Copy link
Author

I wonder if NuGet should really be treating ProjectReferences this way at all. Especially those that don't set IsPackable. Seems to me a confusion about what is a package and not.

Agreed. If ProjectReferences never get pruned (which IMO makes sense when thinking about the P2P over package story) then I wonder how useful the NU1511 warning is. Does it exist to inform customers who have overlapping identities with inbox assemblies to remove those or change their identities? Example: dotnet/winforms@c246b22#diff-e9549b099b67b0b79d6be89fd7ea951de73cd24b15c5079c3bfe809310f6b6dbR5-R8

@nkolev92
Copy link
Member

Does it exist to inform customers who have overlapping identities with inbox assemblies to remove those or change their identities?

That was the idea yes. It's a fringe benefit for sure.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Functionality:Restore Partner:DotNet Priority:1 High priority issues that must be resolved in the current sprint. Type:Bug
Projects
None yet
Development

No branches or pull requests

3 participants