Expose OperatingSystem APIs as intrinsic property functions

[akoeplinger]

These are new in .NET 5.0: https://docs.microsoft.com/en-us/dotnet/api/system.operatingsystem?view=net-5.0

They expose methods for all the common operating systems like OperatingSystem.IsWindows() but also still support the API where you pass in a string: OperatingSystem.IsOSPlatform(string platform).

The new APIs are easier to understand so we should consider exposing them.

[vijaya-lakshmi-venkatraman]

Hi, I would like to take this up if still available. I am a newbie to MSBuild and appreciate any pointers to fix this issue

Thanks.

[akoeplinger]

@vijaya-lakshmi-venkatraman sure, I assigned the issue to you so you can give it a try :)

I think adding an entry similar to RuntimeInformation here should be enough: https://github.com/dotnet/msbuild/blob/5b9216a75e98e19eba84e04a5f30bd35a68f317a/src/Build/Resources/Constants.cs#L379

You'll probably need to wrap it in some ifdef, e.g. FEATURE_OS_APIS, since the API is only available from .NET 5+, so add the define here in a new PropertyGroup for net5.0 here: https://github.com/dotnet/msbuild/blob/5b9216a75e98e19eba84e04a5f30bd35a68f317a/src/Directory.BeforeCommon.targets#L127

Also tests would be good :)

[vijaya-lakshmi-venkatraman]

Thank you for assigning the issue. I will add a new PropertyGroup as below:

 <PropertyGroup Condition="'$(TargetFramework)' == 'net5.0'">
    <DefineConstants>$(DefineConstants);FEATURE_OS_APIS</DefineConstants>
  </PropertyGroup>

Please can you explain a bit on the change related to adding an entry similar to RuntimeInformation. I am afraid I did not get that part :(

[akoeplinger]

It would look something like this:

                        var fileType = new Tuple<string, Type>(null, typeof(File));
                        var runtimeInformationType = new Tuple<string, Type>(null, typeof(RuntimeInformation));
                        var osPlatformType = new Tuple<string, Type>(null, typeof(OSPlatform));
+#if FEATURE_OS_APIS
+                       var operatingSystemType = new Tuple<string, Type>(null, typeof(OperatingSystem));
+#endif

                        // Make specific static methods available (Assembly qualified type names are *NOT* supported, only null which means mscorlib):
                        availableStaticMethods.TryAdd("System.Environment::ExpandEnvironmentVariables", environmentType);
@@ -378,6 +381,9 @@ private static void InitializeAvailableMethods()
                        availableStaticMethods.TryAdd("Microsoft.Build.Utilities.ToolLocationHelper", new Tuple<string, Type>("Microsoft.Build.Utilities.ToolLocationHelper, Microsoft.Build.Utilities.Core, Version=" + MSBuildConstants.CurrentAssemblyVersion + ", Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", null));
                        availableStaticMethods.TryAdd("System.Runtime.InteropServices.RuntimeInformation", runtimeInformationType);
                        availableStaticMethods.TryAdd("System.Runtime.InteropServices.OSPlatform", osPlatformType);
+#if FEATURE_OS_APIS
+                       availableStaticMethods.TryAdd("System.OperatingSystem", operatingSystemType);
+#endif

                        s_availableStaticMethods = availableStaticMethods;
                    }

[akoeplinger]

@vijaya-lakshmi-venkatraman do you still plan to work on this issue or should we put it back into the backlog?

[vijaya-lakshmi-venkatraman]

Hi @akoeplinger yes, I have the changes locally but been trying to figure out reg CLA. Hoping to merge and raise a PR soon.

[vijaya-lakshmi-venkatraman]

Hi @akoeplinger Do you know how to get the Corporate CLA signed for contributions?

[akoeplinger]

@vijaya-lakshmi-venkatraman if you open a PR then you'll see a GitHub check with the CLA link and instructions on how to sign it.

[vijaya-lakshmi-venkatraman]

It'd be my manager that'd sign the CLA. So can I request him to sign on the CLA link?

[danmoseley]

@vijaya-lakshmi-venkatraman as engineers, we unfortunately can't offer legal advice beyond the CLA bot instructions.

[akoeplinger]

From what I remember when signing the CLA five years ago before I started working for Microsoft you just had to acknowledge that your employer is fine with the contribution, i.e. you're responsible for checking with them but no separate signature required for the CLA. Not sure if it has changed since then so best to check with legal if you're unsure.

[ghost]

@akoeplinger @vijaya-lakshmi-venkatraman @danmoseley

Is anyone currently working on this?

The above collaboration makes sense for this specific issue, these Operating Systems APIs are only available in .NET 5.0 so adding the const FEATURE_OS_APIS only when it is available in .NET 5.0 and defining those ifdefs in the constant file to ensure it only gets added to the dictionary at that point in time.

[vijaya-lakshmi-venkatraman]

Hi @WingZer0o, Sorry for the delay in response I could not sort the CLA issue yet. I see you have already raised a PR so I have unassigned myself from the issue.

[jrdodds]

It seems like efforts on this issue ended without completion. If this work should still be done, please assign the issue to me. Thanks

[akoeplinger]

@jrdodds I've assigned the issue to you, thanks for giving it a try 😄

You can look at https://github.com/dotnet/msbuild/pull/8082 for the unfinished PR which had some good discussion.

[jrdodds]

In the unfinished PR, there is a discussion about making the static methods available in net472 builds of MSBuild. From the discussion:

I think the easiest way to implement this would be to create a new type named System.OperatingSystem here in our codebase, with #if around it so it only compiles for .NET Framework.

The System.OperatingSystem type was in .Net Framework 1.1. The static methods were added in .Net 5.0. We can't add a type (as was done for SupportedOSPlatform), because the type exists in net472. The type is a public sealed class and can't be extended as a partial.

Two possible options:

• Don't support net472. The System.OperatingSystem static methods will be available in net7.0 builds only.
• Don't expose the System.OperatingSystem static methods and instead add a set of [MSBuild]:: intrinsic functions that either call the System.OperatingSystem static methods or have an alternate implementation for net472.

Are there other options?

Also, net472 builds can make the assumption that the operating system can only be Windows. What should netstandard2.0 builds do?

[rainersigwald]

• Don't support net472. The System.OperatingSystem static methods will be available in net7.0 builds only.

I do not like this; new functionality should be cross-platform.

• Don't expose the System.OperatingSystem static methods and instead add a set of [MSBuild]:: intrinsic functions that either call the System.OperatingSystem static methods or have an alternate implementation for net472.

We already have [MSBuild]::IsOsPlatform and ::IsOSUnixLike, so this doesn't seem worth doing to me.

The System.OperatingSystem type was in .Net Framework 1.1. The static methods were added in .Net 5.0. We can't add a type (as was done for SupportedOSPlatform), because the type exists in net472. The type is a public sealed class and can't be extended as a partial.

This is all true, but we can tweak the idea: we don't necessarily have to dispatch to the real type when a project says $([System.OperatingSystem]::IsWindows()). We could treat that specially (in the .NET Framework MSBuild) and dispatch to a private type that reimplements it.

Also, net472 builds can make the assumption that the operating system can only be Windows. What should netstandard2.0 builds do?

"Anything that compiles." There is no scenario where netstandard2.0 MSBuild assemblies are used at runtime; they're API-surface-only, and this shouldn't affect the public API surface.

[jrdodds]

we don't necessarily have to dispatch to the real type when a project says $([System.OperatingSystem]::IsWindows()). We could treat that specially (in the .NET Framework MSBuild) and dispatch to a private type that reimplements it.

Cool. Is there an example of how that is done?

Update: I think I see how it works.