facebook / facebook/folly

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

Abierto
#1,470 19 comentarios 8 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
C++
Estrellas
30.5k
Forks
5.9k
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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?

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.