From 2b392d40613354aef3a7c462707e1a905462b953 Mon Sep 17 00:00:00 2001 From: Randolf Jung Date: Thu, 24 Sep 2026 01:38:07 -0700 Subject: [PATCH 1/2] rust: avoid duplicate native flags from direct build scripts --- .../duplicate_link_flags/BUILD.bazel | 3 + .../duplicate_link_flags/build.rs | 8 ++ .../duplicate_link_flags_test.bzl | 83 +++++++++++++++++++ .../duplicate_link_flags/lib.rs | 3 + .../duplicate_link_flags/main.rs | 1 + rust/private/rustc.bzl | 13 ++- 6 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 cargo/tests/cargo_build_script/duplicate_link_flags/BUILD.bazel create mode 100644 cargo/tests/cargo_build_script/duplicate_link_flags/build.rs create mode 100644 cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl create mode 100644 cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs create mode 100644 cargo/tests/cargo_build_script/duplicate_link_flags/main.rs diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/BUILD.bazel b/cargo/tests/cargo_build_script/duplicate_link_flags/BUILD.bazel new file mode 100644 index 0000000000..848d2704f2 --- /dev/null +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/BUILD.bazel @@ -0,0 +1,3 @@ +load(":duplicate_link_flags_test.bzl", "duplicate_link_flags_test_suite") + +duplicate_link_flags_test_suite(name = "duplicate_link_flags_test_suite") diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs new file mode 100644 index 0000000000..167c30ee3e --- /dev/null +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs @@ -0,0 +1,8 @@ +fn main() { + let library = if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") { + "kernel32" + } else { + "c" + }; + println!("cargo:rustc-link-lib=dylib={library}"); +} diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl b/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl new file mode 100644 index 0000000000..1b114e3e05 --- /dev/null +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl @@ -0,0 +1,83 @@ +"""Check when a package build script's native link flags reach a Rustc action.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("//cargo:defs.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_binary", "rust_library") + +def _link_flags_test_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + rustc_actions = [action for action in target.actions if action.mnemonic == "Rustc"] + asserts.equals(env, 1, len(rustc_actions)) + + if rustc_actions: + argv = rustc_actions[0].argv + link_flags = [ + argv[i + 1] + for i in range(len(argv) - 1) + if argv[i] == "--arg-file" and argv[i + 1].endswith("shared_script.linkflags") + ] + asserts.equals(env, ctx.attr.expected_count, len(link_flags)) + + return analysistest.end(env) + +_link_flags_test = analysistest.make( + _link_flags_test_impl, + attrs = {"expected_count": attr.int(mandatory = True)}, +) + +def duplicate_link_flags_test_suite(name): + """Verify a direct library owns its package build script's native flags. + + Args: + name: Name of the test suite. + """ + cargo_build_script( + name = "shared_script", + srcs = ["build.rs"], + ) + + rust_library( + name = "lib", + srcs = ["lib.rs"], + deps = [":shared_script"], + ) + + rust_binary( + name = "bin_with_lib", + srcs = ["main.rs"], + deps = [":lib", ":shared_script"], + ) + + rust_binary( + name = "bin_without_lib", + srcs = ["main.rs"], + deps = [":shared_script"], + ) + + _link_flags_test( + name = "lib_link_flags_test", + target_under_test = ":lib", + expected_count = 1, + ) + + _link_flags_test( + name = "bin_with_lib_link_flags_test", + target_under_test = ":bin_with_lib", + expected_count = 0, + ) + + _link_flags_test( + name = "bin_without_lib_link_flags_test", + target_under_test = ":bin_without_lib", + expected_count = 1, + ) + + native.test_suite( + name = name, + tests = [ + ":lib_link_flags_test", + ":bin_with_lib_link_flags_test", + ":bin_without_lib_link_flags_test", + ], + ) diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs new file mode 100644 index 0000000000..094e370a02 --- /dev/null +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs @@ -0,0 +1,3 @@ +pub fn value() -> i32 { + 1 +} diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs new file mode 100644 index 0000000000..f328e4d9d0 --- /dev/null +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index d3d9d1e981..6acfcd8149 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -2662,7 +2662,7 @@ def _process_build_scripts( build_env_file = build_info.rustc_env if build_info.flags: build_flags_files.append(build_info.flags) - if build_info.linker_flags and include_link_flags: + if build_info.linker_flags and include_link_flags and not _build_script_linked_by_library(build_info, dep_info): build_flags_files.append(build_info.linker_flags) direct_inputs.append(build_info.linker_flags) @@ -2699,6 +2699,17 @@ def _process_build_scripts( depset(build_flags_files, transitive = [dep_info.link_search_path_files]), ) +def _build_script_linked_by_library(build_info, dep_info): + # Cargo applies rustc-link-lib to the package library when one exists. + # Binaries that depend on that library already link its native archives. + for crate in dep_info.direct_crates.to_list(): + if crate.dep.type not in ("lib", "rlib", "dylib") or crate.dep.is_test: + continue + for dep in crate.dep.deps.to_list(): + if dep.build_info == build_info and dep.build_info.linker_flags == build_info.linker_flags: + return True + return False + def _compute_rpaths(toolchain, output_dir, dep_info, use_pic, link_std_dylib, output_file = None, workspace_name = ""): """Determine the artifact's rpaths relative to the bazel root for runtime linking of shared libraries. From 0f201ccc4120ea145c2fb55966a178a0cb1cfce2 Mon Sep 17 00:00:00 2001 From: Randolf Jung Date: Mon, 28 Sep 2026 11:51:48 -0700 Subject: [PATCH 2/2] rust: track build-script link ownership during dependency collection --- .../duplicate_link_flags/build.rs | 9 +++ .../duplicate_link_flags_test.bzl | 59 ++++++++++++++++++- .../duplicate_link_flags/lib.rs | 4 +- .../duplicate_link_flags/main.rs | 9 ++- rust/private/providers.bzl | 2 + rust/private/rustc.bzl | 33 ++++++----- 6 files changed, 98 insertions(+), 18 deletions(-) diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs index 167c30ee3e..c32e4f5c9f 100644 --- a/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/build.rs @@ -1,4 +1,13 @@ fn main() { + let out_dir = std::path::PathBuf::from(std::env::var_os("OUT_DIR").expect("missing OUT_DIR")); + std::fs::write( + out_dir.join("generated.rs"), + "const GENERATED_VALUE: &str = \"from_build_script\";\n", + ) + .expect("could not write the generated fixture"); + println!("cargo:rustc-cfg=build_script_cfg"); + println!("cargo:rustc-env=BUILD_SCRIPT_VALUE=from_build_script"); + let library = if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") { "kernel32" } else { diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl b/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl index 1b114e3e05..9dcf615099 100644 --- a/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/duplicate_link_flags_test.bzl @@ -1,4 +1,4 @@ -"""Check when a package build script's native link flags reach a Rustc action.""" +"""Regression coverage for https://github.com/bazelbuild/rules_rust/issues/4291.""" load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//cargo:defs.bzl", "cargo_build_script") @@ -19,6 +19,20 @@ def _link_flags_test_impl(ctx): ] asserts.equals(env, ctx.attr.expected_count, len(link_flags)) + # Both the library and binary still need the script's non-link outputs. + for flag, suffix in [ + ("--arg-file", "shared_script.flags"), + ("--arg-file", "shared_script.linksearchpaths"), + ("--env-file", "shared_script.env"), + ("--out-dir", "shared_script.out_dir"), + ]: + matches = [ + argv[i + 1] + for i in range(len(argv) - 1) + if argv[i] == flag and argv[i + 1].endswith(suffix) + ] + asserts.equals(env, 1, len(matches), "missing build-script input: " + suffix) + return analysistest.end(env) _link_flags_test = analysistest.make( @@ -55,6 +69,35 @@ def duplicate_link_flags_test_suite(name): deps = [":shared_script"], ) + rust_library( + name = "intermediate_lib", + srcs = ["lib.rs"], + deps = [":lib", ":shared_script"], + ) + + rust_binary( + name = "bin_with_intermediate_lib", + srcs = ["main.rs"], + deps = [":intermediate_lib", ":shared_script"], + ) + + cargo_build_script( + name = "other_script", + srcs = ["build.rs"], + ) + + rust_library( + name = "unrelated_lib", + srcs = ["lib.rs"], + deps = [":other_script"], + ) + + rust_binary( + name = "bin_with_unrelated_lib", + srcs = ["main.rs"], + deps = [":shared_script", ":unrelated_lib"], + ) + _link_flags_test( name = "lib_link_flags_test", target_under_test = ":lib", @@ -73,11 +116,25 @@ def duplicate_link_flags_test_suite(name): expected_count = 1, ) + _link_flags_test( + name = "bin_with_intermediate_lib_link_flags_test", + target_under_test = ":bin_with_intermediate_lib", + expected_count = 0, + ) + + _link_flags_test( + name = "bin_with_unrelated_lib_link_flags_test", + target_under_test = ":bin_with_unrelated_lib", + expected_count = 1, + ) + native.test_suite( name = name, tests = [ ":lib_link_flags_test", ":bin_with_lib_link_flags_test", ":bin_without_lib_link_flags_test", + ":bin_with_intermediate_lib_link_flags_test", + ":bin_with_unrelated_lib_link_flags_test", ], ) diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs index 094e370a02..78d8c97312 100644 --- a/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/lib.rs @@ -1,3 +1,3 @@ -pub fn value() -> i32 { - 1 +pub fn value() -> &'static str { + env!("BUILD_SCRIPT_VALUE") } diff --git a/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs b/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs index f328e4d9d0..13c5d92f38 100644 --- a/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs +++ b/cargo/tests/cargo_build_script/duplicate_link_flags/main.rs @@ -1 +1,8 @@ -fn main() {} +#[cfg(not(build_script_cfg))] +compile_error!("the binary must receive its build script's cfg flags"); + +include!(concat!(env!("OUT_DIR"), "/generated.rs")); + +fn main() { + assert_eq!(env!("BUILD_SCRIPT_VALUE"), GENERATED_VALUE); +} diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index 0aea247efb..612ab31b4b 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -58,7 +58,9 @@ CrateInfo = provider( DepInfo = provider( doc = "A provider containing information about a Crate's dependencies.", fields = { + "build_script_linker_flags": "File, optional: Direct build-script native link flags not already supplied by a direct Rust library.", "dep_env": "File: File with environment variables direct dependencies build scripts rely upon.", + "direct_build_info": "BuildInfo, optional: The build script directly attached to this crate, before deduplicating its native link flags.", "direct_crates": "depset[AliasableDepInfo]", "link_search_path_files": "depset[File]: All transitive files containing search paths to pass to the linker", "transitive_build_infos": "depset[BuildInfo]", diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 6acfcd8149..3662d2a24d 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -208,6 +208,7 @@ def collect_deps( direct_build_infos = [] transitive_build_infos = [] + library_build_infos = [] direct_link_search_paths = [] transitive_link_search_paths = [] @@ -252,6 +253,11 @@ def collect_deps( is_proc_macro = _is_proc_macro(crate_info) + if crate_info.type in ("lib", "rlib", "dylib") and not crate_info.is_test: + library_build_info = getattr(dep_info, "direct_build_info", None) + if library_build_info: + library_build_infos.append(library_build_info) + direct_crates.append(crate_info) if not is_proc_macro: transitive_crates.append(dep_info.transitive_crates) @@ -314,8 +320,17 @@ def collect_deps( fail("rust targets can only depend on rust_library, rust_*_library or cc_library " + "targets.") + # Cargo sends rustc-link-lib only to the package library when one exists. + # Keep the original BuildInfo so dependents can compare the same script, + # and retain its cfg, environment, OUT_DIR and search paths for this crate. + build_script_linker_flags = None + if build_info and build_info not in library_build_infos: + build_script_linker_flags = build_info.linker_flags + return ( rust_common.dep_info( + direct_build_info = build_info, + build_script_linker_flags = build_script_linker_flags, direct_crates = depset( direct_deps, transitive = [extra_named_deps] if extra_named_deps else [], @@ -2662,9 +2677,10 @@ def _process_build_scripts( build_env_file = build_info.rustc_env if build_info.flags: build_flags_files.append(build_info.flags) - if build_info.linker_flags and include_link_flags and not _build_script_linked_by_library(build_info, dep_info): - build_flags_files.append(build_info.linker_flags) - direct_inputs.append(build_info.linker_flags) + linker_flags = getattr(dep_info, "build_script_linker_flags", build_info.linker_flags) + if linker_flags and include_link_flags: + build_flags_files.append(linker_flags) + direct_inputs.append(linker_flags) # `cargo::rustc-link-arg-bins` applies only to binary targets, and (like # cargo) only from the crate's own build script — not transitively. @@ -2699,17 +2715,6 @@ def _process_build_scripts( depset(build_flags_files, transitive = [dep_info.link_search_path_files]), ) -def _build_script_linked_by_library(build_info, dep_info): - # Cargo applies rustc-link-lib to the package library when one exists. - # Binaries that depend on that library already link its native archives. - for crate in dep_info.direct_crates.to_list(): - if crate.dep.type not in ("lib", "rlib", "dylib") or crate.dep.is_test: - continue - for dep in crate.dep.deps.to_list(): - if dep.build_info == build_info and dep.build_info.linker_flags == build_info.linker_flags: - return True - return False - def _compute_rpaths(toolchain, output_dir, dep_info, use_pic, link_std_dylib, output_file = None, workspace_name = ""): """Determine the artifact's rpaths relative to the bazel root for runtime linking of shared libraries.