-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
fix(ios): fix Time.h
patch not being applied
#32961
Conversation
The path to `Time.h` is currently hard-coded and does not take into consideration the `--project-directory` flag when running `pod install`.
@lunaleaps, @kelset: Please also backport this to 0.67 as it is a regression. |
@@ -646,5 +646,6 @@ def __apply_Xcode_12_5_M1_post_install_workaround(installer) | |||
# "Time.h:52:17: error: typedef redefinition with different types" | |||
# We need to make a patch to RCT-Folly - remove the `__IPHONE_OS_VERSION_MIN_REQUIRED` check. | |||
# See https://github.com/facebook/flipper/issues/834 for more details. | |||
`sed -i -e $'s/ && (__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0)//' Pods/RCT-Folly/folly/portability/Time.h` | |||
time_header = "#{Pod::Config.instance.installation_root.to_s}/Pods/RCT-Folly/folly/portability/Time.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha of course! My hack lives on 😆 but I didn't think about making it durable.
Is there an RCT-Folly issue pursuing this for a non-hack fix? Just curious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the non hack fix not be facebook/folly#1688 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Saadnajmi that looks like it exactly! Hopefully it lands soon-ish and react-native 0.68 can include it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the original hacker, this is terrible and I'm saying that about my own hack 😅 , but also this is an obvious improvement. Nice catch + fix
@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @tido64 in 60cef85. When will my fix make it into a release? | Upcoming Releases |
Summary: The path to `Time.h` is currently hard-coded and does not take into consideration the `--project-directory` flag when running `pod install`. ## Changelog [iOS] [Fixed] - Fix `Time.h` patch not being applied when running `pod install --project-directory=ios` Pull Request resolved: #32961 Test Plan: ``` git clone https://github.com/microsoft/react-native-test-app.git cd react-native-test-app npm run set-react-version main yarn cd example pod install --project-directory=ios ../scripts/xcodebuild.sh ios/Example.xcworkspace build ``` Reviewed By: christophpurrer Differential Revision: D33748789 Pulled By: lunaleaps fbshipit-source-id: b125734eba30e552ae139e7ecd4e634c8fa1bcd7
Summary
The path to
Time.h
is currently hard-coded and does not take intoconsideration the
--project-directory
flag when runningpod install
.Changelog
[iOS] [Fixed] - Fix
Time.h
patch not being applied when runningpod install --project-directory=ios
Test Plan