From a000b23a303edb7a614eba03cb4ea632225c8cde Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 16 Feb 2024 13:38:36 -0800 Subject: [PATCH] Enable -dead_strip by default for opt builds (#237) This change makes `-dead_strip` always be enabled for `opt` builds, regardless of `--objc_enable_binary_stripping`. This should be the right default for everyone, if not we can look at adding a way to disable it. This also allows passing `--features=dead_strip` to enable `-dead_strip` in other configurations, which previously wasn't supported if you weren't using `opt`. --- crosstool/cc_toolchain_config.bzl | 12 ++++---- test/linking_tests.bzl | 51 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/crosstool/cc_toolchain_config.bzl b/crosstool/cc_toolchain_config.bzl index 330185a..fba1832 100644 --- a/crosstool/cc_toolchain_config.bzl +++ b/crosstool/cc_toolchain_config.bzl @@ -1051,6 +1051,11 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new ), ], ), + flag_set( + actions = _DYNAMIC_LINK_ACTIONS, + flag_groups = [flag_group(flags = ["-dead_strip"])], + with_features = [with_feature_set(features = ["opt"])], + ), ], ) @@ -2057,14 +2062,9 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new flag_sets = [ flag_set( actions = _DYNAMIC_LINK_ACTIONS, - flag_groups = [ - flag_group( - flags = ["-dead_strip"], - ), - ], + flag_groups = [flag_group(flags = ["-dead_strip"])], ), ], - requires = [feature_set(features = ["opt"])], ) oso_prefix_feature = feature( diff --git a/test/linking_tests.bzl b/test/linking_tests.bzl index ed1032f..504a08f 100644 --- a/test/linking_tests.bzl +++ b/test/linking_tests.bzl @@ -7,6 +7,21 @@ load( default_test = make_action_command_line_test_rule() +opt_test = make_action_command_line_test_rule( + config_settings = { + "//command_line_option:compilation_mode": "opt", + }, +) + +dead_strip_requested_test = make_action_command_line_test_rule( + config_settings = { + "//command_line_option:compilation_mode": "fastbuild", + "//command_line_option:features": [ + "dead_strip", + ], + }, +) + disable_objc_test = make_action_command_line_test_rule( config_settings = { "//command_line_option:features": [ @@ -22,6 +37,11 @@ dsym_test = make_action_command_line_test_rule( ) def linking_test_suite(name): + """Tests for linking behavior. + + Args: + name: The name to be included in test names and tags. + """ default_test( name = "{}_default_apple_link_test".format(name), tags = [name], @@ -35,6 +55,37 @@ def linking_test_suite(name): not_expected_argv = [ "-g", "DSYM_HINT_LINKED_BINARY", + "-dead_strip", + ], + mnemonic = "ObjcLink", + target_under_test = "//test/test_data:macos_binary", + ) + + opt_test( + name = "{}_opt_link_test".format(name), + tags = [name], + expected_argv = [ + "-Xlinker", + "-objc_abi_version", + "-Xlinker", + "2", + "-ObjC", + "-dead_strip", + ], + mnemonic = "ObjcLink", + target_under_test = "//test/test_data:macos_binary", + ) + + dead_strip_requested_test( + name = "{}_dead_strip_requested_test".format(name), + tags = [name], + expected_argv = [ + "-Xlinker", + "-objc_abi_version", + "-Xlinker", + "2", + "-ObjC", + "-dead_strip", ], mnemonic = "ObjcLink", target_under_test = "//test/test_data:macos_binary",