Skip to content

Commit

Permalink
Revert ENABLE_HERMES_PROFILER flag in cocoapods (facebook#37228)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#37228

As discussed offline, the current approach for the Hermes profiler is not the right one.

I'm partially reverting [the commit](facebook@dce9d8d) 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
  • Loading branch information
cipolleschi authored and jeongshin committed May 7, 2023
1 parent 2cd747f commit 387c2c4
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 81 deletions.
68 changes: 0 additions & 68 deletions packages/react-native/scripts/cocoapods/__tests__/utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -677,74 +677,6 @@ def test_applyFlagsForFabric_whenFabricDisabled_doNothing
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

# ===== #
Expand Down
7 changes: 0 additions & 7 deletions packages/react-native/scripts/cocoapods/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,6 @@ def self.update_search_paths(installer)
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 #
# ========= #
Expand Down
9 changes: 3 additions & 6 deletions packages/react-native/scripts/react_native_pods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,10 @@ def use_flipper!(versions = {}, configurations: ['Debug'])
# - 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
Expand All @@ -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

Expand Down

0 comments on commit 387c2c4

Please sign in to comment.