Moving platform guards out of System.Runtime.InteropServices

[terrajobst]

In .NET Core 1.x and 2.x days, we added very limited ability for customers to check which OS they were on. We originally didn't even expose Environment.OSVersion and we built a new API RuntimeInformation.IsOSPlatform(). The assumption was that doing OS checks is exclusively done for P/invoke or interoperating with native code, which is why RuntimeInformation was put in System.Runtime.InteropServices.

However, guarding calls to OS bindings or avoiding unsupported APIs in Blazor isn't really a P/invoke scenario. It's seems counterproductive to force these customers to import the System.Runtime.InteropServices namespace.

API Proposal

Since RuntimeInformation is a very small type today, the proposal is: don't add the API for version checks on RuntimeInformation but instead to add them to Environment.

Environment was the canonical place where people have performed OS checks in the past and are most likely to look for them moving forward. RuntimeInformation is a fairly recent type. And compared to Environment.OSVersion the RuntimeInformation type hasn't gained much adoption yet.

namespace System
{
    public static partial class Environment
    {
        // Primary

        public static bool IsOSPlatform(string platform);
        public static bool IsOSPlatformVersionAtLeast(string platform, int major, int minor = 0, int build = 0, int revision = 0);

        // Accelerators for platforms where versions don't make sense

        public static bool IsBrowser();
        public static bool IsFreeBSD();
        public static bool IsLinux();

        // Accelerators with version checks

        public static bool IsAndroid();
        public static bool IsAndroidVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsiOS();
        public static bool IsiOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsmacOS();
        public static bool IsmacOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IstvOS();
        public static bool IstvOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IswatchOS();
        public static bool IswatchOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsWindows();
        public static bool IsWindowsVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);
    }
}

For the attributes, we'd like to put the core ones people are likely to apply to their own code in the same namespace as the guards, i.e. in System. We'll leave the base type and the TargetPlatformAttribute in System.Runtime.Versioning.

namespace System
{
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class SupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public SupportedOSPlatformAttribute(string platformName);
    }
  
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public UnsupportedOSPlatformAttribute(string platformName);
    }

    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Event |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple=true, Inherited=false)]
    public sealed class ObsoletedInOSPlatformAttribute : OSPlatformAttribute
    {
        public ObsoletedInPlatformAttribute(string platformName);
        public ObsoletedInPlatformAttribute(string platformName, string message);
        public string Message { get; }
        public string Url { get; set; }
    }
}
namespace System.Runtime.Versioning
{
    public abstract class OSPlatformAttribute : Attribute
    {
        private protected OSPlatformAttribute(string platformName);
        public string PlatformName { get; }
    }

    [AttributeUsage(AttributeTargets.Assembly,
                    AllowMultiple=false, Inherited=false)]
    public sealed class TargetPlatformAttribute : OSPlatformAttribute
    {
        public TargetPlatformAttribute(string platformName);
    }
}

This would remove the following APIs:

 namespace System.Runtime.InteropServices
 {
     public static class RuntimeInformation
     {
         // Existing APIs
         public static string FrameworkDescription { get; }
         public static string OSArchitecture { get; }
         public static string OSDescription { get; }
         public static string ProcessArchitecture { get; }
         public static string RuntimeIdentifier { get; }
         public static bool IsOSPlatform(OSPlatform osPlatform);

-        // APIs proposed earlier for .NET 5.0
-        public static bool IsOSPlatformOrLater(string platformName);
-        public static bool IsOSPlatformOrLater(OSPlatform osPlatform, int major);
-        public static bool IsOSPlatformOrLater(OSPlatform osPlatform, int major, int minor);
-        public static bool IsOSPlatformOrLater(OSPlatform osPlatform, int major, int minor, int build);
-        public static bool IsOSPlatformOrLater(OSPlatform osPlatform, int major, int minor, int build, int revision);
-        public static bool IsOSPlatformEarlierThan(string platformName);
-        public static bool IsOSPlatformEarlierThan(OSPlatform osPlatform, int major);
-        public static bool IsOSPlatformEarlierThan(OSPlatform osPlatform, int major, int minor);
-        public static bool IsOSPlatformEarlierThan(OSPlatform osPlatform, int major, int minor, int build);
-        public static bool IsOSPlatformEarlierThan(OSPlatform osPlatform, int major, int minor, int build, int revision);
     }
 }

/cc @jeffhandley @eerhardt @adamsitnik @buyaa-n @mhutch

[jkotas]

public static bool IsOSPlatformOrLater(string platformName);
public static bool IsOSPlatformEarlierThan(string platformName);

The string-based checks like this are slow by design. It means that people will need to cache their results for hot paths.

If all that we got are these string based-checks, the analyzer would need to be smart enough to deal with cached results of these checks.

[jkotas]

cc @adamsitnik

[terrajobst]

public static bool IsOSPlatformOrLater(string platformName);
public static bool IsOSPlatformEarlierThan(string platformName);

The string-based checks like this are slow by design. It means that people will need to cache their results for hot paths.

Will the other overloads be fast enough though or will folks always have to cache if perf is critical? If so, I think we're better off either making the analyzer smarter or just telling people to suppress in hot paths.

[huoyaoyuan]

The string-based checks like this are slow by design.

It should be possible to intrinsify the check: the current OS are constant during JIT, and the the string is typically constant too.

[jkotas]

Yes, it is possible in theory, but it makes the complexity go through the roof once things like AOT are added to the mix.

[terrajobst]

@jkotas, thoughts on this? I'm fine with adding the other overload if that addresses the perf, but if that those aren't good enough than I'd rather improve the analzyer.

[buyaa-n]

If all that we got are these string based-checks, the analyzer would need to be smart enough to deal with cached results of these checks.

For now analyzer only keep track of local cache result, we talked about this here:

void M1()
{
    var canUse = RuntimeInformationHelper.IsOSPlatformOrLater(OSPlatform.Windows, 11);
    if (canUse )
    {
        M2(); // Will not warn
    }
    else
    {
        M2(); // Would warn
    }
}

[MinimumOSPlatform(""Windows10.1.2.3"")]
void M2()
{
}

[jkotas]

@jkotas, thoughts on this?

The current implementation of the OSPlatform overloads caches the result of the expensive string parsing inside struct OSPlatform. RuntimeInformationHelper.IsOSPlatformOrLater(OSPlatform.Windows, 11) does not do any string parsing. It just checks bools and ints.

[terrajobst]

• No final decision made, we'll re-review next week.
• Primary feedback was the string-based one is a performance trap because it requires parsing. We should offer methods that separate platform from version.
• It was concluded that EarlierThan isn't necessary as it's logically equivalent to say if (Environment.IsWindows() && !Environment.IsWindowsVersionAtLeast(10, 0, 19222)).
• We preferred VersionAtLeast as a suffix over OrLater.
• We would like to remove hardcoded strings in favor of platform-specific accelerators.
• We would like to put the core attributes people are likely to apply to their own code in the same namespace as the guards

Current thinking is this:

namespace System
{
    public partial class Environment
    {
        // Primary
        public static bool IsOSPlatform(string platform);
        public static bool IsOSPlatformVersionAtLeast(string platform, int major, int minor = 0, int build = 0, int revision = 0);

        // Accelerators for platforms where versions don't make sense
        public static bool IsBrowser();
        public static bool IsFreeBSD();
        public static bool IsLinux();

        // Accelerators with version checks

        public static bool IsAndroid();
        public static bool IsAndroidVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsiOS();
        public static bool IsiOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsmacOS();
        public static bool IsmacOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IstvOS();
        public static bool IstvOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IswatchOS();
        public static bool IswatchOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsWindows();
        public static bool IsWindowsVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);
    }

    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class SupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public SupportedOSPlatformAttribute(string platformName);
    }
  
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public UnsupportedOSPlatformAttribute(string platformName);
    }
}
namespace System.Runtime.Versioning
{
    public abstract class OSPlatformAttribute : Attribute
    {
        private protected OSPlatformAttribute(string platformName);
        public string PlatformName { get; }
    }

    [AttributeUsage(AttributeTargets.Assembly,
                    AllowMultiple=false, Inherited=false)]
    public sealed class TargetPlatformAttribute : OSPlatformAttribute
    {
        public TargetPlatformAttribute(string platformName);
    }
}

[jkotas]

class Environment

Environment has been a grab-bag for everything. It has ~40 methods for everything from current directory to current thread id and current time. The enriched counterparts of the one-off Environment methods has been separate types. For example, we have Environment.TickCount and the enriched equivalent is Stopwatch. I think this should follow this same pattern and be on some other type. I know this was discussed during the review, just adding my 2 cents.

[akoeplinger]

If we go with the discussed plan of deemphasizing RuntimeInformation then I'd propose undoing the new properties in OSPlatform that are currently slated for .NET 5.0:

namespace System.Runtime.InteropServices
{
    public readonly partial struct OSPlatform
    {
-       public static System.Runtime.InteropServices.OSPlatform Android { get { throw null; } }
-       public static System.Runtime.InteropServices.OSPlatform Browser { get { throw null; } }
        public static System.Runtime.InteropServices.OSPlatform FreeBSD { get { throw null; } }
-       public static System.Runtime.InteropServices.OSPlatform iOS { get { throw null; } }
        public static System.Runtime.InteropServices.OSPlatform Linux { get { throw null; } }
-       public static System.Runtime.InteropServices.OSPlatform macOS { get { throw null; } }
-       [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
        public static System.Runtime.InteropServices.OSPlatform OSX { get { throw null; } }
-       public static System.Runtime.InteropServices.OSPlatform tvOS { get { throw null; } }
-       public static System.Runtime.InteropServices.OSPlatform watchOS { get { throw null; } }
        public static System.Runtime.InteropServices.OSPlatform Windows { get { throw null; } }
        ...
    }
}

[terrajobst]

Video

• We'll capitalize the first letter in Apple platforms when they are part of a longer name, such as IsTvOS (instead of IstvOS).
• We've decided to move all members to OperatingSystem, which doesn't expose any statics today. This avoids creating a dumping ground in Environment.
• We'll try to pull the members from OSPlatform we added in the earlier .NET 5 preview
• FreeBSD has versions, so it should have IsFreeBSDVersionAtLeast
• We agreed to move all attributes to System.Runtime.Versioning. Most developers will apply these attributes through code completion or code fixers, so namespace discoverability won't be a big issue, but polluting the System namespace is a big concern.

namespace System
{
    public partial class OperatingSystem
    {
        // Primary
        public static bool IsOSPlatform(string platform);
        public static bool IsOSPlatformVersionAtLeast(string platform, int major, int minor = 0, int build = 0, int revision = 0);

        // Accelerators for platforms where versions don't make sense
        public static bool IsBrowser();
        public static bool IsLinux();

        // Accelerators with version checks

        public static bool IsFreeBSD();
        public static bool IsFreeBSDVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsAndroid();
        public static bool IsAndroidVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsIOS();
        public static bool IsIOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsMacOS();
        public static bool IsMacOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsTvOS();
        public static bool IsTvOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsWatchOS();
        public static bool IsWatchOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);

        public static bool IsWindows();
        public static bool IsWindowsVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0);
    }
}
namespace System.Runtime.Versioning
{
    public abstract class OSPlatformAttribute : Attribute
    {
        private protected OSPlatformAttribute(string platformName);
        public string PlatformName { get; }
    }

    [AttributeUsage(AttributeTargets.Assembly,
                    AllowMultiple=false, Inherited=false)]
    public sealed class TargetPlatformAttribute : OSPlatformAttribute
    {
        public TargetPlatformAttribute(string platformName);
    }

    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class SupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public SupportedOSPlatformAttribute(string platformName);
    }
  
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public UnsupportedOSPlatformAttribute(string platformName);
    }

    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Event |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple=true, Inherited=false)]
    public sealed class ObsoletedInOSPlatformAttribute : OSPlatformAttribute
    {
        public ObsoletedInPlatformAttribute(string platformName);
        public ObsoletedInPlatformAttribute(string platformName, string message);
        public string Message { get; }
        public string Url { get; set; }
    }
}

namespace System.Runtime.InteropServices
{
    public readonly partial struct OSPlatform
    {
-       public static System.Runtime.InteropServices.OSPlatform Android { get; }
-       public static System.Runtime.InteropServices.OSPlatform Browser { get; }
        public static System.Runtime.InteropServices.OSPlatform FreeBSD { get; }
-       public static System.Runtime.InteropServices.OSPlatform iOS { get; }
        public static System.Runtime.InteropServices.OSPlatform Linux { get; }
-       public static System.Runtime.InteropServices.OSPlatform macOS { get; }
-       [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
        public static System.Runtime.InteropServices.OSPlatform OSX { get; }
-       public static System.Runtime.InteropServices.OSPlatform tvOS { get; }
-       public static System.Runtime.InteropServices.OSPlatform watchOS { get; }
        public static System.Runtime.InteropServices.OSPlatform Windows { get; }
    }
}

[gfoidl]

Should the remaining members of OSPlatform be obsoleted in favor of the new members on OperatingSystem?

With the current proposal we have different "options" to check for Windows / Linux / ..., so questions may arise for

• which one is correct / recommended
• what's the difference between the diffferent options

In short: it might be confusing why to have different apis for the same concern.

And System.OperatingSystem is more discoverable than System.Runtime.InteropServices.OSPlatform (imho).

[jeffhandley]

In short: it might be confusing why to have different apis for the same concern.

Yeah, it certainly is confusing. Obsoleting the OSPlatform members and/or RuntimeInformation.IsOSPlatform is compelling. If we were to do that, we'd want to fix the ~665 references to OSPlatform / RuntimeInformation.IsOSPlatform across the runtime repo.

@terrajobst What do you think about obsoleting the older checks?

[adamsitnik]

While working on the implementation I've realised that on OSX we don't have the Revision number:

runtime/src/libraries/Common/src/Interop/OSX/Interop.libobjc.cs

Lines 34 to 55 in a778b7e

	internal static Version GetOperatingSystemVersion()
	{
	int major = 0;
	int minor = 0;
	int patch = 0;
	IntPtr processInfo = objc_msgSend(objc_getClass("NSProcessInfo"), sel_getUid("processInfo"));
	if (processInfo != IntPtr.Zero)
	{
	NSOperatingSystemVersion osVersion = get_operatingSystemVersion(processInfo, sel_getUid("operatingSystemVersion"));
	checked
	{
	major = (int)osVersion.majorVersion;
	minor = (int)osVersion.minorVersion;
	patch = (int)osVersion.patchVersion;
	}
	}
	return new Version(major, minor, patch);
	}

@eerhardt @terrajobst Should I remove the last parameter from OSX family methods because it is always ignored or keep it because we might add in the future?

-public static bool IsMacOSVersionAtLeast(int major, int minor = 0, int build = 0, int revision = 0)
+public static bool IsMacOSVersionAtLeast(int major, int minor = 0, int build = 0)

I would prefer to remove it because the OSX method that we use returns a type that simply don't have it so it will rather not change in the future:

[eerhardt]

Should I remove the last parameter from OSX family methods because it is always ignored or keep it because we might add in the future?

I think it makes sense to make the APIs conform to the OS they are specific for. We discussed this a bit during the first round of reviews this. And we are also already doing this with:

        public static bool IsBrowser();
        public static bool IsLinux();

Because the versions of those OS platforms aren't useful. Just like the revision number for any OSX family OS isn't useful.

So I agree with you @adamsitnik, I think we should drop the version numbers from OS platforms where it makes sense.

[akoeplinger]

should we also call the third argument int patch instead of int revision for macOS to match the Apple docs?

[am11]

Is<os-name> accelerators are fine; but aren't Is<os-name>VersionAtLeast methods, all with same arguments, redundant?

[akoeplinger]

@am11 you mean because there is IsOSPlatformVersionAtLeast(string platform, int major, int minor, ..) already? That'd mean everyone would need to use the string version and the expectation is that calling these will be pretty common at least for mobile targets so having a short accelerator is very useful.

[adamsitnik]

The PRs to runtime repo has been merged:

#40371
#40457
#40373

The PRs to SDK and ASP.NET got approved and are waiting to be merged:

dotnet/sdk#12775
dotnet/aspnetcore#24652

I've also created new issues in SDK and ASP.NET repo and suggested to switch to use the new APIs for the performance gains:

dotnet/aspnetcore#24653
dotnet/sdk#12825

And ported what was portable in runtime repo:

#40520
#40522

Closing.

[am11]

@akoeplinger, I meant the general shape of public least version APIs (not that particular private method; that was added as part of implementation of these APIs). E.g. instead of Is<os-name>VersionAtLeast, maybe just IsVersionAtLeast(int major, int minor, patch) for the current platform, which I believe would make the usage easier (agnostic of current platform).

[akoeplinger]

@am11 hmm I think that option wasn't discussed in the API review, calling an iOS 13.1+ API would look like this: if (OperatingSystem.IsIOS && OperatingSystem.IsVersionAtLeast(13, 1)) rather than if (OperatingSystem.IsIOSVersionAtLeast(13, 1))

I don't see an immediate issue with that but I haven't thought too deeply about what the implications for the linker and platform analyzer are.

[jeffhandley]

I think the primary benefit of the os-specific version methods is what @eerhardt called out:

I think it makes sense to make the APIs conform to the OS they are specific for. We discussed this a bit during the first round of reviews this. And we are also already doing this with:

Because the versions of those OS platforms aren't useful. Just like the revision number for any OSX family OS isn't useful.

Rewatching that review meeting, I noticed @GrabYourPitchforks mentioned that for Windows, we won't ever care about revision, but #40457 includes that segment for IsWindowsVersionAtLeast. @GrabYourPitchforks, should we remove revision` from that method?

[am11]

So far, we have been living with a single RntimeInformation.IsOSPlatform() method for platform detection. It wouldn't have been a surprise if OperatingSystem class also get one public bool Is() and one public bool IsVersionAtLeast().

If the potential benefit of this work is linker-friendliness, then it is bit of an overkill to add so many last-minute public Is<X> methods with platform names in them; just because linker cannot recognize certain pattern (maybe it can be hardcoded in the linker).

[jeffhandley]

Thanks for the suggestion. We've considered it some more but decided to stick with the OS-specific version methods.

The OS-specific version methods allow us to understand versioning of each OS canonically and guide folks more easily into the right version checks. As a bonus, the XML doc comments for the methods can reference how the version is detected on each platform, as is seen in this comment:

        /// <summary>
        /// Check for the tvOS version (returned by 'libobjc.get_operatingSystemVersion') with a >= version comparison. Used to guard APIs that were added in the given tvOS release.
        /// </summary>
        public static bool IsTvOSVersionAtLeast(int major, int minor = 0, int build = 0)
            => IsTvOS() && IsOSVersionAtLeast(major, minor, build, 0);