Skip to content

Commit

Permalink
Fix post_install_workaround downgrading development targets (#32633)
Browse files Browse the repository at this point in the history
Summary:
The `__apply_Xcode_12_5_M1_post_install_workaround` script changes the `IPHONEOS_DEPLOYMENT_TARGET` to `11.0` for all pods. This causes problems if the pods were targetting `12.0` or higher. Many expo modules are targetting `12.0`.

I fixed this issue by checking the existing version and only bumping the target if it is lower than `11.0`.

See also: this discussion post by mikehardy reactwg/react-native-releases#1 (comment)

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Fixed] - __apply_Xcode_12_5_M1_post_install_workaround causing pods targetting iOS 12 and above to fail

Pull Request resolved: #32633

Test Plan:
### Test (failing before this patch, passing after this patch)

1. pick an iOS Pod that has a minimum deployment target of iOS 12 or higher, I chose the Braintree package
2. `npx react-native init myrnapp`
3. Open `ios/Podfile` and add the pod as a dependency: `pod 'Braintree', '~> 5'` (and upgrade the Podfile target to 12 (`platform :ios, '12.0'`))
4. Compile the app.

Before applying this patch: ❌ Build fails because Braintree uses iOS 12 features and was downgraded to target 11.0
After applying this patch: ✅ Build succeeds

Reviewed By: fkgozali

Differential Revision: D32638171

Pulled By: philIip

fbshipit-source-id: 0487647583057f3cfefcf515820855c7d4b16d31
  • Loading branch information
Yonom authored and facebook-github-bot committed Nov 30, 2021
1 parent 6abf8b4 commit a4a3e67
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions scripts/react_native_pods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -548,13 +548,17 @@ def __apply_Xcode_12_5_M1_post_install_workaround(installer)
# The most reliable known workaround is to bump iOS deployment target to match react-native (iOS 11 now).
installer.pods_project.targets.each do |target|
target.build_configurations.each do |config|
config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = '11.0'
# ensure IPHONEOS_DEPLOYMENT_TARGET is at least 11.0
should_upgrade = config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'].split('.')[0].to_i < 11
if should_upgrade
config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = '11.0'
end
end
end

# But... doing so caused another issue in Flipper:
# "Time.h:52:17: error: typedef redefinition with different types"
# We need to make a patch to RCT-Folly - set `__IPHONE_10_0` to our iOS target + 1.
# 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_10_0/__IPHONE_12_0/' Pods/RCT-Folly/folly/portability/Time.h`
`sed -i -e $'s/ && (__IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_10_0)//' Pods/RCT-Folly/folly/portability/Time.h`
end

3 comments on commit a4a3e67

@billnbell
Copy link
Contributor

@billnbell billnbell commented on a4a3e67 Dec 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not work for me since I get a nil.split()[0] issue. This works:

should_upgrade = config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'].to_f < 11.0 && config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'].to_f != 0.0

Does not error on split().

@billnbell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is some node_modules is not returning IPHONEOS_DEPLOYMENT_TARGET, and you are referencing an array that does not exist.

See this - which is why I recommend to_f. Also puts '10.5.4'.to_f returns 10.5 which is < 11.0 which also works.

$ ruby
settings = Hash.new      
settings['dd'] = '10.5.4'
puts settings['IPHONEOS_DEPLOYMENT_TARGET'].split('.')[0].to_i
-:3:in `<main>': undefined method `split' for nil:NilClass (NoMethodError)

$ ruby
settings = Hash.new
settings['dd'] = '10.5.4'
puts settings['IPHONEOS_DEPLOYMENT_TARGET'].to_f
0.0

@billnbell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we pushing out 0.66.5 ?

Please sign in to comment.