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

Environment.OSVersion returns wrong info on Big Sur #41012

Closed
wfurt opened this issue Aug 19, 2020 · 26 comments · Fixed by #41176
Closed

Environment.OSVersion returns wrong info on Big Sur #41012

wfurt opened this issue Aug 19, 2020 · 26 comments · Fixed by #41176

Comments

@wfurt
Copy link
Member

wfurt commented Aug 19, 2020

This returns 10.16 and may be caveat of the beta.
However, is is curious that OS tools report 11.0 as expected:

furt@macos-11 foo % sysctl kern.osproductversion
kern.osproductversion: 11.0

furt@macos-11 foo % sw_vers
ProductName:	macOS
ProductVersion:	11.0
BuildVersion:	20A5343i

This seems to also impact dotnet --info and perhaps package selection.

cc: @akoeplinger @marek-safar @danmosemsft

@Dotnet-GitSync-Bot
Copy link
Collaborator

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

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 19, 2020
@danmoseley
Copy link
Member

internal static Version GetOperatingSystemVersion()
{
int major = 0;
int minor = 0;
int patch = 0;
IntPtr processInfo = objc_msgSend(objc_getClass("NSProcessInfo"), sel_getUid("processInfo"));
if (processInfo != IntPtr.Zero)
{
NSOperatingSystemVersion osVersion = get_operatingSystemVersion(processInfo, sel_getUid("operatingSystemVersion"));
checked
{
major = (int)osVersion.majorVersion;
minor = (int)osVersion.minorVersion;
patch = (int)osVersion.patchVersion;

@wfurt
Copy link
Member Author

wfurt commented Aug 19, 2020

yah, I saw that. Some objective C messaging...

@danmoseley
Copy link
Member

This suggests everything ultimately reads /System/Library/CoreServices/SystemVersion.plist - maybe not in any more. What is in there on your system?

@wfurt
Copy link
Member Author

wfurt commented Aug 19, 2020

<plist version="1.0">
<dict>
        <key>ProductBuildVersion</key>
        <string>20A5343i</string>
        <key>ProductCopyright</key>
        <string>1983-2020 Apple Inc.</string>
        <key>ProductName</key>
        <string>macOS</string>
        <key>ProductUserVisibleVersion</key>
        <string>11.0</string>
        <key>ProductVersion</key>
        <string>11.0</string>
        <key>iOSSupportVersion</key>
        <string>14.0</string>
</dict>
</plist>

Parsing files seems tricky. What is curious is that we seems to be using API Apple does not use for system utilities.
They may fix it eventually. If supported on mobile devices, systctl seems easies. We already have wrapper and it gives back string.

@vcsjones
Copy link
Member

vcsjones commented Aug 19, 2020

It's dependent on which macOS SDK is used. If the code is built with xcode 12 beta and uses the macOS 11 SDK, then operatingSystemVersion returns 11. If you build using the macOS 10.15 SDK or lower, it returns a "compat" version of 10.16.

I think Apple did this to mitigate older apps making the assumption that major version would be 10 forever, and only return the later version on the new SDK.

@wfurt
Copy link
Member Author

wfurt commented Aug 19, 2020

Interesting. Since we do single build on 10.13, we may do some fix-ups here.

@wfurt
Copy link
Member Author

wfurt commented Aug 20, 2020

This is more tricky than I expected. I added code to get version via sysctl. However it seems to also go through compat layer so when I ask for kern.osproductversion from command line, I get 11.0 but when I ask for the same value from dotnet I get 10.16. Also there is environment variable SYSTEM_VERSION_COMPAT and when I set it to 0, I get 11.0 using existing dotnet builds.

I think we have following options IMHO:

  1. update SDK on build machines
  2. add mapping 10.16 -> 11.0
  3. set SYSTEM_VERSION_COMPAT to 0 if not set already somewhere.
  4. use different method - like parsing files - to detect version

While 1. is probably best option long term, 2. would allow us to move forward quickly and safely. We can itterate on the fix once 11.0 really ships and is stable. I personally don't like fourth option. That seems something hard to maintain over time.

Let me know @danmosemsft what do you want to do.

There is some (unofficial) article suggesting this may be permanent:
https://eclecticlight.co/2020/07/06/why-big-sur-wont-stumble-over-version-numbers/

@jkotas
Copy link
Member

jkotas commented Aug 20, 2020

  1. would allow us to move forward quickly and safely

+1

@danmoseley
Copy link
Member

danmoseley commented Aug 20, 2020

I like # 2 ... and eventually we will build 6.0 (?) using the 11.0 SDK and we can remove it. I assume we will backport this also, but it woudl be for Oct at this point which is fine.

@wfurt
Copy link
Member Author

wfurt commented Aug 20, 2020

ok. SYSTEM_VERSION_COMPAT can be used as workaround by anybody who really want to see 11.0 right now. (and we added compat 10.16 RID as well)

@danmoseley danmoseley removed the untriaged New issue has not been triaged by the area owner label Aug 21, 2020
@danmoseley danmoseley added this to the 5.0.0 milestone Aug 21, 2020
@danmoseley danmoseley added the os-mac-os-x macOS aka OSX label Aug 21, 2020
@danmoseley danmoseley reopened this Aug 24, 2020
@danmoseley danmoseley modified the milestones: 5.0.0, 3.1.9 Aug 24, 2020
@danmoseley
Copy link
Member

/backport to release/5.0

@danmoseley
Copy link
Member

oops, should be on the PR!

@NicoleWang001
Copy link
Member

@danmosemsft , @wfurt ,the OS version is still shown as 10.16 instead of 11.0 with October Update(runtime 3.1.9 and 2.1.23). Here is the result for runtime 3.1.9
_.NET Core SDK (reflecting any global.json):
Version: 3.1.403
Commit: 9e895200cd

Runtime Environment:
OS Name: Mac OS X
OS Version: 10.16
OS Platform: Darwin
RID: osx.10.16-x64
Base Path: /usr/local/share/dotnet/sdk/3.1.403/_
...

@danmoseley
Copy link
Member

This didn't go into 2.1/3.1 yet. I don't recall whether we decided to wait for customer request, or we overlooked it.

@wfurt should we also port #41179 ? I think the only Big Sur change we backported was the RID graph.

Next release would be the Nov release -- 3.1.10 and 2.1.24 I think.

@wfurt
Copy link
Member Author

wfurt commented Sep 28, 2020

real impact of #41179 is probably small but since risk is also small I would do it if we want clean CI runs. Should I create PR for 3.1 and possibly 2.1 @danmosemsft ?

@danmoseley
Copy link
Member

I don't want to change the product just to fix a test. What about customer impact -- it's that this API is broken on Big Sur?

For this one, @NicoleWang001 any particular reason you ask? Or just noticed?

@wfurt
Copy link
Member Author

wfurt commented Sep 28, 2020

yes, GetIcmpV6Statistics does not work on Big Sur. That was discovered by our tests - I have not seen any real use and apisof.net do't show much either.

@danmoseley
Copy link
Member

OK, doesn't meet the servicing bar I think unless and until customers tell us it's essential. (cc @karelz)

Environment.OSVersion I'm thinking is worth porting, as otherwise it will be surprising when folks move code to 5.0+. If we do it now, we break few or nobody because they are'nt using Big Sur yet. What do you think?

@wfurt
Copy link
Member Author

wfurt commented Sep 28, 2020

One could argue the versioning is cosmetic since RID graph can support both. But I think this will be surprising to many users and I would port to at least 3.1 to avoid confusion - under "new OS support" rule - which should be "tell mode", right?

@jkotas
Copy link
Member

jkotas commented Sep 28, 2020

as otherwise it will be surprising when folks move code to 5.0+

All existing programs on Big Sur will have this behavior. I do not think it will be that surprising.

"new OS support" rule - which should be "tell mode", right?

I have not heard about a rule like that. (Also, this is a cosmetic change. It is not required to make things work on the new OS.)

@wfurt
Copy link
Member Author

wfurt commented Sep 28, 2020

https://dev.azure.com/devdiv/DevDiv/_wiki/wikis/DevDiv.wiki/545/NET-Core-Servicing
The "New distro support as needed to stay on supported distros" is not tell mode as I remembered. Maybe that was only for RID.

@danmoseley
Copy link
Member

All existing programs on Big Sur will have this behavior. I do not think it will be that surprising.

What I meant was: if we made the update before Big Sur gets widely deployed, hardly any code will see 10.16 before we change it to 11.0. Fewer people will hve written code like if (OSVersion == 10.16 ){ doBigSurBehavior(); }. Notwithstanding that, I see no reason to fix it unless and until we get evidence of clear customer need as so far nobody's reported it.

I am happy to close this in that case..

@danmoseley
Copy link
Member

Getting another perspective - @marek-safar thoughts about the importance of backporting this change?

If nobody feels strongly -we can close this.

@marek-safar
Copy link
Contributor

It does not meet the bar for 3.1 for me and it'd be breaking change in minor 3.1 version bump. I think it's better to keep the change for 5.0+ only.

@danmoseley
Copy link
Member

OK, closing

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants