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

Upgrade System.Memory to 4.6.0 #15353

Merged
merged 5 commits into from
Dec 20, 2024
Merged

Upgrade System.Memory to 4.6.0 #15353

merged 5 commits into from
Dec 20, 2024

Conversation

mthalman
Copy link
Member

Using a recent daily build of .NET 10 SDK to source build the arcade repo causes the following errors:

/__w/1/vmr/src/arcade/src/VersionTools/Microsoft.DotNet.VersionTools/Microsoft.DotNet.VersionTools.csproj error NU1605: Warning As Error: Detected package downgrade: System.Memory from 4.6.0 to 4.5.5. Reference the package directly from the project to select a different version. 
 Microsoft.DotNet.VersionTools -> System.Formats.Asn1 10.0.0-alpha.1.24610.2 -> System.Memory (>= 4.6.0) 
 Microsoft.DotNet.VersionTools -> NuGet.Packaging 6.11.0 -> System.Security.Cryptography.Pkcs 6.0.4 -> System.Memory (>= 4.5.5) [/__w/1/vmr/src/arcade/Arcade.sln]
/__w/1/vmr/src/arcade/src/Common/Microsoft.Arcade.Common/Microsoft.Arcade.Common.csproj error NU1605: Warning As Error: Detected package downgrade: System.Memory from 4.6.0 to 4.5.5. Reference the package directly from the project to select a different version. 
 Microsoft.Arcade.Common -> System.Collections.Immutable 10.0.0-alpha.1.24610.2 -> System.Memory (>= 4.6.0) 
 Microsoft.Arcade.Common -> Microsoft.Build.Utilities.Core 17.8.3 -> System.Memory (>= 4.5.5) [/__w/1/vmr/src/arcade/Arcade.sln]
/__w/1/vmr/src/arcade/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj error NU1109: Detected package downgrade: System.Memory from 4.6.0 to centrally defined 4.5.5. Update the centrally managed package version to a higher version. 
 Microsoft.DotNet.GenFacades -> Microsoft.CodeAnalysis.CSharp 4.13.0-2.24561.1 -> System.Memory (>= 4.6.0) 
 Microsoft.DotNet.GenFacades -> System.Memory (>= 4.5.5) [/__w/1/vmr/src/arcade/Arcade.sln]

Resolving this by upgrading to 4.6.0.

@mthalman mthalman marked this pull request as ready for review December 19, 2024 16:56
ellahathaway
ellahathaway previously approved these changes Dec 19, 2024
@ViktorHofer ViktorHofer changed the title Upgrade System.Memory to 4.6.0 Upgrade System.Memory to 4.6.0 and M.Bcl.HashCode to 6.0.0 Dec 20, 2024
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the Bcl.HashCode update as well (these packages got produced together at once from maintenance-packages so it makes sense to keep them in sync).

@ViktorHofer ViktorHofer enabled auto-merge (squash) December 20, 2024 14:30
@ViktorHofer ViktorHofer disabled auto-merge December 20, 2024 14:45
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I need to request changes. I just noticed that the System.Memory update impacts .NET Framework msbuild tasks. VS desktop msbuild doesn't yet have those never versions available inbox. We had to approach this differently: https://github.com/dotnet/msbuild/blob/a71903d9684e24db3db261b27ca73c0ef879cf81/eng/Versions.props#L26-L40

This doesn't cause any failures here as the msbuild tasks aren't tested inside a desktop msbuild environment.

Can you please update this change to only update those versions when building from source with a tracking issue to consolidate eventually? cc @carlossanlop @ericstj

@mthalman
Copy link
Member Author

@ViktorHofer - Do you know if the same condition is needed for fsharp: dotnet/fsharp#18166?

@mthalman mthalman requested a review from ViktorHofer December 20, 2024 15:06
@ViktorHofer
Copy link
Member

Do you know if the same condition is needed for fsharp: dotnet/fsharp#18166?

Only for msbuild tasks that are executed on desktop msbuild. I don't think that's the case for fsharp so the change should be good.

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Bcl.HashCode one also needs the conditional update as the Microsoft.DotNet.Build.Tasks.Feed library (which uses that package) is used in a desktop msbuild environment. Please make the same change for it as well.

The rule is that any library that ships inbox in desktop msbuild and is used by an msbuild task either directly or transitively must not go higher than what is provided inbox.

@mthalman
Copy link
Member Author

I think the Bcl.HashCode one also needs the conditional update as the Microsoft.DotNet.Build.Tasks.Feed library (which uses that package) is used in a desktop msbuild environment. Please make the same change for it as well.

The rule is that any library that ships inbox in desktop msbuild and is used by an msbuild task either directly or transitively must not go higher than what is provided inbox.

What part of source build requires a newer version of Microsoft.Bcl.HashCode?

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 20, 2024

Good point, no component here in arcade. I'll revert the version upgrade as it's problematic with desktop msbuild.

EDIT: GH wrote the commit description, no idea why it mentioned BuildTask.props.

@ViktorHofer ViktorHofer changed the title Upgrade System.Memory to 4.6.0 and M.Bcl.HashCode to 6.0.0 Upgrade System.Memory to 4.6.0 Dec 20, 2024
@mthalman mthalman merged commit ddb0b75 into dotnet:main Dec 20, 2024
11 checks passed
@mthalman mthalman deleted the system-memory branch December 20, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants