Skip to content

Commit

Permalink
Add NDEBUG flag for Release builds for both architectures (#41715)
Browse files Browse the repository at this point in the history
Summary:
Currently React Native defines `NDEBUG` flag for all pods in Fabric only. This is useful for other libraries, like Reanimated, because they have no easy way of defining their compilation flags (at least none that I know of). Therefore defining `NDEBUG` for both architectures would be beneficial.

## Changelog:

Pick one each for the category and type tags:

[IOS] [CHANGED] - Add `NDEBUG` flag for Release builds for both architectures

Pull Request resolved: #41715

Test Plan:
Run ruby test suite.

## Notes

For the time being I just copied
`prepare_pod_target_installation_results_mock`
and
`def prepare_installer_for_cpp_flags`
to `utils-test.rb` since I wasn't sure how to handle the installer mock.

Reviewed By: cortinico

Differential Revision: D51708382

Pulled By: cipolleschi

fbshipit-source-id: ff206f8fc151934dbae89aacd1bc69c57b4f28ee
  • Loading branch information
tjzel authored and facebook-github-bot committed Dec 1, 2023
1 parent dbf0984 commit 1a0e174
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ def test_modifyFlagsForNewArch_whenOnNewArchAndIsRelease_updateFlags
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 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1")
assert_nil(react_core_debug_config.build_settings["OTHER_CFLAGS"])
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 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -DNDEBUG")
assert_equal(react_core_release_config.build_settings["OTHER_CFLAGS"], "$(inherited) -DNDEBUG")
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 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1")
assert_nil(react_core_release_config.build_settings["OTHER_CFLAGS"])
assert_equal(yoga_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)")
assert_nil(yoga_debug_config.build_settings["OTHER_CFLAGS"])
assert_equal(yoga_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DNDEBUG")
assert_equal(yoga_release_config.build_settings["OTHER_CFLAGS"], "$(inherited) -DNDEBUG")
assert_equal(yoga_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)")
assert_nil(yoga_release_config.build_settings["OTHER_CFLAGS"])
end

# =================================== #
Expand Down
101 changes: 101 additions & 0 deletions packages/react-native/scripts/cocoapods/__tests__/utils-test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
require_relative "./test_utils/XcodeprojMock.rb"
require_relative "./test_utils/XcodebuildMock.rb"
require_relative "./test_utils/SpecMock.rb"
require_relative "./test_utils/InstallerMock.rb"

class UtilsTests < Test::Unit::TestCase
def setup
Expand Down Expand Up @@ -1008,6 +1009,78 @@ def test_addDependencies_whenSubspecsAndHeaderSearchPathAndVersionWithAdditional
assert_equal(spec.to_hash["pod_target_xcconfig"], {
"HEADER_SEARCH_PATHS" => expected_search_paths})
end

def test_add_flag_to_map_with_inheritance_whenUsedWithBuildConfigBuildSettings
# Arrange
empty_config = BuildConfigurationMock.new("EmptyConfig")
initialized_config = BuildConfigurationMock.new("InitializedConfig", {
"OTHER_CPLUSPLUSFLAGS" => "INIT_FLAG"
})
twiceProcessed_config = BuildConfigurationMock.new("TwiceProcessedConfig");
test_flag = " -DTEST_FLAG=1"

# Act
ReactNativePodsUtils.add_flag_to_map_with_inheritance(empty_config.build_settings, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(initialized_config.build_settings, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(twiceProcessed_config.build_settings, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(twiceProcessed_config.build_settings, "OTHER_CPLUSPLUSFLAGS", test_flag)

# Assert
assert_equal("$(inherited)" + test_flag, empty_config.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited) INIT_FLAG" + test_flag, initialized_config.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited)" + test_flag, twiceProcessed_config.build_settings["OTHER_CPLUSPLUSFLAGS"])
end

def test_add_flag_to_map_with_inheritance_whenUsedWithXCConfigAttributes
# Arrange
empty_xcconfig = XCConfigMock.new("EmptyConfig")
initialized_xcconfig = XCConfigMock.new("InitializedConfig", attributes: {
"OTHER_CPLUSPLUSFLAGS" => "INIT_FLAG"
})
twiceProcessed_xcconfig = XCConfigMock.new("TwiceProcessedConfig");
test_flag = " -DTEST_FLAG=1"

# Act
ReactNativePodsUtils.add_flag_to_map_with_inheritance(empty_xcconfig.attributes, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(initialized_xcconfig.attributes, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(twiceProcessed_xcconfig.attributes, "OTHER_CPLUSPLUSFLAGS", test_flag)
ReactNativePodsUtils.add_flag_to_map_with_inheritance(twiceProcessed_xcconfig.attributes, "OTHER_CPLUSPLUSFLAGS", test_flag)

# Assert
assert_equal("$(inherited)" + test_flag, empty_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited) INIT_FLAG" + test_flag, initialized_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited)" + test_flag, twiceProcessed_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"])
end

def test_add_ndebug_flag_to_pods_in_release
# Arrange
xcconfig = XCConfigMock.new("Config")
default_debug_config = BuildConfigurationMock.new("Debug")
default_release_config = BuildConfigurationMock.new("Release")
custom_debug_config1 = BuildConfigurationMock.new("CustomDebug")
custom_debug_config2 = BuildConfigurationMock.new("Custom")
custom_release_config1 = BuildConfigurationMock.new("CustomRelease")
custom_release_config2 = BuildConfigurationMock.new("Production")

installer = prepare_installer_for_cpp_flags(
[ xcconfig ],
{
"Default" => [ default_debug_config, default_release_config ],
"Custom1" => [ custom_debug_config1, custom_release_config1 ],
"Custom2" => [ custom_debug_config2, custom_release_config2 ]
}
)
# Act
ReactNativePodsUtils.add_ndebug_flag_to_pods_in_release(installer)

# Assert
assert_equal(nil, default_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited) -DNDEBUG", default_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal(nil, custom_debug_config1.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited) -DNDEBUG", custom_release_config1.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal(nil, custom_debug_config2.build_settings["OTHER_CPLUSPLUSFLAGS"])
assert_equal("$(inherited) -DNDEBUG", custom_release_config2.build_settings["OTHER_CPLUSPLUSFLAGS"])
end
end

# ===== #
Expand Down Expand Up @@ -1050,3 +1123,31 @@ def prepare_Code_Signing_build_configuration(name, param)
"CODE_SIGNING_ALLOWED" => param
})
end

def prepare_pod_target_installation_results_mock(name, configs)
target = TargetMock.new(name, configs)
return TargetInstallationResultMock.new(target, target)
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/")
],
:pod_target_installation_results => pod_target_installation_results_map
)
end

24 changes: 4 additions & 20 deletions packages/react-native/scripts/cocoapods/new_architecture.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class NewArchitectureHelper

@@folly_compiler_flags = "#{@@shared_flags} -Wno-comma -Wno-shorten-64-to-32"

@@new_arch_cpp_flags = "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 #{@@shared_flags}"
@@new_arch_cpp_flags = " -DRCT_NEW_ARCH_ENABLED=1 #{@@shared_flags}"

@@cplusplus_version = "c++20"

Expand Down Expand Up @@ -49,17 +49,10 @@ def self.modify_flags_for_new_architecture(installer, is_new_arch_enabled)
unless is_new_arch_enabled
return
end
ndebug_flag = " -DNDEBUG"
# 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

if config_name == "Release"
config_file.attributes['OTHER_CPLUSPLUSFLAGS'] = config_file.attributes['OTHER_CPLUSPLUSFLAGS'] + ndebug_flag
other_cflags = config_file.attributes['OTHER_CFLAGS'] != nil ? config_file.attributes['OTHER_CFLAGS'] : "$(inherited)"
config_file.attributes['OTHER_CFLAGS'] = other_cflags + ndebug_flag
end
ReactNativePodsUtils.add_flag_to_map_with_inheritance(config_file.attributes, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags)

xcconfig_path = aggregate_target.xcconfig_path(config_name)
config_file.save_as(xcconfig_path)
Expand All @@ -71,16 +64,7 @@ def self.modify_flags_for_new_architecture(installer, is_new_arch_enabled)
# The React-Core pod may have a suffix added by Cocoapods, so we test whether 'React-Core' is a substring, and do not require exact match
if pod_name.include? 'React-Core'
target_installation_result.native_target.build_configurations.each do |config|
config.build_settings['OTHER_CPLUSPLUSFLAGS'] = @@new_arch_cpp_flags
end
end

target_installation_result.native_target.build_configurations.each do |config|
if config.name == "Release"
current_flags = config.build_settings['OTHER_CPLUSPLUSFLAGS'] != nil ? config.build_settings['OTHER_CPLUSPLUSFLAGS'] : "$(inherited)"
config.build_settings['OTHER_CPLUSPLUSFLAGS'] = current_flags + ndebug_flag
current_cflags = config.build_settings['OTHER_CFLAGS'] != nil ? config.build_settings['OTHER_CFLAGS'] : "$(inherited)"
config.build_settings['OTHER_CFLAGS'] = current_cflags + ndebug_flag
ReactNativePodsUtils.add_flag_to_map_with_inheritance(config.build_settings, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags)
end
end
end
Expand Down Expand Up @@ -126,7 +110,7 @@ def self.install_modules_dependencies(spec, new_arch_enabled, folly_version)
spec.dependency "glog"

if new_arch_enabled
current_config["OTHER_CPLUSPLUSFLAGS"] = @@new_arch_cpp_flags
ReactNativePodsUtils.add_flag_to_map_with_inheritance(current_config, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags)
end

spec.dependency "React-RCTFabric" # This is for Fabric Component
Expand Down
42 changes: 42 additions & 0 deletions packages/react-native/scripts/cocoapods/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -613,4 +613,46 @@ def self.add_search_path_to_result(result, base_path, additional_paths, include_
}
return result
end

def self.add_ndebug_flag_to_pods_in_release(installer)
ndebug_flag = " -DNDEBUG"

installer.aggregate_targets.each do |aggregate_target|
aggregate_target.xcconfigs.each do |config_name, config_file|
is_release = config_name.downcase.include?("release") || config_name.downcase.include?("production")
unless is_release
next
end
self.add_flag_to_map_with_inheritance(config_file.attributes, 'OTHER_CPLUSPLUSFLAGS', ndebug_flag);
self.add_flag_to_map_with_inheritance(config_file.attributes, 'OTHER_CFLAGS', ndebug_flag);

xcconfig_path = aggregate_target.xcconfig_path(config_name)
config_file.save_as(xcconfig_path)
end
end

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|
is_release = config.name.downcase.include?("release") || config.name.downcase.include?("production")
unless is_release
next
end
self.add_flag_to_map_with_inheritance(config.build_settings, 'OTHER_CPLUSPLUSFLAGS', ndebug_flag);
self.add_flag_to_map_with_inheritance(config.build_settings, 'OTHER_CFLAGS', ndebug_flag);
end
end
end

def self.add_flag_to_map_with_inheritance(map, field, flag)
if map[field] == nil
map[field] = "$(inherited)" + flag
else
unless map[field].include?(flag)
map[field] = map[field] + flag
end
unless map[field].include?("$(inherited)")
map[field] = "$(inherited) " + map[field]
end
end
end
end
1 change: 1 addition & 0 deletions packages/react-native/scripts/react_native_pods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ def react_native_post_install(
ReactNativePodsUtils.apply_ats_config(installer)
ReactNativePodsUtils.updateOSDeploymentTarget(installer)
ReactNativePodsUtils.set_dynamic_frameworks_flags(installer)
ReactNativePodsUtils.add_ndebug_flag_to_pods_in_release(installer)

NewArchitectureHelper.set_clang_cxx_language_standard_if_needed(installer)
NewArchitectureHelper.modify_flags_for_new_architecture(installer, NewArchitectureHelper.new_arch_enabled)
Expand Down

0 comments on commit 1a0e174

Please sign in to comment.