[DCR]: Issues with new NU1507 warning

[Zastai]

NuGet Product(s) Affected

MSBuild.exe, dotnet.exe

Current Behavior

With the VS 2022 17.2 (.NET SDK 6.0.300) release, a change has been introduced in the handling of CentralPackageVersionManagement: if there are multiple package sources defined, a new warning, NU1507, is issued.

Note: the current nuget.exe (6.1.0) does not issue this warning (but I'm assuming the next version will?).

This warning is documented in https://docs.microsoft.com/en-us/nuget/consume-packages/central-package-management#warning-when-using-multiple-package-sources

Where the problems start is:

• it is not uncommon for release builds to have warnings-are-errors set, so this change breaks such builds (and based on the
  following points, apparently for no real reason)
• there is no documentation regarding the rationale for the warning: is it really just a recommendation/best practice, or will
  something break if each package does not resolve to exactly one source?
  • this is important, because without that, it is hard to gauge whether simply adding NU1507 to NoWarn is safe, or likely to
    break things down the line
• there is no command-line way for adding package source mappings yet, making it harder to configure a tree in CI context
• it looks like adding package source mapping, using "*" as pattern for every source, shuts up the warning
  • but isn't that essentially the same as not having explicit source mapping? then why is there no warning for this as well?

Desired Behavior

That depends on the reason for NU1507; if it's only a recommendation or best practice, then at a minimum it should be documented as such, potentially explicitly mentioning that it's safe to disable the warning, or that it can be avoided by mapping all sources to the "*" pattern. (But my absolute preference would be to drop it, or only issue it as part of some new "nuget analyze-config" command.)

If there's actual potential for breakage when multiple potential sources apply to a package, then that should be clearly documented (and perhaps briefly explained in the warning as well). And then a situation where the nuget.config specifies overlapping mapping patterns should likely trigger NU1507 too. Ideally, there would also be some sort of description of how to handle some relatively common scenarios, like having one "real" nuget feed (say, nuget.org), plus one local folder source (to make it easy to do testing with locally-built test packages). In such a case, there may not always be a usable prefix pattern, so using "*" for both would be reasonable.

Additional Context

No response

[mrienhar]

I am hitting this problem, however even turning off warnings as errors and suppressing NU1507 it still throws the error and blocks compilation. Has anyone found a solution to this?

[Zastai]

I am hitting this problem, however even turning off warnings as errors and suppressing NU1507 it still throws the error and blocks compilation. Has anyone found a solution to this?

For me, using NoWarn in Directory.Build.props did work. But in the end I just added a source mapping with * as package pattern to resolve it in one place (my NuGet.config lives above my project checkouts).

[mrienhar]

I am hitting this problem, however even turning off warnings as errors and suppressing NU1507 it still throws the error and blocks compilation. Has anyone found a solution to this?

For me, using NoWarn in Directory.Build.props did work. But in the end I just added a source mapping with * as package pattern to resolve it in one place (my NuGet.config lives above my project checkouts).

I will try that, but adding NoWarn in every project has not worked for me, it still throws... The problem isn't during the build but during the nuget restore, it seems.

[Zastai]

And just to make it clear: I am most definitely not saying there is no problem here. I'd still call it broken by design given that there are 2 package sources present in a clean SDK install, meaning NU1507 always pops up in a clean CI build:

\path\to\foo.csproj : warning NU1507: There are 2 package sources defined in your configuration. When using central package management, please map your package sources with package source mapping (https://aka.ms/nuget-package-source-mapping) or specify a single package source. The following sources are defined: https://api.nuget.org/v3/index.json, C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\

[jeffkl]

You should be able to add this to your Directory.Build.props:

<PropertyGroup>
  <NoWarn>$(NoWarn);NU1507</NoWarn>
</PropertyGroup>

We are working with the .NET SDK folks to see how we can get the fallback folder (C:\Program Files (x86)\Microsoft SDKs\NuGetPackages) configured in such a way that it works with package source mapping.

[Zastai]

An important element on the nuget side is surfacing a command-line way of adding package mappings. Right now, our CI environment creates a blank nuget config with <clear/> fir the sources, and then uses the command line to add sources (different for main vs dev branches vs tags). This can add more than one source, hitting NU1507. Having a command-line way to configure the mapping (whether as extra options when adding the source, or a separate command) would help us there.

[mrienhar]

You should be able to add this to your Directory.Build.props:

<PropertyGroup>
  <NoWarn>$(NoWarn);NU1507</NoWarn>
</PropertyGroup>

We are working with the .NET SDK folks to see how we can get the fallback folder (C:\Program Files (x86)\Microsoft SDKs\NuGetPackages) configured in such a way that it works with package source mapping.

So just a followup, on my local machine using the nowarn worked, but then when I went to build remotely (build pipeline) that failed. So I am now in a situation where there is no solution I can find to build both locally and remotely. So I have to put the nowarn commented out in the directory.build.props, and uncomment when building locally, then comment out again before checking in. This is clearly not ideal, when do we expect this behavior to be consistent between local builds and pipeline builds?

[Zastai]

It seems odd that the presence of a NoWarn would cause build failures. What was the specific failure?

If the failure is "NU1507 not known" (because the pipeline uses an older sdk) then you should be able to add a Condition testing the MSBuild version as being >= 17.2.

(But if the pipeline uses an older sdk, perhaps you should consider using a global.json to specify the use of that exact same sdk for local development as well.)

[mrienhar]

It seems odd that the presence of a NoWarn would cause build failures. What was the specific failure?

If the failure is "NU1507 not known" (because the pipeline uses an older sdk) then you should be able to add a Condition testing the MSBuild version as being >= 17.2.

(But if the pipeline uses an older sdk, perhaps you should consider using a global.json to specify the use of that exact same sdk for local development as well.)

That is exactly what I thought, then had to say to my team that had to redo their approvals on my PR since I had to update it without the no warn. Just to be clear here is exactly what happened in order:

1. I was building and working just fine, no problems whatsoever.
2. Visual Studio said it wanted to do an update.
3. After the update I started getting the NU1507 warning, which was upgraded to an error.
   Note: this was not in the build, but in the nuget restore at the beginning of the build.
4. I tried blocking the warning, and even changing warnings as errors to false in every single project file and that did nothing.
5. I found this article and changed warnings as errors in the directory.build.props file, build worked locally.... but I didn't want to check that in.
6. I removed that, and added the nowarn for 1507, again that worked locally and I thought that would be safe to check in for now. Local build continued to work properly, so I created a PR.
7. When my pipeline ran as part of the PR checks (the one that was working perfectly before) I get this error:
   This is the task:
   Task : .NET Core
   Description : Build, test, package, or publish a dotnet application, or run a custom dotnet command
   Version : 2.208.0
   Author : Microsoft Corporation
   Help : https://docs.microsoft.com/azure/devops/pipelines/tasks/build/dotnet-core-cli

Error:
##[error]Error: The process 'C:__t\dotnet\dotnet.exe' failed with exit code 1
##[warning].NET 5 has some compatibility issues with older Nuget versions(<=5.7), so if you are using an older Nuget version(and not dotnet cli) to restore, then the dotnet cli commands (e.g. dotnet build) which rely on such restored packages might fail. To mitigate such error, you can either: (1) - Use dotnet cli to restore, (2) - Use Nuget version 5.8 to restore, (3) - Use global.json using an older sdk version(<=3) to build
Info: Azure Pipelines hosted agents have been updated and now contain .Net 5.x SDK/Runtime along with the older .Net Core version which are currently lts. Unless you have locked down a SDK version for your project(s), 5.x SDK might be picked up which might have breaking behavior as compared to previous versions. You can learn more about the breaking changes here: https://docs.microsoft.com/en-us/dotnet/core/tools/ and https://docs.microsoft.com/en-us/dotnet/core/compatibility/ . To learn about more such changes and troubleshoot, refer here: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/build/dotnet-core-cli?view=azure-devops#troubleshooting
##[error]Dotnet command failed with non-zero exit code on the following projects : C:__w\1\s\dirs.proj
Finishing: DotNetCore test (windows_build_container)

9. I tried a bunch of things to fix that issue, assuming it was coincidental (given the description did not appear to be related). But all of that had no impact.
10. When I removed the nowarn this error goes away in the pipeline, but then, of course, I can't build locally. So I left it in the file commented out with a note about why it was there.

[jeffkl]

Something I realized recently is that Directory.Build.props is not imported when you build/restore a Visual Studio solution file (.sln). The restore logic won't see the NoWarn setting and they won't be suppressed. If you're restoring a solution file, you'll need to add a NoWarn to Directory.Solution.props

https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#customize-the-solution-build

[jeffkl]

Thanks for all of this feedback. I believe we've addressed everything in this issue and wanted to see if everyone agrees:

it is not uncommon for release builds to have warnings-are-errors set, so this change breaks such builds (and based on the
following points, apparently for no real reason)

NuGet's central package management has always been considered in "preview" so we felt comfortable adding a new warning and fully acknowledge that a warning treated as an error is a breaking change. We'll be removing the "preview" tag from this feature in the next release and will refrain from adding new warnings going forward.

there is no documentation regarding the rationale for the warning: is it really just a recommendation/best practice, or will
something break if each package does not resolve to exactly one source?
this is important, because without that, it is hard to gauge whether simply adding NU1507 to NoWarn is safe, or likely to
break things down the line

I have added documentation around the warning, NU1507, here. It should clear up why package source mapping is important and points to more documentation on package source mapping. I've also changed the warning logic to ignore non-HTTP sources which should help when the .NET SDK injects local sources.

there is no command-line way for adding package source mappings yet, making it harder to configure a tree in CI context

There is an official tool for onboarding to Package Source Mapping available here: https://github.com/NuGet/PackageSourceMapper. There are also pointers to this tool in the official documentation.

An important element on the nuget side is surfacing a command-line way of adding package mappings. Right now, our CI environment creates a blank nuget config with fir the sources, and then uses the command line to add sources (different for main vs dev branches vs tags). This can add more than one source, hitting NU1507. Having a command-line way to configure the mapping (whether as extra options when adding the source, or a separate command) would help us there.

This work is tracked here: #10735.

There's a decent amount of discussion on this issue, do you feel I've covered it all and can close this issue for now?

[Zastai]

I'll have to give this a good read to be able to confirm. My main concern is that #10735 essentially went inactive after this ticket was created. If the intent is to actively rekindle that work after this ticket is closed, then great. Otherwise, I worry that nothing will happen after closing this ticket.

[jeffkl]

As for Visual Studio 17.4 the warning has been updated to only look at remote feeds and feeds added by the .NET SDK like /usr/local/share/dotnet/library-packs are ignored. So now the warning should be correct and only appear if you have more than one remote feed configured. We strongly encourage users to adopt Package Source Mapping.