Make test binary names predictable (#3899)

This PR makes `rust_test` binary names and output paths deterministic by
removing the hash from the test compilation output directory. Instead of
putting test outputs in a hashed subdirectory (e.g.
`my/package/test-<hash>/`), the rules now use a predictable naming
scheme (e.g. a `_test` suffix on the crate name) so test binaries and
their outputs have stable, reproducible paths.

closes #2827

Author: UebelAndre

extensions/wasm_bindgen/private/wasm_bindgen_test.bzl

@@ -76,11 +76,7 @@ def _rust_wasm_bindgen_test_binary_impl(ctx):
 
     output_hash = determine_output_hash(crate.root, ctx.label)
     output = ctx.actions.declare_file(
-        "test-%s/%s%s" % (
-            output_hash,
-            ctx.label.name,
-            toolchain.binary_ext,
-        ),
+        ctx.label.name + toolchain.binary_ext,
     )
 
     srcs = crate.srcs.to_list()
@@ -136,6 +132,7 @@ def _rust_wasm_bindgen_test_binary_impl(ctx):
         attr = ctx.attr,
         toolchain = toolchain,
         crate_info_dict = crate_info_dict,
+        output_hash = output_hash,
         rust_flags = get_rust_test_flags(ctx.attr),
         skip_expanding_rustc_env = True,
     )

rust/private/rust.bzl

@@ -371,21 +371,11 @@ def _rust_test_impl(ctx):
         # Target is building the crate in `test` config
         crate = ctx.attr.crate[rust_common.crate_info] if rust_common.crate_info in ctx.attr.crate else ctx.attr.crate[rust_common.test_crate_info].crate
 
-        if toolchain._incompatible_change_rust_test_compilation_output_directory:
-            crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
-            output = ctx.actions.declare_file(
-                ctx.label.name + toolchain.binary_ext,
-            )
-        else:
-            crate_name = crate.name
-            output_hash = determine_output_hash(crate.root, ctx.label)
-            output = ctx.actions.declare_file(
-                "test-%s/%s%s" % (
-                    output_hash,
-                    ctx.label.name,
-                    toolchain.binary_ext,
-                ),
-            )
+        crate_name = crate.name
+        output_hash = determine_output_hash(crate.root, ctx.label)
+        output = ctx.actions.declare_file(
+            ctx.label.name + toolchain.binary_ext,
+        )
 
         rust_metadata = None
         rustc_rmeta_output = None
@@ -452,19 +442,10 @@ def _rust_test_impl(ctx):
             crate_root = crate_root_src(ctx.attr.name, ctx.attr.crate_name, ctx.files.srcs, crate_root_type)
         srcs, compile_data, crate_root = transform_sources(ctx, ctx.files.srcs, ctx.files.compile_data, crate_root)
 
-        if toolchain._incompatible_change_rust_test_compilation_output_directory:
-            output = ctx.actions.declare_file(
-                ctx.label.name + toolchain.binary_ext,
-            )
-        else:
-            output_hash = determine_output_hash(crate_root, ctx.label)
-            output = ctx.actions.declare_file(
-                "test-%s/%s%s" % (
-                    output_hash,
-                    ctx.label.name,
-                    toolchain.binary_ext,
-                ),
-            )
+        output_hash = determine_output_hash(crate_root, ctx.label)
+        output = ctx.actions.declare_file(
+            ctx.label.name + toolchain.binary_ext,
+        )
 
         rust_metadata = None
         rustc_rmeta_output = None
@@ -513,6 +494,7 @@ def _rust_test_impl(ctx):
         attr = ctx.attr,
         toolchain = toolchain,
         crate_info_dict = crate_info_dict,
+        output_hash = output_hash,
         rust_flags = get_rust_test_flags(ctx.attr),
         skip_expanding_rustc_env = True,
     )

rust/private/rustc.bzl

@@ -1053,13 +1053,13 @@ def construct_arguments(
 
     rustc_flags.add(error_format, format = "--error-format=%s")
 
-    # Mangle symbols to disambiguate crates with the same name. This could
-    # happen only for non-final artifacts where we compute an output_hash,
-    # e.g., rust_library.
+    # Mangle symbols to disambiguate crates with the same name. Used for
+    # rust_library (multiple versions of a crate) and rust_test (shares a
+    # crate name with the underlying binary/library it tests).
     #
-    # For "final" artifacts and ones intended for distribution outside of
-    # Bazel, such as rust_binary, rust_static_library and rust_shared_library,
-    # where output_hash is None we don't need to add these flags.
+    # For final artifacts intended for distribution outside of Bazel, such as
+    # rust_binary, rust_static_library and rust_shared_library, output_hash
+    # is None and these flags are not added.
     if output_hash:
         rustc_flags.add(output_hash, format = "--codegen=metadata=-%s")
         rustc_flags.add(output_hash, format = "--codegen=extra-filename=-%s")
@@ -1468,10 +1468,13 @@ def rustc_compile_action(
     outputs = [crate_info.output]
 
     # The `.o` output file, only used for linking via cc_common.link.
+    # When output_hash is set (e.g. for rust_test targets), include it in the
+    # filename to avoid collisions with other targets sharing the same crate name.
     output_o = None
     if experimental_use_cc_common_link:
         obj_ext = ".o"
-        output_o = ctx.actions.declare_file(crate_info.name + obj_ext, sibling = crate_info.output)
+        obj_basename = crate_info.name + ("-%s" % output_hash if output_hash else "")
+        output_o = ctx.actions.declare_file(obj_basename + obj_ext, sibling = crate_info.output)
         outputs = [output_o]
 
     # For a cdylib that might be added as a dependency to a cc_* target on Windows, it is important to include the

rust/settings/BUILD.bazel

@@ -26,7 +26,6 @@ load(
     "extra_rustc_flag",
     "extra_rustc_flags",
     "incompatible_change_clippy_error_format",
-    "incompatible_change_rust_test_compilation_output_directory",
     "incompatible_do_not_include_data_in_compile_data",
     "lto",
     "no_std",
@@ -110,8 +109,6 @@ extra_rustc_flags()
 
 incompatible_change_clippy_error_format()
 
-incompatible_change_rust_test_compilation_output_directory()
-
 incompatible_do_not_include_data_in_compile_data()
 
 lto()

rust/settings/settings.bzl

@@ -258,16 +258,6 @@ def toolchain_generated_sysroot():
         visibility = ["//visibility:public"],
     )
 
-# buildifier: disable=unnamed-macro
-def incompatible_change_rust_test_compilation_output_directory():
-    """A flag to put rust_test compilation outputs in the same directory as the rust_library compilation outputs.
-    """
-    incompatible_flag(
-        name = "incompatible_change_rust_test_compilation_output_directory",
-        build_setting_default = False,
-        issue = "https://github.com/bazelbuild/rules_rust/issues/2827",
-    )
-
 def experimental_link_std_dylib():
     """A flag to control whether to link libstd dynamically."""
     bool_flag(

rust/toolchain.bzl

@@ -621,7 +621,6 @@ def _rust_toolchain_impl(ctx):
         _experimental_use_cc_common_link = _experimental_use_cc_common_link(ctx),
         _experimental_use_global_allocator = experimental_use_global_allocator,
         _experimental_use_coverage_metadata_files = ctx.attr._experimental_use_coverage_metadata_files[BuildSettingInfo].value,
-        _incompatible_change_rust_test_compilation_output_directory = ctx.attr._incompatible_change_rust_test_compilation_output_directory[IncompatibleFlagInfo].enabled,
         _toolchain_generated_sysroot = ctx.attr._toolchain_generated_sysroot[BuildSettingInfo].value,
         _incompatible_do_not_include_data_in_compile_data = ctx.attr._incompatible_do_not_include_data_in_compile_data[IncompatibleFlagInfo].enabled,
         _no_std = no_std,
@@ -863,9 +862,6 @@ rust_toolchain = rule(
                 "This flag is only relevant when used together with --@rules_rust//rust/settings:experimental_use_global_allocator."
             ),
         ),
-        "_incompatible_change_rust_test_compilation_output_directory": attr.label(
-            default = Label("//rust/settings:incompatible_change_rust_test_compilation_output_directory"),
-        ),
         "_incompatible_do_not_include_data_in_compile_data": attr.label(
             default = Label("//rust/settings:incompatible_do_not_include_data_in_compile_data"),
             doc = "Label to a boolean build setting that controls whether to include data files in compile_data.",

test/unit/linkstamps/linkstamps_test.bzl

@@ -3,7 +3,7 @@
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
 load("@rules_cc//cc:defs.bzl", "cc_library")
 load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
-load("//rust:defs.bzl", "rust_binary", "rust_common", "rust_library", "rust_test")
+load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
 load("//test/unit:common.bzl", "assert_action_mnemonic")
 
 def _is_running_on_linux(ctx):
@@ -25,12 +25,9 @@ def _supports_linkstamps_test(ctx):
     linkstamp_out = linkstamp_action.outputs.to_list()[0]
     asserts.equals(env, linkstamp_out.basename, "linkstamp.o")
     tut_out = tut.files.to_list()[0]
-    is_test = tut[rust_common.crate_info].is_test
     workspace_prefix = _get_workspace_prefix(ctx)
 
-    # Rust compilation outputs coming from a test are put in test-{hash} directory
-    # which we need to remove in order to obtain the linkstamp file path.
-    dirname = "/".join(tut_out.dirname.split("/")[:-1]) if is_test else tut_out.dirname
+    dirname = tut_out.dirname
     expected_linkstamp_path = dirname + "/_objs/" + tut_out.basename + workspace_prefix + "/test/unit/linkstamps/linkstamp.o"
     asserts.equals(
         env,

test/unit/rust_test_codegen_disambiguation/BUILD.bazel

@@ -0,0 +1,4 @@
+load(":codegen_disambiguation_test.bzl", "codegen_disambiguation_test_suite")
+
+############################ UNIT TESTS #############################
+codegen_disambiguation_test_suite(name = "codegen_disambiguation_test_suite")

test/unit/rust_test_codegen_disambiguation/codegen_disambiguation_test.bzl

@@ -0,0 +1,92 @@
+"""Tests that rust_test targets receive codegen disambiguation flags.
+
+rust_test targets pass --codegen=metadata and --codegen=extra-filename to rustc
+so that intermediate compilation artifacts (.o, .d files) get unique names. This
+prevents collisions with rust_binary or rust_library targets that share the same
+crate name, which would otherwise cause link failures on non-sandboxed builds
+(e.g. Windows or --spawn_strategy=standalone). See
+https://github.com/bazelbuild/rules_rust/pull/1434 for the original issue.
+
+rustc flag documentation:
+  https://doc.rust-lang.org/rustc/codegen-options/index.html#metadata
+  https://doc.rust-lang.org/rustc/codegen-options/index.html#extra-filename
+"""
+
+load("@bazel_skylib//lib:unittest.bzl", "analysistest")
+load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
+load("//test/unit:common.bzl", "assert_argv_contains_prefix")
+
+def _codegen_disambiguation_test_impl(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    assert_argv_contains_prefix(env, tut.actions[0], "--codegen=metadata=-")
+    assert_argv_contains_prefix(env, tut.actions[0], "--codegen=extra-filename=-")
+    return analysistest.end(env)
+
+codegen_disambiguation_test = analysistest.make(
+    _codegen_disambiguation_test_impl,
+)
+
+def _codegen_disambiguation_targets():
+    rust_binary(
+        name = "my_binary",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    rust_library(
+        name = "my_library",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    rust_test(
+        name = "my_test_with_srcs",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    codegen_disambiguation_test(
+        name = "rust_test_srcs_codegen_disambiguation_test",
+        target_under_test = ":my_test_with_srcs",
+    )
+
+    rust_test(
+        name = "my_test_with_crate_from_bin",
+        crate = "my_binary",
+        edition = "2018",
+    )
+
+    codegen_disambiguation_test(
+        name = "rust_test_crate_from_bin_codegen_disambiguation_test",
+        target_under_test = ":my_test_with_crate_from_bin",
+    )
+
+    rust_test(
+        name = "my_test_with_crate_from_lib",
+        crate = "my_library",
+        edition = "2018",
+    )
+
+    codegen_disambiguation_test(
+        name = "rust_test_crate_from_lib_codegen_disambiguation_test",
+        target_under_test = ":my_test_with_crate_from_lib",
+    )
+
+def codegen_disambiguation_test_suite(name):
+    """Entry-point macro called from the BUILD file.
+
+    Args:
+        name: Name of the macro.
+    """
+
+    _codegen_disambiguation_targets()
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":rust_test_srcs_codegen_disambiguation_test",
+            ":rust_test_crate_from_bin_codegen_disambiguation_test",
+            ":rust_test_crate_from_lib_codegen_disambiguation_test",
+        ],
+    )