NuGet / Home

Repo for NuGet Client issues
Other
1.49k stars 250 forks source link

NuGet generates consolidation notice for packages implicitly referenced by SDK #13676

Open sbomer opened 1 month ago

sbomer commented 1 month ago

NuGet Product Used

Visual Studio Package Management UI

Product Version

Microsoft Visual Studio Enterprise 2022 (64-bit) - Int Preview Version 17.12.0 Preview 1.0 [35119.72.main]

Worked before?

No response

Impact

It bothers me. A fix would be nice

Repro Steps & Context

The SDK adds an implicit PackageReference to Microsoft.NET.ILLink.Tasks, depending on the TargetFramework. For a project that multitargets to net6.0 and net8.0, this results in a consolidation notice in the package manager UI.

Original repro steps from https://github.com/dotnet/runtime/issues/96249:

Description

I believe this package is being brought in by the .NET 8 SDK. No matter what I do any project multi-targeting .NET 8 with trimming shows that this package needs to be consolidated in the Nuget Package Manager UI for VS 2022. This is on projects that multi-target.

Reproduction Steps

  1. Create a class library that multi-targets to net472 and net6.0.
  2. Build and confirm the Nuget packages UI has no issues.
  3. Update the project to also target net8.0. (this works by itself)
  4. Now conditionally add support for trimming for NET 6+
    <PropertyGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">
      <ImplicitUsings>true</ImplicitUsings>
      <Nullable>disable</Nullable>
      <IsTrimmable>true</IsTrimmable>      
    </PropertyGroup>
  5. Now go back to the Nuget package manager UI and there are 2 packages for ILLink added and a consolidation message for the project (targeting 7.0)

Expected behavior

No consolidation issue for the ILLink package.

Actual behavior

Generates a consolidation warning in the package manager UI.

Regression?

No response

Known Workarounds

Remove trimming from project and the issue goes away.

Configuration

.NET 472, 6 and 8 with trimming Class library

Other information

No response

Verbose Logs

No response

sbomer commented 1 month ago

From https://github.com/dotnet/runtime/issues/96249#issuecomment-2269990635:

Thanks for the repro steps @mtaylorfsmb, I was able to see the same consolidation notice, though it wasn't entirely reliable. I only see it for "Manage NuGet Packages For Solution...", not "Manage NuGet Packages..." for a specific project.

As far as I can tell, the package manager UI shows this notice whenever different versions of a package are referenced across a solution - even if the different versions are referenced for different TFMs. For example, if I add the following, I get a consolidation notice for System.Text.Json:

  <ItemGroup>
      <PackageReference Condition="'$(TargetFramework)' == 'net8.0'" Include="System.Text.Json" Version="8.0.0" />
      <PackageReference Condition="'$(TargetFramework)' == 'net6.0'" Include="System.Text.Json" Version="6.0.0" />
  </ItemGroup>

The SDK effectively adds a PackageReference to Microsoft.NET.ILLink.Tasks, where the referenced version depends on the TFM, so it ends up with the same consolidation notice:

image

Interestingly, the UI is aware that the package is implicitly referenced by an SDK, but still produces a consolidation notice. I also can't select a Version in the UI - if I click that dropdown, it says "Blocked by project", which suggests this has been intentionally blocked for SDK-referenced packages.

I don't think there's anything we can do on our side to fix this - this would need to be handled in NuGet or VS, if it's not intended behavior.

nkolev92 commented 3 weeks ago

Reproducible on both 17.12 P1 and 17.11.

I was worried https://github.com/NuGet/NuGet.Client/commit/51e6ffed88a8f774a86227954da68d0ee2268441 might've regressed things but that's not the case.

jebriede commented 2 weeks ago

Team Triage It seems like the consolidate tab should not be considering implicitly defined packages for consolidation.