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

macOS os.getversion() improvements #1473

Merged
merged 1 commit into from
Jun 21, 2020

Conversation

noresources
Copy link
Contributor

What does this PR do?

  • Read macOS system version from system property list instead of reading kernel version.
  • Add description for latest versions

How does this PR change Premake's behavior?

  • Same behavior for previously supported OS versions
  • Correct version number for any new version of macOS instead of 0.0
  • Maybe a bit slower

Anything else we should know?

  • I have kept the previous method as a fallback if the property list is removed in future releases.
  • The big switch over kernel version has been replaced by info->minorversion = kern_major - 4;. The relation is correct for all previously supported versions and probably for future ones.
  • The description will remain "Mac OS X" for new versions until a manual update of the code.

There is simpler methods to retrieve the OS version but they use private API or Objective-C code.

Add any other context about your changes here.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits

* Use system property list to retrieve current accurate system version
* Add description for latest versions
@samsinsane
Copy link
Member

There is simpler methods to retrieve the OS version but they use private API or Objective-C code.

Is there a simpler Objective-C version that uses public APIs? How much simpler?

@starkos would you object to having a simpler implementation written in a different language? The code is already platform specific, being written in the "platform language" isn't going to make that much of a difference.

@noresources
Copy link
Contributor Author

@samsinsane Objective-C code relies one Cocoa AppKit framework. It is not simpler, just use standard APIs.
Here is an implementation example

@starkos
Copy link
Member

starkos commented Jun 18, 2020

I would not object to using Objective C in the macOS build.

@noresources
Copy link
Contributor Author

I have made some exeperiments with the Objective-C API and the sw_vers command line tool

  1. If I modify the SystemVersion.plist values, both sw_vers and the Objective-C API returns the modified values immediatly. So, the current pull request does exactly what Apple is doing under the hood
  2. The Objective-C API is only available on OSX 10.10+ (with a deprecated alternative)
  3. To compile Objective-C, Bottstrap.mak and premake4/5.lua have to be changed.

@starkos
Copy link
Member

starkos commented Jun 19, 2020

Thanks for doing that research. @samsinsane I'm okay with the current, simpler plist approach, if you have no objections?

@samsinsane samsinsane merged commit 55cd052 into premake:master Jun 21, 2020
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.

3 participants