[source mapping] Having a duplicate source breaks msbuild sdks restore

jeromelaban commented 1 month ago

NuGet Product Used

dotnet.exe, Visual Studio Package Management UI

Product Version

8.0.300

Worked before?

unknown

Impact

Other

Repro Steps & Context

Clear all caches: dotnet nuget locals all -c
Create a library: dotnet new classlib -o test01
Change the test01.csproj sdk to be Sdk="MSTest.Sdk/3.3.1"

In %appdata%\NuGet\NuGet.Config. add the following mapping:

<packageSource key="nuget.org">
    <package pattern="System.*" />
</packageSource>

In the test01 folder, create a nuget.config file:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
<packageSources>
    <add key="NuGet official package source" value="https://api.nuget.org/v3/index.json" />
</packageSources>
</configuration>

In classlib01, run dotnet restore

Notice that if the custom nuget.config file added in the project folder is removed or the duplicate nuget.org source is removed, the restore succeeds.

Verbose Logs

D:\temp\test01\test01.csproj : error : Could not resolve SDK "MSTest.Sdk". Exactly one of the probing messages below indicates why we could not resolve the SDK. Investigate and resolve that message to correctly specify the SDK.
D:\temp\test01\test01.csproj : error :   SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" returned null.
D:\temp\test01\test01.csproj : error :   Unable to resolve 'MSTest.Sdk (= 3.3.1)' for '.NETStandard,Version=v0.0'. PackageSourceMapping is enabled, the following source(s) were not considered: Microsoft Visual Studio Offline Packages, NuGet official package source.
D:\temp\test01\test01.csproj : error MSB4236: The SDK 'MSTest.Sdk' specified could not be found.

JonDouglas commented 1 month ago

/cc @nkolev92 & @donnie-msft related to #13469

nkolev92 commented 1 month ago

What would you have expected to happen @jeromelaban compared to what the error message tells you today. (we have ideas ourselves, but don't want to lead you towards those :D )

jeromelaban commented 1 month ago

In itself, the message is does not feel easily actionable. Knowing the cause, I'd intuitively say that it should fail/warn earlier because the source mapping got overridden.

Also, maybe displaying all the known sources would have told that none of the available ones were considered?

nkolev92 commented 1 month ago

Today it says:

Unable to resolve 'MSTest.Sdk (= 3.3.1)' for '.NETStandard,Version=v0.0'. PackageSourceMapping is enabled, the following source(s) were not considered: Microsoft Visual Studio Offline Packages, NuGet official package source.

A more powerful message would be to say "no sources" were considered potentially, because the package is not mapped. Another idea is to warn the duplicate sources as well.

jeromelaban commented 1 month ago

Both messages would help for sure.

Another problem I see here is that the user for https://github.com/unoplatform/uno/discussions/16758#discussioncomment-9537009 stumbled on this issue had a lot of mappings defined at the user level. It feels like an incorrect use of the source mapping feature, as it forces any new package use to be added to the the mapping list (with errors like https://github.com/NuGet/Home/issues/13469). This situation is what led to this particular issue.

donnie-msft commented 3 weeks ago

had a lot of mappings defined at the user level

I wonder if my rough proposal in https://github.com/NuGet/Home/issues/13173 to have NuGet tooling able to create configs that <clear/> (reset) the list of package sources would have prevented creating those at the user-level. Not the complete solution here, but perhaps a piece of the puzzle.

NuGet / Home