Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In Microsoft.CodeAnalysis.Collections, pin S.C.Immutable to 5.0.0 to allow prebuilt reduction downstream #57129

Closed
dagood opened this issue Oct 13, 2021 · 6 comments
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@dagood
Copy link
Member

dagood commented Oct 13, 2021

If source-build upgrades this from 5.0.0 to 6.0.0[-rc.2.[...]]:

<PackageReference Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableVersion)" />

<SystemCollectionsImmutableVersion>5.0.0</SystemCollectionsImmutableVersion>

then downstream, if dotnet/msbuild consumes Microsoft.CodeAnalysis.Collections as built by dotnet/roslyn to remove a prebuilt dependency, dotnet/sdk then hits a build error when it consumes dotnet/msbuild packages:

/...tarball.../.dotnet/sdk/6.0.100-rc.2.21426.20/Microsoft.Common.CurrentVersion.targets(2315,5):
    error MSB3277: Found conflicts between different versions of "System.Collections.Immutable" that could not be resolved.
There was a conflict between "System.Collections.Immutable, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a".
    "System.Collections.Immutable, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was chosen because it was primary and "System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" was not.
    References which depend on "System.Collections.Immutable, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" [/...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/system.collections.immutable/5.0.0/lib/net461/System.Collections.Immutable.dll].
        /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/system.collections.immutable/5.0.0/lib/net461/System.Collections.Immutable.dll
          Project file item includes which caused reference "/...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/system.collections.immutable/5.0.0/lib/net461/System.Collections.Immutable.dll".
            /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/system.collections.immutable/5.0.0/lib/net461/System.Collections.Immutable.dll
    References which depend on "System.Collections.Immutable, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" [].
        /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.framework/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Framework.dll
          Project file item includes which caused reference "/...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.framework/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Framework.dll".
            /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.framework/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Framework.dll
            /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.utilities.core/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Utilities.Core.dll
        /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.utilities.core/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Utilities.Core.dll
          Project file item includes which caused reference "/...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.utilities.core/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Utilities.Core.dll".
            /...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/package-cache/microsoft.build.utilities.core/17.0.0-preview-21507-06/lib/netstandard2.0/Microsoft.Build.Utilities.Core.dll 
[/...tarball.../src/sdk.d7805ffe7727eff0cd891fc2c40291d150b47135/artifacts/source-build/self/src/src/BlazorWasmSdk/Tasks/Microsoft.NET.Sdk.BlazorWebAssembly.Tasks.csproj]

(Source-build does override it, because the package version follows the overridable convention.)

There are two ways to fix it without adding prebuilts:

  1. Prevent source-build from overriding the System.Collections.Immutable version from 5.0.0. Then when Microsoft.CodeAnalysis.Collections flows downstream, there's no 6.0.0 reference to worry about. 5.0.0 is already in SBRP, so this doesn't add any prebuilts.
    • I think System.Collections.Immutable is used as reference only, but let me know if this is wrong.
  2. Add a specific version of Microsoft.CodeAnalysis.Collections to SBRP as a source-only package and make sure dotnet/msbuild pins to that version.

I'm trying out a patch that implements (1). From a source-build perspective, it's preferred: we wouldn't need to add any more stuff to SBRP or maintain it over time, and Microsoft.CodeAnalysis.Collections could be maintained (by e.g. Red Hat) in a single place rather than having multiple versions to look at and patch.

(2) could also be fine. It would result in a more similar source-build vs. Microsoft build--but that's only the status quo. Is it really intended that dotnet/msbuild never gets auto-updates to Microsoft.CodeAnalysis.Collections? (@sharwell, do you have context, based on dotnet/msbuild#6595?)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 13, 2021
@sharwell
Copy link
Member

Microsoft.CodeAnalysis.Collections

There is no such assembly. Microsoft.CodeAnalysis.Collections is a package containing only source code, and the consumer can reference any version of System.Collections.Immutable it desires.

... if dotnet/msbuild consumes Microsoft.CodeAnalysis.Collections as built by dotnet/roslyn ...

This solution was carefully constructed to make this impossible.

@dagood
Copy link
Member Author

dagood commented Oct 13, 2021

Microsoft.CodeAnalysis.Collections

There is no such assembly. Microsoft.CodeAnalysis.Collections is a package containing only source code, and the consumer can reference any version of System.Collections.Immutable it desires.

I'm referring to the project/package. The package has a package reference:

<PackageReference Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableVersion)" />

It shows up in the nuspec as a nuget dependency, which influences the downstream project.

... if dotnet/msbuild consumes Microsoft.CodeAnalysis.Collections as built by dotnet/roslyn ...

This solution was carefully constructed to make this impossible.

What part of that should be impossible?

@sharwell
Copy link
Member

sharwell commented Oct 14, 2021

It shows up in the nuspec as a nuget dependency, which influences the downstream project.

The downstream project can add an explicit reference to a newer version of System.Collections.Immutable, which will be used instead of the transitive dependency version.

What part of that should be impossible?

Roslyn doesn't build anything for Microsoft.CodeAnalysis.Collections. It's a just a collection of source+resx files in a zip package.

If source-build upgrades this from 5.0.0 to 6.0.0[-rc.2.[...]]:

Source build should not change dependency versions for Roslyn references.

@dagood
Copy link
Member Author

dagood commented Oct 14, 2021

What part of that should be impossible?

Roslyn doesn't build anything for Microsoft.CodeAnalysis.Collections. It's a just a collection of source+resx files in a zip package.

Ok, this was just semantics. I would say that Roslyn builds a zip package that contains the collection of source+resx files (and a nuspec).

If source-build upgrades this from 5.0.0 to 6.0.0[-rc.2.[...]]:

Source build should not change dependency versions for Roslyn references.

That is what I'm proposing in this issue.

@sharwell
Copy link
Member

That is what I'm proposing in this issue.

👍

I think System.Collections.Immutable is used as reference only ...

This is correct.

Is it really intended that dotnet/msbuild never gets auto-updates to Microsoft.CodeAnalysis.Collections

Updating the package can be tedious because of the way it integrates with arcade build customizations (specifically around resx/xlf content). It seems reasonable for MSBuild to auto-update this package, but I would not expect that update process to be tied to source build in any way.

@dagood
Copy link
Member Author

dagood commented Oct 15, 2021

I think given the support limitations in dotnet/msbuild#6960 (comment) it would make sense for MSBuild to not auto-update. (Any problems would hopefully be caught by a PR validation error on the auto-update PR, I suppose those are more likely to be noise than something worth fixing just to get onto the latest version.)

And actually, given closing dotnet/msbuild#6960, this issue isn't needed either. There's no need to build the Microsoft.CodeAnalysis.Collections nupkg in Roslyn during source-build if nobody auto-updates new versions of it when they take new versions of Roslyn. Closing.

@dagood dagood closed this as completed Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

2 participants