[release/6.0.1xx] Eliminate runtime portable build

[ayakael]

Backport of #14549
Backport of #14647
Backport of #14938

Fixes dotnet/source-build#3071

[ayakael]

Of note, as is it currently fails for me with this error:

  /var/build/dotnet6/community/dotnet6-build/src/dotnet-v6.0.110/src/installer/artifacts/source-build/self/src/src/redist/redist.csproj(38,3): error MSB4024: The imported project file "/var/build/dotnet6/community/dotnet6-build/src/dotnet-v6.0.110/src/installer/artifacts/source-build/self/src/src/redist/targets/GenerateLayout.targets" could not be loaded. Could not load file or assembly 'System.Security.Permissions, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.
  /var/build/dotnet6/community/dotnet6-build/src/dotnet-v6.0.110/src/installer/artifacts/source-build/self/src/src/redist/redist.csproj(38,3): error MSB4024: 
  ##vso[task.logissue type=error;sourcepath=/var/build/dotnet6/community/dotnet6-build/src/dotnet-v6.0.110/src/installer/artifacts/source-build/self/src/src/redist/redist.csproj;linenumber=38;columnnumber=3;code=MSB4024;](NETCORE_ENGINEERING_TELEMETRY=Restore) The imported project file "/var/build/dotnet6/community/dotnet6-build/src/dotnet-v6.0.110/src/installer/artifacts/source-build/self/src/src/redist/targets/GenerateLayout.targets" could not be loaded. Could not load file or assembly 'System.Security.Permissions, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.%0A

I found artifacts/obj/x64/Release/blob-feed/packages/System.Security.Permissions.6.0.0.nupkg.

EDIT
Was caused by a typo that has now been found.

[ayakael]

Of note that, as is, this works on Alpine's end. Checks seem to fail because of this:

src/redist/targets/Crossgen.targets(122,5): error MSB6003: (NETCORE_ENGINEERING_TELEMETRY=Build) The specified task executable "crossgen2" could not be run. System.ComponentModel.Win32Exception (8): An error occurred trying to start process '/mnt/vss/_work/1/s/.nuget/packages//microsoft.netcore.app.crossgen2.linux-arm64/6.0.10/tools/crossgen2' with working directory '/mnt/vss/_work/1/s/src/redist'. Exec format error
   at System.Diagnostics.Process.ForkAndExecProcess(ProcessStartInfo startInfo, String resolvedFilename, String[] argv, String[] envp, String cwd, Boolean setCredentials, UInt32 userId, UInt32 groupId, UInt32[] groups, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean usesTerminal, Boolean throwOnNoExec)
   at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo)
   at System.Diagnostics.Process.Start()
   at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands)
   at Microsoft.Build.Utilities.ToolTask.Execute()

[ayakael]

On Alpine, the backport is implemented in this MR: https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/39604

Only failing check for us is use-current-runtime outputting alpine.3.17-$arch rather than expected linux-musl-$arch. Is this something we should care about?

[tmds]

Only failing check for us is use-current-runtime outputting alpine.3.17-$arch rather than expected linux-musl-$arch. Is this something we should care about?

use-current-runtime should use portable rid across all .NET builds for consistent semantics.
For more info, see dotnet/source-build#2932.

What NETCoreSdkPortableRuntimeIdentifier do you see in Microsoft.NETCoreSdk.BundledVersions.props?
If it's alpine*, can you try applying #14647 and see if that makes a difference?

[ayakael]

@tmds This did the trick for Alpine Linux on .net 6. I've yet to try this on .net7.

[MichaelSimons]

Just like the 7.0 backport, my view is backporting the patches to the repos is a requirement for getting approval to merge this.

[ayakael]

Just like the 7.0 backport, my view is backporting the patches to the repos is a requirement for getting approval to merge this.

Absolutely agreed, I'm waiting to get succesful checks before backporting.

[ayakael]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 14816 in repo dotnet/installer

[ayakael]

The crossgen errors don't make sense to me @tmds Does this look weird to you, why would crossgen2 have exec error given them matching the arch they're building in?

Edit: Right, of course, cause we need crossgen2 for x64. I just saw your up-to-date PR.

[ayakael]

I went ahead and backported everything now that checks are looking good

• aspnetcore: [release/6.0] source-build: support building aspnetcore using non-portable runtime packages (backport #43937) aspnetcore#44752
• runtime: [release/6.0] Pass TargetRid and SourceBuildNonPortable to the native scripts (backport of #74504) runtime#77508
• runtime: [release/6.0] source-build: support building runtime using non-portable runtime packages (backport of #75597) runtime#77509
• runtime: [release/6.0] Use generated runtime.json when building shared framework packages (backport of #76068) runtime#77510
• sdk: [release/6.0.1xx] ResolveReadyToRunCompilers: map non-portable rids when targetOS is determined (backport #28380) sdk#28782

@MichaelSimons I suppose we should wait till everything is merged before merging this?

[MichaelSimons]

@MichaelSimons I suppose we should wait till everything is merged before merging this?

Correct. If folks dis-approve of accepting them in servicing then we should not be accepting them in source-build. Additionally, every patch will cause a merge conflict when the changes flow in.

[ayakael]

Only failing check for us is use-current-runtime outputting alpine.3.17-$arch rather than expected linux-musl-$arch. Is this something we should care about?

use-current-runtime should use portable rid across all .NET builds for consistent semantics. For more info, see dotnet/source-build#2932.

What NETCoreSdkPortableRuntimeIdentifier do you see in Microsoft.NETCoreSdk.BundledVersions.props? If it's alpine*, can you try applying #14647 and see if that makes a difference?

Yup, that did it. Including your fix in this backport

[ayakael]

Everything is in. Despite rebase, the runtime commit in Version.Details.xml misses a few relevant PRs.

[crummel]

The builds have started so I think we won't get the Version.Details.xml update until after the release. Due to the holidays we also have a tight schedule around the next release - would it cause you a lot of problems if we delayed this until January?

[ayakael]

The builds have started so I think we won't get the Version.Details.xml update until after the release. Due to the holidays we also have a tight schedule around the next release - would it cause you a lot of problems if we delayed this until January?

Absolutely not, this can wait till January :) Thanks for all the work on y'all's side.

[ayakael]

Rebased and currently failing in installer (build Build_Tarball_x64 CentOS7-Online) with error Could not locate the package or project for "Microsoft.NETCore.App.Runtime.centos.7-x64" during installer build. This makes little sens given that runtime produces that package earlier in the build. Also, this error does not occur on Alpine.

[lbussell]

When this is working and we want to merge it, it will require changes like this in order to fit in the servicing schedule. It's important that we don't break building with N-1 release artifacts. dotnet/source-build#3156 exists to add CI to test this scenario.

[ayakael]

When this is working and we want to merge it, it will require changes like this in order to fit in the servicing schedule. It's important that we don't break building with N-1 release artifacts. dotnet/source-build#3156 exists to add CI to test this scenario.

For convenience, here is the backported patch:

From 2ff083707af48eb9c8ba87f268abccfde2ec2f33 Mon Sep 17 00:00:00 2001
From: Antoine Martin <dev@ayakael.net>
Date: Tue, 10 Jan 2023 07:39:45 -0500
Subject: [PATCH 1/1] Look for portable RID runtime packages by default

---
 .../job/source-build-build-tarball.yml        |  2 +-
 .../tarball/content/repos/runtime.proj        |  1 +
 .../source-build-reference-packages.proj      |  2 +-
 ...portable-runtime-packages-by-default.patch | 44 +++++++++++++++++++
 4 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 src/SourceBuild/tarball/patches/runtime/0004-Look-for-portable-runtime-packages-by-default.patch

diff --git a/src/SourceBuild/Arcade/eng/common/templates/job/source-build-build-tarball.yml b/src/SourceBuild/Arcade/eng/common/templates/job/source-build-build-tarball.yml
index b7318d262..c3056f6bb 100644
--- a/src/SourceBuild/Arcade/eng/common/templates/job/source-build-build-tarball.yml
+++ b/src/SourceBuild/Arcade/eng/common/templates/job/source-build-build-tarball.yml
@@ -41,7 +41,7 @@ jobs:
     - group: AzureDevOps-Artifact-Feeds-Pats
   - ${{ if eq(parameters.usePreviousArtifacts, 'true') }}:
     - name: additionalBuildArgs
-      value: --with-sdk /tarball/.dotnet
+      value: --with-sdk /tarball/.dotnet -- /p:UseNonPortableIlasmPackageOverride=true
   - name: tarballDir
     ${{ if eq(parameters.installerBuildResourceId, '') }}:
       value: $(Build.SourcesDirectory)
diff --git a/src/SourceBuild/tarball/content/repos/runtime.proj b/src/SourceBuild/tarball/content/repos/runtime.proj
index 073cd8bc1..1ff14c4e0 100644
--- a/src/SourceBuild/tarball/content/repos/runtime.proj
+++ b/src/SourceBuild/tarball/content/repos/runtime.proj
@@ -23,6 +23,7 @@
     <BuildCommandArgs>$(BuildCommandArgs) /p:RuntimeOS=$(RuntimeOS)</BuildCommandArgs>
     <BuildCommandArgs>$(BuildCommandArgs) /p:BaseOS=$(BaseOS)</BuildCommandArgs>
     <BuildCommandArgs>$(BuildCommandArgs) /p:SourceBuildNonPortable=true</BuildCommandArgs>
+    <BuildCommandArgs Condition="'$(UseNonPortableIlasmPackageOverride)' == 'true'">$(BuildCommandArgs) /p:UseNonPortableIlasmPackageOverride=true</BuildCommandArgs>
     <BuildCommand>$(StandardSourceBuildCommand) $(BuildCommandArgs)</BuildCommand>
   </PropertyGroup>
 
diff --git a/src/SourceBuild/tarball/content/repos/source-build-reference-packages.proj b/src/SourceBuild/tarball/content/repos/source-build-reference-packages.proj
index 9545b50e4..baa3dbdd3 100644
--- a/src/SourceBuild/tarball/content/repos/source-build-reference-packages.proj
+++ b/src/SourceBuild/tarball/content/repos/source-build-reference-packages.proj
@@ -3,7 +3,7 @@
 
   <PropertyGroup>
     <BuildCommandArgs>$(StandardSourceBuildArgs)</BuildCommandArgs>
-    <BuildCommandArgs>$(BuildCommandArgs) /p:MicrosoftNetCoreIlasmPackageRuntimeId=$(NETCoreSdkRuntimeIdentifier)</BuildCommandArgs>
+    <BuildCommandArgs Condition="'$(UseNonPortableIlasmPackageOverride)' == 'true'">$(BuildCommandArgs) /p:MicrosoftNetCoreIlasmPackageRuntimeId=$(NETCoreSdkRuntimeIdentifier)</BuildCommandArgs>
     <BuildCommand>$(StandardSourceBuildCommand) $(BuildCommandArgs)</BuildCommand>
 
     <NuGetConfigFile>$(ProjectDirectory)NuGet.config</NuGetConfigFile>
diff --git a/src/SourceBuild/tarball/patches/runtime/0004-Look-for-portable-runtime-packages-by-default.patch b/src/SourceBuild/tarball/patches/runtime/0004-Look-for-portable-runtime-packages-by-default.patch
new file mode 100644
index 000000000..9a6e06ceb
--- /dev/null
+++ b/src/SourceBuild/tarball/patches/runtime/0004-Look-for-portable-runtime-packages-by-default.patch
@@ -0,0 +1,44 @@
+From c487c8658a657c30820ddaf2201fc509fec2d503 Mon Sep 17 00:00:00 2001
+From: Antoine Martin <dev@ayakael.net>
+Date: Tue, 10 Jan 2023 07:37:53 -0500
+Subject: [PATCH 1/1] Look for portable runtime packages by default
+
+Change the default behavior to look for portable RID runtime packages (like
+ilasm). If previous source-built artifacts contain non-portable runtime
+packages, build .NET source-build with the
+` -- /p:UseNonPortableIlasmPackageOverride=true` argument.
+
+This patch should not be in the release of .NET Source-build 6.0.114
+---
+ Directory.Build.targets | 2 +-
+ eng/SourceBuild.props   | 1 +
+ 2 files changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/Directory.Build.targets b/Directory.Build.targets
+index 98967298c62..1b4d279bc5b 100644
+--- a/Directory.Build.targets
++++ b/Directory.Build.targets
+@@ -15,7 +15,7 @@
+   When .NET gets built from source, make the SDK aware there are bootstrap packages
+   for Microsoft.NETCore.App.Runtime.<rid> and Microsoft.NETCore.App.Crossgen2.<rid>.
+   -->
+-  <ItemGroup Condition="'$(DotNetBuildFromSource)' == 'true'">
++  <ItemGroup Condition="'$(DotNetBuildFromSource)' == 'true' and '$(UseNonPortableIlasmPackageOverride)' == 'true'">
+     <KnownFrameworkReference Update="@(KnownFrameworkReference->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->WithMetadataValue('TargetFramework', '$(NetCoreAppCurrent)'))">
+       <RuntimePackRuntimeIdentifiers>$(PackageRID)</RuntimePackRuntimeIdentifiers>
+     </KnownFrameworkReference>
+diff --git a/eng/SourceBuild.props b/eng/SourceBuild.props
+index fcbe9fa75e2..aa6f769a294 100644
+--- a/eng/SourceBuild.props
++++ b/eng/SourceBuild.props
+@@ -55,6 +55,7 @@
+       <InnerBuildArgs>$(InnerBuildArgs) /p:BuildDebPackage=false</InnerBuildArgs>
+       <InnerBuildArgs>$(InnerBuildArgs) /p:RuntimeOS=$(RuntimeOS)</InnerBuildArgs>
+       <InnerBuildArgs>$(InnerBuildArgs) /p:AdditionalRuntimeIdentifierParent=$(BaseOS)</InnerBuildArgs>
++      <InnerBuildArgs Condition="'$(UseNonPortableIlasmPackageOverride)' == 'true'">$(InnerBuildArgs) /p:UseNonPortableIlasmPackageOverride=true</InnerBuildArgs>
+     </PropertyGroup>
+   </Target>
+ 
+-- 
+2.36.3
+
-- 
2.36.3

Of note, I think I found the source of the error: Despite 6.0.112, aspnetcore is still 6.0.11 (edit: only in release/6.0.1xx, v6.0.112 tag uses 6.0.12), which was released before the aspnetcore changes were integrated. To test this hypothesis, I've manually upgraded aspnetcore to 6.0.12.

[ayakael]

Current failure is due to dotnet/source-build#3207

[ayakael]

The issue seems fixed - it was due to a mismatch in override variables. The current failure with Build_Fedora_29_Debug_x64 looks like a fluke .

[ayakael]

@crummel Is this too late for inclusion for next servicing now that checks pass?

[crummel]

I'll review this quick and ask for approval if it looks good, I think end of day today is the deadline to get everything merged.

[crummel]

I don't think we'll be able to get this in for February, I was talking about it with @MichaelSimons and there is additional work necessary after this goes in (the same changes we made in 7.0 after removing the portable build) so I think we'll have to hold off. I'll still review and get approval so we're ready to go as soon as the branches open up again.

[MichaelSimons]

[Triage] We discussed this in the source-build planning meeting today and the consensus was that this was too risky to merge into 6.0 (LTS).

This change had a number of side effects (some we foresaw and other we didn't). There is some uncertainty if the full set of required backports is known. Additionally we are still working through UX impact from this change.

@ayakael - I appreciate your work on this and wish this was less risky so that we could accept the backport.