From 71da21243c85283445c6cefa64d1ace13823ab69 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Fri, 17 Jun 2022 17:11:51 -0700 Subject: [PATCH] Move New Architecture setup to `new_architecture.rb` file (#33990) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/33990 This diff moves the setting of some CPP flags from the main React native pods file to a dedicated file. It also introduces some tests and it improves the Test Mocks we have ## Changelog [iOS][Changed] - Move the `modify_flags_for_new_architecture` method to separate ruby file Reviewed By: cortinico Differential Revision: D37040927 fbshipit-source-id: 037ddaf123d01f3a2fd622b8a0cd10535da70b92 --- packages/rn-tester/Podfile | 4 +- .../__tests__/new_architecture-test.rb | 120 +++++++++++++++++- .../__tests__/test_utils/InstallerMock.rb | 49 ++++++- scripts/cocoapods/new_architecture.rb | 59 ++++++--- scripts/react_native_pods.rb | 36 +----- 5 files changed, 210 insertions(+), 58 deletions(-) diff --git a/packages/rn-tester/Podfile b/packages/rn-tester/Podfile index 8ead6c2d6eb1ff..24130f1874ce5c 100644 --- a/packages/rn-tester/Podfile +++ b/packages/rn-tester/Podfile @@ -1,5 +1,4 @@ require_relative '../../scripts/react_native_pods' -require_relative '../../scripts/cocoapods/new_architecture' source 'https://cdn.cocoapods.org/' platform :ios, '12.4' @@ -29,7 +28,7 @@ def pods(options = {}, use_flipper: false) # Custom fabric component is only supported when using codegen discovery. pod 'MyNativeView', :path => "NativeComponentExample" end - + use_react_native!( path: @prefix_path, fabric_enabled: fabric_enabled, @@ -67,5 +66,4 @@ end post_install do |installer| react_native_post_install(installer, @prefix_path) __apply_Xcode_12_5_M1_post_install_workaround(installer) - set_clang_cxx_language_standard_if_needed(installer) end diff --git a/scripts/cocoapods/__tests__/new_architecture-test.rb b/scripts/cocoapods/__tests__/new_architecture-test.rb index 1188b3add31eff..d1593b83cb8def 100644 --- a/scripts/cocoapods/__tests__/new_architecture-test.rb +++ b/scripts/cocoapods/__tests__/new_architecture-test.rb @@ -9,18 +9,17 @@ require_relative "./test_utils/PodMock.rb" class NewArchitectureTests < Test::Unit::TestCase - def setup - File.enable_testing_mode! - end - def teardown Pod::UI.reset() end + # ============================= # + # Test - Set Clang Cxx Lang Std # + # ============================= # def test_setClangCxxLanguageStandardIfNeeded_whenReactCoreIsPresent installer = prepare_mocked_installer_with_react_core - set_clang_cxx_language_standard_if_needed(installer) + NewArchitectureHelper.set_clang_cxx_language_standard_if_needed(installer) assert_equal(installer.aggregate_targets[0].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal(installer.aggregate_targets[1].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") @@ -30,7 +29,7 @@ def test_setClangCxxLanguageStandardIfNeeded_whenReactCoreIsPresent def test_setClangCxxLanguageStandardIfNeeded_whenReactCoreIsNotPresent installer = prepare_mocked_installer_without_react_core - set_clang_cxx_language_standard_if_needed(installer) + NewArchitectureHelper.set_clang_cxx_language_standard_if_needed(installer) assert_equal(installer.aggregate_targets[0].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], nil) assert_equal(installer.aggregate_targets[1].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], nil) @@ -40,15 +39,81 @@ def test_setClangCxxLanguageStandardIfNeeded_whenReactCoreIsNotPresent def test_setClangCxxLanguageStandardIfNeeded_whenThereAreDifferentValuesForLanguageStandard_takesTheFirstValue installer = prepare_mocked_installer_with_react_core_and_different_language_standards - set_clang_cxx_language_standard_if_needed(installer) + NewArchitectureHelper.set_clang_cxx_language_standard_if_needed(installer) assert_equal(installer.aggregate_targets[0].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal(installer.aggregate_targets[1].user_project.build_configurations[0].build_settings["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal(installer.pods_project.targets[1].received_resolved_build_setting_parameters, [ReceivedCommonResolvedBuildSettings.new("CLANG_CXX_LANGUAGE_STANDARD", true)]) assert_equal(Pod::UI.collected_messages, ["Setting CLANG_CXX_LANGUAGE_STANDARD to c++17 on /test/path.xcproj", "Setting CLANG_CXX_LANGUAGE_STANDARD to c++17 on /test/path2.xcproj"]) end + + # =================== # + # Test - Modify Flags # + # =================== # + def test_modifyFlagsForNewArch_whenOnOldArch_doNothing + # Arrange + first_xcconfig = prepare_xcconfig("First") + second_xcconfig = prepare_xcconfig("Second") + react_core_debug_config = prepare_CXX_Flags_build_configuration("Debug") + react_core_release_config = prepare_CXX_Flags_build_configuration("Release") + yoga_debug_config = prepare_CXX_Flags_build_configuration("Debug") + yoga_release_config = prepare_CXX_Flags_build_configuration("Release") + + installer = prepare_installer_for_cpp_flags( + [ first_xcconfig, second_xcconfig ], + { + "React-Core" => [ react_core_debug_config, react_core_release_config ], + "Yoga" => [ yoga_debug_config, yoga_release_config ], + } + ) + # Act + NewArchitectureHelper.modify_flags_for_new_architecture(installer, false) + + # Assert + assert_equal(first_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(first_xcconfig.save_as_invocation, []) + assert_equal(second_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(second_xcconfig.save_as_invocation, []) + assert_equal(react_core_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(react_core_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(yoga_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(yoga_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + end + + def test_modifyFlagsForNewArch_whenOnNewArch_updateFlags + # Arrange + first_xcconfig = prepare_xcconfig("First") + second_xcconfig = prepare_xcconfig("Second") + react_core_debug_config = prepare_CXX_Flags_build_configuration("Debug") + react_core_release_config = prepare_CXX_Flags_build_configuration("Release") + yoga_debug_config = prepare_CXX_Flags_build_configuration("Debug") + yoga_release_config = prepare_CXX_Flags_build_configuration("Release") + + installer = prepare_installer_for_cpp_flags( + [ first_xcconfig, second_xcconfig ], + { + "React-Core" => [ react_core_debug_config, react_core_release_config ], + "Yoga" => [ yoga_debug_config, yoga_release_config ], + } + ) + # Act + NewArchitectureHelper.modify_flags_for_new_architecture(installer, true) + + # Assert + assert_equal(first_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") + assert_equal(first_xcconfig.save_as_invocation, ["a/path/First.xcconfig"]) + assert_equal(second_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") + assert_equal(second_xcconfig.save_as_invocation, ["a/path/Second.xcconfig"]) + assert_equal(react_core_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") + assert_equal(react_core_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") + assert_equal(yoga_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(yoga_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + end end +# ================ # +# Test - Utilities # +# ================ # def prepare_mocked_installer_with_react_core return InstallerMock.new( PodsProjectMock.new([ @@ -131,3 +196,44 @@ def prepare_mocked_installer_without_react_core ] ) end + +def prepare_xcconfig(name) + return XCConfigMock.new(name, :attributes => {"OTHER_CPLUSPLUSFLAGS" => "$(inherited)"}) +end + +def prepare_CXX_Flags_build_configuration(name) + return BuildConfigurationMock.new(name, { + "OTHER_CPLUSPLUSFLAGS" => "$(inherited)" + }) +end + +def prepare_pod_target_installation_results_mock(name, configs) + return PodTargetInstallationResultsMock.new( + :name => name, + :native_target => TargetMock.new(name, configs) + ) +end + +def prepare_installer_for_cpp_flags(xcconfigs, build_configs) + xcconfigs_map = {} + xcconfigs.each do |config| + xcconfigs_map[config.name.to_s] = config + end + + pod_target_installation_results_map = {} + build_configs.each do |name, build_configs| + pod_target_installation_results_map[name.to_s] = prepare_pod_target_installation_results_mock( + name.to_s, build_configs + ) + end + + return InstallerMock.new( + PodsProjectMock.new, + [ + AggregatedProjectMock.new(:xcconfigs => xcconfigs_map, :base_path => "a/path/") + ], + :target_installation_results => TargetInstallationResultsMock.new( + :pod_target_installation_results => pod_target_installation_results_map + ) + ) +end diff --git a/scripts/cocoapods/__tests__/test_utils/InstallerMock.rb b/scripts/cocoapods/__tests__/test_utils/InstallerMock.rb index f7ad218714b362..c09f0659b6fe83 100644 --- a/scripts/cocoapods/__tests__/test_utils/InstallerMock.rb +++ b/scripts/cocoapods/__tests__/test_utils/InstallerMock.rb @@ -39,10 +39,12 @@ class InstallerMock attr_reader :pods_project attr_reader :aggregate_targets + attr_reader :target_installation_results - def initialize(pods_project = PodsProjectMock.new, aggregate_targets = [AggregatedProjectMock.new]) + def initialize(pods_project = PodsProjectMock.new, aggregate_targets = [AggregatedProjectMock.new], target_installation_results: []) @pods_project = pods_project @aggregate_targets = aggregate_targets + @target_installation_results = target_installation_results end def target_with_name(name) @@ -80,9 +82,18 @@ def save() class AggregatedProjectMock attr_reader :user_project + attr_reader :xcconfigs - def initialize(user_project = UserProjectMock.new) + @base_path + + def initialize(user_project = UserProjectMock.new, xcconfigs: {}, base_path: "") @user_project = user_project + @xcconfigs = xcconfigs + @base_path = base_path + end + + def xcconfig_path(config_name) + return File.join(@base_path, "#{config_name}.xcconfig") end end @@ -106,6 +117,22 @@ def save() end end +class XCConfigMock + attr_reader :name + attr_accessor :attributes + attr_reader :save_as_invocation + + def initialize(name, attributes: {}) + @name = name + @attributes = attributes + @save_as_invocation = [] + end + + def save_as(file_path) + @save_as_invocation.push(file_path) + end +end + ReceivedCommonResolvedBuildSettings = Struct.new(:key, :resolve_against_xcconfig) class TargetMock @@ -136,3 +163,21 @@ def initialize(name, build_settings = {}) @build_settings = build_settings end end + +class TargetInstallationResultsMock + attr_reader :pod_target_installation_results + + def initialize(pod_target_installation_results: {}) + @pod_target_installation_results = pod_target_installation_results + end +end + +class PodTargetInstallationResultsMock + attr_reader :name + attr_reader :native_target + + def initialize(name: "", native_target: TargetMock.new()) + @name = name + @native_target = native_target + end +end diff --git a/scripts/cocoapods/new_architecture.rb b/scripts/cocoapods/new_architecture.rb index 82ac79c678abcf..166a2ed7879953 100644 --- a/scripts/cocoapods/new_architecture.rb +++ b/scripts/cocoapods/new_architecture.rb @@ -3,28 +3,57 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -def set_clang_cxx_language_standard_if_needed(installer) - language_standard = nil +class NewArchitectureHelper - installer.pods_project.targets.each do |target| - if target.name == 'React-Core' - language_standard = target.resolved_build_setting("CLANG_CXX_LANGUAGE_STANDARD", resolve_against_xcconfig: true).values[0] + @@new_arch_cpp_flags = '$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1' + + def self.set_clang_cxx_language_standard_if_needed(installer) + language_standard = nil + + installer.pods_project.targets.each do |target| + if target.name == 'React-Core' + language_standard = target.resolved_build_setting("CLANG_CXX_LANGUAGE_STANDARD", resolve_against_xcconfig: true).values[0] + end end - end - unless language_standard.nil? - projects = installer.aggregate_targets - .map{ |t| t.user_project } - .uniq{ |p| p.path } + unless language_standard.nil? + projects = installer.aggregate_targets + .map{ |t| t.user_project } + .uniq{ |p| p.path } - projects.each do |project| - Pod::UI.puts("Setting CLANG_CXX_LANGUAGE_STANDARD to #{ language_standard } on #{ project.path }") + projects.each do |project| + Pod::UI.puts("Setting CLANG_CXX_LANGUAGE_STANDARD to #{ language_standard } on #{ project.path }") - project.build_configurations.each do |config| - config.build_settings["CLANG_CXX_LANGUAGE_STANDARD"] = language_standard + project.build_configurations.each do |config| + config.build_settings["CLANG_CXX_LANGUAGE_STANDARD"] = language_standard + end + + project.save() end + end + end - project.save() + def self.modify_flags_for_new_architecture(installer, is_new_arch_enabled) + unless is_new_arch_enabled + return + end + + # Add RCT_NEW_ARCH_ENABLED to Target pods xcconfig + installer.aggregate_targets.each do |aggregate_target| + aggregate_target.xcconfigs.each do |config_name, config_file| + config_file.attributes['OTHER_CPLUSPLUSFLAGS'] = @@new_arch_cpp_flags + xcconfig_path = aggregate_target.xcconfig_path(config_name) + config_file.save_as(xcconfig_path) + end + end + + # Add RCT_NEW_ARCH_ENABLED to generated pod target projects + installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result| + if pod_name == 'React-Core' + target_installation_result.native_target.build_configurations.each do |config| + config.build_settings['OTHER_CPLUSPLUSFLAGS'] = @@new_arch_cpp_flags + end + end end end end diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb index ee20ac9061c117..f8f36a411d4c31 100644 --- a/scripts/react_native_pods.rb +++ b/scripts/react_native_pods.rb @@ -11,14 +11,13 @@ require_relative './cocoapods/fabric.rb' require_relative './cocoapods/codegen.rb' require_relative './cocoapods/utils.rb' +require_relative './cocoapods/new_architecture.rb' $CODEGEN_OUTPUT_DIR = 'build/generated/ios' $CODEGEN_COMPONENT_DIR = 'react/renderer/components' $CODEGEN_MODULE_DIR = '.' $REACT_CODEGEN_PODSPEC_GENERATED = false $REACT_CODEGEN_DISCOVERY_DONE = false -DEFAULT_OTHER_CPLUSPLUSFLAGS = '$(inherited)' -NEW_ARCH_OTHER_CPLUSPLUSFLAGS = '$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1' $START_TIME = Time.now.to_i @@ -148,38 +147,13 @@ def react_native_post_install(installer, react_native_path = "../node_modules/re ReactNativePodsUtils.exclude_i386_architecture_while_using_hermes(installer) ReactNativePodsUtils.fix_library_search_paths(installer) - - cpp_flags = DEFAULT_OTHER_CPLUSPLUSFLAGS - if ENV['RCT_NEW_ARCH_ENABLED'] == '1' - cpp_flags = NEW_ARCH_OTHER_CPLUSPLUSFLAGS - end - modify_flags_for_new_architecture(installer, cpp_flags) - ReactNativePodsUtils.set_node_modules_user_settings(installer, react_native_path) - puts "Pod install took #{Time.now.to_i - $START_TIME} [s] to run" -end - -def modify_flags_for_new_architecture(installer, cpp_flags) - # Add RCT_NEW_ARCH_ENABLED to Target pods xcconfig - installer.aggregate_targets.each do |aggregate_target| - aggregate_target.xcconfigs.each do |config_name, config_file| - config_file.attributes['OTHER_CPLUSPLUSFLAGS'] = cpp_flags - xcconfig_path = aggregate_target.xcconfig_path(config_name) - Pod::UI.puts xcconfig_path - config_file.save_as(xcconfig_path) - end - end + NewArchitectureHelper.set_clang_cxx_language_standard_if_needed(installer) + is_new_arch_enabled = ENV['RCT_NEW_ARCH_ENABLED'] == '1' + NewArchitectureHelper.modify_flags_for_new_architecture(installer, is_new_arch_enabled) - # Add RCT_NEW_ARCH_ENABLED to generated pod target projects - installer.target_installation_results.pod_target_installation_results - .each do |pod_name, target_installation_result| - if pod_name == 'React-Core' - target_installation_result.native_target.build_configurations.each do |config| - config.build_settings['OTHER_CPLUSPLUSFLAGS'] = cpp_flags - end - end - end + Pod::UI.puts "Pod install took #{Time.now.to_i - $START_TIME} [s] to run".green end def get_react_codegen_spec(options={})