sdk: Conditional PackageReference version splitting analyzer presence fails

If a project references two different versions of the same package in two different TargetFrameworks, and the package has added or removed a Roslyn analyzer, the analyzer will be added from both versions of the package, even the one where it’s not present.

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

  <PropertyGroup>
    <TargetFrameworks>net6.0;net472</TargetFrameworks>
    <RootNamespace>two_systemtextjson</RootNamespace>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.Text.Json" Version="4.7.0" Condition=" '$(TargetFramework)' == 'net472' " />
    <PackageReference Include="System.Text.Json" Version="6.0.0-preview.4.21253.7" Condition=" '$(TargetFramework)' == 'net6.0' " />
  </ItemGroup>

</Project>

two-systemtextjson on master .NET v6.0.100-preview.5.21302.13 🎯 net6.0;net472 
❯ dotnet build  
Microsoft (R) Build Engine version 17.0.0-preview-21302-02+018bed83d for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored S:\work\two-systemtextjson\two-systemtextjson.csproj (in 135 ms).
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
CSC : error CS0006: Metadata file 'C:\Users\raines\.nuget\packages\system.text.json\4.7.0\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.dll' could not be found [S:\work\two-systemtextjson\two-systemtextjson.csproj]
  two-systemtextjson -> S:\work\two-systemtextjson\bin\Debug\net6.0\two-systemtextjson.dll

Build FAILED.

CSC : error CS0006: Metadata file 'C:\Users\raines\.nuget\packages\system.text.json\4.7.0\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.dll' could not be found [S:\work\two-systemtextjson\two-systemtextjson.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:01.20

Because 4.7.0 of that package didn’t have an analyzer/source generator, the file isn’t found in that package version. The reference is only there because it was found in the other version.

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 9
Comments: 19 (11 by maintainers)

Commits related to this issue

Work around dotnet/sdk#18148 Referencing differing versions of System.Text.Json confused the SDK, so consistently push it to a higher version for Bootstrap with the thing that drags it up (Dependency... — committed to dotnet/msbuild by rainersigwald 3 years ago
Fix matching analyzers from packages to TargetFramework Fixes #18148 — committed to dsplaisted/sdk by dsplaisted 3 years ago
Remove workaround for https://github.com/dotnet/sdk/issues/18148. — committed to ExRam/ExRam.Gremlinq by danielcweber 2 years ago

Most upvoted comments

We just ran into this with the newly-introduced analyzer in Microsoft.Extensions.Logging.Abstractions 6.0.0-preview.5.21301.5. We found a fairly easy workaround for this, as long as you don’t actually need the analyzer in the project:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netstandard2.1;net6.0-android</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup Condition="'$(TargetFramework.Contains(net6.0))' == 'true'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.0-preview.5.21301.5" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework.Contains(net6.0))' != 'true'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="5.0.0" />
  </ItemGroup>

  <!-- Workaround for this bug (replace the analyzer name with the one you need to exclude (filename only, no extension) -->
  <Target Name="RemoveLoggingAnalyzer" BeforeTargets="CoreCompile">
    <ItemGroup>
      <Analyzer Remove="@(Analyzer)" Condition="%(FileName) == 'Microsoft.Extensions.Logging.Generators'" />
    </ItemGroup>
  </Target>
</Project>

Eilon on Jun 14, 2021

I went ahead and sent a PR to fix this: #18877

Thanks to @ericst for identifying where and what the fix was.

I still would really like to see NuGet fix this class of issue by treating analyzers as a proper asset type, though I recognize that’s a lot more complicated.

dsplaisted on Jul 13, 2021

I think NuGet/Home#6279 if fixed would fix this issue since it would let the SDK delete the code with this bug (and replace it with something else).

@ericstj The reply from @zkat (https://github.com/NuGet/Home/issues/6279#issuecomment-870000004) on that issues was that:

Our understanding is that dotnet/sdk#18148 is not strictly blocked by this issue, as it can be implemented without NuGet’s help. If this is not the case, please update us (and @ us). /cc @bruno-garcia

Bringing this to light as this completely blocks our ability to add a .NET 6 target to our package target Microsoft.Extensions.Logging so pretty critical on our end:

https://github.com/getsentry/sentry-dotnet/blob/7d008a9fc437d86d6eeff33300f2ba0160c74a9c/src/Sentry.Extensions.Logging/Sentry.Extensions.Logging.csproj#L16-L29

That’s unless we move all the TFMs to depend on the package version 6.0 which can cause some diamond dependency and/or other breakages and we’d like to avoid. For reference this package is relatively popular with 6M downloads.

bruno-garcia on Jul 8, 2021

I think NuGet/Home#6279 if fixed would fix this issue since it would let the SDK delete the code with this bug (and replace it with something else).

In lieu of that, I believe this can be fixed in the SDK.

Bug is here: https://github.com/dotnet/sdk/blob/529c048b7d7d4b04974fa40ce4073693eb8a0916/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs#L836-L855

This enumerates every library referenced by the all TargetFrameworks of the library. If any library contains an analyzer (on any targetframework) it then uses the package for the current _runtimeTarget and creates an item as if the asset came from that package.

This is broken in a couple ways:

using the runtime target for compile time assets (analyzers) is wrong. Compile target should be used.
using all packages as the input is wrong. Need to use compile target as the input, all packages should serve as the lookup (to get file list and lookup analyzer).

This algorithm should instead be:

Enumerate each package referenced in the _compileTarget.
Lookup that package/version in the _lockFile.Libraries.
Enumerate files.
If a file is an applicable analyzer, raise that as an analyzer.

ericstj on Jun 28, 2021