[release/9.0] Stable branding for GA by carlossanlop · Pull Request #108899 · dotnet/runtime

carlossanlop · 2024-10-15T17:49:15Z

No description provided.

dotnet-policy-service · 2024-10-15T17:49:41Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

carlossanlop · 2024-10-15T17:53:34Z

The emsdk stable branding deps flow PR is #108898

mmitche · 2024-10-15T22:12:44Z

@carlossanlop @akoeplinger Test is failing on check a stable branding string. emsdk is merged but I don't see any failrues here that are a result of not flowing emsdk bits yet. Test failure is here:

#if STABILIZE_PACKAGE_VERSION
            // a stabilized version looks like 8.0.0
            Assert.DoesNotContain("-", version);
            Assert.True(Version.TryParse(version, out Version _));
#else
            // a non-stabilized version looks like 8.0.0-preview.5.23280.8 or 8.0.0-dev
            Assert.Contains("-", version);
            var versionNumber = version.Substring(0, version.IndexOf("-"));
            Assert.True(Version.TryParse(versionNumber, out Version _));
#endif

carlossanlop · 2024-10-15T22:38:36Z

The test is expecting to not see the "-" character in the version, but PR CI results are (AFAIK) always expected to have the "-ci" suffix added to them, and local builds are expected to have the "-dev" suffix.

[19:03:40] info: [FAIL] System.Runtime.InteropServices.RuntimeInformationTests.DescriptionNameTests.VerifyFrameworkDescriptionContainsCorrectVersion
[19:03:40] info: Assert.DoesNotContain() Failure: Sub-string found
[19:03:40] info:               ↓ (pos 5)
[19:03:40] info: String: "9.0.0-ci"
[19:03:40] info: Found:  "-"
[19:03:40] info:    at System.Runtime.InteropServices.RuntimeInformationTests.DescriptionNameTests.VerifyFrameworkDescriptionContainsCorrectVersion()
[19:03:40] info:    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[19:03:40] info:    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[19:03:40] info: Finished:    System.Runtime.InteropServices.RuntimeInformation.Tests.dll

I suspect the test will pass in rolling builds where "-ci" or "-dev" is not added.

Fixing the test by making it more specific.

…fic in its checks when stable.

mmitche · 2024-10-16T00:18:36Z

The test is expecting to not see the "-" character in the version, but PR CI results are (AFAIK) always expected to have the "-ci" suffix added to them, and local builds are expected to have the "-dev" suffix.

[19:03:40] info: [FAIL] System.Runtime.InteropServices.RuntimeInformationTests.DescriptionNameTests.VerifyFrameworkDescriptionContainsCorrectVersion
[19:03:40] info: Assert.DoesNotContain() Failure: Sub-string found
[19:03:40] info:               ↓ (pos 5)
[19:03:40] info: String: "9.0.0-ci"
[19:03:40] info: Found:  "-"
[19:03:40] info:    at System.Runtime.InteropServices.RuntimeInformationTests.DescriptionNameTests.VerifyFrameworkDescriptionContainsCorrectVersion()
[19:03:40] info:    at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[19:03:40] info:    at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
[19:03:40] info: Finished:    System.Runtime.InteropServices.RuntimeInformation.Tests.dll

I suspect the test will pass in rolling builds where "-ci" or "-dev" is not added.

Fixing the test by making it more specific.

Hmm, the shipping assets maybe should not have -ci. At least, when i build winforms stable with -ci, shipping packages are 9.0.0. Maybe this is just specific to the framework description? We def need to verify on an official build.

dkurepa · 2024-10-16T07:30:02Z

We didn't see any -ci suffix in 9.0.0 test build https://dev.azure.com/dnceng/internal/_build/results?buildId=2531164&view=results. So I guess it's just a PR build?

akoeplinger · 2024-10-16T08:53:27Z

...untime/tests/System.Runtime.InteropServices.RuntimeInformation.Tests/DescriptionNameTests.cs

+            // A stabilized version looks like 9.0.0
+            // In local builds, the version looks like 9.0.0-dev, and in PRs CI, the version looks like 9.0.0-ci.
+            // Both are acceptable.
+            if (!version.Contains("-dev") && !version.Contains("-ci"))


This is not correct. The FrameworkDescription should always contain the stabilized version irrespective of the build environment.

This is a regression caused by #102164. Before it was reading the AssemblyInformationalVersion from System.Private.CoreLib.dll which is 9.0.0*, now it's reading it from the generators assembly and that contains 9.0.0-dev (basically what Stephen noticed in #102164 (comment))

* this is because we override it here:

runtime/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj

Lines 46 to 48 in 588bc45



<InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion>

<InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>

I pushed a fix. There is potential to clean up overriding the version in SPC.dll now I think but that can be done separately.

…re specific in its checks when stable." This reverts commit 2dd6cd4.

ViktorHofer · 2024-10-16T09:17:05Z

Should these lines now get removed (at least in main when we forward port this change)?

runtime/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj

Lines 47 to 49 in ee99d7c

    
           <!-- Override InformationalVersion during servicing as it's returned via public api. --> 
        
           <InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion> 
        
           <InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>

runtime/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj

Lines 34 to 36 in ee99d7c

    
           <!-- Override InformationalVersion during servicing as it's returned via public api. --> 
        
           <InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion> 
        
           <InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>

runtime/src/coreclr/nativeaot/Directory.Build.props

Lines 28 to 30 in ee99d7c

    
            <!-- Override InformationalVersion during servicing as it's returned via public api. -->  
        
           <InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion>  
        
           <InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>

akoeplinger · 2024-10-16T09:18:18Z

Yeah I mentioned it above.

mmitche · 2024-10-16T14:13:40Z

Looks like some additional test failures that may be test issues rather than product issues

carlossanlop · 2024-10-16T16:29:08Z

Build Analysis says most failures are known. There is one in Installer Host Activation that I have not seen before. @NikolaMilosavljevic does this look familiar? https://dev.azure.com/dnceng-public/public/_build/results?buildId=844459&view=logs&j=34fcf1e9-20e8-52c9-c21d-1ed3608ac124&t=3f5386ba-6526-5d7f-4342-42281a824185&l=489

Edit: I re-ran Build Analysis and the failure was captured byhttps://github.com//issues/108921 but I am worried it might be overreacting and capturing too much.

mmitche · 2024-10-16T16:42:51Z

Those are the ones i think are interesting.

carlossanlop · 2024-10-16T17:08:49Z

Those are the ones i think are interesting.

The failure is coming from the build output argument passed to this method:

runtime/src/mono/wasi/Wasi.Build.Tests/BuildTestBase.cs

Lines 345 to 355 in 9305d7f

    
           static void AssertRuntimePackPath(string buildOutput, string targetFramework) 
        
           { 
        
               var match = s_runtimePackPathRegex.Match(buildOutput); 
        
               if (!match.Success || match.Groups.Count != 2) 
        
                   throw new XunitException($"Could not find the pattern in the build output: '{s_runtimePackPathPattern}'.{Environment.NewLine}Build output: {buildOutput}"); 
        
               string expectedRuntimePackDir = s_buildEnv.GetRuntimePackDir(targetFramework); 
        
               string actualPath = match.Groups[1].Value; 
        
               if (string.Compare(actualPath, expectedRuntimePackDir) != 0) 
        
                   throw new XunitException($"Runtime pack path doesn't match.{Environment.NewLine}Expected: '{expectedRuntimePackDir}'{Environment.NewLine}Actual:   '{actualPath}'"); 
        
           }

Log: https://helixre107v0xd1eu3ibi6ka.blob.core.windows.net/dotnet-runtime-refs-pull-108899-merge-651f2a552cc9403fa1/Workloads-ST-Wasm.Build.Tests/1/console.8c4328cb.log?helixlogtype=result

  Discovered:  Wasi.Build.Tests (found 11 of 13 test cases)
  Starting:    Wasi.Build.Tests (parallel test collections = on [2 threads], stop on fail = off)
    Wasi.Build.Tests.WasiTemplateTests.ConsoleBuildAndRunAOT(config: "Debug", singleFileBundle: False) [STARTING]
    Wasi.Build.Tests.SdkMissingTests.SimpleBuildDoesNotFail(config: "Debug") [STARTING]
    Wasi.Build.Tests.SdkMissingTests.SimpleBuildDoesNotFail(config: "Debug") [FAIL]
      Runtime pack path doesn't match.
      Expected: '/root/helix/work/workitem/e/dotnet-latest/packs/Microsoft.NETCore.App.Runtime.Mono.wasi-wasm/9.0.0-ci'
      Actual:   '/root/helix/work/workitem/e/dotnet-latest/packs/Microsoft.NETCore.App.Runtime.Mono.wasi-wasm/9.0.0'
      Stack Trace:
        /_/src/mono/wasi/Wasi.Build.Tests/BuildTestBase.cs(354,0): at Wasm.Build.Tests.BuildTestBase.AssertRuntimePackPath(String buildOutput, String targetFramework)
        /_/src/mono/wasi/Wasi.Build.Tests/BuildTestBase.cs(259,0): at Wasm.Build.Tests.BuildTestBase.BuildProject(BuildArgs buildArgs, String id, BuildProjectOptions options)
        /_/src/mono/wasi/Wasi.Build.Tests/SdkMissingTests.cs(70,0): at Wasi.Build.Tests.SdkMissingTests.BuildWithInvalidSdkPath(String config, String extraProperties, Boolean publish, Boolean expectSuccess)
        /_/src/mono/wasi/Wasi.Build.Tests/SdkMissingTests.cs(58,0): at Wasi.Build.Tests.SdkMissingTests.SimpleBuildDoesNotFail(String config)
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
           at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
      Output:
        [] Executing (Captured Output) - /root/helix/work/workitem/e/dotnet-latest/dotnet new wasiconsole  -  in pwd /root/helix/work/workitem/e/wbt artifacts/Debug_qpld2yyi_a5x
        [] The template "Wasi Console App" was created successfully.
        []

@lewing @akoeplinger

carlossanlop · 2024-10-16T17:14:53Z

We generate the build out of this portion of the code, which we then pass to AssertRuntimePackPath. I wonder if we need to pass an environment variable to indicate that we are using a stabilizing build.

runtime/src/mono/wasi/Wasi.Build.Tests/BuildTestBase.cs

Lines 243 to 253 in 9305d7f

    
           var envVars = s_buildEnv.EnvVars; 
        
           if (options.ExtraBuildEnvironmentVariables is not null) 
        
           { 
        
               envVars = new Dictionary<string, string>(s_buildEnv.EnvVars); 
        
               foreach (var kvp in options.ExtraBuildEnvironmentVariables!) 
        
                   envVars[kvp.Key] = kvp.Value; 
        
           } 
        
           envVars["NUGET_PACKAGES"] = _nugetPackagesDir; 
        
           if (UseWBTOverridePackTargets && s_buildEnv.IsWorkload) 
        
               envVars["WBTOverrideRuntimePack"] = "true"; 
        
           result = AssertBuild(sb.ToString(), id, expectSuccess: options.ExpectSuccess, envVars: envVars);

Edit: I just realized the problem is not in the actual but in the expected string, which is coming from:

runtime/src/mono/wasm/Wasm.Build.Tests/Common/BuildEnvironment.cs

Lines 170 to 175 in def5e32

    
           public string GetRuntimePackDir(string tfm = BuildTestBase.DefaultTargetFramework, RuntimeVariant runtimeType = RuntimeVariant.SingleThreaded) 
        
               => Path.Combine(WorkloadPacksDir, 
        
                       runtimeType is RuntimeVariant.SingleThreaded 
        
                           ? $"Microsoft.NETCore.App.Runtime.Mono.{DefaultRuntimeIdentifier}" 
        
                           : $"Microsoft.NETCore.App.Runtime.Mono.multithread.{DefaultRuntimeIdentifier}", 
        
                       GetRuntimePackVersion(tfm));

It seems we are getting the value of the suffix from an environment variable called RUNTIME_PACK_VER9. I wonder if we need to indicate that we're working with a stabilized build in these tests as well.

carlossanlop · 2024-10-16T18:45:05Z

Opened two issues to get the failures fixed separately:

[release/9.0][wasi/wasm] SimpleBuildDoesNotFail compares with the wrong expected value #108938 - This test fails because the expected string is wrong, but the actual string is correct.
[release/9.0][wasi/wasm] Expected the multithreading runtime pack to be available when running on CI (after branch stabilization) #108937 - This test fails in PRs because we're running in a ContinuousIntegrationBuild and the exception is thrown when that is true

I'll merge this to unblock stable branding.

@lewing @akoeplinger

carlossanlop · 2024-10-17T03:55:18Z

Good news: The release/9.0 rolling build that included this change did not hit the failures reported above: https://dev.azure.com/dnceng/internal/_build/results?buildId=2562281&view=results

So they indeed are only happening in PRs.

[release/9.0] Stable branding for GA

c8b2715

carlossanlop added the area-Infrastructure label Oct 15, 2024

carlossanlop added this to the 9.0.0 milestone Oct 15, 2024

carlossanlop requested review from ViktorHofer, dkurepa, ericstj and mmitche October 15, 2024 17:49

carlossanlop self-assigned this Oct 15, 2024

carlossanlop added the Servicing-approved Approved for servicing release label Oct 15, 2024

ViktorHofer approved these changes Oct 15, 2024

View reviewed changes

carlossanlop mentioned this pull request Oct 15, 2024

[release/9.0] Update dependencies from dotnet/emsdk #108898

Merged

build-analysis bot mentioned this pull request Oct 15, 2024

[Wasm][Test Failure] ConsolePublishAndRun(config: "Release", aot: True, relinking: False) #106162

Closed

Merge remote-tracking branch 'dotnet/release/9.0' into StableBranding9

1d36cdb

Make VerifyFrameworkDescriptionContainsCorrectVersion test more speci…

2dd6cd4

…fic in its checks when stable.

dkurepa approved these changes Oct 16, 2024

View reviewed changes

akoeplinger requested changes Oct 16, 2024

View reviewed changes

akoeplinger added 2 commits October 16, 2024 11:05

Revert "Make VerifyFrameworkDescriptionContainsCorrectVersion test mo…

19f895d

…re specific in its checks when stable." This reverts commit 2dd6cd4.

Fix InformationalVersion in corelib generators

ee99d7c

akoeplinger approved these changes Oct 16, 2024

View reviewed changes

build-analysis bot mentioned this pull request Oct 16, 2024

DebuggerTests other than WasmTestOnChrome timeouts in CI #108921

Closed

This was referenced Oct 16, 2024

[release/9.0][wasi/wasm] Expected the multithreading runtime pack to be available when running on CI (after branch stabilization) #108937

Closed

[release/9.0][wasi/wasm] SimpleBuildDoesNotFail compares with the wrong expected value #108938

Closed

carlossanlop merged commit d398172 into dotnet:release/9.0 Oct 16, 2024

carlossanlop deleted the StableBranding9 branch October 16, 2024 18:45

github-actions bot locked and limited conversation to collaborators Nov 16, 2024

	<!-- Override InformationalVersion during servicing as it's returned via public api. -->
	<InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion>
	<InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>

Conversation

carlossanlop commented Oct 15, 2024

Uh oh!

dotnet-policy-service bot commented Oct 15, 2024

Uh oh!

carlossanlop commented Oct 15, 2024

Uh oh!

mmitche commented Oct 15, 2024

Uh oh!

carlossanlop commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmitche commented Oct 16, 2024

Uh oh!

dkurepa commented Oct 16, 2024

Uh oh!

akoeplinger Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akoeplinger Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

ViktorHofer commented Oct 16, 2024

Uh oh!

akoeplinger commented Oct 16, 2024

Uh oh!

mmitche commented Oct 16, 2024

Uh oh!

carlossanlop commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmitche commented Oct 16, 2024

Uh oh!

carlossanlop commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlossanlop commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlossanlop commented Oct 16, 2024

Uh oh!

carlossanlop commented Oct 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

carlossanlop commented Oct 15, 2024 •

edited

Loading

akoeplinger Oct 16, 2024 •

edited

Loading

carlossanlop commented Oct 16, 2024 •

edited

Loading

carlossanlop commented Oct 16, 2024 •

edited

Loading

carlossanlop commented Oct 16, 2024 •

edited

Loading