Fix setting of the DotNetCliVersion for ASP.NET Core runtime

[premun]

• This issue is blocking
• This issue is causing unreasonable pain

Context
This is a follow-up for #6975.

Arcade is now dogfooding the .NET SDK and was updated to .NET 6 Preview 1 (#6975).
In DotNetCli.props, we set the version to the ASP.NET Core runtime's version that is contained in this SDK's full version.

Green is the SDK's full version, yellow is the runtime contained inside:

The problem
We construct the URL to get the runtime package like this:

arcade/src/Microsoft.DotNet.Helix/Sdk/FindDotNetCliPackage.cs

Line 89 in 15246f4

"aspnetcore-runtime" => $"{DotNetCliAzureFeed}/aspnetcore/Runtime/{Version}/aspnetcore-runtime-{Version}-{Runtime}.{extension}",

This URL is getting 404 as we put in the version twice. I am not sure what the correct link should be.
This worked before because everything was 5.0.100

What is blocked
Currently, we won't be able to update the dotnet SDK version in the global.json.
To unblock ourselves, we have hardcoded the ASP.NET Core runtime's version to be the major one in so it should work for now but we need a stable flexible solution.

The hardcode places are in DotNetCli.props and XHarnessRunner.targets as per #6975.

[premun]

The version we hardcode in can be also found dynamically using something like:

@(KnownFrameworkReference->WithMetadataValue('TargetingPackName', 'Microsoft.AspNetCore.App.Ref')->Metadata('DefaultRuntimeFrameworkVersion'))

but not sure this is the right solution..

[garath]

@alexperovich Do you have any insight here?

[alexperovich]

The DotNetCli.props file has logic that figures out what version of the sdk we are building with and uses that as the defaults. The xharness stuff is doing the same thing, but it appears to be defaulting to the aspnetcore runtime package. If the default in the xharness targets is supposed to be the aspnetcore runtime package, then it needs to have code in it to figure out what the matching version is. I'm confused why the xharness stuff defaults that aspnetcore-runtime package though.

[premun]

@alexperovich this is not about XHarness at all, Arcade breaks on its own, XHarness just happens to use the same version as DotNetCli.props. Please read the thread or how this happened in #6975.

Nothing to do with #10420

[alexperovich]

It is a bug in the xharness code though. This file https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Helix/Sdk/tools/xharness-runner/XHarnessRunner.targets#L7 incorrectly assumes that setting DotNetCliPackageType to aspnetcore-runtime will work with the default version. There is a nasty workarround 2 lines later, but this code needs to be updated to figure out what the correct version is if it is going to change the package type.

[premun]

@alexperovich please forget about XHarness for the moment, that is just trying to fix a symptom of an issue that already happens earlier in DotNetCli.props.

The problem this issue is about in DotNetCli.props. Originally, this file had this:

<DotNetCliVersion Condition=" '$(DotNetCliVersion)' == '' AND '$(DotNetCliPackageType)' == 'aspnetcore-runtime' ">$(BundledNETCoreAppPackageVersion)</DotNetCliVersion>

This doesn't work for preview versions of .NET as these just don't match between ASP and the full SDK which is where this property comes from. So anyone trying to get the CLI installed will fail (XHarness is just one customer of the Arcade SDK in this scenario and we can have possibly other customers who might break the same way XHarness got broken).

I hardcoded there a specific version so that Viktor can branch out runtime with a newer version, however the problem really lies in the way we structure the URL to get the package. The URL should contain the full version and then the included runtime version but we just insert this one version twice (this is shown above).

So I am not sure if the real fix is in reaching into the KnownFrameworkReference and setting some existing full version or whether we need to start constructing the link in a different way so that we don't get 404s. But definitely the issue is in the FindDotNetCli or in DotNetCli.props.

[alexperovich]

Then the change that added aspnetcore-runtime to DotNetCli.props was wrong, it should find the correct version in there.

[premun]

Yes, and that should be the subject of this issue.

[alexperovich]

The correctly solution here is to have the Microsoft.NETCoreSdk.BundledVersions.props file contain the asp.net core runtime version as a property in it. Then we can read that property. Until that happens we can't really do much more than hard-code the version.

[premun]

I already spoke with Daniel Plaisted about this and he suggested to not float the property from there, not accessing KnownFramewokReference but to use the install scripts which use the releases.json file.

Other option - close to what you're proposing - is to reach to the KnownFrameWorkReference like I showed above, since these are generated from GenerateBundledVersions.targets.

The problem is that even if we get the runtime version there, the way we construct the download link won't work - the link should probably contain the full version and then the runtime version (not sure about this hence this issue) but we just insert the runtime version into the URL template twice.

[alexperovich]

using the install scripts is a non-starter, they are unstable and the usage we have right now of them is costing tons of money because they aren't cached at all. If there is a way to use the scripts to get a download url instead of actually doing the installation then we could probably do that, but I have never seen that functionality.

[premun]

@alexperovich maybe we could use the aka.ms links that Michelle created to get the right version?

[premun]

@garath can you please re-triage this? It was originally mistriaged into the iOS/Android epic but we cleared that up

[premun]

Looks like there's some new links we can use?

https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=6.0.0-preview.2.21154.6&arch=x64&rid=osx.10.15-x64

[markwilkie]

@premun - is this really blocking? What are the next steps?

(it's been lingering so want to make sure things are sorted)

[premun]

Right, this is not blocking anymore as we have a workaround. But it requires attention for sure. For example, when we bumped to 6.0 preview 2 from preview 1, we didn't update the hardcoded version. So whoever would use FindDotNetCli (which is used for example in Helix SDK) and would not specify a version, would get an older version (quite silently).

So to not forget every 2 months when we bump SDKs, we should resolve this.

I am, however, not sure what is the right fix and I am currently not able to work on this. Not sure whether this should go though and whether there is some "Arcade Phase 2" epic where we put this?

[markwilkie]

@adiaaida / @mmitche - is using the new aka links something that makes sense here?

If doing the work is "cheap", then it arguably fits into FR (cc'ing @ilyas1974 to check my work) because it breaks builds. However, if it's more expensive, then it'll likely have to go into our technical debt backlog.

[michellemcdaniel]

Premek and I discussed this last week, and I believe the conclusion we came to was that, no, the aka.ms links won't work, because we need specific versions that are related to the sdk being used. Or something like that. My memory is a little fuzzy on exactly why we decided that the aka.ms links weren't the right solution here.

[MattGal]

[Async Triage] I got nothin'.

[riarenas]

Since we don't really have any good alternatives so far, would it at least make sense to add a test to Arcade that would fail in the same way this failed in consumer repos? that way we won't be able to update the .NET sdk in arcade until we've dealt with updating the version manually.

If we go with that approach it seems like a good candidate for FR

[garath]

@ilyas1974 What do you think? Good candidate for FR?

[premun]

@riarenas Since we don't really have any good alternatives so far, would it at least make sense to add a test to Arcade that would fail in the same way this failed in consumer repos? that way we won't be able to update the .NET sdk in arcade until we've dealt with updating the version manually.

Yes, this is a nice way to deal with this. We need the test to not only that things work but that we have the same major/minor/whatever of the runtime since it happened we were on preview 2 for about 2 weeks but this string was still hardcoded to preview 1. So might get a little tricky.

[ilyas1974]

Adding a test for this condition is something that we can handle within FR.