Skip to content

Conversation

@akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Nov 16, 2022

While debugging #29029 I noticed that we hit an unhandled exception because later macOS versions don't have an extra dot in the RID version number so the string split failed.

Switched to using float parsing for the version like we do for Ubuntu.

@ghost
Copy link

ghost commented Nov 16, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

string osxVersionString = restOfRid.Split('-')[0];

// Starting with macOS 12.0 (Monterey), the RID is "osx.12-x64" instead of the "osx.10.14-x64" format used before, so the version won't have a dot in it
if (!osxVersionString.Contains('.'))
Copy link
Member

Choose a reason for hiding this comment

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

Instead of relying on the fact that the RID doesn't contain a period in it, could we make the OS parsing resilient to not having a dot? Then this condition would be something like macOSMajorVersion >= 12.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dsplaisted good point, I switched to using float parsing for the version like we do for Ubuntu.

@dsplaisted
Copy link
Member

@akoeplinger I'm not sure float parsing is a good idea here either, since floats can't represent decimals exactly and you might get a result something like 16.0400000000001. I was going to suggest something like Version.Parse, but that wouldn't be able to tell the difference between 16.04 and 16.4. So probably Decimal.Parse would be better.

@akoeplinger
Copy link
Member Author

@dsplaisted I tried and float.Parse("10.12") == 10.12f returns true, which is the only case that counts since others use </>

@akoeplinger
Copy link
Member Author

besides that, 10.12 is ancient so we could probably get rid of that specific version check :)

While debugging dotnet#29029 I noticed that we hit an unhandled exception because later macOS versions don't have an extra dot in the RID version number so the string split failed.

Switched to using float parsing for the version like we do for Ubuntu.
@akoeplinger akoeplinger force-pushed the fix-unhandled-exception branch from 145c8e8 to 77c0980 Compare November 21, 2022 12:24
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.

2 participants