[release/7.0.1xx] Wrong version of aspnetcore.runtime.internal is pulled by GenerateLayout

[ayakael]

Wrong version of aspnetcore-runtime-internal-$(MicrosoftAspNetCoreAppRuntimePackageVersion)-$(AspNetCoreArchiveRid)$(ArchiveExtension) is pulled by installer when building source-build on Alpine Linux. Version 7.0.0-rc.1.22404 is expected, but aspnetcore builds 7.0.0-rc.2.22452.11. The following patch is used as workaround:

From db54f968bc5f0bcb327ca1e5362b314b3d952860 Mon Sep 17 00:00:00 2001
From: Antoine Martin <[email protected]
Date: Sun, 13 Feb 2022 21:59:46 +0000
Subject: [PATCH 2/2] 
 installer_forgotten-MicrosoftAspNetCoreAppRuntimePackageVersion-fix

 Somewhere along the way, installer forgets MicrosoftAspNetCoreApp-
 RuntimePackageVersion, thus expects version 6.0.0 when building 6.0.x.
 This reminds installer what version AspNetCoreappRuntime is by 
 re-setting it as what it is usually set.

---
 src/redist/targets/GenerateLayout.targets | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/installer/src/redist/targets/GenerateLayout.targets b/src/installer/src/redist/targets/GenerateLayout.targets
index e05004e17..d3573f401 100644
--- a/src/installer/src/redist/targets/GenerateLayout.targets
+++ b/src/installer/src/redist/targets/GenerateLayout.targets
@@ -96,6 +96,7 @@
       <DownloadedAspNetTargetingPackInstallerFileName Condition=" '$(InstallerExtension)' == '.msi' ">aspnetcore-targeting-pack-$(MicrosoftAspNetCoreAppRefInternalPackageVersion)-$(AspNetCoreInstallerRid)$(InstallerExtension)</DownloadedAspNetTargetingPackInstallerFileName>
       <DownloadedAspNetCoreV2ModuleInstallerFileName Condition=" '$(InstallerExtension)' == '.msi' ">aspnetcoremodule_$(Architecture)_en_v2_$(MicrosoftAspNetCoreAppRuntimePackageVersion)$(InstallerExtension)</DownloadedAspNetCoreV2ModuleInstallerFileName>
       <AspNetTargetingPackArchiveFileName>aspnetcore-targeting-pack-$(MicrosoftAspNetCoreAppRefPackageVersion)-$(AspNetCoreArchiveRid)$(ArchiveExtension)</AspNetTargetingPackArchiveFileName>
+      <MicrosoftAspNetCoreAppRuntimePackageVersion>$(MicrosoftAspNetCoreAppRuntimewinx64PackageVersion)</MicrosoftAspNetCoreAppRuntimePackageVersion>
       <AspNetCoreSharedFxArchiveFileName>aspnetcore-runtime-internal-$(MicrosoftAspNetCoreAppRuntimePackageVersion)-$(AspNetCoreArchiveRid)$(ArchiveExtension)</AspNetCoreSharedFxArchiveFileName>
 
       <!-- Used to detect if ASP.NET Core is built against the same version of Microsoft.NETCore.App. -->
-- 
2.35.1

This issue also affects .NET 6

[tmds]

MicrosoftAspNetCoreAppRuntimewinx64PackageVersion is the default value of MicrosoftAspNetCoreAppRuntimePackageVersion in eng/Versions.props.

MicrosoftAspNetCoreAppRuntimePackageVersion is changed by ExtraPackageVersionPropsPackageInfo to MicrosoftAspNetCoreAppRuntimeLinuxx64PackageVersion.

This causes a mismatch between the version produced by aspnetcore (MicrosoftAspNetCoreAppRuntimewinx64PackageVersion), and the version expected by installer (MicrosoftAspNetCoreAppRuntimeLinuxx64PackageVersion).

Either aspnetcore needs to change to build MicrosoftAspNetCoreAppRuntimeLinuxx64PackageVersion, or we should remove the ExtraPackageVersionPropsPackageInfo for MicrosoftAspNetCoreAppRuntimePackageVersion.

I don't know which one is the proper fix.

@crummel @MichaelSimons ptal.

[crummel]

I think we'll need to add a MUSL version of the runtime package here with the other alternative versions: https://github.com/dotnet/installer/blob/314db6c4f5199bad4bf06e2d826aded06e2b177b/src/SourceBuild/tarball/content/Directory.Build.props#L237. This seems to be the same issue we run into on Windows/Mac when we're not producing the linux-x64 runtime. Would you mind opening a PR for this?

[tmds]

In dotnet/installer#14549 I also run into this issue.

Can we use MicrosoftAspNetCoreAppRuntimewinx64PackageVersion (or something else) when we build aspnetcore, and then again when we consume it in installer, independent of the target platform?

[ayakael]

I think we'll need to add a MUSL version of the runtime package here with the other alternative versions:

https://github.com/dotnet/installer/blob/314db6c4f5199bad4bf06e2d826aded06e2b177b/src/SourceBuild/tarball/content/Directory.Build.props#L237

. This seems to be the same issue we run into on Windows/Mac when we're not producing the linux-x64 runtime. Would you mind opening a PR for this?

Do you mean we ought to do something like this:

diff --git a/eng/Versions.props b/eng/Versions.props
index 8f69d9cb0..279539a63 100644
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -51,6 +51,7 @@
   <PropertyGroup>
     <!-- Dependencies from https://github.com/aspnet/AspNetCore -->
     <MicrosoftAspNetCoreAppRuntimewinx64PackageVersion>7.0.0-rc.2.22465.19</MicrosoftAspNetCoreAppRuntimewinx64PackageVersion>
+    <MicrosoftAspNetCoreAppRuntimemuslx64PackageVersion>$(MicrosoftAspNetCoreAppRuntimewinx64PackageVersion)</MicrosoftAspNetCoreAppRuntimewinx64PackageVersion>
     <MicrosoftAspNetCoreAppRefPackageVersion>7.0.0-rc.2.22465.19</MicrosoftAspNetCoreAppRefPackageVersion>
     <MicrosoftAspNetCoreAppRefInternalPackageVersion>7.0.0-rc.2.22465.19</MicrosoftAspNetCoreAppRefInternalPackageVersion>
     <VSRedistCommonAspNetCoreSharedFrameworkx6470PackageVersion>7.0.0-rc.2.22465.19</VSRedistCommonAspNetCoreSharedFrameworkx6470PackageVersion>
diff --git a/src/SourceBuild/tarball/content/Directory.Build.props b/src/SourceBuild/tarball/content/Directory.Build.props
index 797e54fa6..2daf16661 100644
--- a/src/SourceBuild/tarball/content/Directory.Build.props
+++ b/src/SourceBuild/tarball/content/Directory.Build.props
@@ -236,6 +236,7 @@
     <!-- OSX needs the OSX version instead of Linux.  We don't have a lot of flexibility in how we output these properties so we're relying on the previous one being blank if the Linux vers
ion of the package is missing.  -->
     <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" Version="%24(MicrosoftAspNetCoreAppRuntimeOsxX64PackageVersion)" DoNotOverwrite="true" />
     <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" Version="%24(MicrosoftAspNetCoreAppRuntimewinx64PackageVersion)" DoNotOverwrite="true" />
+    <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" Version="%24(MicrosoftAspNetCoreAppRuntimemuslx64PackageVersion)" DoNotOverwrite="true" />
 
     <!-- Used by installer to determine sdk version -->
     <ExtraPackageVersionPropsPackageInfo Include="MicrosoftDotnetToolsetInternalPackageVersion" Version="%24(MicrosoftNETSdkPackageVersion)" />

[tmds]

Do you mean we ought to do something like this:

Any fix that doesn't cause aspnetcore and installer to read the version from the same source is a bad one imo.

[ayakael]

Do you mean we ought to do something like this:

Any fix that doesn't cause aspnetcore and installer to read the version from the same source is a bad one imo.

I'm a bit out of my depth so I can refer to ya'll for proper implementation.

[tmds]

I mean that the source tarball should pass the version number to build to aspnetcore and then use that same version number when installer gets the artifacts.

@crummel is that possible?

I don't understand how the tarball tells aspnetcore what version it should build.

Is this done using?

https://github.com/dotnet/installer/blob/429fe082c9fa5d67efc070f669f24bac6f48fc6a/src/SourceBuild/tarball/content/Directory.Build.props#L223-L224

[ayakael]

Given @tmds already exploring a fix in dotnet/installer#14549, I am closing my own PR.

[tmds]

Based on how this behaves, it seems aspnetcore picks up one of these MicrosoftAspNetCoreAppRuntimeXXXPackageVersion if XXX is the platform being built. It's a mystery to me how this happens.
If that property is missing, it falls back to the MicrosoftAspNetCoreAppRuntimewinx64PackageVersion which gets set to aspnetcoreOutputPackageVersion by ExtraPackageVersionPropsPackageInfo.

Then the logic for ExtraPackageVersionPropsPackageInfo tries to set MicrosoftAspNetCoreAppRuntimePackageVersion to the version that was built based on what MicrosoftAspNetCoreAppRuntimeXXXPackageVersion are set. That is the version consumed by installer.

I think it would be best if we can make aspnetcore always produce aspnetcoreOutputPackageVersion, and installer always consume aspnetcoreOutputPackageVersion. Then there is no possible mismatch.

I don't know how to do this because I don't know what the proper way is to tell aspnetcore it should always build aspnetcoreOutputPackageVersion even in the presence of MicrosoftAspNetCoreAppRuntimeXXXPackageVersion properties.

[ayakael]

Based on how this behaves, it seems aspnetcore picks up one of these MicrosoftAspNetCoreAppRuntimeXXXPackageVersion if XXX is the platform being built. It's a mystery to me how this happens. If that property is missing, it falls back to the MicrosoftAspNetCoreAppRuntimewinx64PackageVersion which gets set to aspnetcoreOutputPackageVersion by ExtraPackageVersionPropsPackageInfo.

Then the logic for ExtraPackageVersionPropsPackageInfo tries to set MicrosoftAspNetCoreAppRuntimePackageVersion to the version that was built based on what MicrosoftAspNetCoreAppRuntimeXXXPackageVersion are set. That is the version consumed by installer.

I think it would be best if we can make aspnetcore always produce aspnetcoreOutputPackageVersion, and installer always consume aspnetcoreOutputPackageVersion. Then there is no possible mismatch.

I don't know how to do this because I don't know what the proper way is to tell aspnetcore it should always build aspnetcoreOutputPackageVersion even in the presence of MicrosoftAspNetCoreAppRuntimeXXXPackageVersion properties.

I agree, code that's magic is bad code. Those logics shouldn't be so opaque. When does this version have to be any other than aspnetcoreOutputPackageVersion is the question for me. As for aspnetcore, the way I know is to give it a /p:VersionSuffix= flag with whatever is after the main version. Usually,it's $version-servicing.22000.1 or something, thus VersionSuffix would lose the $version. Some parsing would have to happen to remove the undesirable bit.

[tmds]

What I have in dotnet/installer#14549 renames MicrosoftAspNetCoreAppRuntimePackageVersion to MicrosoftAspNetCoreAppRuntimePackageVersionBuilt in installer causing it to ignore the ExtraPackageVersionPropsPackageInfo stuff for MicrosoftAspNetCoreAppRuntimePackageVersion.

Maybe the fix is to remove ExtraPackageVersionPropsPackageInfo for MicrosoftAspNetCoreAppRuntimePackageVersion?

@crummel wdyt?

[crummel]

Yes, I think what has changed here is that once upon a time ASP.NET produced and installer consumed some unstable-versioned packages and some stable-version packages. aspnetcoreOutputPackageVersion was what we used for the unstable version, and the stable versions were picked up by having these multiple options for picking up the relevant version depending on the package that was built for the platform. Note that this is a one-way flow:

1. git-info/aspnetcore.props looks like

<Project>
  <PropertyGroup>
    <GitCommitHash>2651d9202b0bf5fdd081930cd7a4438ced351410</GitCommitHash>
    <IsStable>false</IsStable>
    <OfficialBuildId>20221004.29</OfficialBuildId>
    <OutputPackageVersion>7.0.0-rtm.22504.29</OutputPackageVersion>
    <PreReleaseVersionLabel>rtm</PreReleaseVersionLabel>
  </PropertyGroup>
</Project>

2. OfficialBuildId is fed into the ASP.NET build by source-build to produce packages versioned as OutputPackageVersion. OutputPackageVersion is also loaded in to the source-build build as aspnetcoreOutputPackageVersion.
3. When PackageVersion.props is generated, we add extra entries to either override or supplement all of the entries that are auto-generated by virtue of the package existing. I believe MicrosoftAspNetCoreAppRuntimePackageVersion used to be expected to be a stabilized version (i.e. no -rtm.22504.29 suffix) so we needed to override it with the runtime package version with was produced stable. The ExtraPackageVersionPropsPackageInfo with multiple platforms was required so that the build could pick up whichever of the runtime packages was produced by the current build.

It now appears both the MS.ASPNetCore.App.Runtime package and the MS.ASPNetCore.App.Runtime.platform packages are produced with the same version number (which is unstabilized), so I think you're right that we can eliminate this old workaround. I opened dotnet/installer#14769 to do so and had a successful build with it, I'm going to try to build with the work you two have been doing but I'd appreciate it if you tried it out as well.

[MichaelSimons]

Another way to test this would be to pull the changes into @omajid's PR to add an Alpine CI leg - dotnet/installer#14417. The PR hit this issue.

[ayakael]

Of note, @tmds fix, that is to say changing the name of MicrosoftAspNetCoreAppRuntimePackageVersion to MicrosoftAspNetCoreAppRuntimePackageVersionBuilt is what we've been using for dotnet6 and dotnet7 Alpine package. In fact, it is what I backported torelease/6.0.1xx in dotnet/installer#14816. Has this been identified as an adequate fix?

[tmds]

changing the name of MicrosoftAspNetCoreAppRuntimePackageVersion to MicrosoftAspNetCoreAppRuntimePackageVersionBuilt

That's fine as a patch in the packages. I renamed the property to stop it from being overridden. The proper fix is to stop overriding it, which is what @crummel was doing in dotnet/installer#14769. We should take another look at that and figure out why it didn't work. I think it may be because pre-PR installer stuff gets used which causes the version still to be overridden.

[tmds]

I think it may be because pre-PR installer stuff gets used which causes the version still to be overridden.

When I extract PackageVersions.props from Private.SourceBuilt.Artifacts.7.0.100-rc.2.tar.gz, that has a PackageVersions.props. This file contains the MicrosoftAspNetCoreAppRuntimePackageVersion that were added when the tar file was generated.

So we need to remove all <ExtraPackageVersionPropsPackageInfo Include="MicrosoftAspNetCoreAppRuntimePackageVersion" from installer, and also remove the MicrosoftAspNetCoreAppRuntimePackageVersion from PackageVersions.props in the artifacts tar.gz, and then things should be working.

We can rename the property as my patch did, and what is included in dotnet/installer#14816.

And once artifacts tar.gz files no longer include MicrosoftAspNetCoreAppRuntimePackageVersion, we should be able to change the name back. Maybe we only do that for 8.0.

@crummel @MichaelSimons how does that sound to you?

[tmds]

dotnet/installer#14938 implements it. It combines my patch with @crummel's PR.
For 8.0 we should be able to change back to the original property name (MicrosoftAspNetCoreAppRuntimePackageVersion) when the Artifacts tarball no longer includes this name.