Commit 4a8837f3 authored by Adam Gleitman's avatar Adam Gleitman Committed by Facebook GitHub Bot

fix: Flip signs in Time.h's Apple version checks (#1688)

Summary:
When trying to bring react-native-macos up to date with 0.66, I was running into an issue getting RNTester to compile due to an error regarding redefining `clockid_t`. Other people have been seeing similar issues as per [these search results](https://github.com/facebook/folly/issues?q=clockid_t).

The history behind this appears to be as follows:

Several declarations in `<time.h>` were not available on Apple platforms until macOS 10.12 and iOS 10, which is why Folly needs to check the minimum version and set `FOLLY_HAVE_CLOCK_GETTIME` as needed. The problem is, the current logic as it stands right now is to set `FOLLY_HAVE_CLOCK_GETTIME = 1` (which implies that we don't need to declare them ourselves as the OS provides them for us) if...
* ...we're building for macOS, and the minimum required version is less than 10.12, or...
* ...we're building for iOS, and the minimum required version is less than 10.

However, this doesn't make any sense. This is saying that we don't need to declare these missing APIs if we could be compiling Folly for use on an older version (i.e., macOS 10.11/iOS 9 or earlier), which is totally wrong! Instead, we ought to be checking if the versions are *at least* macOS 10.12 or iOS 10.

React Native currently works around this by eliminating the minimum version check entirely with [this PR](https://github.com/facebook/react-native/pull/32715/), which is certainly a valid local fix ([the minimum iOS version for React Native apps is currently iOS 11](https://github.com/facebook/react-native/blob/1b31d6bb582768ccbe92d3c1a9e43354a8c531e5/template/ios/Podfile#L4)), but doesn't solve the problem at its core. This PR does solve the problem.

I have not tested building this with a minimum version below the above thresholds for use on a modern version of macOS/iOS, but considering the discussion in https://github.com/facebook/folly/issues/1545, I think we should be safe to ignore these older versions from now on.

Pull Request resolved: https://github.com/facebook/folly/pull/1688

Reviewed By: yfeldblum

Differential Revision: D33483705

Pulled By: Orvid

fbshipit-source-id: 0fe7c556af7e5b79a7679f75d003cf81a8f412ce
parent c6981b29
......@@ -18,6 +18,7 @@
#include <folly/CPortability.h>
#include <folly/Likely.h>
#include <folly/Utility.h>
#include <cassert>
......@@ -99,7 +100,7 @@ static int clock_thread_cputime(struct timespec* ts) {
}
FOLLY_ATTR_WEAK int clock_gettime(clockid_t clk_id, struct timespec* ts) {
switch (clk_id) {
switch (folly::to_underlying(clk_id)) {
case CLOCK_REALTIME: {
auto now = std::chrono::system_clock::now().time_since_epoch();
duration_to_ts(now, ts);
......
......@@ -27,11 +27,11 @@
// solve that by pretending we have it here in the header and
// then enable our implementation on the source side so that
// gets linked in instead.
#if defined(__MACH__) && \
((!defined(TARGET_OS_OSX) || TARGET_OS_OSX) && \
(MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12)) || \
(defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE != 0 && \
(__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0))
#if defined(__MACH__) && \
((!defined(TARGET_OS_OSX) || TARGET_OS_OSX) && \
(MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12)) || \
(defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE != 0 && \
(__IPHONE_OS_VERSION_MIN_REQUIRED >= __IPHONE_10_0))
#ifdef FOLLY_HAVE_CLOCK_GETTIME
#undef FOLLY_HAVE_CLOCK_GETTIME
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment