Do not forward ARFLAGS

This is a partial revert of #3704. It does NOT revert logic that
properly finds the hermetic archiver (binary and associated tests).

cc-rs needs full ownership of flag handling for the archiver. This is in
part due to `ar` having very specific positional flag behaviors, and
partially due to some special logic in how cc-rs constructs static libraries.

The change to enable ARFLAGS was causing many toolchains that pass flags
to the static archiver to error out due to conflicting expectations for
flag handling. Example bad `ar` invocation, where `cq` is treated as the
destination archive instead of the `ar` tool mode:

  ar rcsD cq dest.a src1.o src2.o

This drops forwarding of `ARFLAGS` to fix the common case.

Author: armandomontanez

cargo/private/cargo_build_script.bzl

@@ -131,7 +131,6 @@ def get_cc_compile_args_and_env(cc_toolchain, feature_configuration):
         tuple: A tuple of the following items:
             - (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(
@@ -148,17 +147,12 @@ 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_ar_args, cc_env
+    return cc_c_args, cc_cxx_args, cc_env
 
 def _pwd_flags_sysroot(args):
     """Prefix execroot-relative paths of known arguments with ${pwd}.
@@ -429,12 +423,13 @@ def _cargo_build_script_impl(ctx):
     env["CC"] = "${{pwd}}/{}".format(ctx.executable._fallback_cc.path)
     env["CXX"] = "${{pwd}}/{}".format(ctx.executable._fallback_cxx.path)
     env["AR"] = "${{pwd}}/{}".format(ctx.executable._fallback_ar.path)
+    env["ARFLAGS"] = ""
     env["CFLAGS"] = ""
     env["CXXFLAGS"] = ""
 
     if cc_toolchain:
         # MSVC requires INCLUDE to be set
-        cc_c_args, cc_cxx_args, cc_ar_args, cc_env = get_cc_compile_args_and_env(cc_toolchain, feature_configuration)
+        cc_c_args, cc_cxx_args, cc_env = get_cc_compile_args_and_env(cc_toolchain, feature_configuration)
         include = cc_env.get("INCLUDE")
         if include:
             env["INCLUDE"] = include
@@ -464,7 +459,9 @@ def _cargo_build_script_impl(ctx):
         # 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))
+        # It may be tempting to forward ARFLAGS, but cc-rs is opinionated enough
+        # that doing so is more likely to hurt than help. If you need to change
+        # ARFLAGS, make changes to cc-rs.
 
     # Inform build scripts of rustc flags
     # https://github.com/rust-lang/cargo/issues/9600

test/cargo_build_script/cc_args_and_env/BUILD.bazel

@@ -1,6 +1,5 @@
 load(
     "cc_args_and_env_test.bzl",
-    "ar_flags_test",
     "bindir_absolute_test",
     "bindir_relative_test",
     "fsanitize_ignorelist_absolute_test",
@@ -31,6 +30,4 @@ 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")

test/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl

@@ -207,9 +207,12 @@ def _cc_args_and_env_analysis_test_impl(ctx):
         )
 
     _ENV_VAR_TO_EXPECTED_ARGS = {
-        "ARFLAGS": ctx.attr.expected_arflags,
         "CFLAGS": ctx.attr.expected_cflags,
         "CXXFLAGS": ctx.attr.expected_cxxflags,
+        # ARFLAGS is intentionally always empty. cc-rs tightly controls the
+        # archiver flags in such a way that forwarding standard flags as
+        # set up by most Bazel C/C++ toolchains is extremely error-prone.
+        "ARFLAGS": [],
     }
 
     for env_var, expected_flags in _ENV_VAR_TO_EXPECTED_ARGS.items():
@@ -227,7 +230,6 @@ 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_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),
@@ -394,17 +396,6 @@ def fsanitize_ignorelist_absolute_test(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,