Generalise how package version isn't stabilized for dev/preview stages

[RussKie]

Follow up on #5632

Microsoft Reviewers: Open in CodeFlow

[joperezr]

Thank you so much for the help with this @RussKie! this is great. Given these properties could be impacted by race condition, can we also add a target that runs after GenerateNuspec that validates that version contains a prerelease label?

[joperezr]

Also, I'm assuming you did, but in case not you should be able to validate if this works by running build.cmd -ci -pack /p:OfficialBuildId=20241212.01

[RussKie]

build.cmd -ci -pack /p:OfficialBuildId=20241212.01

This command doesn't work (don't ask me how I know).

 D:\Development\dotnet-extensions\.dotnet\sdk\9.0.100-rtm.24479.2\NuGet.RestoreEx.targets(19,5): error :
      '9.1.0-preview.1.24612.01' is not a valid version string. (Parameter 'value')

The following do:

.\build.cmd -ci -restore
.\build.cmd -ci -build /p:OfficialBuildId=20241120.4
.\build.cmd -ci -pack /p:Restore=false /p:Build=false /p:OfficialBuildId=20241120.4

I added a pack-time check to ensure the version suffix is correct:

[RussKie]

Version suffix calculations are performed in Version.BeforeCommonTargets.targets, which too late for the functionality placed in eng/MSBuild/ProjectStaging.targets
Arguably, we could add eng/MSBuild/ProjectStaging.props, but it feels more logical to have versioning-related configurations in one place (which is here).

[joperezr]

Will Stage be defined this early though?

[joperezr]

Oh, I see it will because you are creating a Directory.Build.props everywhere to make sure you control the order of imports. While that would work, I'd argue that we are probably defeating the point of the benefit of the change. The initial motivation was to simplify things, and reducing 2 lines in projects (setting the stage, and setting suppressFinalVersion) to a single one, but this is instead now making us having to add new files per project, and it also means that now you need to know to look into this file to see if a package is stable or not and also to know what is the stage.

Thoughts?

[joperezr]

Probably an even more meta question (sorry for having this discussion in your PR) is: do we even really need package stages? What benefit are we getting out of them? if it is literally just that the description has a few other words as a prefix, then perhaps we should just have stable and unstable packages, and adjust the descriptions of packages accordingly. That way, we can go back to having a single property in the package that marks it's "stableness"

[RussKie]

Probably an even more meta question (sorry for having this discussion in your PR) is: do we even really need package stages? What benefit are we getting out of them? if it is literally just that the description has a few other words as a prefix, then perhaps we should just have stable and unstable packages, and adjust the descriptions of packages accordingly. That way, we can go back to having a single property in the package that marks it's "stableness"

The stages drive the test coverage requirements

extensions/eng/MSBuild/ProjectStaging.targets

Lines 19 to 23 in c08e5ac

	<!-- Produce errors if we don't have all the right property values for normal stage -->
	<Target Name="_CheckNormalStageProps" Condition="'$(Stage)' == 'normal'" BeforeTargets="Build">
	<Error Condition="'$(MinCodeCoverage)' != 'n/a' AND ('$(MinCodeCoverage)' &lt; 75)" Text="MinCodeCoverage property must be >= 75 for normal stage." />
	<Error Condition="'$(MinMutationScore)' != 'n/a' AND ('$(MinMutationScore)' &lt; 50)" Text="MinMutationScore property must be >= 50 for normal stage." />
	</Target>

[joperezr]

Right but that could easily just be controlled without stages, right? You can basically just change that logic to read whether a package is stable or not via supressfinalpackageversion

[joperezr]

NIT: No need to unset it here as that is the default already.

Suggested change

<SuppressFinalPackageVersion />

[RussKie]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dotnet-comment-bot]

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	75	86
Microsoft.Extensions.AI.Ollama	0	80
Microsoft.Extensions.AI.AzureAIInference	0	77
Microsoft.Extensions.AI.Abstractions	0	83
Microsoft.Extensions.Diagnostics.Probes	70	76
Microsoft.Extensions.AI.OpenAI	0	66
Microsoft.Extensions.AI	0	83

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=879138&view=codecoverage-tab