From 1ea2c9bad91bc2bed06abf14893ee86426b5ec4e Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Thu, 4 May 2023 03:01:50 -0700 Subject: [PATCH] Revert ENABLE_HERMES_PROFILER flag in cocoapods (#37228) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37228 As discussed offline, the current approach for the Hermes profiler is not the right one. I'm partially reverting [the commit](https://github.com/facebook/react-native/commit/dce9d8d5de381fe53760ddda0d6cbbdfb5be00e4) which introduced it. The commit did also a bit of refactoring to improve the quality of the cocoapods scripts we would like to keep. ## Changelog: [iOS][Removed] - Remove support of Hermes profiler as that's not the right approach. Reviewed By: cortinico Differential Revision: D45527507 fbshipit-source-id: acea5f8b610d8b67ee7a6a91993bb8e4592d093f --- .../scripts/cocoapods/__tests__/utils-test.rb | 68 ------------------- .../react-native/scripts/cocoapods/utils.rb | 7 -- .../react-native/scripts/react_native_pods.rb | 9 +-- 3 files changed, 3 insertions(+), 81 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb index 510c4032825..843b58336fb 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/utils-test.rb @@ -677,74 +677,6 @@ class UtilsTests < Test::Unit::TestCase assert_equal(config.build_settings["OTHER_CFLAGS"], "$(inherited)") end end - - # ============================= # - # Test - Enable Hermes Profiler # - # ============================= # - - def test_enableHermesProfiler_whenEnableHermesProfileIsTrue_setsFlagsInRelease - # Arrange - first_target = prepare_target("FirstTarget") - second_target = prepare_target("SecondTarget") - third_target = prepare_target("ThirdTarget", "com.apple.product-type.bundle") - user_project_mock = UserProjectMock.new("a/path", [ - prepare_config("Debug"), - prepare_config("Release"), - ], - :native_targets => [ - first_target, - second_target - ] - ) - pods_projects_mock = PodsProjectMock.new([third_target], {"hermes-engine" => {}}) - installer = InstallerMock.new(pods_projects_mock, [ - AggregatedProjectMock.new(user_project_mock) - ]) - - # Act - ReactNativePodsUtils.enable_hermes_profiler(installer, enable_hermes_profiler: true) - - # Assert - installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result| - target_installation_result.native_target.build_configurations.each do |config| - if config.name != "Release" - assert_nil(config.build_settings["OTHER_CFLAGS"]) - else - assert_equal(config.build_settings["OTHER_CFLAGS"], "$(inherited) -DRCT_REMOTE_PROFILE=1") - end - end - end - end - - def test_enableHermesProfiler_whenEnableHermesProfileIsFalse_doesNothing - # Arrange - first_target = prepare_target("FirstTarget") - second_target = prepare_target("SecondTarget") - third_target = prepare_target("ThirdTarget", "com.apple.product-type.bundle") - user_project_mock = UserProjectMock.new("a/path", [ - prepare_config("Debug"), - prepare_config("Release"), - ], - :native_targets => [ - first_target, - second_target - ] - ) - pods_projects_mock = PodsProjectMock.new([third_target], {"hermes-engine" => {}}) - installer = InstallerMock.new(pods_projects_mock, [ - AggregatedProjectMock.new(user_project_mock) - ]) - - # Act - ReactNativePodsUtils.enable_hermes_profiler(installer) - - # Assert - installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result| - target_installation_result.native_target.build_configurations.each do |config| - assert_nil(config.build_settings["OTHER_CFLAGS"]) - end - end - end end # ===== # diff --git a/packages/react-native/scripts/cocoapods/utils.rb b/packages/react-native/scripts/cocoapods/utils.rb index 92a621d994d..79c216f74ed 100644 --- a/packages/react-native/scripts/cocoapods/utils.rb +++ b/packages/react-native/scripts/cocoapods/utils.rb @@ -220,13 +220,6 @@ class ReactNativePodsUtils end end - def self.enable_hermes_profiler(installer, enable_hermes_profiler: false) - return if !enable_hermes_profiler - - Pod::UI.puts "[Hermes Profiler] Enable Hermes Sample profiler" - self.add_compiler_flag_to_pods(installer, "-DRCT_REMOTE_PROFILE=1", configuration: "Release") - end - # ========= # # Utilities # # ========= # diff --git a/packages/react-native/scripts/react_native_pods.rb b/packages/react-native/scripts/react_native_pods.rb index 55bf9dbc7c7..b33575a95dc 100644 --- a/packages/react-native/scripts/react_native_pods.rb +++ b/packages/react-native/scripts/react_native_pods.rb @@ -222,11 +222,10 @@ end # - mac_catalyst_enabled: whether we are running the Pod on a Mac Catalyst project or not. # - enable_hermes_profiler: whether the hermes profiler should be turned on in Release mode def react_native_post_install( - installer, react_native_path = "../node_modules/react-native", - mac_catalyst_enabled: false, - enable_hermes_profiler: false + installer, + react_native_path = "../node_modules/react-native", + mac_catalyst_enabled: false ) - enable_hermes_profiler = enable_hermes_profiler || ENV["ENABLE_HERMES_PROFILER"] == "1" ReactNativePodsUtils.turn_off_resource_bundle_react_core(installer) ReactNativePodsUtils.apply_mac_catalyst_patches(installer) if mac_catalyst_enabled @@ -242,13 +241,11 @@ def react_native_post_install( ReactNativePodsUtils.update_search_paths(installer) ReactNativePodsUtils.set_node_modules_user_settings(installer, react_native_path) ReactNativePodsUtils.apply_flags_for_fabric(installer, fabric_enabled: fabric_enabled) - ReactNativePodsUtils.enable_hermes_profiler(installer, enable_hermes_profiler: enable_hermes_profiler) 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) - Pod::UI.puts "Pod install took #{Time.now.to_i - $START_TIME} [s] to run".green end