Skip to content

map macOS compat 10.16 version to 11.0 #41176

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 24, 2020
Merged

map macOS compat 10.16 version to 11.0 #41176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7163c7
map macOS compat 10.16 version to 11.0
wfurt Aug 21, 2020
45e1c74
feedback from review
wfurt Aug 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions src/installer/corehost/cli/hostmisc/pal.unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,6 @@ pal::string_t pal::get_current_os_rid_platform()
// We will, instead, use kern.osrelease and use its major version number
Copy link
Member

@danmoseley danmoseley Aug 21, 2020

Choose a reason for hiding this comment

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

This SO answer suggests that kern.osrelease is not a future proof way to get macOS versions -- https://stackoverflow.com/questions/11072804/how-do-i-determine-the-os-version-at-runtime-in-os-x-or-ios-without-using-gesta#comment40165087_11697362
but again we're clearly already doing it..

Copy link
Member Author

@wfurt wfurt Aug 21, 2020

Choose a reason for hiding this comment

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

We should to kern.osproductversion once we update SDK and we only support versions where supported.
It returns 11.0 on Big Sure and 10.15.5 on Catalina.
10.12 does not have it and I did not check 10.13 and 10.14 yet.
We can also use it when exist and fall-back to kernel mapping for old versions where it is unlikely to change.

// as a means to formulate the OSX 10.X RID.
//
// Needless to say, this will need to be updated if OSX RID were to become 11.* ever.
size_t size = sizeof(str);
int ret = sysctlbyname("kern.osrelease", str, &size, nullptr, 0);
if (ret == 0)
Expand All @@ -562,18 +561,31 @@ pal::string_t pal::get_current_os_rid_platform()
size_t pos = release.find('.');
if (pos != std::string::npos)
{
// Extract the major version and subtract 4 from it
// to get the Minor version used in OSX versioning scheme.
// That is, given a version 10.X.Y, we will get X below.
int minorVersion = stoi(release.substr(0, pos)) - 4;
if (minorVersion < 10)
int majorVersion = stoi(release.substr(0, pos));
Copy link
Contributor

@scalablecory scalablecory Aug 24, 2020

Choose a reason for hiding this comment

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

Can the integer be negative? Should this be stoul?

// compat path with 10.x
if (majorVersion < 20)
{
// On OSX, our minimum supported RID is 10.12.
minorVersion = 12;
}
// Extract the major version and subtract 4 from it
// to get the Minor version used in OSX versioning scheme.
// That is, given a version 10.X.Y, we will get X below.
//
// macOS Cataline 10.15.5 has kernel 19.5.0
int minorVersion = majorVersion - 4;
if (minorVersion < 10)
Copy link
Member

@danmoseley danmoseley Aug 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is a pre-existing bug -- it should be 12, not 10.

{
// On OSX, our minimum supported RID is 10.12.
minorVersion = 12;
}

ridOS.append(_X("osx.10."));
ridOS.append(pal::to_string(minorVersion));
ridOS.append(_X("osx.10."));
ridOS.append(pal::to_string(minorVersion));
}
else
{
// 11.0 shipped with kernel 20.0
ridOS.append(_X("osx.11."));
ridOS.append(pal::to_string(majorVersion - 20));
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/libraries/Common/src/Interop/OSX/Interop.libobjc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ internal static Version GetOperatingSystemVersion()
}
}

if (major == 10 && minor == 16)
{
// We get "compat" version for 11.0 unless we build with updated SDK.
// Hopefully that will be before 11.x comes out
// For now, this maps 10.16 to 11.0.
major = 11;
minor = 0;
}

return new Version(major, minor, patch);
}

Expand Down