Let <PackageVersion> handle package-specific warning suppression

[crhaglun]

NuGet Product(s) Involved

MSBuild.exe

The Elevator Pitch

We can suppress warnings for specific packages when using <PackageReference>. We can even place suppressions in a central location to ensure that we have common documentation / tracking of why we need to suppress certain warnings.

When using central package versioning with transitive pinning, this unfortunately breaks down. We need to go either suppress warnings globally (not great for a large and active repo) or explicitly include the reference which defeats the purpose of using transitive pinning in the first place.

Can we update <PackageVersion> to support <NoWarn> and have centralized tracking / suppression of warnings for specific packages?

Additional Context and Details

The repo I work on uses Microsoft.Build.CentralPackageVersions but we're looking to migrate to the built-in central package versioning feature instead.

We currently have some issues with transitive dependencies, requiring us to explicitly include packages in order to control the version -- exactly the problem which <CentralPackageTransitivePinningEnabled> solves which means we have an opportunity to de-clutter the repo by migrating.

Unfortunately, we also depend on older NuGet packages that do not yet support .Net Core. That means we have a couple of package declarations where we need to suppress NU1701. We can do all this in a central location, i.e. in .csproj files we'd have something like

<PackageReference Include="LegacyPackage" />

And in Packages.props

<PackageReference Update="LegacyPackage" Version="1.0.0">
  <!-- 
    Suppress NU1701 for this package. It does not yet support .Net Core, but does not use any .Net Framework
    specific APIs at runtime and should be okay to use.
    Upstream work tracked by https://github.com/...
  -->
  <NoWarn>NU1701</NoWarn>
</PackageReference>

When we move to central package versioning with transitive pinning we can do the equivalent by placing the item update directive in Directory.Build.targets, and the .csproj which explicitly declares the reference suppress the warning as expected - but every downstream project now trigger NU1701.

Adding the reference to downstream projects of course pick up the suppression as expected, but then we're again explicitly adding upstream references and increasing clutter.

If we could add the <NoWarn> on the relevant <PackageVersion> item, we'd be able to track everything package-related in one central file just like we do with Microsoft.Build.CentralPackageVersion

[crhaglun]

Just to follow up: Adding <NoWarn> to <PackageVersion> would be one way to solve this, but perhaps there are other more general solutions to the problem?

[jeffkl]

Can we update to support and have centralized tracking / suppression of warnings for specific packages?

We did spend time designing Central Package Management and could not find a good user experience around defining other metadata on transitive dependencies other than the version. Since a <PackageVersion /> item applies to all projects, it's not a good candidate for IncludeAssets, ExcludeAssets, PrivateAssets, or NoWarn.

Without Central Package Management and transitive pinning, the only good option to override transitive metadata is with an explicit <PackageReference /> item which essentially elevates that PackageReference to a direct dependency, allowing your specified metadata to take precedence.

You can specify <NoWarn /> at the project level which is also decent, it just applies to all packages referenced by a particular project.

[crhaglun]

Right - none of those options are ideal in our case.

The repo has ~120 projects, so any change that requires us to go touch a majority of the .csproj files is hard to motivate. Promoting transitive references to direct references has historically led to excessive dependencies as the project evolves and packages are added or removed from the dependency tree.

At the same time, suppressing warnings even on a per-project basis has also historically shown to cause problems over time, which means we try to be explicit and document why warnings are suppressed and track resolution.

Right now it looks like NuGet central package versioning solves more problems than it introduces compared to using Microsoft.Build.CentralPackageVersions, the only thing it's really missing is support for setting other package metadata in Directory.Packages.props

[zivkan]

@crhaglun as a mitigation, you can abuse the fact that MSBuild is a scripting language. Add something like this to one of your Directory.* files

<Project>
  <!-- other stuff you already have in the file -->
  <Target name="SetNoWarnOnCommonPackageReferences" BeforeTargets="CollectPackageReferences">
     <ItemGroup>
      <PackageReference Update="LegacyPackage" NoWarn="NU1701" />
    </ItemGroup>
  </Target>
</Project>

edit: as rightfully pointed out below, this does not work with transitive pinning.

[crhaglun]

@zivkan thanks for the suggestion, but doesn't this still require direct references?

Keeping with the example so far, imagine I have two projects:

BaseProject.csproj:

<PropertyGroup>
  <TargetFrameworks>net472;net7.0</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
  <PackageReference Include="LegacyPackage" />
</ItemGroup>

DerivedProject.csproj:

<PropertyGroup>
  <TargetFrameworks>net472;net7.0</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
    <ProjectReference Include="..\BaseProject\BaseProject.csproj" />
</ItemGroup>

The package "LegacyPackage" depends on "DependentPackage" version 1.0.0. This version has a security issue causing the build to fail (this is a first-party Microsoft repo with Component Governance), we need to override the version of "DependentPackage" to 1.0.1 so we have a Directory.Packages.props that looks like

<ItemGroup>
  <PackageVersion Include="LegacyPackage" Version="1.0.0" />
  <PackageVersion Include="DependentPackage" Version="1.0.1" />
</ItemGroup>

With the following settings in Directory.Build.props:

<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled>

Only BaseProject.csproj will trigger NU1701 when building against net7.0, and the build will eventually fail because it's picking up the "bad" version of DependentPackage.

With the settings

<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>

The build will eventually succeed, but now both BaseProject.csproj and DerivedProject.csproj will raise NU1701 when targeting net7.0

Adding the target you proposed suppress NU1701 only for BaseProject.csproj, it persists for DerivedProject.csproj

Is there some other extension point to the Restore target where we could modify <PackageReference> items once we have evaluated all packages to include?

[zivkan]

oops, yes, it won't work with transitive pinning. Sorry, I didn't read the issue carefully enough. There's no alternative.