facebook / facebook/folly

Time.h `typedef redefinition` with iOS 10 set as a deployment target

Open
#1,470 19 comments 8 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

Hey,

I am working on [a Hermes support for React Native](https://github.com/facebook/react-native/pull/29914) and while working on it, I have run into the following issue, when building the React Native project:

Screenshot_2020-10-01_at_11 07 41

React Native has deployment target set to `10.0` for all its parts, including third party pod specs such as [`RCT-Folly`](https://github.com/facebook/react-native/blob/master/third-party-podspecs/RCT-Folly.podspec), that it defines to download and compile Folly with pieces that it needs. After changing the deployment target for `RCT-Folly` specifically from `10.0` to `9.0`, the issue goes away.

This is [a temporary workaround](https://github.com/facebook/react-native/pull/29914/commits/16452d0c7509a839a3887a8e26a6147a0060e132) that we have implemented to unblock the PR.

I believe it is caused by [the following code](https://github.com/facebook/folly/blob/master/folly/portability/Time.h#L30-L33):
```cpp
#if __MACH__ && \
((!defined(TARGET_OS_OSX) || TARGET_OS_OSX) && \
(MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12)) || \
(TARGET_OS_IPHONE && (__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0))

#ifdef FOLLY_HAVE_CLOCK_GETTIME
#undef FOLLY_HAVE_CLOCK_GETTIME
#endif

#define FOLLY_HAVE_CLOCK_GETTIME 1
#define FOLLY_FORCE_CLOCK_GETTIME_DEFINITION 1

#endif
```

The way I read it is - when building for iPhone and targeting iOS versions lower than 10.0 (e.g. 9.0), don't define macros. When building for newer iOS release, such as 10.0, define it. Shouldn't it be the opposite, if the purpose was to provide backwards compatibility?

The issue is being brought up from time to time in various projects - that's where I have learnt about the "deployment target" workaround:
- https://github.com/facebook/react-native/issues/28810
- https://github.com/facebook/react-native/issues/27845
- https://github.com/facebook/flipper/issues/834

Here are my questions:
- What was the original intent behind this particular `if` condition? Where is the "__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0" requirement coming from?
- Is the workaround applied in our PR a valid fix?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.