Skip to content

Commit

Permalink
Fix: Xcode 12.5+ build of iPhone Simulator on Apple M1 (#32284)
Browse files Browse the repository at this point in the history 
Summary:
Since Apple released its own silicon M1, an ARM64, the react-native build is broken or at least not as effective as it should.

This PR stops excluding `arm64` simulator (this is not needed on the M1 neither on Intel devices) and removes the problematic `$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)` from `LIBRARY_SEARCH_PATHS`, since on Xcode 12.5 and 13.0 this folder contains only `i386/x86_64` binaries and will fail compilation.

Instead this PR forces `$(SDKROOT)/usr/lib/swift` while it removes the incorrect directory. Ideally we could just remove `LIBRARY_SEARCH_PATHS` altogether if `$(inherited)` and `$(SDKROOT)/usr/lib/swift` were the only entries, but it would require us a **newer CocoaPods**, since that was fixed with `1.11` (see  CocoaPods/CocoaPods@6985cbf). Since we don't enforce that, lets keep the `$(SDKROOT)/usr/lib/swift` and call it done.

Last but not least, deprecate the `__apply_Xcode_12_5_M1_post_install_workaround()` as it's not needed anymore, at least with recent versions of the dependencies, no patching is required with RCT-Folly, neither we need to force `IPHONEOS_DEPLOYMENT_TARGET=11.0`

## Changelog

[iOS] [Fixed] - Xcode 12.5+ build of iPhone Simulator on Apple M1
[iOS] [Changed] - Do not exclude the arm64 iphonesimulator
[iOS] [Deprecated] - __apply_Xcode_12_5_M1_post_install_workaround()

Pull Request resolved: #32284

Test Plan:
* Build `packages/rn-tester` on M1 and see it still works properly
* Run `pod install` on x86_64 and arm64 (m1) and see the `project.pbxproj` is not changed

## References:
* Closes #31480
* The initial fix ac4ddec
* Upgrading CocoaPods to 1.11 would bring us CocoaPods/CocoaPods@6985cbf and we could avoid adding `$(SDKROOT)/usr/lib/swift` ourselves

Reviewed By: lunaleaps

Differential Revision: D31248460

Pulled By: fkgozali

fbshipit-source-id: 5a0d69593e889e296a2ba2e7b4387ecbd56fc08d
  • Loading branch information
@barbieri @facebook-github-bot
barbieri authored and facebook-github-bot committed Sep 29, 2021
1 parent 132d1d0 commit a1c445a
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 41 deletions.
1 change: 0 additions & 1 deletion packages/rn-tester/Podfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,4 @@ end

post_install do |installer|
react_native_post_install(installer)
__apply_Xcode_12_5_M1_post_install_workaround(installer)
end
2 changes: 1 addition & 1 deletion packages/rn-tester/Podfile.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,6 @@ SPEC CHECKSUMS:
Yoga: c0d06f5380d34e939f55420669a60fe08b79bd75
YogaKit: f782866e155069a2cca2517aafea43200b01fd5a

PODFILE CHECKSUM: 21be0a8894b752aaab399f9d4075cf41baf345f3
PODFILE CHECKSUM: f2609ec079811d7784b6e802b8351314bd41de51

COCOAPODS: 1.10.1
6 changes: 2 additions & 4 deletions packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,6 @@
LIBRARY_SEARCH_PATHS = (
"\"$(SDKROOT)/usr/lib/swift\"",
"$(inherited)",
"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)",
"$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)",
);
"LIBRARY_SEARCH_PATHS[arch=*]" = "$(inherited)";
Expand Down Expand Up @@ -820,7 +819,6 @@
LIBRARY_SEARCH_PATHS = (
"\"$(SDKROOT)/usr/lib/swift\"",
"$(inherited)",
"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)",
"$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)",
);
OTHER_LDFLAGS = (
Expand Down Expand Up @@ -873,7 +871,7 @@
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
ENABLE_TESTABILITY = YES;
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "arm64 ";
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "";
GCC_C_LANGUAGE_STANDARD = gnu11;
GCC_DYNAMIC_NO_PIC = NO;
GCC_NO_COMMON_BLOCKS = YES;
Expand Down Expand Up @@ -957,7 +955,7 @@
ENABLE_BITCODE = NO;
ENABLE_NS_ASSERTIONS = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "arm64 ";
"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "";
GCC_C_LANGUAGE_STANDARD = gnu11;
GCC_NO_COMMON_BLOCKS = YES;
GCC_TREAT_INCOMPATIBLE_POINTER_TYPE_WARNINGS_AS_ERRORS = YES;
Expand Down
72 changes: 39 additions & 33 deletions scripts/react_native_pods.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,49 @@ def exclude_architectures(installer)
.uniq{ |p| p.path }
.push(installer.pods_project)

arm_value = `/usr/sbin/sysctl -n hw.optional.arm64 2>&1`.to_i

# Hermes does not support `i386` architecture
excluded_archs_default = has_pod(installer, 'hermes-engine') ? "i386" : ""

projects.each do |project|
project.build_configurations.each do |config|
if arm_value == 1 then
config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = excluded_archs_default
else
config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = "arm64 " + excluded_archs_default
config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = excluded_archs_default
end

project.save()
end
end

def fix_library_search_paths(installer)
def fix_config(config)
lib_search_paths = config.build_settings["LIBRARY_SEARCH_PATHS"]
if lib_search_paths
if lib_search_paths.include?("$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)") || lib_search_paths.include?("\"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)\"")
# $(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME) causes problem with Xcode 12.5 + arm64 (Apple M1)
# since the libraries there are only built for x86_64 and i386.
lib_search_paths.delete("$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)")
lib_search_paths.delete("\"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)\"")
if !(lib_search_paths.include?("$(SDKROOT)/usr/lib/swift") || lib_search_paths.include?("\"$(SDKROOT)/usr/lib/swift\""))
# however, $(SDKROOT)/usr/lib/swift is required, at least if user is not running CocoaPods 1.11
lib_search_paths.insert(0, "$(SDKROOT)/usr/lib/swift")
end
end
end
end

projects = installer.aggregate_targets
.map{ |t| t.user_project }
.uniq{ |p| p.path }
.push(installer.pods_project)

projects.each do |project|
project.build_configurations.each do |config|
fix_config(config)
end
project.native_targets.each do |target|
target.build_configurations.each do |config|
fix_config(config)
end
end
project.save()
end
end
Expand All @@ -151,6 +180,7 @@ def react_native_post_install(installer)
end

exclude_architectures(installer)
fix_library_search_paths(installer)
end

def use_react_native_codegen!(spec, options={})
Expand Down Expand Up @@ -337,32 +367,8 @@ def use_react_native_codegen!(spec, options={})
# See https://github.com/facebook/react-native/issues/31480#issuecomment-902912841 for more context.
# Actual fix was authored by https://github.com/mikehardy.
# New app template will call this for now until the underlying issue is resolved.
#
# It's not needed anymore and will be removed later
def __apply_Xcode_12_5_M1_post_install_workaround(installer)
# Apple Silicon builds require a library path tweak for Swift library discovery to resolve Swift-related "symbol not found".
# Note: this was fixed via https://github.com/facebook/react-native/commit/eb938863063f5535735af2be4e706f70647e5b90
# Keeping this logic here but commented out for future reference.
#
# installer.aggregate_targets.each do |aggregate_target|
# aggregate_target.user_project.native_targets.each do |target|
# target.build_configurations.each do |config|
# config.build_settings['LIBRARY_SEARCH_PATHS'] = ['$(SDKROOT)/usr/lib/swift', '$(inherited)']
# end
# end
# aggregate_target.user_project.save
# end

# Flipper podspecs are still targeting an older iOS deployment target, and may cause an error like:
# "error: thread-local storage is not supported for the current target"
# 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'
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.
# See https://github.com/facebook/flipper/issues/834 for more details.
`sed -i -e $'s/__IPHONE_10_0/__IPHONE_12_0/' #{installer.sandbox.root}/RCT-Folly/folly/portability/Time.h`
puts "__apply_Xcode_12_5_M1_post_install_workaround() is not needed anymore"
end
2 changes: 0 additions & 2 deletions template/ios/HelloWorld.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,6 @@
LIBRARY_SEARCH_PATHS = (
"\"$(SDKROOT)/usr/lib/swift\"",
"\"$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)\"",
"\"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)\"",
"\"$(inherited)\"",
);
MTL_ENABLE_DEBUG_INFO = YES;
Expand Down Expand Up @@ -495,7 +494,6 @@
LIBRARY_SEARCH_PATHS = (
"\"$(SDKROOT)/usr/lib/swift\"",
"\"$(TOOLCHAIN_DIR)/usr/lib/swift/$(PLATFORM_NAME)\"",
"\"$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)\"",
"\"$(inherited)\"",
);
MTL_ENABLE_DEBUG_INFO = NO;
Expand Down

4 comments on commit a1c445a

@barbieri
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkgozali just noticed you forgot to remove __apply_Xcode_12_5_M1_post_install_workaround from template/ios/Podfile, this will result in the puts "__apply_Xcode_12_5_M1_post_install_workaround() is not needed anymore" being called in new projects

@fkgozali
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I missed that. We can fix forward (or feel free to send another PR and I'll merge it)

@barbieri
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fkgozali #32296 there I also have the other commit to warn if Rosetta2 is in use, harmless, but you may drop if you think that should not get in for 0.66

@leotm
Copy link
Contributor

@leotm leotm commented on a1c445a Nov 13, 2021

Choose a reason for hiding this comment

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

@fkgozali just noticed you forgot to remove __apply_Xcode_12_5_M1_post_install_workaround from template/ios/Podfile, this will result in the puts "__apply_Xcode_12_5_M1_post_install_workaround() is not needed anymore" being called in new projects

jus noticed this too for 1st time on new proj 0.67.0-rc.0, before upgrading to 0.67.0-rc.3

Screenshot 2021-11-13 at 10 03 24

looking ahead before doing so

#32296

ac4ddec

03a0907

looks like its still needed after all 🔄

Please sign in to comment.