Fix: Xcode 12.5+ build of iPhone Simulator on Apple M1 #32284
Conversation
facebook-github-bot
added
CLA Signed
|
@hramos has enabled "commitly" releases on CircleCI (like nightly builds, but for every commit) and perhaps he could enable it for all branches or PRs ? Otherwise it will be difficult for me to test this It appears that the core of it is all in the text file If you ran |
Base commit: f3fe7a0 |
|
CI Failures:
|
barbieri
force-pushed
the
fix/xcode12.5-and-m1
branch
from
a91d85f
to
7f68e04
Compare
Base commit: f3fe7a0 |
|
While it passed most tests, I'll do some improvements to |
Remove __apply_Xcode_12_5_M1_post_install_workaround() usage and make that function empty. Instead just force `$(SDKROOT)/usr/lib/swift` into the LIBRARY_SEARCH_PATHS, removing any `$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)` as that contains only i386+x86_64 in iphonesimulator, failing the build for M1 arm64 simulators.
The "project.pbxproj" file was being updated by team members running on x86_64 and arm64 as the file was being changed based on their hw.optional.arm64 value. Xcode 12.4 and on (at least, couldn't test on older versions) properly handle builds on both x86_64 and arm64.
barbieri
force-pushed
the
fix/xcode12.5-and-m1
branch
from
7f68e04
to
4eb4461
Compare
|
@mikehardy see if the following patch helps. I did it on top of diff --git a/node_modules/react-native/scripts/react_native_pods.rb b/node_modules/react-native/scripts/react_native_pods.rb
index 3e79bd6..ad619f8 100644
--- a/node_modules/react-native/scripts/react_native_pods.rb
+++ b/node_modules/react-native/scripts/react_native_pods.rb
@@ -123,20 +123,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
@@ -147,6 +176,7 @@ def react_native_post_install(installer)
end
exclude_architectures(installer)
+ fix_library_search_paths(installer)
end
def use_react_native_codegen!(spec, options={})
diff --git a/node_modules/react-native/template/ios/HelloWorld.xcodeproj/project.pbxproj b/node_modules/react-native/template/ios/HelloWorld.xcodeproj/project.pbxproj
index acb72e4..4aee980 100644
--- a/node_modules/react-native/template/ios/HelloWorld.xcodeproj/project.pbxproj
+++ b/node_modules/react-native/template/ios/HelloWorld.xcodeproj/project.pbxproj
@@ -435,8 +435,8 @@
"$(inherited)",
);
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;
@@ -492,8 +492,8 @@
"$(inherited)",
);
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; |
|
Thanks a lot for the fix @barbieri! Just so I understand this: we need this PR if we want to stay with CocoaPods 1.10.x, or we could require upgrade (in main and next release) to CocoaPods 1.11.x and this PR is no longer necessary? |
He's landing that support as we speak, so should be available later today hopefully. |
|
@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@fkgozali this works and is required on both 1.10 and 1.11. If we force 1.11 we could do a different
In every case, we MUST remove The difference in CocoaPods is that 1.11 was fixed to automatically link with Swift if any of the dependencies uses that (ie: |
|
@barbieri Thanks for the explanation. I'm trying to figure out if:
I think we should proceed in that order. Now, whether this fix should be considered for 0.65.1 or 0.66.1 (patched releases), we'll discuss that separately. I don't think we need to block on this to make 0.66.0 stable though. cc @lunaleaps |
|
@fkgozali I just got my M1, but it seems it's worth to require the following tools and versions:
|
|
Re tooling upgrades: we can track that separately for main and next releases, but probably not worth bringing those upgrades requirements to 0.65, 0.66 for now. |
|
re: tooling, does react-native really get into what ruby a user is using? That said, seems like just strongly recommending a non-Rosetta ruby execution environment is the goal, and detecting it is possible (snippets on stackoverflow, and my tested version below). For cocoapods, I think enforcing 1.11.x is a pretty easy requirement for the next release unless I'm missing something, isn't switching to it from 1.10 trivial? I don't recall any breaking changes? (I just scanned their releases list again, I can't see any) Bash arch detection, exits on Rosetta while wagging a finger at you - ruby syntax port is an exercise for the reader 😅 # We do not want to run under Rosetta 2, brew doesn't work and compiles might not work after
arch_name="$(uname -m)"
if [ "${arch_name}" = "x86_64" ]; then
if [ "$(sysctl -in sysctl.proc_translated)" = "1" ]; then
echo "Running on Rosetta 2"
echo "This is not supported. Run \`env /usr/bin/arch -arm64 /bin/bash --login\` then try again"
exit 1
else
echo "Running on native Intel"
fi
elif [ "${arch_name}" = "arm64" ]; then
echo "Running on ARM"
else
echo "Unknown architecture: ${arch_name}"
fi
|
Yeah, that's right, the only question is do we want to put that line in 0.66.0 release note or wait for later release. |
Well, it's not a requirement for rc4 so I wouldn't add it now. If you merge this as is, still doesn't need it. If we go for a cleaner version then and only then it's needed if I understand correctly. So for me, later release (0.67) I'm responding fast just because I happen to be paying attention about this right now, I don't feel super strongly about it and I'm just expressing opinion, to be clear. I do have an opinion but at the end of the day if it builds for me from npx react-native init on an M1 I'm happy :-) |
|
I'm gonna land this as is to main, per the convo above. |
|
Ah cool, then with the commitlies everyone interested can start playing with it with a notice to |
Since it does
Agreed. I don't think there is any breaking change, but needs more testing to confirm (as usual). As for your script to detect rosetta, that makes sense. |
Summary: * Remove left over from a1c445a#commitcomment-57240925 * Add a warning if running with Rosetta2 as per #32284 (comment) ## Changelog [iOS] [Fixed] - Removed __apply_Xcode_12_5_M1_post_install_workaround [iOS] [Changed] - Warn if Rosetta2 is being used (x86_64 on arm64) Pull Request resolved: #32296 Test Plan: Build on macOS Apple devices without any warnings during `pod install` Reviewed By: RSNara Differential Revision: D31291567 Pulled By: fkgozali fbshipit-source-id: 65e54507dedcdba39c1b441aad85e940eedc8b52
Having experienced this ruby version spec / Gemfile stuff in the template for the last couple months, I have to say it adds friction to my workflows. Specifically if you do The workaround for me is to add If I could propose deleting all of that ruby version thing from the template I'd be a happy person. Then if cocoapods was going to fail, it would fail anyway, but if it was going to work (with a newer version) at least it would actually work which would be nice... |
|
@mikehardy deleting is okay, but would make life to "grant the tools are all known to work" harder. However, since 2.7.4 is not pre-installed in the M1, I'd be all for go with 3.1.x... I used 2.7.4 at the time because it was more well known and widely used (ie: in CI). |
|
Any solution that does not allow for friction-less compatibility with future workings is a bug in my opinion
That's literally it, that should work :-) If there was some way to have an open-ended range so we could enforce minimum versions that's great but constraining max is the issue for me |
|
doctor checks can do things more finely I think, by the way - we could enforce a range there if we determine that there is some problem with current stable interop with cocoapods (that did happen with ruby-3.0.0 initially, it took a little while for cocoapods to work well with it) |
|
with each solution comes a problem, some people use rvm, others asdf, rbenv... Other than that, ok. The idea was to help unify, but we can just document/recommend and let people execute the installations. I'd still provide a |
Exactly - for that reason the find-node.sh script was also retired, something intended to help caused more trouble than it's worth. We just need to assume node is installed and callable and move on, so find-node went away Similarly, something that restricts ruby is causing friction. I'm agnostic on the .bundle/config, my use case is as stated, so if it causes no friction with rvm installing 3.1.1, works for me... but in general I think it's okay to focus on doctor, let it verify compatible ruby / cocoapods install and prompt people to install them if not |
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
arm64simulator (this is not needed on the M1 neither on Intel devices) and removes the problematic$(TOOLCHAIN_DIR)/usr/lib/swift-5.0/$(PLATFORM_NAME)fromLIBRARY_SEARCH_PATHS, since on Xcode 12.5 and 13.0 this folder contains onlyi386/x86_64binaries and will fail compilation.Instead this PR forces
$(SDKROOT)/usr/lib/swiftwhile it removes the incorrect directory. Ideally we could just removeLIBRARY_SEARCH_PATHSaltogether if$(inherited)and$(SDKROOT)/usr/lib/swiftwere the only entries, but it would require us a newer CocoaPods, since that was fixed with1.11(see CocoaPods/CocoaPods@6985cbf). Since we don't enforce that, lets keep the$(SDKROOT)/usr/lib/swiftand 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 forceIPHONEOS_DEPLOYMENT_TARGET=11.0Changelog
[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()
Test Plan
packages/rn-testeron M1 and see it still works properlypod installon x86_64 and arm64 (m1) and see theproject.pbxprojis not changedReferences:
$(SDKROOT)/usr/lib/swiftourselves