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

Convert ref and runtime packs to use the SharedFramework SDK and Arcade's installer tooling #58612

Open
wants to merge 56 commits into
base: main
Choose a base branch
from

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Oct 24, 2024

Convert ref and runtime packs to use the SharedFramework SDK and Arcade's installer tooling (for Linux installers)

Replace the custom infrastructure in ASP.NET Core for building shared frameworks, .debs, and .rpms with the Arcade infrastructure

Description

Replace the custom infrastructure in this repo with the Arcade infrastructure. This provides a number of benefits:

  • Crossgen2/R2R support is hooked up correctly through the SDK support
  • Construction of targeting and runtime packs uses standard tooling, so less for aspnetcore to maintain and free updates to "best practices" from Arcade
  • Support for building deb and rpm packages on any Linux distro

Fixes #48013
Fixes #49486
Fixes #58073
Contributes to dotnet/source-build#3986

Depends on dotnet/arcade#15200
Depends on an SDK with dotnet/sdk#44436 (#58862)
Depends on dotnet/runtime#109169
Replaces #58600
Depends on dotnet/arcade#15208
Depends on dotnet/arcade#15209
Depends on #58934

@jkoritzinsky jkoritzinsky added the blocked The work on this issue is blocked due to some dependency label Oct 24, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 24, 2024
@wtgodbe
Copy link
Member

wtgodbe commented Oct 28, 2024

Whoa, awesome! Feel free to ping me on Teams if/when you want me to take a look

@jkoritzinsky jkoritzinsky added the blocked The work on this issue is blocked due to some dependency label Nov 5, 2024
@jkoritzinsky jkoritzinsky marked this pull request as ready for review November 8, 2024 15:50
@ViktorHofer
Copy link
Member

@jkoritzinsky do you plan to touch this code path as well?

<ItemGroup>
<PackageReference Include="Microsoft.DotNet.Build.Tasks.Installers" Version="$(MicrosoftDotNetBuildTasksInstallersVersion)" />
</ItemGroup>
<PropertyGroup>
<MicrosoftDotNetBuildTasksInstallersTaskAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(NuGetPackageRoot)microsoft.dotnet.build.tasks.installers\$(MicrosoftDotNetBuildTasksInstallersVersion)\tools\netcoreapp2.1\Microsoft.DotNet.Build.Tasks.Installers.dll</MicrosoftDotNetBuildTasksInstallersTaskAssembly>
<MicrosoftDotNetBuildTasksInstallersTaskAssembly Condition="'$(MSBuildRuntimeType)' != 'Core'">$(NuGetPackageRoot)microsoft.dotnet.build.tasks.installers\$(MicrosoftDotNetBuildTasksInstallersVersion)\tools\net472\Microsoft.DotNet.Build.Tasks.Installers.dll</MicrosoftDotNetBuildTasksInstallersTaskAssembly>
</PropertyGroup>
<UsingTask TaskName="CreateLightCommandPackageDrop" AssemblyFile="$(MicrosoftDotNetBuildTasksInstallersTaskAssembly)" />
<UsingTask TaskName="CreateLitCommandPackageDrop" AssemblyFile="$(MicrosoftDotNetBuildTasksInstallersTaskAssembly)" />

The hardcodes there are problematic and shouldn't exist at all. But before someone fixing this (by just using proper PackageReference) I wanted to double check if you intend to change this.

@jkoritzinsky
Copy link
Member Author

I was going to handle messing with any Wix installers in a different PR (as I'm less confident in getting the upgrade codes and things right).

@@ -31,13 +31,15 @@
<_InstallersToPublish Include="$(ArtifactsDir)installers\**\*.version" UploadPathSegment="Runtime" Condition="'$(PublishInstallerBaseVersion)' == 'true'" />

<!-- The following installers create checksums -->
<_InstallersToPublish Include="$(ArtifactsDir)installers\**\*.deb" UploadPathSegment="Runtime" ChecksumPath="%(FullPath).sha512" />
<_InstallersToPublish Include="$(ArtifactsDir)packages\**\*.deb" UploadPathSegment="Runtime" ChecksumPath="%(FullPath).sha512" />
Copy link
Member

Choose a reason for hiding this comment

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

Is it the common pattern to publish installers to the packages dir?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, dotnet/runtime and dotnet/windowsdesktop publish all installers to the packages dir.

"Microsoft.DotNet.Helix.Sdk": "10.0.0-beta.24560.1"
"Microsoft.DotNet.Helix.Sdk": "10.0.0-beta.24560.1",
"Microsoft.DotNet.SharedFramework.Sdk" : "10.0.0-beta.24560.1",
"Microsoft.Build.NoTargets": "3.7.0",
Copy link
Member

Choose a reason for hiding this comment

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

Why these versions/when should we update them?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the versions used by dotnet/runtime today. I believe they're the newest versions. We generally try to update them when new versions come out, but we don't have a regular cadence.

@@ -0,0 +1,66 @@
<Project Sdk="Microsoft.Build.Traversal">
<PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this proj?

Copy link
Member Author

Choose a reason for hiding this comment

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

This project builds an archive containing the .NET runtime and the ASP.NET Core runtime with the Composite ReadyToRun image (which is used in some of our docker images).

@wtgodbe
Copy link
Member

wtgodbe commented Nov 12, 2024

Can you run an internal build of this branch so we can inspect the produced SharedFx/Targeting packs packages/installers?

@jkoritzinsky
Copy link
Member Author

@ViktorHofer
Copy link
Member

According to the official build, the targeting pack project is built and published from multiple legs:

##[error].packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.24560.1\tools\SdkTasks\PublishBuildAssets.proj(42,5): error : Repeated Package entry: 'Microsoft.AspNetCore.App.Ref' - '10.0.0-alpha.2.24562.3' 
D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.24560.1\tools\SdkTasks\PublishBuildAssets.proj(42,5): error : InvalidOperationException: Duplicate package entries are not allowed for publishing to BAR, as this can cause race conditions and unexpected behavior
D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.24560.1\tools\SdkTasks\PublishBuildAssets.proj(42,5): error :    at Microsoft.DotNet.Maestro.Tasks.PushMetadataToBuildAssetRegistry.MergeManifests(List`1 parsedManifests) in /_/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs:line 557
D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.24560.1\tools\SdkTasks\PublishBuildAssets.proj(42,5): error :    at Microsoft.DotNet.Maestro.Tasks.PushMetadataToBuildAssetRegistry.PushMetadataAsync(CancellationToken cancellationToken) in /_/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs:line 105
D:\a\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\10.0.0-beta.24560.1\tools\SdkTasks\PublishBuildAssets.proj(42,5): error : 

@jkoritzinsky
Copy link
Member Author

Failures in the Linux x64 leg will be fixed by #58934

Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework blocked The work on this issue is blocked due to some dependency pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
4 participants