Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename RuntimeInformation.OSDescription to RuntimeInformation.OperatingSystemVersion #17127

Closed
Priya91 opened this issue Apr 26, 2016 · 12 comments
Assignees
Milestone

Comments

@Priya91
Copy link
Contributor

Priya91 commented Apr 26, 2016

The OSDescription API returns the output from RtlGetVersion on windows and uname on Unix. The proposal is to rename the api to be used for runtime decisions rather than a display string, since we are making an OS runtime call, instead of setting the values at build time. If there were a new api added to get the runtime OS version, it will mostly be doing the same OS calls, and it's redundant to have multiple APIs doing the same work.
#17126
#15824

cc @stephentoub @weshaggard @KrzysztofCwalina @terrajobst @davkean

@Priya91 Priya91 self-assigned this Apr 26, 2016
@mellinoe
Copy link
Contributor

I don't understand the rename. Doesn't this type include the actual name of the OS, i.e. "Windows", "Ubuntu", etc.? Why is "Version" better than "Description"?

@Priya91
Copy link
Contributor Author

Priya91 commented Apr 26, 2016

Why is "Version" better than "Description"?

As it is making a runtime query to the OS Version APIs. The return is a string which is customized by the managed wrapper, which can be changed. After the API rename, I expect this API to just return the output from OS call without any managed customizations.

@terrajobst
Copy link
Member

Looks reasonable to me.

@weshaggard @KrzysztofCwalina?

@weshaggard
Copy link
Member

The proposal is to rename the api to be used for runtime decisions rather than a display string,

I still do not believe this should be used for runtime decisions and I agree with Eric that it we shouldn't rename it to OperatingSystemVersion. If we really want to expose the version information, which I'm still not convinced of either, it should be a version object and not a string coming from an underlying OS call.

@terrajobst
Copy link
Member

terrajobst commented Apr 27, 2016

If we really want to expose the version information, which I'm still not convinced of either, it should be a version object and not a string coming from an underlying OS call.

Good point. Wes just mentioned in person that we discussed this in FXDC and specifically used the term "description" to make it clear that this is a display name, rather than a value that can be compared.

So now I believe we shouldn't make this change.

@Priya91
Copy link
Contributor Author

Priya91 commented Apr 27, 2016

which I'm still not convinced of either

Why is this scenario not convincing?

it should be a version object and not a string coming from an underlying OS call.

I dont think so, that is what we did in Desktop and it lead to problems finally getting the version returned to be a fixed value. Returning the version as string gives more flexibility especially in xplat with different version syntax among OSes. And in future when more OSes are supported, how do we maintain this Version object.

@weshaggard
Copy link
Member

Your tow statements seem contradicting to me. You want people to start using this property to make runtime decisions but you don't want them to use it as a Version object because of the past issues we have had with people making runtime decisions on it? I'm not sure I understand the difference.

If people start making runtime decisions based on this string they have no hope of actually working correctly when new versions come out, at least with a Version object people have a chance of working correctly because they can do the greater than or equal to checks.

@mellinoe
Copy link
Contributor

If I'm understanding correctly, the problem isn't that people are using it to make runtime decisions, but that the concept of a "Version" is not well-defined or consistent, especially across different operating systems. Trying to shoe-horn all of the different version concepts into a single object was problematic, so the new proposal is to just return the operating system's own concept of a "version" without doing any post-processing on it. Does that sound correct?

@Priya91
Copy link
Contributor Author

Priya91 commented Apr 28, 2016

You want people to start using this property to make runtime decisions but you don't want them to use it as a Version object because of the past issues we have had with people making runtime decisions on it?

As I see it, I view making runtime decisions and version object as different issues. From the discussions in the issues tagged in this issue description, it is clear that there is gap in the APIs to provide OS version information, that can be used to make runtime decisions. So I support the idea of having an API that can be used to make runtime decisions.

In Desktop this was solved by defining a Version object that mimicked Windows OS version syntax. In .NET Core that object adds very little value, when we are supporting multiple platforms, that don't conform to a particular version standard. So I am proposing to get rid of having a Version object idea and just return the Version as a string. What benefits are obtained with a Version object? The example about numeric comparison can still be achieved by parsing the string, following the rules of the underlying OS. Moreover, other lang like Java, Android apis, python, R etc also provide the OS version information as a system property string, if it can be evaluated.

Does that sound correct?

Yes

@weshaggard
Copy link
Member

Let's continue the conversation in an API review please work with @terrajobst to set one up.

@Priya91
Copy link
Contributor Author

Priya91 commented May 6, 2016

This issue will be revisited after API port effort of #15824

@Priya91
Copy link
Contributor Author

Priya91 commented May 10, 2016

Closing this, as it will be a breaking change with rc2. This API Change was proposed to change the behavior of the API to provide a better OS Version scenario for runtime decisions. Since we are still debating about bringing back the old Environment.OSVersion we will first explore to support it cross plat, and if still there is a need, will come up with a new proposal for xplat Operating system version.

cc @KrzysztofCwalina @weshaggard

@Priya91 Priya91 closed this as completed May 10, 2016
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants