CLRInformationVersion contains the buildNumber or version suffix

[Anipik]

Description

In one of the tests in the coreclr we are matching the Informational version with ClrProductVersion. We are always including the build number in the InformationalVersion of an assembly but we dont do that for clr Product verison which is leading to the test failure.

This is reproing when we enable the stable bits flag.

#60482
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-60482-merge-8ea93e618a844dc487/PayloadGroup0/1/console.cff9299f.log?sv=2019-07-07&se=2021-11-07T04%3A53%3A55Z&sr=c&sp=rl&sig=IBViKwqkqcGqRgvTnM98zHnFWUGmK6yIVdcwvTBcAU8%3D

// failure test https://github.com/dotnet/runtime/blob/release/6.0/src/tests/tracing/eventpipe/processinfo2/processinfo2.cs#L244

is this a test failure or a bug ?

cc @hoyosjs @ericstj @ViktorHofer @jeffschwMSFT

Reproduction Steps

Turn on stable flag

Expected behavior

  Finished:    tracing.eventlistener.XUnitWrapper
  Discovering: tracing.eventpipe.XUnitWrapper (method display = ClassAndMethod, method display options = None)
  Discovered:  tracing.eventpipe.XUnitWrapper (found 14 test cases)
  Starting:    tracing.eventpipe.XUnitWrapper (parallel test collections = on, max threads = 4)
    tracing/eventpipe/processinfo2/processinfo2/processinfo2.sh [FAIL]
      Unhandled exception. System.Exception: ClrProductVersion must match. Expected: "6.0.0-ci", received: "6.0.0"
         at Tracing.Tests.Common.Utils.Assert(Boolean predicate, String message)
         at Tracing.Tests.ProcessInfoValidation.ProcessInfoValidation.Main(String[] args)
      apply_reg_state: ip and cfa unchanged; stopping here (ip=0x7fa83a7aac)
      /root/helix/work/workitem/e/tracing/eventpipe/processinfo2/processinfo2/processinfo2.sh: line 374:  1677 Aborted                 (core dumped) $LAUNCHER $ExePath "${CLRTestExecutionArguments[@]}"
      
      Return code:      1
      Raw output file:      /root/helix/work/workitem/uploads/Reports/tracing.eventpipe/processinfo2/processinfo2/processinfo2.output.txt
      Raw output:
      BEGIN EXECUTION
      /root/helix/work/correlation/corerun -p System.Reflection.Metadata.MetadataUpdater.IsSupported=false processinfo2.dll ''
        0.0s: Test PID: 1677
        0.7s: Wrote: { Header={ Magic=System.Byte[]; Size=20; CommandSet=4; CommandId=4; Reserved=0 }; }
        1.1s: Received: <omitted>
        1.1s: Total size of Payload = 358 bytes
        1.1s: pid: 1677
        1.1s: runtimeCookie: 29acf19e-51a3-4d25-b80b-1bdba44bbc91

Actual behavior

I am not sure if these versions should match or its just a test failure

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[hoyosjs]

@josalem

[jeffschwMSFT]

@agocke / @trylek can y'all take a look?

[trylek]

@Anipik - have you got any idea whether the failure started to repro recently or whether the issue was previously just hidden for some reason? Initially I've been thinking that perhaps my CoreCLR script changes may have caused this but I no longer think that is likely as I understand your explanation so that the CLRProductVersion suffix is missing in the CoreCLR product build and I have only modified the test build scripts, not the product build. I noticed the product version suffix in the past, I'll try to remind myself how it gets handled in the CoreCLR build.

[Anipik]

i dont think its regression, I am not sure why we are comparing the coreclrProductVersion with the informationalVersion of Executing assembly ? are they both the same thing ?

I have seen the version suffix being included in the informational version for coreclr for a while now.

[josalem]

This test is comparing the value retrieved as part of the ProcessInfo2 Diagnostics IPC command to the value retrieved from the Assembly.

The Diagnostics Server gets this information from

runtime/src/coreclr/inc/clrversion.h

Line 19 in 57bfe47

#define CLR_PRODUCT_VERSION QUOTE_MACRO(RuntimeProductVersion)

and the test compares this value to https://github.com/dotnet/runtime/blob/release/6.0/src/tests/tracing/eventpipe/processinfo2/processinfo2.cs#L244

Based on the test failure, it looks like the assembly version had the -ci suffix, while the raw RuntimeProductVersion does not.

The value of RuntimeProductVersion gets set here:

runtime/eng/versioning.targets

Lines 170 to 186 in e544ccf

	<_RuntimeVersionFileContents>
	<![CDATA[
	#define RuntimeAssemblyMajorVersion $(MajorVersion)
	#define RuntimeAssemblyMinorVersion $(MinorVersion)
	#define RuntimeFileMajorVersion $(FileVersion.Split('.')[0])
	#define RuntimeFileMinorVersion $(FileVersion.Split('.')[1])
	#define RuntimeFileBuildVersion $(FileVersion.Split('.')[2])
	#define RuntimeFileRevisionVersion $(FileVersion.Split('.')[3])
	#define RuntimeProductMajorVersion $(Version.Split(".-")[0])
	#define RuntimeProductMinorVersion $(Version.Split(".-")[1])
	#define RuntimeProductPatchVersion $(Version.Split(".-")[2])
	#define RuntimeProductVersion $(Version)
	]]>
	</_RuntimeVersionFileContents>

This test has been in for a long while now and hasn't been failing as far as I'm aware. When the test went in, I recall noticing both values had suffixes on them.

Whether this is a bug or not depends on the consensus of whether those two values should be the same. If they shouldn't be the same, then this is a test bug where we comapre to the wrong value. The value being returned by the diagnostic server is what is being tested, and that appears to be correct but compared to an incorrect value. If they should be the same, then it's an actual test failure.

[trylek]

Thanks John for your detailed explanation. Based on your clarification it's possible it's indeed a bug I made in the test build script refactoring where perhaps my changes inadvertently stopped propagating the suffix to the test build. I'm taking a deeper look.

[trylek]

I'm able to repro the problem locally after Anirudh kindly explained to me how to enable the stable bits flag. It's basically an inconsistency between coreclr.dll and System.Private.CoreLib.dll - when the stable bits are on, coreclr.dll stores CLR_PRODUCT_VERSION as "7.0.0" (in dotnet/runtime main) whereas System.Private.CoreLib.dll has "7.0.0-dev" in its AssemblyInformationalVersionAttribute. Based on the previous clarifications I assume that the bug is that "System.Private.CoreLib.dll" doesn't switch its version to just "7.0.0" in the stable mode; @Anipik / @mmitche, can you please confirm if this assumption is correct?

[trylek]

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

Line 48 in efa6163

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

[Anipik]

Based on the previous clarifications I assume that the bug is that "System.Private.CoreLib.dll" doesn't switch its version to just "7.0.0" in the stable mode; @Anipik / @mmitche, can you please confirm if this assumption is correct?

i think thats correct.

[Anipik]

I think InformationalVersion might be getting set to $(Version) by default.

[Anipik]

https://github.com/dotnet/runtime/blob/main/Directory.Build.targets#L25

[trylek]

Nice, thanks Anirudh for looking that up! I leave it up to those more familiar with our release versioning to decide whether to take my targeted fix for CoreLib or fix this globally in the script you quoted.

[Anipik]

Its being correctly set in libraries subset, I think bigger issue here will be to fix the versionSuffix. Not sure why its not getting set properly but it might be causing some other side effects. (i know about the ones in pkgprojs but there might be more hidden)

[trylek]

I ran the clr.corelib build with the binlog switch and I'm seeing the VersionSuffix is set at line 136 in the script Version.BeforeCommonTargets.targets in the Arcade SDK:

    <!--
      Some projects want to remain producing prerelease packages even if we are doing a final stable build because
      they don't ship or aren't ready to ship stable. Those projects can set SuppressFinalPackageVersion property to true.

      TODO: BlockStable is obsolete. Remove once repos update. https://github.com/dotnet/arcade/issues/1213
    -->
    <VersionSuffix Condition="'$(BlockStable)' == 'true' or '$(SuppressFinalPackageVersion)' == 'true'">$(_PreReleaseLabel)$(_BuildNumberLabels)</VersionSuffix>

So far I haven't found the script in the Arcade or SDK GitHub repo though, maybe it's coming from the VS CLI somehow (but that's kind of weird the same error happens on Unix).

[trylek]

Apparently we do set SuppressFinalPackageVersion in a couple places, I'm especially wondering about the version in

runtime/src/installer/prepare-artifacts.proj

Line 29 in efa6163

<SuppressFinalPackageVersion>true</SuppressFinalPackageVersion>

[trylek]

OK, so for CoreLib it gets set here,

runtime/Directory.Build.props

Line 271 in efa6163

<SuppressFinalPackageVersion Condition="'$(SuppressFinalPackageVersion)' == '' and ($(IsExperimentalAssembly) or $(IsPrivateAssembly))">true</SuppressFinalPackageVersion>

as the IsPrivateAssembly flag is set.

[trylek]

In light of this I'm wondering whether the appropriate fix is to remove the IsPrivateAssembly flags from CoreLib and / or from this conditional check, the final call here is far beyond my current expertise in this field as I have no idea what other purposes / semantics the IsPrivateAssembly flag has.

[hoyosjs]

Looks like we got tripped by a blanket assertion to prevent stabilizing assets that are "implementation details":

runtime/Directory.Build.props

Lines 268 to 272 in efa6163

	<IsExperimentalAssembly>$(MSBuildProjectName.Contains('Experimental'))</IsExperimentalAssembly>
	<IsPrivateAssembly>$(MSBuildProjectName.Contains('Private'))</IsPrivateAssembly>
	<!-- Experimental packages should not be stable -->
	<SuppressFinalPackageVersion Condition="'$(SuppressFinalPackageVersion)' == '' and ($(IsExperimentalAssembly) or $(IsPrivateAssembly))">true</SuppressFinalPackageVersion>
	<IsShippingAssembly Condition="$(IsExperimentalAssembly)">false</IsShippingAssembly>

For 6.0 the most targeted thing might be setting SuppressFinalPackageVersion and IsPrivateAssembly to false for SPC.

[DamianEdwards]

Almost certainly cause of this regressing again #45812

[akoeplinger]

This was fixed by #60572