Move magic compiler wrapper args to env vars

crosstool/cc_toolchain_config.bzl

@@ -759,12 +759,13 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
 
     strip_debug_symbols_feature = feature(
         name = "strip_debug_symbols",
-        flag_sets = [
-            flag_set(
+        env_sets = [
+            env_set(
                 actions = _DYNAMIC_LINK_ACTIONS,
-                flag_groups = [
-                    flag_group(
-                        flags = ["STRIP_DEBUG_SYMBOLS"],
+                env_entries = [
+                    env_entry(
+                        key = "STRIP_DEBUG_SYMBOLS",
+                        value = "true",
                         expand_if_available = "strip_debug_symbols",
                     ),
                 ],
@@ -1815,13 +1816,14 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
                 ],
                 flag_groups = [flag_group(flags = ["-g"])],
             ),
-            flag_set(
+        ],
+        env_sets = [
+            env_set(
                 actions = _DYNAMIC_LINK_ACTIONS,
-                flag_groups = [
-                    flag_group(
-                        flags = [
-                            "DSYM_HINT_DSYM_PATH=%{dsym_path}",
-                        ],
+                env_entries = [
+                    env_entry(
+                        key = "DSYM_HINT_DSYM_PATH",
+                        value = "%{dsym_path}",
                         # We need to check this for backwards compatibility with bazel 7
                         expand_if_available = "dsym_path",
                     ),

crosstool/wrapped_clang.cc

@@ -12,16 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //
-// wrapped_clang.cc: Pass args to 'xcrun clang' and zip dsym files.
+// wrapped_clang.cc: Pass args to 'xcrun clang' and optionally produce dSYM
+// files.
 //
-// wrapped_clang passes its args to clang, but also supports a separate set of
-// invocations to generate dSYM files.  If "DSYM_HINT" flags are passed in, they
-// are used to construct that separate set of invocations (instead of being
-// passed to clang).
-// The following "DSYM_HINT" flags control dsym generation.  If any one if these
-// are passed in, then they all must be passed in.
-// "LINKED_BINARY": Workspace-relative path to binary output of the link action.
-// "DSYM_HINT_DSYM_PATH": Workspace-relative path to dSYM dwarf file.
 
 #include <libgen.h>
 #include <spawn.h>
@@ -256,14 +249,12 @@ static std::unique_ptr<TempFile> WriteResponseFile(
 
 void ProcessArgument(const std::string arg, const std::string developer_dir,
                      const std::string sdk_root, const std::string cwd,
-                     std::string &linked_binary, std::string &dsym_path,
-                     bool &strip_debug_symbols, std::string toolchain_path,
+                     std::string &linked_binary, std::string toolchain_path,
                      std::function<void(const std::string &)> consumer);
 
 bool ProcessResponseFile(const std::string arg, const std::string developer_dir,
                          const std::string sdk_root, const std::string cwd,
-                         std::string &linked_binary, std::string &dsym_path,
-                         bool &strip_debug_symbols, std::string toolchain_path,
+                         std::string &linked_binary, std::string toolchain_path,
                          std::function<void(const std::string &)> consumer) {
   auto path = arg.substr(1);
   std::ifstream original_file(path);
@@ -277,8 +268,7 @@ bool ProcessResponseFile(const std::string arg, const std::string developer_dir,
     // Arguments in response files might be quoted/escaped, so we need to
     // unescape them ourselves.
     ProcessArgument(Unescape(arg_from_file), developer_dir, sdk_root, cwd,
-                    linked_binary, dsym_path, strip_debug_symbols,
-                    toolchain_path, consumer);
+                    linked_binary, toolchain_path, consumer);
   }
 
   return true;
@@ -360,28 +350,19 @@ std::string GetToolchainPath(const std::string &toolchain_id) {
 
 void ProcessArgument(const std::string arg, const std::string developer_dir,
                      const std::string sdk_root, const std::string cwd,
-                     std::string &linked_binary, std::string &dsym_path,
-                     bool &strip_debug_symbols, std::string toolchain_path,
+                     std::string &linked_binary, std::string toolchain_path,
                      std::function<void(const std::string &)> consumer) {
   auto new_arg = arg;
   if (arg[0] == '@') {
     if (ProcessResponseFile(arg, developer_dir, sdk_root, cwd, linked_binary,
-                            dsym_path, strip_debug_symbols, toolchain_path,
-                            consumer)) {
+                            toolchain_path, consumer)) {
       return;
     }
   }
 
   if (SetArgIfFlagPresent(arg, "LINKED_BINARY", &linked_binary)) {
     return;
   }
-  if (SetArgIfFlagPresent(arg, "DSYM_HINT_DSYM_PATH", &dsym_path)) {
-    return;
-  }
-  if (arg == "STRIP_DEBUG_SYMBOLS") {
-    strip_debug_symbols = true;
-    return;
-  }
 
   FindAndReplace("__BAZEL_EXECUTION_ROOT__", cwd, &new_arg);
   FindAndReplace("__BAZEL_EXECUTION_ROOT_CANONICAL__", GetCanonicalPath(cwd), &new_arg);
@@ -436,8 +417,7 @@ int main(int argc, char *argv[]) {
 
   std::string developer_dir = GetMandatoryEnvVar("DEVELOPER_DIR");
   std::string sdk_root = GetMandatoryEnvVar("SDKROOT");
-  std::string linked_binary, dsym_path;
-  bool strip_debug_symbols = false;
+  std::string linked_binary;
 
   const std::string cwd = GetCurrentDirectory();
   std::vector<std::string> invocation_args = {"/usr/bin/xcrun", tool_name};
@@ -449,8 +429,8 @@ int main(int argc, char *argv[]) {
   for (int i = 1; i < argc; i++) {
     std::string arg(argv[i]);
 
-    ProcessArgument(arg, developer_dir, sdk_root, cwd, linked_binary, dsym_path,
-                    strip_debug_symbols, toolchain_path, consumer);
+    ProcessArgument(arg, developer_dir, sdk_root, cwd, linked_binary,
+                    toolchain_path, consumer);
   }
 
   char *modulemap = getenv("APPLE_SUPPORT_MODULEMAP");
@@ -482,7 +462,9 @@ int main(int argc, char *argv[]) {
     output.close();
   }
 
-  bool generate_dsym = !dsym_path.empty();
+  const char *dsym_path_raw_value = getenv("DSYM_HINT_DSYM_PATH");
+  bool generate_dsym = dsym_path_raw_value != nullptr;
+  bool strip_debug_symbols = getenv("STRIP_DEBUG_SYMBOLS") != nullptr;
   if (!generate_dsym && !strip_debug_symbols) {
     return 0;
   }
@@ -495,6 +477,7 @@ int main(int argc, char *argv[]) {
   }
 
   if (generate_dsym) {
+    std::string dsym_path = dsym_path_raw_value;
     const std::string bundle_suffix = ".dSYM";
     bool is_bundle = dsym_path.rfind(bundle_suffix) ==
                      dsym_path.length() - bundle_suffix.length();

test/linking_tests.bzl

@@ -65,9 +65,11 @@ def linking_test_suite(name):
         ],
         not_expected_argv = [
             "-g",
-            "DSYM_HINT_DSYM_PATH",
             "-dead_strip",
         ],
+        not_expected_env_keys = [
+            "DSYM_HINT_DSYM_PATH",
+        ],
         mnemonic = "ObjcLink",
         target_under_test = "//test/test_data:macos_binary",
     )
@@ -88,9 +90,11 @@ def linking_test_suite(name):
         ],
         not_expected_argv = [
             "-g",
-            "DSYM_HINT_DSYM_PATH",
             "-dead_strip",
         ],
+        not_expected_env_keys = [
+            "DSYM_HINT_DSYM_PATH",
+        ],
         mnemonic = "ObjcLink",
         target_under_test = "//test/test_data:ios_binary",
     )
@@ -184,7 +188,8 @@ def linking_test_suite(name):
         tags = [name],
         expected_argv = [
             "-g",
-            "LINKED_BINARY",
+        ],
+        expected_env_keys = [
             "DSYM_HINT_DSYM_PATH",
         ],
         mnemonic = "ObjcLink",
@@ -194,8 +199,7 @@ def linking_test_suite(name):
     dsym_test(
         name = "{}_generate_cpp_dsym_test".format(name),
         tags = [name],
-        expected_argv = [
-            "LINKED_BINARY",
+        expected_env_keys = [
             "DSYM_HINT_DSYM_PATH",
         ],
         mnemonic = "CppLink",

test/rules/action_command_line_test.bzl

@@ -102,6 +102,28 @@ def _action_command_line_test_impl(ctx):
                 ),
             )
 
+    for expected_key in ctx.attr.expected_env_keys:
+        if expected_key not in action.env:
+            unittest.fail(
+                env,
+                "{}expected env to contain key '{}', but it did not: {}".format(
+                    message_prefix,
+                    expected_key,
+                    action.env,
+                ),
+            )
+
+    for not_expected_key in ctx.attr.not_expected_env_keys:
+        if not_expected_key in action.env:
+            unittest.fail(
+                env,
+                "{}expected env to not contain key '{}', but it did: {}".format(
+                    message_prefix,
+                    not_expected_key,
+                    action.env,
+                ),
+            )
+
     return analysistest.end(env)
 
 def make_action_command_line_test_rule(config_settings = {}):
@@ -132,6 +154,20 @@ space-delimited string.
 A list of strings representing substrings expected not to appear in the action
 command line, after concatenating all command line arguments into a single
 space-delimited string.
+""",
+            ),
+            "expected_env_keys": attr.string_list(
+                mandatory = False,
+                doc = """\
+A list of strings representing environment variable keys expected to be set
+in the action environment.
+""",
+            ),
+            "not_expected_env_keys": attr.string_list(
+                mandatory = False,
+                doc = """\
+A list of strings representing environment variable keys expected not to be
+set in the action environment.
 """,
             ),
             "mnemonic": attr.string(