map macOS compat 10.16 version to 11.0 #41176
Conversation
|
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. |
| // certain APIs work till 10.9 and have been deprecated and others require linking against | ||
| // UI frameworks to get the data. | ||
| // | ||
| // We will, instead, use kern.osrelease and use its major version number |
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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.
|
In the issue, we discussed the minimal change on the managed side -- which is super low risk and can be backported. Do we need to consider backporting the RID change too? I don't have context on what it achieves.. cc @ericstj |
| // | ||
| // macOS Cataline 10.15.5 has kernel 19.5.0 | ||
| int minorVersion = majorVersion - 4; | ||
| if (minorVersion < 10) |
There was a problem hiding this comment.
This is a pre-existing bug -- it should be 12, not 10.
| // 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)); |
There was a problem hiding this comment.
Can the integer be negative? Should this be stoul?
|
/backport to release/5.0 |
|
Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/222641990 |
|
This seems to have caused a new regression for me? |
|
Is that executed on macOS 11? I cannot tell from the log @directhex |
|
|
|
Some tests setups use stale |
|
I'll check if the test pass locally. I did not run this test set before. |
|
@wfurt how did it look locally? I'm still getting errors out of Helix |
|
It fails for me locally as well @directhex. But if I replace |
|
/backport to release/5.0 |
|
I think we probably want to propose this change to Tactics for 5.0, since it's very localized, and to accommodate the OS which will be released 1 year before 6.0. |
|
Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/234887530 |
|
@danmosemsft backporting to release/5.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: map macOS compat 10.16 version to 11.0
Using index info to reconstruct a base tree...
M src/installer/corehost/cli/hostmisc/pal.unix.cpp
M src/libraries/Common/src/Interop/OSX/Interop.libobjc.cs
Falling back to patching base and 3-way merge...
Auto-merging src/installer/corehost/cli/hostmisc/pal.unix.cpp
CONFLICT (content): Merge conflict in src/installer/corehost/cli/hostmisc/pal.unix.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 map macOS compat 10.16 version to 11.0
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
Never mind- it's already there. |
ghost
locked as resolved and limited conversation to collaborators
6 participants
It seems like we get "wrong" os version because we build and link against older macOS SDK.
With that we get 10.16 instead of 11.0 as expected.
For time being this adds mapping from 10.16 to 11.0.
It also updates RID calculation. That one is based on kernel version.
11.0 ships with 20.0 and the current logic needed to be updated as well.
fixes #41012