Properly fix build error on old macOS / Xcode with clockid_t #12074
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR vim#10549 attempted to fix a build break on older Xcode versions (Xcode 8 / macOS 10.11 SDK), but it did it in a way that's not very reliable. In particular, it did it just by checking whether `MAC_OS_X_VERSION_10_12` exists, but that macro is just the version number for 10.12 SDK. In MacVim, we manually define it to allow us to do version checks. As a result, MacVim builds would fail on macOS 10.11 SDKs (or below). Instead, the proper fix is to check `MAC_OS_X_VERSION_MAX_ALLOWED` against `MAC_OS_X_VERSION_10_12`. The "max allowed" value is the version of the SDK we are building with and it will be properly defined. The previous bug that the PR was trying to fix was that it was using `MAC_OS_X_VERSION_MIN_REQUIRED` instead, which is the "deployment target" of the build and wasn't the right value to use. See macvim-dev/macvim#1372
Codecov Report
@@ Coverage Diff @@
## master #12074 +/- ##
==========================================
+ Coverage 81.31% 81.88% +0.57%
==========================================
Files 164 164
Lines 193906 194031 +125
Branches 43822 43851 +29
==========================================
+ Hits 157665 158879 +1214
+ Misses 23510 22296 -1214
- Partials 12731 12856 +125
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #10549 attempted to fix a build break on older Xcode versions (Xcode 8 / macOS 10.11 SDK), but it did it in a way that's not very reliable. In particular, it did it just by checking whether
MAC_OS_X_VERSION_10_12exists, but that macro is just the version number for 10.12 SDK. In MacVim, we manually define it to allow us to do version checks. As a result, MacVim builds would fail on macOS 10.11 SDKs (or below).Instead, the proper fix is to check
MAC_OS_X_VERSION_MAX_ALLOWEDagainstMAC_OS_X_VERSION_10_12. The "max allowed" value is the version of the SDK we are building with and it will be properly defined. The previous bug that the PR was trying to fix was that it was usingMAC_OS_X_VERSION_MIN_REQUIREDinstead, which is the "deployment target" of the build and wasn't the right value to use.See macvim-dev/macvim#1372