Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NDEBUG flag for Release builds for both architectures #41715

Closed
wants to merge 6 commits into from
Closed

Add NDEBUG flag for Release builds for both architectures #41715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a65fe5c
init
tjzel Nov 30, 2023
3dce6f0
problematic tests
tjzel Nov 30, 2023
45df9bd
fix
tjzel Nov 30, 2023
b2ad71a
remove extra argument
tjzel Nov 30, 2023
84506fe
fail-safe React-Core
tjzel Nov 30, 2023
0a7a0fe
inheritance
tjzel Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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}"
cipolleschi marked this conversation as resolved.
Show resolved Hide resolved
@@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
tjzel marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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