don't emit --codegen={metadata,extra-filename} for rust_static_librar…

…y and rust_shared_library (#1222)

Extracted from https://github.com/bazelbuild/rules_rust/pull/1219/files, addressing this part:
* currently we don't include the output hash for cdylib, but we do for staticlib. It's more accurate to not include the output hash for staticlib as well.


Related to, but does not address #1063.

@scentini suggested to not even emit --codegen flags in these cases.

Co-authored-by: Marcel Hlopko <hlopko@google.com>

rust/private/rust.bzl

@@ -245,14 +245,14 @@ def _rust_library_common(ctx, crate_type):
     toolchain = find_toolchain(ctx)
 
     # Determine unique hash for this rlib.
-    # Note that we don't include a hash for `cdylib` since they are meant to be consumed externally and having a
-    # deterministic name is important since it ends up embedded in the executable. This is problematic when one needs
-    # to include the library with a specific filename into a larger application.
+    # Note that we don't include a hash for `cdylib` and `staticlib` since they are meant to be consumed externally
+    # and having a deterministic name is important since it ends up embedded in the executable. This is problematic
+    # when one needs to include the library with a specific filename into a larger application.
     # (see https://github.com/bazelbuild/rules_rust/issues/405#issuecomment-993089889 for more details)
-    if crate_type != "cdylib":
-        output_hash = determine_output_hash(crate_root, ctx.label)
-    else:
+    if crate_type in ["cdylib", "staticlib"]:
         output_hash = None
+    else:
+        output_hash = determine_output_hash(crate_root, ctx.label)
 
     crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
     rust_lib_name = _determine_lib_name(

rust/private/rustc.bzl

@@ -714,12 +714,20 @@ def construct_arguments(
     if hasattr(attr, "_error_format"):
         rustc_flags.add("--error-format=" + attr._error_format[ErrorFormatInfo].error_format)
 
-    # Mangle symbols to disambiguate crates with the same name
-    extra_filename = "-" + output_hash if output_hash else ""
-    rustc_flags.add("--codegen=metadata=" + extra_filename)
+    # 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.
+    #
+    # 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.
+    if output_hash:
+        extra_filename = "-" + output_hash
+        rustc_flags.add("--codegen=metadata=" + extra_filename)
+        rustc_flags.add("--codegen=extra-filename=" + extra_filename)
+
     if output_dir:
         rustc_flags.add("--out-dir=" + output_dir)
-    rustc_flags.add("--codegen=extra-filename=" + extra_filename)
 
     compilation_mode = get_compilation_mode_opts(ctx, toolchain)
     rustc_flags.add("--codegen=opt-level=" + compilation_mode.opt_level)

test/unit/crate_name/crate_name_test.bzl

@@ -1,8 +1,8 @@
 """Unit tests for crate names."""
 
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
-load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
-load("//test/unit:common.bzl", "assert_argv_contains")
+load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_shared_library", "rust_static_library", "rust_test")
+load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_prefix_not")
 
 def _default_crate_name_library_test_impl(ctx):
     env = analysistest.begin(ctx)
@@ -68,9 +68,22 @@ def _slib_library_name_test_impl(ctx):
     """
     env = analysistest.begin(ctx)
     tut = analysistest.target_under_test(env)
+    assert_argv_contains(env, tut.actions[0], "--codegen=metadata=-2102077805")
     assert_argv_contains(env, tut.actions[0], "--codegen=extra-filename=-2102077805")
     return analysistest.end(env)
 
+def _no_extra_filename_test_impl(ctx):
+    """Check that no extra filename is used.
+
+    Args:
+      ctx: rule context.
+    """
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+    assert_argv_contains_prefix_not(env, tut.actions[0], "--codegen=metadata=")
+    assert_argv_contains_prefix_not(env, tut.actions[0], "--codegen=extra-filename=")
+    return analysistest.end(env)
+
 default_crate_name_library_test = analysistest.make(
     _default_crate_name_library_test_impl,
 )
@@ -100,6 +113,9 @@ invalid_custom_crate_name_test = analysistest.make(
 slib_library_name_test = analysistest.make(
     _slib_library_name_test_impl,
 )
+no_extra_filename_test = analysistest.make(
+    _no_extra_filename_test_impl,
+)
 
 def _crate_name_test():
     rust_library(
@@ -153,6 +169,16 @@ def _crate_name_test():
         srcs = ["slib.rs"],
     )
 
+    rust_shared_library(
+        name = "shared_lib",
+        srcs = ["lib.rs"],
+    )
+
+    rust_static_library(
+        name = "static_lib",
+        srcs = ["lib.rs"],
+    )
+
     slib_library_name_test(
         name = "slib_library_name_test",
         target_under_test = ":slib",
@@ -198,6 +224,16 @@ def _crate_name_test():
         target_under_test = ":invalid-custom-crate-name",
     )
 
+    no_extra_filename_test(
+        name = "no_extra_filename_for_shared_library_test",
+        target_under_test = ":shared_lib",
+    )
+
+    no_extra_filename_test(
+        name = "no_extra_filename_for_static_library_test",
+        target_under_test = ":static_lib",
+    )
+
 def crate_name_test_suite(name):
     """Entry-point macro called from the BUILD file.