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

RID work for macOS 12 #59066

Merged
merged 4 commits into from
Sep 30, 2021
Merged

RID work for macOS 12 #59066

merged 4 commits into from
Sep 30, 2021

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Sep 13, 2021

Note this is weird as with 11.0 the naming has changed as well as and OS was lying us of a while about actual os version, reporting 10.17 instead. Since we seems to use updated SDK this does not seems to be necessary any more so as the game of mapping kernel to OS version.

It seems like starting with 11.0, the releases we used to track as 10.x are really now major releases and the 'x' becomes insignificant. So I changed the 12 rid to match Win & RHEL.

We may do more cleanup for Environment.OSVersion but since that seems to war I'll do it separately if needed.
kern.osproductversion was missing in some very old OS X versions but it is available in 10.13 and later.

@ghost
Copy link

ghost commented Sep 13, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Note this is weird as with 11.0 the naming has changed as well as and OS was lying us of a while about actual os version, reporting 10.17 instead. Since we seems to use updated SDK this does not seems to be necessary any more so as the game of mapping kernel to OS version.

It seems like starting with 11.0, the releases we used to track as 10.x are really now major releases and the 'x' becomes insignificant. So I changed the 12 rid to match Win & RHEL.

We may do more cleanup for Environment.OSVersion but since that seems to war I'll do it separately if needed.
kern.osproductversion was missing in some very old OS X versions but it is available in 10.13 and later.

Author: wfurt
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@wfurt wfurt requested a review from stephentoub September 29, 2021 19:11
@wfurt
Copy link
Member Author

wfurt commented Sep 29, 2021

ping. anybody brave?

@ghost
Copy link

ghost commented Sep 29, 2021

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Note this is weird as with 11.0 the naming has changed as well as and OS was lying us of a while about actual os version, reporting 10.17 instead. Since we seems to use updated SDK this does not seems to be necessary any more so as the game of mapping kernel to OS version.

It seems like starting with 11.0, the releases we used to track as 10.x are really now major releases and the 'x' becomes insignificant. So I changed the 12 rid to match Win & RHEL.

We may do more cleanup for Environment.OSVersion but since that seems to war I'll do it separately if needed.
kern.osproductversion was missing in some very old OS X versions but it is available in 10.13 and later.

Author: wfurt
Assignees: -
Labels:

area-Host

Milestone: -

@marek-safar
Copy link
Contributor

Are you planning to backport this to 6.0?

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2021

yes. I think this would qualify for servicing. I will get ready separate change for 5.x

@marek-safar
Copy link
Contributor

@agocke please speed up the review

@agocke
Copy link
Member

agocke commented Sep 30, 2021

The precedent we're setting with Windows 11 is that we match the OS product version, not the marketing version. It looks to me like that's what this PR is doing -- that the OS is reporting a new product version. Is that correct?

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2021

There is really no marketing version here. With this, we should match what sw_vers reports and what you see one the About dialog @agocke.

There are two parts: In the old days, there was no convent API to get that so we had mapping from kernel version to OS version. AFAIK that is no longer needed.
For bits but agains older SDK the OS would lie and report "compact" version eg. 10.x instead of 11.0 and 12.0. That impacts 5.0 (will need separate change) but not 6.0 as far as I can tell.

And there is the "normal" rid work to add 12.0 to the graph.

@vitek-karas
Copy link
Member

I personally don't know versioning story around macOS.

On Windows we decided (See #58588) that there's little point in introducing win11 RIDs. Core of the reasoning is that OS still returns 10 as the major version and that we don't introduce new RIDs for Windows Server releases either.

If on macOS the OS actually returns the new version and it makes a material difference, then we should add a new RID (I don't know this though). I guess this is just a "small pushback" to make sure that we need to add the new RID (trying to keep the always growing list of RIDs in check).

@wfurt
Copy link
Member Author

wfurt commented Sep 30, 2021

We already made the leap with 11.0 @vitek-karas. (with 5.0 & ARM based machines)

@agocke
Copy link
Member

agocke commented Sep 30, 2021

It looks like Big Sur reported itself as 11.0, except if you set SYSTEM_VERSION_COMPAT=1. I don't think we should operate based on compat versions, so by that metric, the system version was 11.0 and Monterey should be 12.0.

Change LGTM

@vitek-karas
Copy link
Member

OK - makes sense 😄

@wfurt wfurt merged commit 28f2f86 into dotnet:main Sep 30, 2021
@wfurt
Copy link
Member Author

wfurt commented Oct 28, 2021

I verified the outcome with recent builds on two separate machines using dotnet --info

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  12.0
 OS Platform: Darwin
 RID:         osx.12-x64
 Base Path:   /Users/furt/dotnet-7/sdk/7.0.100-alpha.1.21518.14/


Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  11.6
 OS Platform: Darwin
 RID:         osx.11.0-arm64
 Base Path:   /Users/furt/dotnet-7/sdk/7.0.100-alpha.1.21528.2/

First one is my 12.0 VM. It shows the expected RID matching the change.
Second system us my ARM64 macBook running 11.6. It shows the same legacy osx.11.0 as it used to before the change (because we did not add more rids for the 11.x releases)

Unless there are concerns I'll pick up all the changes and I'll create servicing request for 6.0.x.

wfurt added a commit to wfurt/runtime that referenced this pull request Oct 30, 2021
Anipik pushed a commit that referenced this pull request Nov 10, 2021
* RID work for macOS 12 (#59066)

* fix rid processing on macOS (#60494)

* fix rid processing on macOS

* Update src/native/corehost/hostmisc/pal.unix.cpp

* Update src/native/corehost/hostmisc/pal.unix.cpp

* remove extra size calculation

* Fix "fix rid processing on macOS" (#60668)

The `else if (major == 12)` is dead code, since the previous if `if (major > 11)` would be true for `major == 12`. Judging by the comment and code, it looks like the intention of this `else if` statement was to match `major == 11`.

* add the packaging for platforms package

Co-authored-by: Austin Wise <[email protected]>
Co-authored-by: Anirudh Agnihotry <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Nov 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants