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

[release/9.0-staging] Fix IsOSVersionAtLeast when build or revision are not provided #109332

Open
wants to merge 18 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 29, 2024

Backport of #108748 to release/9.0-staging

/cc @kotlarmilos

Customer Impact

  • Customer reported
  • Found internally

This PR fixes #108694 by updating IsOSVersionAtLeast to treat unspecified build or revision components as zero. Previously, the IsOSVersionAtLeast didn't normalize components, leading to incorrect version checks on MacCatalyst when only two components (major and minor) were provided.

Regression

  • Yes
  • No

On MacCatalyst, the version does not include build or revision components, and tests only covered the four-parameter overload of IsOSVersionAtLeast.

Testing

The OperatingSystem tests have been extended to cover the three- and two-parameter overloads of IsOSVersionAtLeast.

Risk

Low. Only MacCatalyst should be affected, as it is the only platform that does not provide a build component. Also, this change does not introduce a breaking change.

kotlarmilos and others added 17 commits October 29, 2024 09:51
…Version.MacCatalyst.cs

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
…m.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
…m.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
…m.cs

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
…m.cs

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 29, 2024
@kotlarmilos kotlarmilos self-assigned this Oct 29, 2024
@kotlarmilos kotlarmilos added area-System.Runtime os-maccatalyst MacCatalyst OS and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 29, 2024
@kotlarmilos kotlarmilos added this to the 9.0.1 milestone Oct 29, 2024
@kotlarmilos kotlarmilos linked an issue Oct 29, 2024 that may be closed by this pull request
@kotlarmilos kotlarmilos added the Servicing-consider Issue for next servicing release review label Oct 29, 2024
@kotlarmilos kotlarmilos requested a review from jkotas October 29, 2024 11:03
Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

This looks good to me for servicing.

This fix targets only the IsOSVersionAtLeast so that it returns the expected value when a version.Build or version.Revision contains a value less than 0. That is different from the original fix in #108748 where the OperatingSystem instance itself is normalized to have 0 for those properties.

@jeffhandley
Copy link
Member

@artl93 This is ready for Servicing now

@artl93
Copy link
Member

artl93 commented Oct 29, 2024

Thanks! I'd like, to get clearer on the customer scenario for my education.

What does the app see right now? Is the problem that version checks apps do to, for example, make their code selectable based on versions of macOS work properly? How long has MacCatalyst worked this way?

@kotlarmilos
Copy link
Member

What does the app see right now? Is the problem that version checks apps do to, for example, make their code selectable based on versions of macOS work properly?

This issue doesn’t make the code selectable based on different versions but leads to incorrect behavior.

For example, on MacCatalyst, if the version is set to 18.0 , it appears as 18.0.-1.-1. In this case, OperatingSystem.IsMacCatalystVersionAtLeast(18,0) would return false, since it compared a default build component (0) with the OS-provided build component (-1).

How long has MacCatalyst worked this way?

We have no evidence that iOSSupportVersion in SupportVersion.plist has ever included build or revision components.

@kotlarmilos kotlarmilos added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 30, 2024
@carlossanlop carlossanlop modified the milestones: 9.0.1, 9.0.2 Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime os-maccatalyst MacCatalyst OS Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MacCatalyst] OperatingSystem is currently not working
5 participants