search

NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

NoWarn on a package reference does not apply transitively to its dependencies #5740

Open bording opened 7 years ago

bording commented 7 years ago

If I have added NoWarn to a package reference to suppress NU1701, it only applies to the specific package and not its dependencies.

For example, in a netcoreapp2.0 project, if I have the following package reference:

<PackageReference Include="ApprovalTests" Version="3.0.13" NoWarn="NU1701" />

The warning is suppressed for the ApprovalTests package itself, but I still see a warning for the ApprovalUtilities package that it depends on: image

I currently have to add an additional package reference to suppress that warning:

<PackageReference Include="ApprovalUtilities" Version="3.0.13" NoWarn="NU1701" />

It doesn't make sense to me that I should have to do this. If I've suppressed the warning on the package I've added to the project, it should apply to all of its dependencies implicitly since they are also referenced implicitly.

While I could work around this by globally suppressing the warning, that isn't a great solution either. That means I'll won't see the warning for any new packages I might add later.

VS Version: 15.3.0 Preview 7.0

anangaur commented 7 years ago

@bording We did discuss this issue and there doesn't seem to be a clear way to fix this. Suppressing per package is meant to suppress only for that package so that the user knows the issue is only for a given package and can know about warnings in future for other package dependencies he/she adds. However, I do see this use case may not be a rare one.

May be we need another property NoWarnTransitive="NU1701" that does this trick for us? But this would mean we would move away from the standard suppression mechanism of MSBuild/C#

mishra14 commented 7 years ago

@bording thank you for the detailed issue. You can work around this by adding a project wide no warn - 

 <PropertyGroup>
    <NoWarn>NU1603</NoWarn>
  </PropertyGroup>

Please let us know if that resolves your problem or if you would still prefer a nowarn for the package closure?

bording commented 7 years ago

@mishra14 As mentioned on the issue already, globally suppressing the warning doesn't really solve the problem, because that will hide it for all packages, including ones I add in the future.

If I add a new package that should show this warning, then I want to see the warning so I know that I need to spend some extra time ensuring the package will work (and then add NoWarn to that package reference) or decide to not use that package after all.

mishra14 commented 7 years ago

@bording thanks for clarifying that. I will leave this open as a design change request.

mishra14 commented 7 years ago

Assigning to @anangaur to get more clarity on the need/requirement for this feature.

paulbatum commented 6 years ago

I just want to capture that this functionality would be useful for us (azure functions team). Our dependency chain pulls in Microsoft.AspNet.WebApi.Client via Microsoft.AspNetCore.Mvc.WebApiCompatShim and we'd like to suppress this particular warning without adding a global suppression or an explicit reference. Right now we don't have a good workaround.

anangaur commented 6 years ago

@paulbatum Lets say we have the following dependencies for a project: PackageA depends on PackageC PackageB depends on Package C

Now lets say we have a capability to suppress warning for PackageA and all its transitive dependencies. Should the suppression apply to PackageC which is also being referenced from PackageB as a transitive dependency and there is no NoWarn on PackageB?

@mishra14, Do we retain the whole dependency graph after the package dependencies resolution to be able to suppress transitively?

paulbatum commented 6 years ago

I do not feel strongly either way, though I have a slight preference for the nowarn to not apply for the b->c case. On Tue, Nov 21, 2017 at 11:13 PM Anand Gaurav notifications@github.com wrote:

@paulbatum https://github.com/paulbatum Lets say we have the following dependencies for a project: PackageA depends on PackageC PackageB depends on Package C

Now lets say we have a capability to suppress warning for PackageA and all its transitive dependencies. Should the suppression apply to PackageC which is also being referenced from PackageB as a transitive dependency and there is no NoWarn on PackageB?

@mishra14 https://github.com/mishra14, Do we retain the whole dependency graph after the package dependencies resolution to be able to suppress transitively?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NuGet/Home/issues/5740#issuecomment-346262905, or mute the thread https://github.com/notifications/unsubscribe-auth/AAApp8p6nIwwME5DYs5a-g7z9V_857kNks5s48mAgaJpZM4Oz5i2 .

bording commented 6 years ago

@anangaur I would want to see separate warnings for PackageA and PackageB. The reason is that while they both depend on PackageC, they could both use different APIs from PackageC. That would mean that using PackageA with the compat shim could be fine, but PackageB could fail.

If I don't get a warning about PackageB because PackageC is also referenced by PackageA, then I don't get any warning that lets me know I need to verify that PackageB works correctly in all the scenarios I'm using it for.

cosminstirbu commented 6 years ago

Hello,

Any news on this issue? I'm interested in suppressing this at the package level as well.

Thank you, Cosmin

CaCTuCaTu4ECKuu commented 5 years ago

As far as there is issue when migration from NetCore 2.1 to 2.2 you need to define Version in Microsoft.AspNetCore.App package reference (due to ef packages problems) there is no way to supress NETSDK1071 warning delicately. Adding NoWarn="NETSDK1071" attribute doesn't work

rrelyea commented 5 years ago

@cristinamanum @anangaur - nowarn needs to be considered for central version management.

AdamRiddick commented 4 years ago

A valid workaround is to directly reference the transitive dependeny and specify nowarn against that package.

Not ideal, but it prevents disabling the warning globally.

For example, with the Microsoft,TeamFoundationServer.ExtendedClient package;

<ItemGroup>
    <PackageReference Include="Microsoft.AspNet.WebApi.Core" Version="5.2.3" NoWarn="NU1701" />
     <PackageReference Include="Microsoft.TeamFoundationServer.ExtendedClient" Version="16.153.0"/>
</ItemGroup>

Then for anything referencing the project that has Microsoft,TeamFoundationServer.ExtendedClient as a transitive;

<ItemGroup>
    <PackageReference Include="Microsoft.AspNet.WebApi.Core" Version="5.2.3" NoWarn="NU1701" />
</ItemGroup>

<ItemGroup>
    <ProjectReference Include="..\Installs.DataExtraction\Installs.DataExtraction.csproj" />
</ItemGroup>
bording commented 4 years ago

@AdamRiddick I don't really consider that a workaround when that is exactly the problem I was reporting in the first place.

fowl2 commented 4 years ago

A few things: =)

Is there someway of marking these package references as "transitive"? ie,. only existing to resolve a transitive dependency conflict? Otherwise it's going to get confusing seeing apparently unused references. Especially since these will be called out automatically soon.

Is is possible to include the "requested" version number(/range) in the suppression? Otherwise in a A->B->C situation when B upgrades its' version of C, A might be forgotten.

Could the error message suggest adding the direct reference instead of simply "widen its version constraints"? Possibly would have to split this warning for the transitive/direct reference case.

megakid commented 4 years ago

Still interested in this as we distribute .NET Standard 2.0 packages that depend on legacy .NET 4.6.1 packages (e.g. Apache.NMS.ActiveMq) that are 100% API compatible with .NET Standard 2.0 (as reported by the .NET Portability Analyzer tool.

b-kurczynski commented 4 years ago

Can I join the discussion? I have faced similar scenario that @bording described in this issue so I truly feel it. However I also do understand that having "NoWarn" applied transitively is not a good idea as it is still "All or nothing" solution solving one problem and generating another.

I It would be a very nice feature to have a full control over NuGet warnings in transitive way so what about nesting? Here is a proposed syntax to suppress NuGet warnings for implicitly referenced packages:

<PackageReference Include="ApprovalTests" Version="3.0.13">
    <PackageReference Include="ApprovalUtilities" NoWarn="NU1701" />
</PackageReference>
jasase commented 1 year ago

Is there any progress on this topic? We also need this currently we are having to much warnings due to transitiv warnings.

jasase commented 2 months ago

Any progress on this topic? Would be great to get some feedback.

Suchiman commented 1 month ago

Indeed, i've just switched to CPM and have the scenario where i have Project A depending on Project B where Project B depends on some .NET FX nuget with a NoWarn="NU1701" on it, which worked fine until i've enabled <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled> which then made the warning pop up on Project A where it did not before.