SupportedPlatform calculation for platform compatibility analyzer is wrong

[ViktorHofer]

Problem statement

The platform compatibility analyzer depends on a set of msbuild items as inputs based on the platform exclusion spec.

From the spec:

In order to indicate which platforms the analyzer should warn about, we're adding some metadata to MSBuild: <SupportedPlatform Include="..." />

Based on the spec, the SupportedPlatform item defines the list of platforms that the platform compatibility analyzer should warn about.

The SDK unconditionally adds supported platforms and one of those is Windows. The SDK itself doesn't make it possible to only target platforms like macOS or Linux via a target platform, but we in dotnet/runtime allow that via the Microsoft.DotNet.TargetFramework.Sdk.

As reported in #69980 (comment), the warnings are currently not correct in certain cases, i.e. when a target framework is platform agnostic.

Here you can see that the SupportedPlatform metadata is added for Browser when the platform is Browser, or when it's agnostic and the library doesn't have a Browser specific implementation.

I think we would want the same for Unix and for any other platform.

Possible solution

Please see ViktorHofer#2 (my fork, just to get feedback). That logic calculates the SupportedPlatforms items correctly based on the RID graph. This implementation is completely static but solves the noted issues.

To make this dynamic, an msbuild task in the TargetFramework.Sdk would be required that takes the build rid graph (OSGroups.json), TargetFrameworks, TargetFramework and TargetPlatformIdentifier as an input and outputs the SupportedPlatform item to be added.

cc @buyaa-n @jeffhandley @ericstj

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Moving this discussion out from #69980 (comment).

@buyaa-n @ViktorHofer the File.SetUnixFileMode method is marked as [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")] and this particular C# file is compiled only for Unix:

The SDK unconditionally adds supported platforms and one of those is Windows. The SDK itself doesn't make it possible to only target Windows, macOS or Linux via a target platform, but we in dotnet/runtime allow that via the Microsoft.DotNet.TargetFramework.Sdk.

Because of that, we would need to remove these unconditionally set SupportedPlatform items and instead add them conditionally similar to how it is done for Browser.

Here you can see that the SupportedPlatform metadata is added for Browser when the platform is Browser, or when it's agnostic and the library doesn't have a Browser specific implementation.

I think we would want the same for Unix and for any other platform.

EDIT:
Please see ViktorHofer#2 (my fork, just to get feedback). That logic calculates the SupportedPlatforms items correctly based on the RID graph. This implementation is completely static but solves the noted issues.

To make this dynamic, an msbuild task in the TargetFramework.Sdk would be required that takes the build rid graph (OSGroups.json), TargetFrameworks, TargetFramework and TargetPlatformIdentifier as an input and outputs the SupportedPlatform item to be added.

cc @buyaa-n @jeffhandley @ericstj

Author:	ViktorHofer
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	-

[ViktorHofer]

@buyaa-n commented on the related issue:

SDK supported platforms cause warnings only for cross platform targets, has no effect targeted builds. I might missing something but still do not understand why we could not use Linux or Unix targets for the above mentioned build and keep the $(NetCoreAppCurrent) only for real cross platform targets.

I tried to explain that in the related issue:

Some libraries intentionally define a TargetPlatform agnostic target framework which serves as the default configuration to reduce the number of inner builds in the build graph. Such a TargetPlatform agnostic target framework is either truly platform agnostic or it targets a specific platform that is believed to be a good default (in most / all cases Unix).

If you would introduce a net7.0-Unix tfm in

runtime/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj

Line 4 in 911da68

<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)</TargetFrameworks>

, you would also need a platform agnostic tfm: net7.0 as that's required to compose the shared framework.

That would increase the inner builds from two (net7.0-windows, net7.0) to three (net7.0-windows, net7.0-Unix, net7.0). In projects which also ship via packages, you would increase the number of inner builds further, because of net6.0 which would need to follow the same pattern. For that project, we chose Unix to be the default implementation: net7.0.

[ViktorHofer]

@ericstj commented on the PR:

Is the problem that TargetPlatformIdentifier isn't set at this point of the evaluation? If so - could we move these to a target?

No, in the context of versioning.targets, the TargetPlatformIdentifier property is fully evaluated, that's not the problem. Please see the top post in which I tried to explain the existing issue.

[ericstj]

I see.

To make this dynamic, an msbuild task in the TargetFramework.Sdk would be required that takes the build rid graph (OSGroups.json), TargetFrameworks, TargetFramework and TargetPlatformIdentifier as an input and outputs the SupportedPlatform item to be added.

Is that enough? Don't we also need to know what items in that set are actual support vs which are PNSEs? I imagine we're trying to apply these SupportedFramework items on the RID-less configuration and the reference assembly so that there is some design time checking of support.

One solution could be to evaluate the best TargetFramework value for each known supported platform value we might include, then call a target in the project evaluated for that TargetFramework to determine if it's a PNSE or not.

Perhaps another way might be to change how we represent PNSEs so that they can be known statically from the outer build (EG: represent them as a list that's a subset of TargetFrameworks).

[buyaa-n]

@ViktorHofer thanks for the explanation, question: are all TFMs like <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)</TargetFrameworks> are equivalent to <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix</TargetFrameworks>?

I assume there is a case for a real cross platform target/assembly, what will be the TFM for that case? <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>?