Build rust_test targets using a crate name different from the underlying lib (#2828)

This is a rollforward of
#2803, but behind the
`incompatible_change_rust_test_compilation_output_directory`
incompatible flag.

This PR also makes `rust_test` put its compilation outputs in the same
directory as the `rust_library` rule (i.e. not in a `test-{hash}`
subdirectory anymore).

After this change both the `rust_library` and `rust_test` rules will put
all its compilation outputs in the same directory, but there won't be
any name collisions in non-sandboxed environments (see
#1427 for more context).

Issue with context: #2827

Author: felipeamp

docs/src/flatten.md

@@ -1050,9 +1050,7 @@ rust_test(
 )
 ```
 
-Run the test with `bazel test //hello_lib:hello_lib_test`. The crate
-will be built using the same crate name as the underlying ":hello_lib"
-crate.
+Run the test with `bazel test //hello_lib:hello_lib_test`.
 
 ### Example: `test` directory
 

docs/src/rust.md

@@ -567,9 +567,7 @@ rust_test(
 )
 ```
 
-Run the test with `bazel test //hello_lib:hello_lib_test`. The crate
-will be built using the same crate name as the underlying ":hello_lib"
-crate.
+Run the test with `bazel test //hello_lib:hello_lib_test`.
 
 ### Example: `test` directory
 

rust/private/rust.bzl

@@ -309,14 +309,21 @@ 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
 
-        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,
-            ),
-        )
+        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,
+                ),
+            )
 
         srcs, crate_root = transform_sources(ctx, ctx.files.srcs, getattr(ctx.file, "crate_root", None))
 
@@ -342,7 +349,7 @@ def _rust_test_impl(ctx):
 
         # Build the test binary using the dependency's srcs.
         crate_info_dict = dict(
-            name = crate.name,
+            name = crate_name,
             type = crate_type,
             root = crate.root,
             srcs = depset(srcs, transitive = [crate.srcs]),
@@ -368,14 +375,19 @@ def _rust_test_impl(ctx):
             crate_root = crate_root_src(ctx.attr.name, ctx.attr.crate_name, ctx.files.srcs, crate_root_type)
         srcs, crate_root = transform_sources(ctx, ctx.files.srcs, crate_root)
 
-        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,
-            ),
-        )
+        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,
+                ),
+            )
 
         data_paths = depset(direct = getattr(ctx.attr, "data", [])).to_list()
         rustc_env = expand_dict_value_locations(
@@ -1348,9 +1360,7 @@ rust_test = rule(
         )
         ```
 
-        Run the test with `bazel test //hello_lib:hello_lib_test`. The crate
-        will be built using the same crate name as the underlying ":hello_lib"
-        crate.
+        Run the test with `bazel test //hello_lib:hello_lib_test`.
 
         ### Example: `test` directory
 

rust/settings/BUILD.bazel

@@ -1,6 +1,7 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag")
 load("//rust/private:unpretty.bzl", "rust_unpretty_flag")
+load(":incompatible.bzl", "incompatible_flag")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -90,6 +91,13 @@ bool_flag(
     build_setting_default = True,
 )
 
+# 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",
+)
+
 # A flag to control whether to link libstd dynamically.
 bool_flag(
     name = "experimental_link_std_dylib",

rust/toolchain.bzl

@@ -18,6 +18,7 @@ load(
     "is_std_dylib",
     "make_static_lib_symlink",
 )
+load("//rust/settings:incompatible.bzl", "IncompatibleFlagInfo")
 
 rust_analyzer_toolchain = _rust_analyzer_toolchain
 rustfmt_toolchain = _rustfmt_toolchain
@@ -696,6 +697,7 @@ 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,
         _no_std = no_std,
     )
@@ -881,6 +883,9 @@ 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"),
+        ),
         "_no_std": attr.label(
             default = Label("//:no_std"),
         ),

test/unit/rust_test_outputs_are_in_same_directory/BUILD.bazel

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

test/unit/rust_test_outputs_are_in_same_directory/foo.rs

@@ -0,0 +1 @@
+pub fn main() {}

test/unit/rust_test_outputs_are_in_same_directory/rust_test_outputs.bzl

@@ -0,0 +1,92 @@
+"""Tests for rust_test outputs directory."""
+
+load("@bazel_skylib//lib:paths.bzl", "paths")
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("//rust:defs.bzl", "rust_binary", "rust_common", "rust_library", "rust_test")
+
+def _rust_test_outputs_test(ctx):
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+
+    output = tut[rust_common.crate_info].output
+
+    # Check compilation output is in directory with same name as package
+    test_target_label = ctx.attr.target_under_test[0].label
+    asserts.true(env, output.dirname.split("/")[-1] == test_target_label.package.split("/")[-1])
+
+    # Check compilation output has same name as crate name, ignoring possible binary extension
+    output_filename_without_ext = paths.split_extension(output.basename)[0]
+    asserts.true(env, output_filename_without_ext == tut[rust_common.crate_info].name)
+
+    return analysistest.end(env)
+
+rust_test_outputs_test = analysistest.make(
+    _rust_test_outputs_test,
+    config_settings = {
+        str(Label("//rust/settings:incompatible_change_rust_test_compilation_output_directory")): True,
+    },
+)
+
+def _rust_test_outputs_targets():
+    rust_binary(
+        name = "bin_outputs",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    rust_library(
+        name = "lib_outputs",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    rust_test(
+        name = "test_outputs_with_srcs",
+        srcs = ["foo.rs"],
+        edition = "2018",
+    )
+
+    rust_test_outputs_test(
+        name = "rust_test_outputs_using_srcs_attr",
+        target_under_test = ":test_outputs_with_srcs",
+    )
+
+    rust_test(
+        name = "test_outputs_with_crate_from_bin",
+        crate = "bin_outputs",
+        edition = "2018",
+    )
+
+    rust_test_outputs_test(
+        name = "rust_test_outputs_using_crate_attr_from_bin",
+        target_under_test = ":test_outputs_with_crate_from_bin",
+    )
+
+    rust_test(
+        name = "test_outputs_with_crate_from_lib",
+        crate = "lib_outputs",
+        edition = "2018",
+    )
+
+    rust_test_outputs_test(
+        name = "rust_test_outputs_using_crate_attr_from_lib",
+        target_under_test = ":test_outputs_with_crate_from_lib",
+    )
+
+def rust_test_outputs_test_suite(name):
+    """Entry-point macro called from the BUILD file.
+
+    Args:
+        name: Name of the macro.
+    """
+
+    _rust_test_outputs_targets()
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":rust_test_outputs_using_srcs_attr",
+            ":rust_test_outputs_using_crate_attr_from_bin",
+            ":rust_test_outputs_using_crate_attr_from_lib",
+        ],
+    )