Properly configure AR for the cargo_build_script rule

armandomontanez · armandomontanez · commit dc09d9190f79 · 2025-10-28T13:40:26.000-07:00
There were two issues with AR in the cargo_build_script rule:

  1. ARFLAGS was not set.
  2. For many toolchains, the ar tool was not properly configured.

This change fixes both of those problems.
diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl
@@ -130,9 +130,10 @@ def get_cc_compile_args_and_env(cc_toolchain, feature_configuration):
 
     Returns:
         tuple: A tuple of the following items:
-            - (sequence): A flattened C command line flags for given action.
-            - (sequence): A flattened CXX command line flags for given action.
-            - (dict): C environment variables to be set for given action.
+            - (sequence): A flattened list of C command line flags.
+            - (sequence): A flattened list of CXX command line flags.
+            - (sequence): A flattened list of AR command line flags.
+            - (dict): C environment variables to be set for this configuration.
     """
     compile_variables = cc_common.create_compile_variables(
         feature_configuration = feature_configuration,
@@ -148,12 +149,17 @@ def get_cc_compile_args_and_env(cc_toolchain, feature_configuration):
         action_name = ACTION_NAMES.cpp_compile,
         variables = compile_variables,
     )
+    cc_ar_args = cc_common.get_memory_inefficient_command_line(
+        feature_configuration = feature_configuration,
+        action_name = ACTION_NAMES.cpp_link_static_library,
+        variables = compile_variables,
+    )
     cc_env = cc_common.get_environment_variables(
         feature_configuration = feature_configuration,
         action_name = ACTION_NAMES.c_compile,
         variables = compile_variables,
     )
-    return cc_c_args, cc_cxx_args, cc_env
+    return cc_c_args, cc_cxx_args, cc_ar_args, cc_env
 
 def _pwd_flags_sysroot(args):
     """Prefix execroot-relative paths of known arguments with ${pwd}.
@@ -423,7 +429,7 @@ def _cargo_build_script_impl(ctx):
     env["LDFLAGS"] = " ".join(_pwd_flags(link_args))
 
     # MSVC requires INCLUDE to be set
-    cc_c_args, cc_cxx_args, cc_env = get_cc_compile_args_and_env(cc_toolchain, feature_configuration)
+    cc_c_args, cc_cxx_args, cc_ar_args, cc_env = get_cc_compile_args_and_env(cc_toolchain, feature_configuration)
     include = cc_env.get("INCLUDE")
     if include:
         env["INCLUDE"] = include
@@ -444,11 +450,17 @@ def _cargo_build_script_impl(ctx):
             action_name = ACTION_NAMES.cpp_link_static_library,
         )
 
+        # Many C/C++ toolchains are missing an action_config for AR because
+        # one was never included in the unix_cc_toolchain_config.
+        if not env["AR"]:
+            env["AR"] = cc_toolchain.ar_executable
+
         # Populate CFLAGS and CXXFLAGS that cc-rs relies on when building from source, in particular
         # to determine the deployment target when building for apple platforms (`macosx-version-min`
         # for example, itself derived from the `macos_minimum_os` Bazel argument).
         env["CFLAGS"] = " ".join(_pwd_flags(cc_c_args))
         env["CXXFLAGS"] = " ".join(_pwd_flags(cc_cxx_args))
+        env["ARFLAGS"] = " ".join(_pwd_flags(cc_ar_args))
 
     # Inform build scripts of rustc flags
     # https://github.com/rust-lang/cargo/issues/9600
diff --git a/test/cargo_build_script/cc_args_and_env/BUILD.bazel b/test/cargo_build_script/cc_args_and_env/BUILD.bazel
@@ -1,11 +1,13 @@
 load(
     "cc_args_and_env_test.bzl",
+    "ar_flags_test",
     "bindir_absolute_test",
     "bindir_relative_test",
     "fsanitize_ignorelist_absolute_test",
     "fsanitize_ignorelist_relative_test",
     "isystem_absolute_test",
     "isystem_relative_test",
+    "legacy_cc_toolchain_test",
     "sysroot_absolute_test",
     "sysroot_next_absolute_test",
     "sysroot_relative_test",
@@ -28,3 +30,7 @@ bindir_absolute_test(name = "bindir_absolute_test")
 fsanitize_ignorelist_absolute_test(name = "fsanitize_ignorelist_absolute_test")
 
 fsanitize_ignorelist_relative_test(name = "fsanitize_ignorelist_relative_test")
+
+ar_flags_test(name = "ar_flags_test")
+
+legacy_cc_toolchain_test(name = "legacy_cc_toolchain_test")
diff --git a/test/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl b/test/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl
@@ -8,12 +8,18 @@ To verify the processed cargo cc_args, we use cc_args_and_env_analysis_test().
 """
 
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
-load("@rules_cc//cc:action_names.bzl", "ACTION_NAME_GROUPS")
-load("@rules_cc//cc:cc_toolchain_config_lib.bzl", "feature", "flag_group", "flag_set")
+load("@rules_cc//cc:action_names.bzl", "ACTION_NAMES", "ACTION_NAME_GROUPS")
+load("@rules_cc//cc:cc_toolchain_config_lib.bzl", "action_config", "feature", "flag_group", "flag_set", "tool", "tool_path")
 load("@rules_cc//cc:defs.bzl", "cc_toolchain")
 load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
 load("//cargo:defs.bzl", "cargo_build_script")
 
+_EXPECTED_CC_TOOLCHAIN_TOOLS = {
+    "AR": "/usr/fake/ar",
+    "CC": "/usr/fake/gcc",
+    "CXX": "/usr/fake/g++",
+}
+
 def _test_cc_config_impl(ctx):
     features = [
         feature(
@@ -30,9 +36,96 @@ def _test_cc_config_impl(ctx):
                 ),
             ],
         ),
+        feature(
+            name = "default_cpp_flags",
+            enabled = True,
+            flag_sets = [
+                flag_set(
+                    actions = ACTION_NAME_GROUPS.all_cpp_compile_actions,
+                    flag_groups = ([
+                        flag_group(
+                            flags = ctx.attr.extra_cxx_compile_flags,
+                        ),
+                    ]),
+                ),
+            ],
+        ),
+        feature(
+            name = "default_ar_flags",
+            enabled = True,
+            flag_sets = [
+                flag_set(
+                    actions = [ACTION_NAMES.cpp_link_static_library],
+                    flag_groups = ([
+                        flag_group(
+                            flags = ctx.attr.extra_ar_flags,
+                        ),
+                    ]),
+                ),
+            ],
+        ),
     ]
+
+    tool_paths = []
+    action_configs = []
+    if ctx.attr.legacy_cc_toolchain:
+        tool_paths = [
+            tool_path(
+                name = "gcc",
+                path = _EXPECTED_CC_TOOLCHAIN_TOOLS["CC"],
+            ),
+            tool_path(
+                name = "cpp",
+                path = _EXPECTED_CC_TOOLCHAIN_TOOLS["CXX"],
+            ),
+            tool_path(
+                name = "ar",
+                path = _EXPECTED_CC_TOOLCHAIN_TOOLS["AR"],
+            ),
+            # These need to be set to something to create a toolchain, but
+            # is not tested here.
+            tool_path(
+                name = "ld",
+                path = "/usr/ignored/false",
+            ),
+            tool_path(
+                name = "nm",
+                path = "/usr/ignored/false",
+            ),
+            tool_path(
+                name = "objdump",
+                path = "/usr/ignored/false",
+            ),
+            tool_path(
+                name = "strip",
+                path = "/usr/ignored/false",
+            ),
+        ]
+    else:
+        action_configs = (
+            action_config(
+                action_name = ACTION_NAMES.c_compile,
+                tools = [
+                    tool(path = _EXPECTED_CC_TOOLCHAIN_TOOLS["CC"]),
+                ],
+            ),
+            action_config(
+                action_name = ACTION_NAMES.cpp_compile,
+                tools = [
+                    tool(path = _EXPECTED_CC_TOOLCHAIN_TOOLS["CXX"]),
+                ],
+            ),
+            action_config(
+                action_name = ACTION_NAMES.cpp_link_static_library,
+                tools = [
+                    tool(path = _EXPECTED_CC_TOOLCHAIN_TOOLS["AR"]),
+                ],
+            ),
+        )
+
     config_info = cc_common.create_cc_toolchain_config_info(
         ctx = ctx,
+        action_configs = action_configs,
         toolchain_identifier = "test-cc-toolchain",
         host_system_name = "unknown",
         target_system_name = "unknown",
@@ -42,13 +135,17 @@ def _test_cc_config_impl(ctx):
         abi_version = "unknown",
         abi_libc_version = "unknown",
         features = features,
+        tool_paths = tool_paths,
     )
     return config_info
 
 test_cc_config = rule(
     implementation = _test_cc_config_impl,
     attrs = {
+        "extra_ar_flags": attr.string_list(),
         "extra_cc_compile_flags": attr.string_list(),
+        "extra_cxx_compile_flags": attr.string_list(),
+        "legacy_cc_toolchain": attr.bool(default = False),
     },
     provides = [CcToolchainConfigInfo],
 )
@@ -84,27 +181,66 @@ def _cc_args_and_env_analysis_test_impl(ctx):
     env = analysistest.begin(ctx)
     tut = analysistest.target_under_test(env)
     cargo_action = tut[DepActionsInfo].actions[0]
-    cflags = cargo_action.env["CFLAGS"].split(" ")
-    for flag in ctx.attr.expected_cflags:
-        asserts.true(
+
+    for env_var, expected_path in _EXPECTED_CC_TOOLCHAIN_TOOLS.items():
+        if ctx.attr.legacy_cc_toolchain and env_var == "CXX":
+            # When using the legacy tool_path toolchain configuration approach,
+            # the CXX tool is forced to be the same as the the CC tool.
+            # See: https://github.com/bazelbuild/bazel/blob/14840856986f21b54330e352b83d687825648889/src/main/starlark/builtins_bzl/common/cc/toolchain_config/legacy_features.bzl#L1296-L1304
+            expected_path = _EXPECTED_CC_TOOLCHAIN_TOOLS["CC"]
+
+        actual_path = cargo_action.env.get(env_var)
+        asserts.false(
+            env,
+            actual_path == None,
+            "error: '{}' tool unset".format(env_var),
+        )
+        asserts.equals(
             env,
-            flag in cflags,
-            "error: expected '{}' to be in cargo CFLAGS: '{}'".format(flag, cflags),
+            expected_path,
+            actual_path,
+            "error: '{}' tool path '{}' does not match expected '{}'".format(
+                env_var,
+                actual_path,
+                expected_path,
+            ),
         )
 
+    _ENV_VAR_TO_EXPECTED_ARGS = {
+        "ARFLAGS": ctx.attr.expected_arflags,
+        "CFLAGS": ctx.attr.expected_cflags,
+        "CXXFLAGS": ctx.attr.expected_cxxflags,
+    }
+
+    for env_var, expected_flags in _ENV_VAR_TO_EXPECTED_ARGS.items():
+        actual_flags = cargo_action.env[env_var].split(" ")
+        for flag in expected_flags:
+            asserts.true(
+                env,
+                flag in actual_flags,
+                "error: expected '{}' to be in cargo {}: '{}'".format(flag, env_var, actual_flags),
+            )
+
     return analysistest.end(env)
 
 cc_args_and_env_analysis_test = analysistest.make(
     impl = _cc_args_and_env_analysis_test_impl,
     doc = """An analysistest to examine the custom cargo flags of an cargo_build_script target.""",
     attrs = {
-        "expected_cflags": attr.string_list(),
+        "expected_arflags": attr.string_list(default = ["-x"]),
+        "expected_cflags": attr.string_list(default = ["-Wall"]),
+        "expected_cxxflags": attr.string_list(default = ["-fno-rtti"]),
+        "legacy_cc_toolchain": attr.bool(default = False),
     },
 )
 
 def cargo_build_script_with_extra_cc_compile_flags(
+        *,
         name,
-        extra_cc_compile_flags):
+        extra_cc_compile_flags = ["-Wall"],
+        extra_cxx_compile_flags = ["-fno-rtti"],
+        extra_ar_flags = ["-x"],
+        legacy_cc_toolchain = False):
     """Produces a test cargo_build_script target that's set up to use a custom cc_toolchain with the extra_cc_compile_flags.
 
     This is achieved by creating a cascade of targets:
@@ -115,12 +251,19 @@ def cargo_build_script_with_extra_cc_compile_flags(
 
     Args:
       name: The name of the test target.
-      extra_cc_compile_flags: Extra args for the cc_toolchain.
+      extra_cc_compile_flags: Extra C/C++ args for the cc_toolchain.
+      extra_cxx_compile_flags: Extra C++-specific args for the cc_toolchain.
+      extra_ar_flags: Extra archiver args for the cc_toolchain.
+      legacy_cc_toolchain: Enables legacy tool_path configuration of the cc
+        cc toolchain.
     """
 
     test_cc_config(
         name = "%s/cc_toolchain_config" % name,
         extra_cc_compile_flags = extra_cc_compile_flags,
+        extra_cxx_compile_flags = extra_cxx_compile_flags,
+        extra_ar_flags = extra_ar_flags,
+        legacy_cc_toolchain = legacy_cc_toolchain,
     )
     cc_toolchain(
         name = "%s/test_cc_toolchain_impl" % name,
@@ -250,3 +393,25 @@ def fsanitize_ignorelist_absolute_test(name):
         target_under_test = "%s/cargo_build_script" % name,
         expected_cflags = ["-fsanitize-ignorelist=/test/absolute/path"],
     )
+
+def ar_flags_test(name):
+    cargo_build_script_with_extra_cc_compile_flags(
+        name = "%s/cargo_build_script" % name,
+        extra_ar_flags = ["-static"],
+    )
+    cc_args_and_env_analysis_test(
+        name = name,
+        target_under_test = "%s/cargo_build_script" % name,
+        expected_arflags = ["-static"],
+    )
+
+def legacy_cc_toolchain_test(name):
+    cargo_build_script_with_extra_cc_compile_flags(
+        name = "%s/cargo_build_script" % name,
+        legacy_cc_toolchain = True,
+    )
+    cc_args_and_env_analysis_test(
+        name = name,
+        target_under_test = "%s/cargo_build_script" % name,
+        legacy_cc_toolchain = True,
+    )
