Change Environment.Version to return product version

[jkotas]

Contributes to https://github.com/dotnet/corefx/issues/31099

[jkotas]

cc @richlander

[stephentoub]

It doesn't appear to be set by corerun, unless I'm just missing it. Can we do that?

[jkotas]

What should we set it to in corerun?

[stephentoub]

Whatever we expect it to be set to by dotnet? It otherwise seems strange to me that the same build of coreclr would produce different results for Environment.Version. I guess my concern is that this is configurable in this way at all by the host. Why is that the approach? Maybe I just don't understand what Version is meant to be, but the docs say it's the version of the common language runtime.

[jkotas]

We do not have product version (ie the marketing version) available when building CoreCLR repo: https://github.com/dotnet/corefx/issues/31099#issuecomment-407190291

I guess we can redo how the build works to make it available somehow, but I have no idea what it would take.

[stephentoub]

I guess we can redo how the build works to make it available somehow

This makes the most sense to me, Environment.Version always returning the same version for the same CoreLib.dll build, regardless of who is hosting it, what an appcontext variable was set to, etc. It just doesn't seem like something that should be dynamic in any way.

But I also don't know what it would take. How does dotnet know the marketing version? Is that not configured in its build somehow?

[jkotas]

Environment.Version always returning the same version for the same CoreLib.dll build

+1

How does dotnet know the marketing version?

It reads it from *.deps.json file (either from shared framework, or from the one in self-contained app).

I also don't know what it would take.

It seems to be available here: https://github.com/dotnet/coreclr/blob/master/eng/Versions.props#L13 so we just need to embed this property in the CoreLib.

[jkotas]

@mmitche Could you please confirm whether MicrosoftNETCoreAppVersion property is set to the current product version during the official builds?

[mmitche]

It wouldn't be (neither for 2.1 or 2.2), The version is flowed from the last build of core-setup that was done. Since this is a circular dependency, there's no to have pre-knowledge of what the netcore app version will be.

[jkotas]

Ok, I think the best compromise is to use FX_PRODUCT_VERSION when it is set, and use approximate version used to build CoreCLR as fallback (https://github.com/dotnet/coreclr/blob/master/eng/Versions.props#L5).

We use similar logic for AppContext.BaseDirectory: https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/shared/System/AppContext.cs#L26

[stephentoub]

the best compromise

Sounds good.

[davidfowl]

This won't work for pre-releases, it'll return an empty or incomplete version. This is the reason why I stopped pushing on this for to be returned from Environment.Version even though what we return today is pretty useless so maybe this is better than nothing. Is it worth having another property on environment that is a string ProductVersion or something? We need to do this https://github.com/dotnet/corefx/issues/13526 😄

[jkotas]

on environment that is a string ProductVersion or something?

We do have RuntimeInformation.FrameworkDescription that returns string. It contains bogus version today as well.

I think we should first fix existing APIs to return sensible versions. If that proves to be insufficient, we can add a yet another version string returning API.

[davidfowl]

Sure we can, and people can always get FX_PRODUCT_VERSION directly as a workaround. It's just that it'll show slightly less accurate information but it's better than what we have today.