From f2447e6048a6b519c3333767d950dbf567149b75 Mon Sep 17 00:00:00 2001 From: JP Date: Tue, 29 Aug 2023 02:46:51 -0700 Subject: [PATCH] iOS: don't override EXCLUDED_ARCHS when installing Hermes (#39060) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: - if an existing iOS project specifies `EXCLUDED_ARCHS`, these settings are overwritten by the `react_native_post_install` step as part of the Cocoapods install - this can break the build of existing iOS apps that want to include React Native - a previous PR (https://github.com/facebook/react-native/pull/38132) updated the Cocoapods utils to that we only update `EXCLUDED_ARCHS` when using Hermes - this worked around the issue for apps that opted to use JSC - however if you _do_ want to use Hermes (the default) this problem persists ### Existing Behaviour - one of the functions called as part of the `react_native_post_install` step is `exclude_i386_architecture_while_using_hermes` - see [/packages/react-native/scripts/cocoapods/utils.rb](https://github.com/facebook/react-native/blob/v0.72.1/packages/react-native/scripts/cocoapods/utils.rb#L56-L69) ``` def self.exclude_i386_architecture_while_using_hermes(installer) projects = self.extract_projects(installer) # Hermes does not support `i386` architecture excluded_archs_default = self.has_pod(installer, 'hermes-engine') ? "i386" : "" projects.each do |project| project.build_configurations.each do |config| config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = excluded_archs_default end project.save() end end ``` 🐛 **However** 🐛 - this means existing projects that have `EXCLUDED_ARCHS` set, the existing value will be overwritten either to `"i386"` or a blank string ### Changed Behaviour - appends `"i386"` to existing string if set, or just sets the value to `"i386"` if there is no existing value ## Changelog: [IOS] [FIXED] don't override `EXCLUDED_ARCHS` when installing Hermes Pull Request resolved: https://github.com/facebook/react-native/pull/39060 Reviewed By: dmytrorykun Differential Revision: D48515441 Pulled By: cipolleschi fbshipit-source-id: 8cb3c8b680d92272da0b106553179af051d0f84e --- .../scripts/cocoapods/__tests__/utils-test.rb | 54 +++++++++++++++++-- .../react-native/scripts/cocoapods/utils.rb | 26 ++++++--- 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb index 4e4aa2ef4e6bd6..2553261e3e3ee7 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb @@ -220,7 +220,7 @@ def test_SetGCCPreprocessorDefinitionForHermes_itSetsThePreprocessorForDebug # ============================ # # Test - Exclude Architectures # # ============================ # - def test_excludeArchitectures_whenHermesEngineIsNotIncluded_excludeNothing + def test_excludeArchitectures_whenHermesEngineIsNotIncluded_withNoValue_leaveUnset # Arrange user_project_mock = prepare_empty_user_project_mock() pods_projects_mock = PodsProjectMock.new() @@ -233,13 +233,36 @@ def test_excludeArchitectures_whenHermesEngineIsNotIncluded_excludeNothing # Assert user_project_mock.build_configurations.each do |config| - assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "") + assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], nil) end - assert_equal(user_project_mock.save_invocation_count, 1) + assert_equal(user_project_mock.save_invocation_count, 0) + assert_equal(pods_projects_mock.save_invocation_count, 0) + end + + def test_excludeArchitectures_whenHermesEngineIsNotIncluded_withExistingValue_preserveExistingValue + # Arrange + user_project_mock = prepare_empty_user_project_mock() + user_project_mock.build_configurations.each do |config| + config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = "arm64" + end + pods_projects_mock = PodsProjectMock.new() + installer = InstallerMock.new(pods_projects_mock, [ + AggregatedProjectMock.new(user_project_mock) + ]) + + # Act + ReactNativePodsUtils.exclude_i386_architecture_while_using_hermes(installer) + + # Assert + user_project_mock.build_configurations.each do |config| + assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "arm64") + end + + assert_equal(user_project_mock.save_invocation_count, 0) assert_equal(pods_projects_mock.save_invocation_count, 0) end - def test_excludeArchitectures_whenHermesEngineIsIncluded_excludeI386 + def test_excludeArchitectures_whenHermesEngineIsIncluded_withNoValue_onlyExcludeI386 # Arrange user_project_mock = prepare_empty_user_project_mock() pods_projects_mock = PodsProjectMock.new([], {"hermes-engine" => {}}) @@ -259,6 +282,29 @@ def test_excludeArchitectures_whenHermesEngineIsIncluded_excludeI386 assert_equal(pods_projects_mock.save_invocation_count, 1) end + def test_excludeArchitectures_whenHermesEngineIsIncluded_withExistingValue_appendI386 + # Arrange + user_project_mock = prepare_empty_user_project_mock() + user_project_mock.build_configurations.each do |config| + config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = "arm64" + end + pods_projects_mock = PodsProjectMock.new([], {"hermes-engine" => {}}) + installer = InstallerMock.new(pods_projects_mock, [ + AggregatedProjectMock.new(user_project_mock) + ]) + + # Act + ReactNativePodsUtils.exclude_i386_architecture_while_using_hermes(installer) + + # Assert + user_project_mock.build_configurations.each do |config| + assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "arm64 i386") + end + + assert_equal(user_project_mock.save_invocation_count, 1) + assert_equal(pods_projects_mock.save_invocation_count, 1) + end + # ================= # # Test - Fix Config # # ================= # diff --git a/packages/react-native/scripts/cocoapods/utils.rb b/packages/react-native/scripts/cocoapods/utils.rb index e26f0cf1b2a4a0..e9bf0fb2388657 100644 --- a/packages/react-native/scripts/cocoapods/utils.rb +++ b/packages/react-native/scripts/cocoapods/utils.rb @@ -59,17 +59,27 @@ def self.turn_off_resource_bundle_react_core(installer) end def self.exclude_i386_architecture_while_using_hermes(installer) - projects = self.extract_projects(installer) + is_using_hermes = self.has_pod(installer, 'hermes-engine') - # Hermes does not support `i386` architecture - excluded_archs_default = self.has_pod(installer, 'hermes-engine') ? "i386" : "" + if is_using_hermes + key = "EXCLUDED_ARCHS[sdk=iphonesimulator*]" - projects.each do |project| - project.build_configurations.each do |config| - config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = excluded_archs_default - end + projects = self.extract_projects(installer) - project.save() + projects.each do |project| + project.build_configurations.each do |config| + current_setting = config.build_settings[key] || "" + + excluded_archs_includes_I386 = current_setting.include?("i386") + + if !excluded_archs_includes_I386 + # Hermes does not support `i386` architecture + config.build_settings[key] = "#{current_setting} i386".strip + end + end + + project.save() + end end end