Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the default SupportedOSPlatformVersion value #9260

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mattleibow
Copy link
Member

All the other .NET SDKs and workloads don't do anything and
the SDK will set it to the TFM version.

This default here also makes it hard to know if the user has set
the value and we should warn about the value, or if we are allowed
to set it. By keeping it blank, any value means the user set it, but
blank means we can set it to a better default in the MAUI SDK.

All the other .NET SDKs and workloads don't do anything and the SDK will set it to the TFM version.

This default here also makes it hard to know if the user has set the value and we should warn about the value, or if we are allowed to set it. By keeping it blank, any value means the user set it, but blank means we can set it to a better default in the MAUI SDK.
@dellis1972
Copy link
Contributor

Do all the templates set this value to a default then? (if its not in the targets)

Comment on lines -29 to -30
<!-- Use $(AndroidMinimumSupportedApiLevel) for $(SupportedOSPlatformVersion) if unset -->
<SupportedOSPlatformVersion Condition=" '$(SupportedOSPlatformVersion)' == '' ">$(AndroidMinimumSupportedApiLevel)</SupportedOSPlatformVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rolfbjarne from your memory of the meeting, should we actually leave this in and add a similar default in xamarin/xamarin-macios?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about doing it either way:

  • Switch the default to the minimum across all workloads.
  • Not set a default in any SDK workload, and make MAUI/Blazor/whomever set a default value.

Note that the spec says "Project files may specify a SupportedOSPlatformVersion property, and if they do not it will default to OS Version equivalent to the OS API version specified in the TargetPlatformVersion value."

I don't know what Google does with Android, but Xcode does more or less the same thing, defaults to the latest OS version.

I think either way can work, although I lean a little bit towards the latter (it's what the spec says, but I might also be influenced by the fact that I wouldn't have to do anything...). I also think the latter way works better if MAUI/Blazor/whomever wants a different default than the min supported by the SDK.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple test failures for SupportedOSPlatformVersion*, so we'd probably also need to update the tests if this behavior changes.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we had some discussion about 941e04c here, and why it set SupportedOSPlatformVersion=21 by default.

The problem is:

  • I don't have SupportedOSPlatformVersion set in my Android project
  • I am using an API 34 emulator and .NET 8
  • I upgrade to .NET 9
  • All of a sudden, I can't deploy to the API 34 emulator anymore

Removing SupportedOSPlatformVersion=21 by default would make the behavior match the .NET SDK, and so it would be 35... This sets minSdkVersion="35" making the app not installable on an API-34 emulator.

I'm not sure we should make this change, but is there something this "fixes" we should look at?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants