When a ProjectReference and PackageReference in different TFMs have the same identity/version, Restore throws "An item with the same key has already been added. [...]"

[dagood]

Details about Problem

Full repro details at https://github.com/dagood/repro/tree/repro-restore-ref-same-key. I used the mcr.microsoft.com/dotnet/sdk:5.0 Docker image to minimally repro, but this also repros the same on my Windows machine.

$ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.100
 Commit:    5044b93829

Runtime Environment:
 OS Name:     debian
 OS Version:  10
 OS Platform: Linux
 RID:         debian.10-x64
 Base Path:   /usr/share/dotnet/sdk/5.0.100/

Host (useful for support):
  Version: 5.0.0
  Commit:  cf258a14b7

.NET SDKs installed:
  5.0.100 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 5.0.0 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 5.0.0 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

System.Security.Cryptography.Cng.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <Version>5.0.0-rc.2.20475.5</Version>
  </PropertyGroup>
</Project>

System.Security.Cryptography.Pkcs.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net5.0;netcoreapp3.1</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'net5.0'">
    <ProjectReference Include="../System.Security.Cryptography.Cng/System.Security.Cryptography.Cng.csproj" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'">
    <PackageReference Include="System.Security.Cryptography.Cng" Version="5.0.0-rc.2.20475.5" />
  </ItemGroup>
</Project>

As far as I know I haven't seen this work before.

We hit it in source-build while trying to build dotnet/runtime: dotnet/source-build#1845 (comment)

This might not be supported--it doesn't make a whole lot of sense to me for a project to intentionally do this--but the error message should be improved at least! 😄 It took a while to figure out what the message was complaining about, work around it in source-build, and write this small repro.

Detailed repro steps so we can see the same problem

1. Clone https://github.com/dagood/repro/tree/repro-restore-ref-same-key
   (git clone -b repro-restore-ref-same-key https://github.com/dagood/repro && cd repro)

2. Run dotnet build ./System.Security.Cryptography.Pkcs/System.Security.Cryptography.Pkcs.csproj /bl

3. See this error:
   /usr/share/dotnet/sdk/5.0.100/NuGet.targets(131,5): error : An item with the same key has already been added. Key: (System.Security.Cryptography.Cng, 5.0.0-rc.2.20475.5) [/work/System.Security.Cryptograp hy.Pkcs/System.Security.Cryptography.Pkcs.csproj]

4. Open msbuild.binlog to see full stack trace:

System.ArgumentException: An item with the same key has already been added. Key: (System.Security.Cryptography.Cng, 5.0.0-rc.2.20475.5)
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](List`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](IEnumerable`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](IEnumerable`1 source, Func`2 keySelector)
   at NuGet.Commands.LockFileBuilder.CreateLockFile(LockFile previousLockFile, PackageSpec project, IEnumerable`1 targetGraphs, IReadOnlyList`1 localRepositories, RemoteWalkContext context)
   at NuGet.Commands.RestoreCommand.ExecuteAsync(CancellationToken token)
   at NuGet.Commands.RestoreRunner.ExecuteAsync(RestoreSummaryRequest summaryRequest, CancellationToken token)
   at NuGet.Commands.RestoreRunner.ExecuteAndCommitAsync(RestoreSummaryRequest summaryRequest, CancellationToken token)
   at NuGet.Commands.RestoreRunner.CompleteTaskAsync(List`1 restoreTasks)
   at NuGet.Commands.RestoreRunner.RunAsync(IEnumerable`1 restoreRequests, RestoreArgs restoreContext, CancellationToken token)
   at NuGet.Commands.RestoreRunner.RunAsync(RestoreArgs restoreContext, CancellationToken token)
   at NuGet.Build.Tasks.BuildTasksUtility.RestoreAsync(DependencyGraphSpec dependencyGraphSpec, Boolean interactive, Boolean recursive, Boolean noCache, Boolean ignoreFailedSources, Boolean disableParallel, Boolean force, Boolean forceEvaluate, Boolean hideWarningsAndErrors, Boolean restorePC, Boolean cleanupAssetsForUnsupportedProjects, ILogger log, CancellationToken cancellationToken)
   at NuGet.Build.Tasks.RestoreTask.ExecuteAsync(ILogger log)

Verbose Logs

Binlog: repro-restore-ref-same-key.zip

[nkolev92]

Known issue that @erdembayar is actively working on.
Maybe he can find the dup.

[erdembayar]

Known issue that @erdembayar is actively working on.
Maybe he can find the dup.

It's duplicate of Issue6795.
Draft PR for fix is here:
NuGet/NuGet.Client#3789

[dagood]

Looking at that PR, it seems like a different issue. It mentions "Prefer project reference over package reference, so remove the the package reference.". This isn't what I'd expect in this situation.

I would expect that (in this example) when building net5.0, it selects the project reference. When building netcoreapp3.1, it selects the package reference. That's based on these conditions:

  <ItemGroup Condition="'$(TargetFramework)' == 'net5.0'">
    <ProjectReference Include="../System.Security.Cryptography.Cng/System.Security.Cryptography.Cng.csproj" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'">
    <PackageReference Include="System.Security.Cryptography.Cng" Version="5.0.0-rc.2.20475.5" />
  </ItemGroup>

It sounds like that PR might actually make things worse by silently transforming the PackageReference into a ProjectReference when that isn't what you'd expect from the MSBuild code. 😕

[nkolev92]

It sounds like that PR might actually make things worse by silently transforming the PackageReference into a ProjectReference when that isn't what you'd expect from the MSBuild code

Project over package is the expectation for restore in general.

I think this scenario is slightly different from the linked one, and as such I am re-opening it.

[dagood]

Not quite sure my expectations are clear... restating just in case:

• When TargetFramework == net5.0, there's a ProjectReference.
• When TargetFramework == netcoreapp3.1, there's a PackageReference.

They seem very isolated from the project author point of view... I don't know why one would ever have to be considered "over" another. Why do they have a chance to collide and force NuGet to choose something that only exists in the "wrong" TargetFramework?

At the moment this feels like an arbitrary limitation (maybe of the project.assets.json format?) rather than something intentional. The scenario is pretty wild, so I understand we might run into a problem like this, but if this is intentional, it shakes up something I thought I understood about NuGet, so I'd like to know more.

[ViktorHofer]

Absolutely agree with @dagood on this one. Filed #10617 which was closed as a dup of this one but which includes a very simple repro. Note that in the other issue, the error is slightly different:

C:\temp\nugetnodes\parent>dotnet restore
  Determining projects to restore...
C:\temp\nugetnodes\parent\parent.csproj : error NU1201: Project System.Numerics.Vectors is not compatible with netstandard2.0 (.NETStandard,Version=v2.0). Project System.Numerics.Vectors supports: net5.0 (.NETCoreApp,Version=v5.0)
  Restored C:\temp\nugetnodes\child1\child1.csproj (in 276 ms).
  Failed to restore C:\temp\nugetnodes\parent\parent.csproj (in 276 ms).

This is causing unreasonable pain in dotnet/runtime in which we often reference via <PackageReference /> and <ProjectReference /> based on the TFM but IMHO is a bug that any customer could easily encounter when multi-targeting and conditionally referencing based on the inner-build TFM.

@nkolev92 could we please raise the priority of this bug?

[ViktorHofer]

This is still causing unreasonable pain. @nkolev92 @rrelyea have you had a chance to take a look?

[erdembayar]

@ViktorHofer
We already have fix for this. NuGet/NuGet.Client#3789
But this is hot code path so need perf analysis testing to make sure new change is doesn't add weight. Due other urgent priorities I didn't finish that part. I'll try to finish in next 2-3 week, sorry for delay.

[dagood]

We already have fix for this. NuGet/NuGet.Client#3789

I don't understand how that applies to the current issue. @erdembayar, can you give some more details?

What will the behavior be with Viktor's repro project, or my repro project?

The PR description mentions this currently:

Since we already have project over package preference I currently implemented this approach in my code.

This makes me concerned, because as a user, "Project over package" seems unrelated to this issue and (if it's forced in the way that comes to mind) would make the situation even worse than it is now. (See my comments above.)

[dagood]

Oh, that PR is the same one that this issue was mistakenly closed as a duplicate of once before. #10368 (comment)

It's duplicate of Issue6795.
Draft PR for fix is here:
NuGet/NuGet.Client#3789

I think this scenario is slightly different from the linked one, and as such I am re-opening it.

We already have fix for this. NuGet/NuGet.Client#3789

It seems to me like the actual status is that there has been no progress towards solving this issue, and there has been progress towards a fix for a different issue that will make this issue worse. (Significantly harder to detect/diagnose.)

[ViktorHofer]

I agree with Davis and his comment above which is a perfect summary of how I would expect NuGet to behave in a situation where a ProjectReference and a PackageReference to the same identity is present but both references being conditioned on a different TFM (inner build).

[erdembayar]

@ViktorHofer
Sorry, it was different issue. As you can see you already upvoted this issue.

[ViktorHofer]

As this still affects us in dotnet/runtime and makes it impossible to use ProjectReferences in multi-targeting projects without bringing in potentially vulnerable prebuilts, it would be good if could get some traction on this issue. Any idea where this issue originates? I would be willing to help but I have no idea where to start.

cc @aortiz-msft @ericstj

[erdembayar]

I would be willing to help but I have no idea where to start.

@ViktorHofer
I believe NuGet/NuGet.Client#3789 is very close one to what you're trying to do. Possible need to add TFM check before selecting the package vs project reference, tbh I haven't looked back at that code since then, so I don't know if it needs more changes other than TFM check.

[ViktorHofer]

Looking into this again as the issue is still impacting us in dotnet/runtime when using ProjectReferences and PackageReferences in different TFMs in the same project.

When projects reference System.Numerics.Vectors.csproj from one TFM and another TFM package references System.Memory which transitively package references System.Numerics.Vectors, S.N.V incorrectly gets resolved as a P2P. I.e.: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj#L67-L75

Error: C:\temp\tfmnugetoverlap\main\main.csproj : error NU1201: Project System.Numerics.Vectors is not compatible with netstandard2.0 (.NETStandard,Version=v2.0). Project System.Numerics.Vectors supports: net8.0 (.NETCoreApp,Version=v8.0)

---

I looked into this more closely and here's what I got so far.

Example 1 - Direct dependencies

The following scenario behaves as expected:

main.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net8.0</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\System.Numerics.Vectors\System.Numerics.Vectors.csproj" Condition="'$(TargetFramework)' == 'net8.0'" />
    <PackageReference Include="System.Numerics.Vectors" Version="4.4.0" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
  </ItemGroup>

</Project>

System.Numerics.Vectors.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>

</Project>

The ProjectReference is correctly identified as project dependency in the graph walker because the LibraryRange's TypeConstraint is Project (for the net8.0 TFM) and the TypeConstraint for the package LibraryRange is Package (for the netstandard2.0 TFM).

Example 2 - Transitive dependency only

The following scenario also behaves as expected:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Memory" Version="4.5.5" />
  </ItemGroup>

</Project>

The direct dependency is resolved as package because the TypeConstraint is Package. The transitive dependency (System.Numerics.Vectors/4.4.0)'s TypeConstraint is resolved as PackageProjectExternal because that's what NuGet does for transitives to allow P2Ps to win over PackageReferences.

In this case example, a ProjectReference for System.Numerics.Vectors doesn't exist and hence the transitive dependency is resolved as a package.

So far so good...

Example 3 - Matching transitive and direct dependency

The following scenario doesn't behave as expected:

main.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net8.0</TargetFrameworks>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\System.Numerics.Vectors\System.Numerics.Vectors.csproj" Condition="'$(TargetFramework)' == 'net8.0'" />
    <!-- System.Memory transitively references System.Numerics.Vectors for netstandard2.0. -->
    <PackageReference Include="System.Memory" Version="4.5.5" Condition="'$(TargetFramework)' == 'netstandard2.0'" />
  </ItemGroup>

</Project>

System.Numerics.Vectors.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>

</Project>

Error: C:\temp\tfmnugetoverlap\main\main.csproj : error NU1201: Project System.Numerics.Vectors is not compatible with netstandard2.0 (.NETStandard,Version=v2.0). Project System.Numerics.Vectors supports: net8.0 (.NETCoreApp,Version=v8.0)

First, the direct dependency for the net8.0 TFM is resolved as a project because the TypeConstraint is Project. That's expected and correct.

Now the netstandard2.0 TFM has a direct dependency to System.Memory which is resolved as package as the TypeConstraint of the dependency is Package. Still correct.

Now, the transitive dependency System.Numerics.Vectors is calculated. That dependency has a TypeConstraint of PackageProjectExternal (same as in example 2 above - because that's how NuGet implements P2Ps over PackageReferences). Now, because a ProjectReference with the same identity exists in the graph (even though not for the current evaluated TFM), the dependency is incorrectly resolved as a project.

Here's the code in question: https://github.com/NuGet/NuGet.Client/blob/41e98d33e40293d8276015d7a01e4d90610fa9d8/src/NuGet.Core/NuGet.DependencyResolver.Core/ResolverUtility.cs#L161

The tricky part (which I haven't yet figured out) is to make this code aware of which projects (in the projectProviders instance (?)) apply and which don't.

[ViktorHofer]

I came up with a "solution" and pushed a draft PR: NuGet/NuGet.Client#5397

Note that I had to touch public API (ResolverUtility) as the current API doesn't provide any information about the library's referencee.

With these changes, restoring the project tree from the above "Example 3" now works. I'm happy to jump on a call with experts if that helps to explain the problem and the current solution in more detail, cc @nkolev92 @zivkan.