From ba5bf2e789fc0bc8bc44677d1e56c11592547b93 Mon Sep 17 00:00:00 2001 From: JP Date: Thu, 17 Aug 2023 09:39:38 -0700 Subject: [PATCH 1/4] iOS: don't override excluded archs when installing Hermes --- packages/react-native/scripts/cocoapods/utils.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/utils.rb b/packages/react-native/scripts/cocoapods/utils.rb index e26f0cf1b2a4a0..ed3de89dfd67b7 100644 --- a/packages/react-native/scripts/cocoapods/utils.rb +++ b/packages/react-native/scripts/cocoapods/utils.rb @@ -61,12 +61,14 @@ def self.turn_off_resource_bundle_react_core(installer) 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 + currentValue = config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] || "" + + unless currentValue.include?("i386") + # Hermes does not support `i386` architecture + config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = "#{currentValue} i386".strip + end end project.save() From a54a23ddd9ac84837fe76dbb4512da9f6be46fe0 Mon Sep 17 00:00:00 2001 From: JP Date: Sat, 19 Aug 2023 10:55:49 -0700 Subject: [PATCH 2/4] fixup logic --- packages/react-native/scripts/cocoapods/utils.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/utils.rb b/packages/react-native/scripts/cocoapods/utils.rb index ed3de89dfd67b7..a2e54046d846cd 100644 --- a/packages/react-native/scripts/cocoapods/utils.rb +++ b/packages/react-native/scripts/cocoapods/utils.rb @@ -59,15 +59,23 @@ def self.turn_off_resource_bundle_react_core(installer) end def self.exclude_i386_architecture_while_using_hermes(installer) + isUsingHermes = self.has_pod(installer, 'hermes-engine') + key = "EXCLUDED_ARCHS[sdk=iphonesimulator*]" + projects = self.extract_projects(installer) projects.each do |project| project.build_configurations.each do |config| - currentValue = config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] || "" + current_setting = config.build_settings[key] || "" + + excludesI386 = current_setting.include?("i386") - unless currentValue.include?("i386") + if isUsingHermes and !excludesI386 # Hermes does not support `i386` architecture - config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"] = "#{currentValue} i386".strip + config.build_settings[key] = "#{current_setting} i386".strip + else + # leave EXCLUDED_ARCHS unchanged or set it to "" if it doesn't already exist + config.build_settings[key] = current_setting end end From 9ee6b071731e1a248a90ed8418f77052f4ba0e32 Mon Sep 17 00:00:00 2001 From: JP Date: Sat, 19 Aug 2023 10:56:09 -0700 Subject: [PATCH 3/4] add new tests --- .../scripts/cocoapods/__tests__/utils-test.rb | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb index 4e4aa2ef4e6bd6..246775d5cff5e5 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb @@ -259,6 +259,52 @@ def test_excludeArchitectures_whenHermesEngineIsIncluded_excludeI386 assert_equal(pods_projects_mock.save_invocation_count, 1) end + def test_excludeArchitectures_whenHermesEngineIsNotIncluded_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, 1) + assert_equal(pods_projects_mock.save_invocation_count, 1) + end + + def test_excludeArchitectures_whenHermesEngineIsIncluded_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 # # ================= # From 9e047d2dab24677c8085e313474c839d4e0f9c19 Mon Sep 17 00:00:00 2001 From: JP Date: Thu, 24 Aug 2023 13:00:09 -0700 Subject: [PATCH 4/4] address PR comments --- .../scripts/cocoapods/__tests__/utils-test.rb | 32 +++++++++---------- .../react-native/scripts/cocoapods/utils.rb | 30 ++++++++--------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb index 246775d5cff5e5..4882195d1b6d8d 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,16 +233,19 @@ 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_whenHermesEngineIsIncluded_excludeI386 + + def test_excludeArchitectures_whenHermesEngineIsNotIncluded_withExistingValue_preserveExistingValue # Arrange user_project_mock = prepare_empty_user_project_mock() - pods_projects_mock = PodsProjectMock.new([], {"hermes-engine" => {}}) + 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) ]) @@ -252,20 +255,17 @@ def test_excludeArchitectures_whenHermesEngineIsIncluded_excludeI386 # Assert user_project_mock.build_configurations.each do |config| - assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "i386") + assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "arm64") end - assert_equal(user_project_mock.save_invocation_count, 1) - assert_equal(pods_projects_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_preserveExistingValue + def test_excludeArchitectures_whenHermesEngineIsIncluded_withNoValue_onlyExcludeI386 # 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() + pods_projects_mock = PodsProjectMock.new([], {"hermes-engine" => {}}) installer = InstallerMock.new(pods_projects_mock, [ AggregatedProjectMock.new(user_project_mock) ]) @@ -275,14 +275,14 @@ def test_excludeArchitectures_whenHermesEngineIsNotIncluded_preserveExistingValu # Assert user_project_mock.build_configurations.each do |config| - assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "arm64") + assert_equal(config.build_settings["EXCLUDED_ARCHS[sdk=iphonesimulator*]"], "i386") end assert_equal(user_project_mock.save_invocation_count, 1) assert_equal(pods_projects_mock.save_invocation_count, 1) end - def test_excludeArchitectures_whenHermesEngineIsIncluded_appendI386 + def test_excludeArchitectures_whenHermesEngineIsIncluded_withExistingValue_appendI386 # Arrange user_project_mock = prepare_empty_user_project_mock() user_project_mock.build_configurations.each do |config| diff --git a/packages/react-native/scripts/cocoapods/utils.rb b/packages/react-native/scripts/cocoapods/utils.rb index a2e54046d846cd..a9c96e7e1b63f1 100644 --- a/packages/react-native/scripts/cocoapods/utils.rb +++ b/packages/react-native/scripts/cocoapods/utils.rb @@ -59,27 +59,27 @@ def self.turn_off_resource_bundle_react_core(installer) end def self.exclude_i386_architecture_while_using_hermes(installer) - isUsingHermes = self.has_pod(installer, 'hermes-engine') - key = "EXCLUDED_ARCHS[sdk=iphonesimulator*]" + is_using_hermes = self.has_pod(installer, 'hermes-engine') + + if is_using_hermes + key = "EXCLUDED_ARCHS[sdk=iphonesimulator*]" - projects = self.extract_projects(installer) + projects = self.extract_projects(installer) - projects.each do |project| - project.build_configurations.each do |config| - current_setting = config.build_settings[key] || "" + projects.each do |project| + project.build_configurations.each do |config| + current_setting = config.build_settings[key] || "" - excludesI386 = current_setting.include?("i386") + build_will_skip_I386 = current_setting.include?("i386") - if isUsingHermes and !excludesI386 - # Hermes does not support `i386` architecture - config.build_settings[key] = "#{current_setting} i386".strip - else - # leave EXCLUDED_ARCHS unchanged or set it to "" if it doesn't already exist - config.build_settings[key] = current_setting + unless build_will_skip_I386 + # Hermes does not support `i386` architecture + config.build_settings[key] = "#{current_setting} i386".strip + end end - end - project.save() + project.save() + end end end