Add back Environment.OSVersion

[stephentoub]

Whatever the current Windows-related philosophy is about version numbers, the non-Windows world does use version numbers as a way of indicating when features become available, version information is consumable via functions like uname, etc.

I've heard concerns about our not exposing it because we're concerned about developers misusing it, but developers often need this information and, if we don't expose it, developers will end up finding other ways to get at it, parsing things that shouldn't be parsed, doing their own P/Invokes in a non-portable fashion, and so on, resulting in a worse situation than if we'd just made it easy in the first place.

We should add Environment.OSVersion back to the System.Runtime.Extensions contract so that folks can rely on it.

[davkean]

And on Windows, just always return the shimmed version?

[stephentoub]

On Windows as on Linux and OS X ask the OS for its version and give back whatever it gives back. If it lies, that's the OS' problem.

[jkotas]

+1

[JonHanna]

👍

Should more info be available when kernels can be updated separately to the rest of the OS? For example, I've a partition running Linux Mint 17.2, which is based on (and hence sometimes reports itself as) Ubuntu 14.04.1, and which has the kernel version 3.16.0-38-generic.

I guess I should see OSVersion as 3.16.0.38. Should the 17.2 or the 14.4.1 be visible anywhere? Should the "generic"?

[KrzysztofCwalina]

Could you provide concrete scenarios when the version information is needed? I do know that people use it, but it is riddled with problems (especially for reusable libraries distributed as binaries), and so we would prefer to add query methods for common scenarios, if the scenarios are well known.

Maybe we can do both?, i.e.

bool IsSupported(Featrue feature, string version = null);

enum Feature {
Feature1,
Feature2,
...
OSVersion,
}

[davidfowl]

E.g.

• Windows <= 7 doesn't support websockets
• https://github.com/Microsoft/referencesource/blob/master/System/net/System/Net/HttpListener.cs#L296

Windows <= 7 has a bug with synchronous completion of IOCP events.

I don't think it makes sense to try to make a feature detection API, there's just too many random things people check.

[stephentoub]

There are tons of examples on Linux. Do a search in the man pages for something like "prior to version" and you'll get tons of hits about how behaviors changed after a particular point. For example, getrandom was introduced in 3.17 but its limits changed in 3.19. eventfd's flags argument must be 0 until version 2.6.26 and then after that is respected. There are POSIX behaviors that are undefined and vary by kernel version, e,g, "POSIX does not specify whether this also should happen when root does the chown(); the Linux behavior depends on the kernel version". or "Whether PROT_EXEC has any effect different from PROT_READ is architecture- and kernel version-dependent. On some hardware architectures (e.g., i386), PROT_WRITE implies PROT_READ.". Sometimes functionality is supported but is much faster after the OS provides support for it, e.g. "epoll_wait() was added to the kernel in version 2.6". Sometimes the interpretation of values changes, e.g. "In kernels before 2.6.37, a timeout value larger than approximately LONG_MAX / HZ milliseconds is treated as -1 (i.e., infinity)." The list goes on and on.

[davidsh]

+1

In a perfect world, we would only use "IsSupported*" type of methods instead of checking version numbers. Design-wise it is better because support for various things might go "down-level" to previous versions of OS's / components. So, doing a version number check isn't ideal.

But considering the reality of the state of world as others have pointed out, we should allow developers the flexibility in querying for version numbers and using them when appropriate.

[JonHanna]

If nothing else, just being able to dump Environment.OSVersion into an error log has utility in itself.

[davkean]

@davidfowl Version checking on Windows isn't going to work via traditional methods. Since, Windows 8.1 it has been lying to applications.

[KrzysztofCwalina]

I understand, that's why the API I proposed has both: check for feature (like web sockets) and for version.

[stephentoub]

@KrzysztofCwalina, how does your proposal address the examples I cited?

@davkean, if you include the appropriate manifest, doesn't Windows provide you the info you need? Regardless, RtlGetVersion provides me with correct version info on Windows even without such a manifest.

[davkean]

@stephentoub Yes, but my point is that websockets isn't a good example - the framework would never be able to rely on it consistently.

[davkean]

@stephentoub RtlGetVersion is a kernel-mode method, this works in user-space?

[KrzysztofCwalina]

@stephentoub

if(!IsSupported(Feature.OSVersion, "2.6.37") && timeout > LONG_MAX / HZ){
timeout = -1; /// it's infinity anyway
}

[davidsh]

RtlGetVersion is a kernel-mode method, this works in user-space?

http://simplecodewithsanat.blogspot.com/2015/01/get-correct-os-version-on-win-81.html

[stephentoub]

using System;
using System.Runtime.InteropServices;

class Program
{
    [DllImport("ntdll", CharSet = CharSet.Unicode)]
    private static extern int RtlGetVersion(ref RTL_OSVERSIONINFOW lpVersionInformation);

    private unsafe struct RTL_OSVERSIONINFOW
    {
        internal uint dwOSVersionInfoSize;
        internal uint dwMajorVersion;
        internal uint dwMinorVersion;
        internal uint dwBuildNumber;
        internal uint dwPlatformId;
        internal fixed char szCSDVersion[128];
    }

    static void Main()
    {
        RTL_OSVERSIONINFOW v = default(RTL_OSVERSIONINFOW);
        v.dwOSVersionInfoSize = (uint)Marshal.SizeOf<RTL_OSVERSIONINFOW>();

        RtlGetVersion(ref v);
        Console.WriteLine(v.dwMajorVersion);
        Console.WriteLine(v.dwMinorVersion);
        Console.WriteLine(v.dwBuildNumber);
        Console.WriteLine(v.dwPlatformId);
    }
}

on my Win10 machine prints out:

10
0
10586
2

[stephentoub]

if(!IsSupported(Feature.OSVersion, "2.6.37")

What are the semantics of this, telling you if you're on an OS version that's at least 2.6.37? If so, it'd take someone about 5 minutes to write a "GetVersion" function that does a binary search on the version to get the actual version, post that sample code, and then everyone uses it to get the current version and repeatedly asks why we didn't just provide it in the first place.

[davidsh]

One thing to consider is whether we can call this Windows API on both CoreFx DNXCORE50 target as well as UWP (NETCORE50) target (which needs to be validated against WACK).

 [DllImport("ntdll", CharSet = CharSet.Unicode)]
    private static extern int RtlGetVersion(ref RTL_OSVERSIONINFOW lpVersionInformation);

[davidfowl]

@davkean just going to leave this right here https://github.com/Microsoft/referencesource/blob/master/System/net/System/Net/WebSockets/WebSocketHelpers.cs#L47 😄

[davkean]

@davidfowl That works because it's Win8, the change to version lie was introduced in Win81. If we introduced Web Sockets in 8.1, that same check would be broken.

[davidfowl]

Then just add the manifest so it reports the right version. See https://github.com/dotnet/corefx/pull/4334/files#diff-46758d6fea22e74878ebdcf80c434b0eR13

[davkean]

@davidfowl The app needs to do that, that doesn't help the framework workaround "bugs" in OS versions or light-up features on higher level OSes.

The link you pointed to has nothing to do with a manifest - they are calling RtlGetVersion, which I've started an internal thread to figure if this is a viable replacement.

[OtherCrashOverride]

doing their own P/Invokes in a non-portable fashion

There currently is no way to do portable p/invoke (dotnet/coreclr#930) so that should not be a concern. Testing against a specific version automatically makes the code non portable anyway (see next paragraph)

What is important is that OSVersion is misleading on unix. Is it the distro version (ubuntu 14.04, CentOS 7, etc)? Is it the kernel version (linux 3.x.y, linux 4.x.y)? is it both? (FreeBSD 10, NetBSD, etc?). Should a developer be testing against a kernel version if they can't tell if they are testing a linux or freebsd environment?

Seems this information is also misleading on Windows if its misreported unless certain conditions are met.

Rather than adding this API back, perhaps a new API should be introduced to report this information unambiguously across supported platforms. Also worth noting for consideration is that a kernel version can dramatically change within a single distro version. This happens when new LTS kernel versions are released on linux.

[comdiv]

1. old OSVersion was not very correct
2. correct version detecting is complex problem
3. dealing with versions is special case (for many things you even can be agnostic about if it's Linux or Windows)

So my opinion is that it's not concern of existing System.Environment class.
But may be it's good idea add System.Diagnostics.OperationSystem assembly and collect such things in it. It can be version checkers, cross-platform Path resolvers, installed compilers, dependency, package checker and so on for those developers who require more sofisticated reflection over underlying OS.

[weshaggard]

I agree we need to figure out how to provide something to solve some of these scenarios. However given Eviroment.OSVersion exposes an OperatingSystem object model which has a number of issues such as the PlatformID enum which we don't have a good way to update given it is bound to the full .NET Framework. I think we should add some helper APIs for common scenarios onto our new RuntimeInformation type.

Regardless of whether or not we directly expose the version we should also add some APIs that make doing the right thing easier. Perhaps something like IsOSVersionOrHigher(version) and IsOSFeatureSupported(string).

[comdiv]

IsOSVersionOrHigher and IsOSFeatureSupported - wrong - it is too coupled to OS detection, while it's funcitonality more complex.
Better extend existing Version class and provide support not just for X.X.X.X format but for more various variances. Most things are versioned on manner prodname-1.2-rc233 или prod.1.2-build-43ab433 - it should be able to get Version as set of parts, and determine numeric comparable things.
Add it as dotnet/corefx#4847

[OtherCrashOverride]

I also think we should ask "What value is there in knowing the kernel version?" CoreCLR runs in userspace and has no provision for making syscalls directly to the kernel. I think developers are more interested in making version based decisions based on OS (userland) versions. While in Windows and OSX, kernel version implies OS version and associated "features". On linux, its really OS version (Ubuntu x.x, CentOS x.x) that determines what userland API calls and "features" are available for p/invoke. Kernel versions in linux can vary wildly for a given OS version.

If the API call is going to be named "OSVersion" then it should report the OSVersion (Ubuntu x.x, CentOS x.x, WIndows x.x, FreeBSD x.x). This implies that there should be a API called "KernelVersion" if that information is desired.

As to the version number itself, the linux kernel has major.minor.revision.extra. This probably means that "Version" should be an abstract base class with "LinuxKernelVersion" as a concrete result. The class should also probably implement its own comparison functions to return greater than, less than, equal.

var osVersion = FutureApiCall.OSVersion;
if (osVersion > new LinuxOSVersion(LinuxOS.Ubuntu, 14, 4))
{
  // do something
}

Type Safe Version Checking(tm): it throws if you are checking a windows environment for a linux version.
😉

Version
  |
  |---- KernelVersion
  |        |
  |        |--- LinuxKernelVersion
  |        |--- WindowsKernelVersion
  |        |--- OsxKernelVersion
  |        |--- (others as needed)
  |
  |---- OSVersion
  |       |
  |       |--- LinuxOSVersion
  |       |--- WindowsOSVersion
  |       |--- OsxOSVersion  

[OtherCrashOverride]

Upon further reflection (no pun intended), a version check is only useful for specific distributions, so rather than:

if (osVersion > new LinuxOSVersion(LinuxOS.Ubuntu, 14, 4))

LinuxOSVersion should also be an abstract base class with UbuntuLinuxOSVersion as concrete:

if (osVersion > new UbuntuLinuxOSVersion(14, 4))

This means testing for an ubuntu version on centos or freebsd will throw.

Again, on linux, the presence of a specific kernel version does not imply that a particular feature is exposed and/or available from userland. So ultimately, a developer may need to check both LinuxOSVersion and LinuxKernelVersion.

[Edit]
Addendum: Yes, there was such a thing as DebianFreeBsdOSVersion.

[stephentoub]

I have two key goals here:

1. Enable existing code to "just work" as much as possible. Every time there's a difference between the APIs exposed in the full framework and in corefx, we impede portability that much more, and we need to close these gaps significantly.
2. Enable code that needs OS-related version information to get what it needs. That typically falls into two camps:
   (a) Logging information about versions.
   (b) Making run-time decisions based on version (e.g. whether a flag is available and understood as of a particular version, whether we're before or after some behavioral change went into effect, etc.)

As such, I think we should do the following two things....

1. Expose Environment.OSVersion. This is primarily about the first goal, but also helps in many cases for the second. On Windows, it's primarily about ensuring that existing code on Windows continues to work as the developer expected (if the developer's code was working as expected with whatever version number Windows returns, then it will here, too.) On both Linux and OS X, we return kernel version (kernel version is useful in many cases, with reasons previously cited, and is the most consistent thing we can provide without additional information exposed about how to interpret it). As other platforms come online, if there's a choice to be made about version number, we choose on a case-by-case basis. For the PlatformID enum, we simply stick with the values we have: on Windows we continue to return what we return today, on OS X we return the existing MacOSX value (even though the "Mac" part is outdated in terminology), and on everything else we return the existing Unix value. For existing code that's using OSVersion on Windows successfully, the code continues to "just work." For existing code that's using OSVersion on Windows for Windows-related decision making and where they want to then run cross-platform, that code would need to change anyway, as using version information for run-time decision making is inherently platform-specific in nature. For existing and new code that's using OSVersion for logging purposes, it'll "just work" cross-platform. For new code that wants to use it for decision making, it may or may not work, depending on how much information is needed about other operating systems, but it'll at least provide a basis, and will likely suffice for many cases.

2. In addition to that, expose any additional desired information from the new RuntimeInformation library. We could have an OperatingSystemName property (e.g. "Windows" on Windows, and on POSIX-compliant system what's returned by uname's sysname), OperatingSystemVersion property, and a KernelVersion property for cases where they need to be distinguished (on a platform where there is no difference, they could return the same thing). All of these could return strings, allowing for cases where they don't strictly follow the major.minor.build.revision scheme enforced by the Version class. A higher-level library could put an object model on top of these, if desired, but all of that policy and complicated hierarchy should be kept out of corefx. Most of this seems to already be done as part of Add new apis proposed in #4137 to System.Runtime.InteropServices.RuntimeInformation corefx#4334, potentially with a few tweaks.

[OtherCrashOverride]

While I feel strongly that the correct thing to do here is introduce a new API rather than "hack" an existing one, in the absence of doing the correct thing, the next best thing is "just do what mono does":

https://github.com/mono/mono/blob/b7a308f660de8174b64697a422abfc7315d07b8c/mono/metadata/environment.c#L61

#elif defined(HAVE_SYS_UTSNAME_H)
    struct utsname name;

    if (uname (&name) >= 0) {
        return mono_string_new (mono_domain_get (), name.release);
    }
#endif

http://man7.org/linux/man-pages/man2/uname.2.html
"uname - get name and information about current kernel"

I should also reiterate that a kernel version is not the same nor a substitute for an os version on linux. Features such as systemd depend on OS version, not kernel version. Features available on Ubuntu 14.04 may or may not be available on CentOS 7 or FreeBSD 10.x. All this does is "kick the can down the road".

I should also state that as the milestone for this issue is "1.0.0-rc2", I completely understand its far too late for a radical API change. At some point in the future there needs to be thought and design put into making CoreCLR cross-platform (dllimport is the shining star there). So please do not mark this issue closed without opening a new future issue with the goal of resolving items such as these.

[stephentoub]

While I feel strongly that the correct thing to do here is introduce a new API rather than "hack" an existing one

I'm not sure what you're arguing against, as I just said I thought it was important to both expose the existing API and introduce an additional set.

uname

Yes, that's what I was suggesting.

I should also reiterate that a kernel version is not the same nor a substitute for an os version on linux.

No one is arguing with this. I explicitly acknowledged this in my last reply.

goal of resolving items such as these

There are already multiple issues open tracking concerns around DllImport. If you have other specific concerns about the cross-platform nature of CoreCLR, please open issues for them so they can be discussed, ideally in the coreclr repo. I know you've been very involved in the other existing issues.

[OtherCrashOverride]

I'm not sure what you're arguing against

I think we should see [Obsolete] Environment.OSVersion if the API is added back so that its clear its not meant to be a long term solution (unless it is). As mentioned before, CoreCLR has breaking changes. There is not much developer guidance provided for them at this point. The [Obsolete] attribute will at least give developers a clue that while their code will not break now, it will in the future.

Yes, that's what I was suggesting.

I meant the behavior should be as mono's version is and not introduce anything new or deviate. Otherwise, this truly is a new API. (Its not the end of the world if it does.)

There are already multiple issues open tracking concerns around DllImport.

This issue is loosely related to DllImport in that an example of a version based decision would be the choice of API points to p/invoke. This is dependent on actual OS version and not kernel version. I think there should be "Add cross-platform, unambiguous platform version API" as a new issue if there is a desire by the maintainers to provide a long term solution to the issue. As mentioned in the first post, developers can (and will) implement their own solutions so its not a critical issue and not one that I will personally raise. Its only worthwhile to pursue if the maintainers are interested and fell its a good fit.

When ARM platforms are supported, we will likely be revisiting this issue.

[krytarowski]

What is important is that OSVersion is misleading on unix. Is it the distro version (ubuntu 14.04, CentOS 7, etc)? Is it the kernel version (linux 3.x.y, linux 4.x.y)? is it both? (FreeBSD 10, NetBSD, etc?). Should a developer be testing against a kernel version if they can't tell if they are testing a linux or freebsd environment?

Regarding NetBSD, I suggest to go for 'netbsdN', where N is major version release, for example 'netbsd7'.

[roji]

+1 on the importance of this.

One thought for a new API would be to have a class hierarchy, where OS-specific version fields would become accessible if the user downcasts an abstract base class (which could be similar to .NET Framework's current OperatingClass).

This would allow cleanly exposing things like kernel versions, distribution names and versions on Linux, vs. things like service packs on Windows.

[ianhays]

On Windows as on Linux and OS X ask the OS for its version and give back whatever it gives back. If it lies, that's the OS' problem.

👍

[davidsh]

On Windows as on Linux and OS X ask the OS for its version and give back whatever it gives back. If it lies, that's the OS' problem.

I don't think it is useful using a version API that lies especially for developers using an API. The OS lied mainly to preserve app-compat behavior for legacy applications that didn't expect to see an updated version. So, it would be required that an app have an updated manifest so that it could be considered tested/ready for handling a newer version being reported.

There are Windows APIs like RtlGetVersion() that will never lie and will give complete version number information especially the build number. That makes it useful to find out if it is Windows 10 10240 (original RTM) or 10586 (1511 November Update), etc.

I really think we should consider using APIs that are 100% truthful.

[niemyjski]

+1 I really need this.

[niemyjski]

@stephentoub Was this issue closed because it's not happening? Would love some context.

[stephentoub]

Was this issue closed because it's not happening?

It was closed because it was implemented:
dotnet/corefx#9851