Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Duplicate NuGetAuditSuppress warnings use code NU1508 #5921

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Duplicate NuGetAuditSuppress warnings use code NU1508 #5921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkolev92 why does NuGet.exe disable duplicate checking?

NuGet.Client/src/NuGet.Clients/NuGet.CommandLine/MsBuildUtility.cs

Line 265 in 128a506

AddProperty(args, "DisableCheckingDuplicateNuGetItems", bool.TrueString);

Copy link
Member

@nkolev92 nkolev92 Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because TreatWarningsAsErrors would fail the dg spec generation and fail the operation with a message that's not very actionable.
It wasn't worth the effort back then.

Spec has more details: https://github.com/NuGet/Home/blob/dev/accepted/2022/duplicate-nuget-item-error-handling.md#technical-explanation.

Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<CheckForDuplicateNuGetItemsTask
Items="@(NuGetAuditSuppress)"
ItemName="NuGetAuditSuppress"
LogCode="NU1505"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkolev92 from NuGet/Home#13620

This should be a new log code.

Why do we use a different code for each MSBuild item type?

The docs are the same for each item type, since the resolution is the same. I feel like all this design decision does is cause us extra busy work. I feel like one error code, where in the docs we mention that this can happen for multiple item types, would just be easier, with no downside.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The owner of the action isn't always the same.
In particular the SDK sets PackageDownload items, so any issues are likely problems on the SDK side.

For example, if you search with NU1505, you end up with dotnet/sdk#24747.
NU1504 gives you something else.

It's duplicative sure, but I think there's an eventual limit to the number of items we'll be using :)

LogCode="NU1508"
MSBuildProjectFullPath="$(MSBuildProjectFullPath)"
TreatWarningsAsErrors="$(TreatWarningsAsErrors)"
WarningsAsErrors="$(WarningsAsErrors)"
Expand Down
5 changes: 5 additions & 0 deletions src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ public enum NuGetLogCode
/// </summary>
NU1507 = 1507,

/// <summary>
/// Duplicate NuGetAuditSuppress found
/// </summary>
NU1508 = 1508,

/// <summary>
/// Dependency bumped up
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/NuGet.Core/NuGet.Common/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
NuGet.Common.NuGetLogCode.NU1302 = 1302 -> NuGet.Common.NuGetLogCode
NuGet.Common.HashAlgorithmName.SHA1 = 4 -> NuGet.Common.HashAlgorithmName
NuGet.Common.NuGetLogCode.NU3043 = 3043 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1905 = 1905 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1905 = 1905 -> NuGet.Common.NuGetLogCode
NuGet.Common.NuGetLogCode.NU1508 = 1508 -> NuGet.Common.NuGetLogCode
1 change: 1 addition & 0 deletions test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetRestoreTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1699,6 +1699,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
[Theory]
[InlineData("PackageReference", "NU1504")]
[InlineData("PackageDownload", "NU1505")]
[InlineData("NuGetAuditSuppress", "NU1508")]
public async Task DotnetRestore_WithDuplicateItem_WarnsWithLogCode(string itemName, string logCode)
{
using (SimpleTestPathContext pathContext = _dotnetFixture.CreateSimpleTestPathContext())
Expand Down
Loading