Add a System.Runtime.Experimental package so users can still use the generic math preview

[tannergooding]

This re-enables the generic math feature by providing a System.Runtime.Experimental package so it can still be used/referenced via explicit opt-in without disrupting other tools/compilers, such as C++/CLI.

CC. @jeffhandley, @ericstj, @ViktorHofer, @terrajobst, @davidwrighton, @MadsTorgersen

[ghost]

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

[tannergooding]

I've marked this draft as we will need to version the experimental package "higher" than the regular package and am still trying to determine the appropriate way to do that.

[ericstj]

draft as we will need to version the experimental package "higher" than the regular package and am still trying to determine the appropriate way to do that.

I think we should just compute a new FileVersion as a transformation of the existing one. Similar to what we do here

runtime/src/libraries/Microsoft.VisualBasic.Core/Directory.Build.props

Line 4 in 8de6d05

<AssemblyVersion>$([MSBuild]::Add($(MajorVersion), 5)).$(MinorVersion).0.0</AssemblyVersion>

except for $(FileVersion), perhaps parsing like

runtime/eng/versioning.targets

Lines 175 to 178 in 57bfe47

	#define RuntimeFileMajorVersion $(FileVersion.Split('.')[0])
	#define RuntimeFileMinorVersion $(FileVersion.Split('.')[1])
	#define RuntimeFileBuildVersion $(FileVersion.Split('.')[2])
	#define RuntimeFileRevisionVersion $(FileVersion.Split('.')[3])

I believe that is set as part of the https://github.com/dotnet/arcade/blob/e7ede87875f41a9b3df898ae08da5ebc96e24f56/src/Microsoft.DotNet.Arcade.Sdk/tools/Version.targets#L14
So we'd need to run a target after that.

[tannergooding]

Do we need to also update the File/Informational versions or is simply marking this as <AssemblyVersion>$(MajorVersion).$(MinorVersion).0.1</AssemblyVersion> enough?

[ericstj]

We shouldn't change AssemblyVersion as that would change the binaries emitted by the compiler and require a corresponding runtime change. By keeping AssemblyVersion the same we are ensuring that we don't need to change the runtime assembly.

[davidwrighton]

This looks good to me. This was a fantastic turnaround on getting all of this put together.

[ericstj]

LGTM, but I'd want @Anipik and/or @safern to review since we're doing some funny stuff with packaging here.

[lambdageek]

Mono "change" LGTM.

Just so I'm clear on how this will work: the real runtime S.P.C is built with the generic math types defined. But the normal System.Runtime ref assembly will not have the extra interfaces. So a normal C# (or C++/CLI) build will only look at the non-experimental ref assembly and everything will work as before -System.Int32 is the same plain old type it is today. At execution time the new interfaces will be there, but we assume C++/CLI apps will not notice or care. Is that right?

[tannergooding]

Just so I'm clear on how this will work: the real runtime S.P.C is built with the generic math types defined. But the normal System.Runtime ref assembly will not have the extra interfaces. So a normal C# (or C++/CLI) build will only look at the non-experimental ref assembly and everything will work as before -System.Int32 is the same plain old type it is today. At execution time the new interfaces will be there, but we assume C++/CLI apps will not notice or care. Is that right?

Yes. Corelib and the System.Runtime implementation (which only contains type forwards) both contain all the new interfaces/methods. The ref assembly however is missing them which will allow incompatible tools (such as C++/CLI) to continue working by default.

We then have an additional System.Runtime.Experimental nuget package that only contains a ref assembly. This ref assembly does contain the new interfaces/methods and has a higher file version so it "wins" (specifically the build [3rd] component is 100 higher). This means if a C# developer explicitly references this NuGet package, they will have all the necessary bits to use and experiment with the feature.

[safern]

can this value change?

[tannergooding]

I don't believe its meant to ever change. The project was largely copied identically from the main System.Runtime and new property/items are under their own groups.

[MichalStrehovsky]

It's meant to change whenever we change the assembly file format (like we did now for the static virtual methods). It would tell the old tools that the file is not compatible.

We haven't been changing the version despite doing breaking changes to the format in the hopes that things will "just work" for tools that don't know what the new things are. It sometimes works and sometimes it leads to obscure failure modes (like now for the C++ compiler in the presence of static virtuals).

[safern]

Some questions but LGTM.

[tannergooding]

CI notifications don't seem to be going through:

• https://dev.azure.com/dnceng/public/_build/results?buildId=1239413&view=results
• https://dev.azure.com/dnceng/public/_build/results?buildId=1239415&view=results
• https://dev.azure.com/dnceng/public/_build/results?buildId=1239414&view=results
• https://dev.azure.com/dnceng/public/_build/results?buildId=1239416&view=results

[ghost]

Hello @tannergooding!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[tannergooding]

@lambdageek, there appears to be some WASM failures but AFAICT all the tests should be disabled. Any pointers?

[tannergooding]

I don't see anything obvious from the console log: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-55678-merge-f9f65ddd16a64b0cbc/System.Runtime.Experimental.Tests/console.069fc379.log?sv=2019-07-07&se=2021-08-04T16%3A58%3A33Z&sr=c&sp=rl&sig=69E%2Bwi1j4Mf3eA8aLI3rW2AsrlVNC4cQwimWNn%2FmlnI%3D
or
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-55678-merge-c51e1599037142e1ad/System.Runtime.Experimental.Tests/console.f51fb755.log?sv=2019-07-07&se=2021-08-04T16%3A57%3A05Z&sr=c&sp=rl&sig=TFXg9aDGfUSGVHw%2BNSEOVxUJuidayB9QFAd%2FTYiFkL0%3D

(from https://dev.azure.com/dnceng/public/_build/results?buildId=1240169&view=ms.vss-test-web.build-test-results-tab&runId=36885132&resultId=170234&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab)

[lambdageek]

I don't see anything obvious from the console log: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-55678-merge-f9f65ddd16a64b0cbc/System.Runtime.Experimental.Tests/console.069fc379.log?sv=2019-07-07&se=2021-08-04T16%3A58%3A33Z&sr=c&sp=rl&sig=69E%2Bwi1j4Mf3eA8aLI3rW2AsrlVNC4cQwimWNn%2FmlnI%3D
or
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-55678-merge-c51e1599037142e1ad/System.Runtime.Experimental.Tests/console.f51fb755.log?sv=2019-07-07&se=2021-08-04T16%3A57%3A05Z&sr=c&sp=rl&sig=TFXg9aDGfUSGVHw%2BNSEOVxUJuidayB9QFAd%2FTYiFkL0%3D

(from https://dev.azure.com/dnceng/public/_build/results?buildId=1240169&view=ms.vss-test-web.build-test-results-tab&runId=36885132&resultId=170234&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab)

@tannergooding the first log seems like there are problems running the AOT compiler - I suspect it's some ref assembly confusion (most likely the build tooling for wasm aot tests isn't passing the right things to the AOT compiler).
You can disable building the whole project by adding a <ProjectExclusions> to src/libraries/tests.proj under

<ItemGroup Condition="'$(TargetOS)' == 'Browser' and '$(BuildAOTTestsOnHelix)' == 'true' and '$(RunAOTCompilation)' == 'true'">

Not sure about the second failure.

[jeffhandley]

Post-merge question (I was offline yesterday afternoon and today):

Should the package instead be named System.Runtime.PreviewFeatures to align with the naming of RequiresPreviewFeatures and EnablePreviewFeatures. We had moved away from "experimental" in the design for the preview features.

[tannergooding]

We have existing tooling and infrastructure around packages and assemblies named *.Experimental. I don't think we want to be changing that infrastructure this late in the cycle, even if the attributes and SDK tooling involves PreviewFeatures

[jeffhandley]

Ah; got it. Thanks!

[hez2010]

Will EnablePreviewFeatures automatically reference the System.Runtime.Experimental nuget package?

[tannergooding]

That isn't currently planned. EnablePreviewFeatures itself is a broader concept and this (creating a separate Experimental package) is ideally a 1 time thing that won't be applicable to other features in the future.