nuspec with `any` framework version not handled by PackageSourceGenerator

[mthalman]

The PackageSourceGenerator for SBRP doesn't correctly generate ProjectReferences in the csproj for a package whose nuspec file defines a dependency with the any framework version.

Repro:

.\generate.cmd -p Microsoft.CodeAnalysis.CSharp,2.9.0 -e

You'll notice that the nuspec for that package defines this:

<dependencies>
  <dependency id="Microsoft.CodeAnalysis.Common" version="[2.9.0]" />
</dependencies>

There's no group element with a targetFramework element. This implicitly means the target framework is any. This is the value that will be returned when retrieving the TargetFramework metadata from the package dependency.

This causes the generated csproj to be missing a ProjectReference to the dependency because it fails to match on the target framework here: https://github.com/dotnet/source-build-reference-packages/blob/f9dc7282be60127425b58b40972374cdec0348ed/src/packageSourceGenerator/PackageSourceGeneratorTask/GenerateProject.cs#L75

Continuing the example, this means that Microsoft.CodeAnalysis.CSharp.4.0.1.csproj is generated with no reference to Microsoft.CodeAnalysis.Common.2.9.0.csproj.

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ViktorHofer]

The package source generator fundamentally depends on target frameworks and respective nuspec dependency nodes. I strongly suggest to explore removing the prebuild in the source repository instead of trying to bring a super old package from 2018 in.

Do we know which repository depends on that version of Microsoft.CodeAnalysis.CSharp?

[mthalman]

Do we know which repository depends on that version of Microsoft.CodeAnalysis.CSharp?

This was done as part of dotnet/source-build-reference-packages#595, for arcade.

[ViktorHofer]

Just checked. Arcade should just update MicrosoftCodeAnalysisCSharpVersion to something newer. It doesn't need to be pinned down to such an old version. We had a similar issue / requirement in dotnet/sdk: https://github.com/dotnet/sdk/blob/0961189a48b0763d7cccba637539c876ed29f43f/src/Microsoft.DotNet.ApiSymbolExtensions/Microsoft.DotNet.ApiSymbolExtensions.csproj#L8-L11

We should be able to follow that approach in arcade as well: Use 4.0.1 during the normal build and use the very latest during source build.

[ViktorHofer]

@MichaelSimons based on the above observation, I think we should remove these very old packages from SBRP and upgrade consumers (probably just Arcade). Ideally we would do this in combination with removing netstandard1.x assets from SBRP. I would be surprised if the stack still depend on netstandard1.x assets. By removing netstandard1.x support, we make sure that such old packages won't get added again to SBRP and we will be able to remove all netstandard.library targeting packs aside from the latest (2.0.3).

Related: #3350

[MichaelSimons]

We should be able to follow that approach in arcade as well: Use 4.0.1 during the normal build and use the very latest during source build.

In order for this to work, it must be implemented at the repo level. What I mean by that is the repo must have a dependency declared and darc subscription so that the repo's source-build leg builds with the latest. We don't want it to only pick up the latest in the production source-build. This would be a recipe for breaks when the repo flows into the VMR.

In general, I am not a fan of repos conditioning which versions they use based on source-build. It is preferrable for one version to be used IMO.

I think we should remove these very old packages from SBRP and upgrade consumers (probably just Arcade). Ideally we would do this in combination with removing netstandard1.x assets from SBRP

I have no objection to that.

I would be surprised if the stack still depend on netstandard1.x assets.

There are a number still being referenced across the product.

[MichaelSimons]

Recently NetStandard1.x support was dropped from the rep, all of the unsupported packages were removed, and checks were enabled to ensure NetStandard1.x dependencies are not taken within source-build. Given that, any framework support should not be encountered and we should not need to support it. Closing issue. @ViktorHofer, @mthalman, feel free to comment if I missed a reason this is still needed.