Ensure `MinimumOSVersion` is is consistent with`SupportedOSPlatformVersion`

[mhutch]

We should ensure that the MinimumOSVersion property in the Info.plist is consistent with the SupportedOSPlatformVersion MSBuild property. SupportedOSPlatformVersion is a standardized across platforms and controls the SDK's platform analyzers.

https://github.com/dotnet/designs/search?q=SupportedOSPlatformVersion

A few options:

• emit a build error if the MinimumOSVersion property and the SupportedOSPlatformVersion MSBuild property do not match
• merge the SupportedOSPlatformVersion MSBuild property into the MinimumOSVersion plist value during app build
• set the SupportedOSPlatformVersion MSBuild property from the MinimumOSVersion plist property in a build target at start of build. the downside would be that it would not be available during evaluation.

[mhutch]

It appears we do emit the SupportedOSPlatformAttribute annotations, so the SupportedOSPlatformVersion property will be used by the platform check analyzers when building in Mac/iOS projects.

[rolfbjarne]

1. Support a platform-specific version (iOSMinimumVersion, tvOSMinimumVersion, etc), and if set, use it to set SupportedOSPlatformVersion:

   <SupportedOSPlatformVersion Condition="'$(iOSMinimumVersion)'!=''">$(iOSMinimumVersion)</SupportedOSPlatformVersion>

   One question is which value should take precedence if both are specified: I feel that iOSMinimumVersion is more specific than SupportedOSPlatformVersion, and if I set both, I'd expect iOSMinimumVersion to be applied to iOS project (only).

   We do this before .NET defaults SupportedOSPlatformVersion to TargetPlatformVersion (in Xamarin.Shared.Sdk.props for instance),

2. This means that we'll always have a value for SupportedOSPlatformVersion. Now the question becomes what to do if the Info.plist specifies a value (where it's historically been set). Typically all current projects will have a value set, so this will be an issue for all legacy projects that have been upgraded to .NET.

   • Show an error if the values don't match.
     • Con: this can be annoying for upgraded projects that already have a value in Info.plist, if that value doesn't happen to match whatever SupportedOSPlatformVersion ends up being.
     • Pro: predictable behavior. We should be able to write a good error message that tells people what to do.
   • Make Info.plist override whatever is specified in SupportedOSPlatformVersion.
     • Con: this can result in confusing/incorrect behavior if other build logic reads SupportedOSPlatformVersion before we can set it to the value from the Info.plist.
     • Pro: this matches how other values in Info.plist work (whatever is set in the Info.plist takes precedence over any MSBuild properties).

   I lean towards the first option, because:

   • It's matches how other platforms work.
   • Any problems with upgraded projects is a one-time cost, and a good error message should mitigate any problems.
   • Less chance of random/confusing/incorrect behavior if someone reads SupportedOSPlatformVersion before we've been able to set it to the correct value.

[dalexsoto]

I lean towards the first option

+1, while painful on migration it is a one time thing instead of the can of worms of option 2

[mhutch]

1. Support a platform-specific version (iOSMinimumVersion, tvOSMinimumVersion, etc), and if set, use it to set SupportedOSPlatformVersion:

Yes, the spec originally recommended this {PlatformIdentifier}MinimumVersion per-platform property naming convention, but I'm no longer sure we should use it. There are many other properties that may need to be set per platform, and it would be inconsistent and confusing if this pattern were only available for some properties on some platforms.

It would be much better if MSBuild had a clean, consistent way to specify per-TFM and per-platform property values. It might look something like this:

<!-- this kind of platform condition doesn't actually exist -->
<PropertyGroup Platform="ios">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

Unfortunately, this kind of simple platform condition does not exist today. There were several discussions on this topic but we did not settle upon a solution. So right now there isn't a good option for this.

If you tried to do a condition like this with MSBuild today, you might try something like this:

<!-- this kind of platform condition cannot be used for properties in a csproj -->
<PropertyGroup Condition="'$(TargetPlatformIdentifier)'=='ios'">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

This platform condition is indeed valid, but only works in some places. It doesn't work for properties and property groups in a csproj, but does work in most targets files. This is because the TargetPlatformIdentifier property is not an intrinsic feature, but is set by the SDK targets, which are evaluated after the csproj file, and decompose the TargetFramework into its components.

Somewhat confusingly, the TargetPlatformIdentifier property is able to be used in conditions on items and item groups. This is because item evaluation takes place in a second pass after the property evaluation pass.

So how do you do it for real? The "correct" way is this incredibly ugly and difficult to read condition:

<!-- this platform condition is correct but ugly -->
<PropertyGroup Condition="'$([MSBuild]::GetTargetPlatformIdentifier(&quot;$(TargetFramework)&quot;'))'=='ios'">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

Checking the verbatim TargetFramework value looks cleaner but does not accurately express intent, and it's fragile, as it will break if you update the framework version or platform version on the TFM without updating all the conditions:

<!-- this platform condition is correct but fragile -->
<PropertyGroup Condition="'$(TargetFramework)'=='net6.0-ios15.0'">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

Right now, in my own projects I'd would probably define TargetPlatformIdentifier myself:

<!-- this platform condition is correct, and limits ugliness to one line -->
<PropertyGroup>
   <TargetFrameworks>net6.0;net6.0-android;net6.0-ios</TargetFrameworks>
   <TargetPlatformIdentifier Condition="'$(TargetFramework)'==''">$([MSBuild]::GetTargetPlatformIdentifier("$(TargetFramework)")</TargetPlatformIdentifier>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetPlatformIdentifier)'=='ios'">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetPlatformIdentifier)'=='android'">
   <SupportedOSPlatformVersion>13.0<SupportedOSPlatformVersion>
</PropertyGroup>

Obviously this isn't something we'd want projects to have to do. We might have to just go with the pattern, at least for now.

/cc @richlander @KathleenDollard @Redth @dsplaisted @terrajobst @rainersigwald

[dsplaisted]

What does MinimumOSVersion do? Why do we have it in addition to SupportedOSPlatformVersion?

[mhutch]

What does MinimumOSVersion do? Why do we have it in addition to SupportedOSPlatformVersion?

@dsplaisted yeah there's some overloading going on here. MinimumOSVersion is a property in the Info.plist app manifest on Apple platforms. It's not an MSBuild property.

MinimumOSVersion was originally also the name I originally used for an MSBuild property in the "minimum OS version" spec, but it got unified with the more general-purpose platform analyzers feature and the spec was updated to use SupportedOSPlatformVersion instead. There was never a real MSBuild property called MinimumOSVersion AFAIK.

[dsplaisted]

I see. In that case I would suggest this option, or something similar:

• merge the SupportedOSPlatformVersion MSBuild property into the MinimumOSVersion plist value during app build

[filipnavara]

This was [at least to some extent] addressed with #12516.