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.

Sign up for GitHub

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

Move to central package pinning #10716

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Move to central package pinning #10716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06b9bd1
Move to central package pinning
jaredpar Aug 7, 2024
9cbc17b
more
jaredpar Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<RepositoryRoot>$(MSBuildThisFileDirectory)</RepositoryRoot>
<EnableWindowsTargeting Condition="'$(DotNetBuildFromSource)' != 'true'">true</EnableWindowsTargeting>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>

<SharedSourceRoot>$(MSBuildThisFileDirectory)src\Shared\</SharedSourceRoot>

Expand Down
12 changes: 6 additions & 6 deletions Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<_MicrosoftCodeAnalysisAnalyzersPackageVersion>3.11.0-beta1.24170.2</_MicrosoftCodeAnalysisAnalyzersPackageVersion>
<_MicrosoftVisualStudioLanguageServicesPackageVersion>$(MicrosoftVisualStudioLanguageServicesPackageVersion)</_MicrosoftVisualStudioLanguageServicesPackageVersion>
<_XunitPackageVersion>2.6.3</_XunitPackageVersion>
<_MicrosoftBuildPackageVersion>17.3.0-preview-22364-05</_MicrosoftBuildPackageVersion>
<_MicrosoftBuildPackageVersion>17.11.0-preview-24309-01</_MicrosoftBuildPackageVersion>
</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -89,7 +89,7 @@
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.11.8" />
<PackageVersion Include="Microsoft.VisualStudio.Shell.Framework" Version="$(_MicrosoftVisualStudioShellPackagesVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Shell.15.0" Version="$(_MicrosoftVisualStudioShellPackagesVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Telemetry" Version="17.9.102" />
<PackageVersion Include="Microsoft.VisualStudio.Telemetry" Version="17.11.8" />
<PackageVersion Include="Microsoft.VisualStudio.Text.Data" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Text.Implementation" Version="$(_MicrosoftVisualStudioPackagesVersion)" NoWarn="NU1701" />
<PackageVersion Include="Microsoft.VisualStudio.Text.Logic" Version="$(_MicrosoftVisualStudioPackagesVersion)" />
Expand All @@ -109,13 +109,13 @@
<PackageVersion Include="NuGet.VisualStudio" Version="17.9.1" />
<PackageVersion Include="Roslyn.Diagnostics.Analyzers" Version="$(_MicrosoftCodeAnalysisAnalyzersPackageVersion)" />
<PackageVersion Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutablePackageVersion)" />
<PackageVersion Include="System.Resources.Extensions" Version="6.0.0" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="6.0.0" />
<PackageVersion Include="System.Resources.Extensions" Version="8.0.0" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="8.0.0" />
<PackageVersion Include="System.Private.Uri" Version="4.3.2" />
<PackageVersion Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataPackageVersion)" />
<PackageVersion Include="System.Security.Cryptography.Xml" Version="6.0.1" />
<PackageVersion Include="System.Security.Cryptography.Xml" Version="7.0.1" />
<PackageVersion Include="System.Text.Encodings.Web" Version="8.0.0" />
<PackageVersion Include="System.Text.Json" Version="6.0.0" />
<PackageVersion Include="System.Text.Json" Version="8.0.4" />
<PackageVersion Include="System.Threading.Tasks.Extensions" Version="4.5.4" />
<PackageVersion Include="xunit" Version="$(_XunitPackageVersion)" />
<PackageVersion Include="Xunit.Combinatorial" Version="1.5.25" />
Expand Down
3 changes: 3 additions & 0 deletions ...pNetCore.Razor.Language/legacyTest/Microsoft.AspNetCore.Razor.Language.Legacy.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

<PackageReference Include="Newtonsoft.Json" />
<PackageReference Include="Moq" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
Copy link
Member

Choose a reason for hiding this comment

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

Just for my own understanding, why are these neccessary?

Copy link
Member

Choose a reason for hiding this comment

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

I had the same question. Does arcade do something to these packages? Because it seems like the override version would otherwise always match the real version.

Copy link
Member

Choose a reason for hiding this comment

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

I assume it's because these projects depend on these packages implicitly, and if we didn't set the version explicitly here, they'd be set to the version in the root project, not the overridden version in the compiler projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

What is going on here is that there are three sources of two sources of references for MS.CA.CSharp (and the other projects):

  1. The project ref to Microsoft.CodeAnalysis.Razor.Compiler.csproj which uses version 4.9.2
  2. The project ref to Microsoft.AspNetCore.Razor.Test.Common.csproj which uses version 4.12.0-1.24379.11

Standard MSBuild logic will say the 4.12.0-1.24379.11 reference is higher therefore lets use that one. But in this case we're using central transitive pinning which means that the version in Directory.Packages.props wins over standard MSBuild logic. For this project it uses src\Compiler\Directory.Packages.props which has MS.CA.CSharp at 4.9.2. Without this VersionOverride we'd end up with un-runnable binaries.

The good news here is that an essential feature of central transitive package pinning is that it will warn aggressively on any silent downgrades like this. That is essential because central transitive pinning is somewhat spooky action at a distance. It's saying any transitive dependency that matches an entry in Directory.Packages.props will be done at the version specified in that file. Hence it aggressively warns if this ever results in a silent downgrade because that is almost certainly what you did not want.

That is exactly what happened here and why I was lead to make the correct override notations.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the explanation. I didn't realise there still was a Directory.Packages.props in the Compiler folder, so I couldn't think of a place where a different version could possibly come from.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's still there because we haven't fully gotten the torn state solution into the .NET SDK. Once we get to say .NET 9 SDK RC1 we don't have to pin the compiler backwards any more and we can start undoing this nest of weirdness.

<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions .../Microsoft.AspNetCore.Razor.Language/test/Microsoft.AspNetCore.Razor.Language.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

<PackageReference Include="Newtonsoft.Json" />
<PackageReference Include="Moq" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
6 changes: 4 additions & 2 deletions ...zor.Microbenchmarks.Generator/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
<ItemGroup>
<PackageReference Include="BenchmarkDotNet" />
<PackageReference Include="BenchmarkDotNet.Diagnostics.Windows" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.Build.Locator" />
<PackageReference Include="System.Security.Cryptography.Xml" />
</ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions ...NET.Sdk.Razor.SourceGenerators.Tests/Microsoft.NET.Sdk.Razor.SourceGenerators.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
<PackageReference Include="Microsoft.Build.Framework" />
<PackageReference Include="Microsoft.Build.Utilities.Core" />
<PackageReference Include="Moq" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisCSharpWorkspacesPackageVersion)" />
<PackageReference Include="xunit.extensibility.execution" />
</ItemGroup>
Expand Down