From c94fa17fa5167c8cd19743131e2e0d6791e1dde9 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Fri, 1 Aug 2025 16:03:43 +0100 Subject: [PATCH 1/9] Include transitive out directories when running rustc Previously only the out directories of build scripts that are direct dependencies were included when running rustc. This caused errors when linking when a transitive dependency's build script builds an archive into the out directory and adds a link argument to it via `cargo::rustc-link-lib`. Now all out directories of transitive dependencies are collected into a depset and included as dependencies in the rustc compile action. --- rust/private/providers.bzl | 1 + rust/private/rustc.bzl | 7 +++++- test/linking_out_dir/BUILD.bazel | 43 ++++++++++++++++++++++++++++++++ test/linking_out_dir/bar.rs | 7 ++++++ test/linking_out_dir/build.rs | 15 +++++++++++ test/linking_out_dir/foo.c | 3 +++ test/linking_out_dir/main.rs | 5 ++++ 7 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 test/linking_out_dir/BUILD.bazel create mode 100644 test/linking_out_dir/bar.rs create mode 100644 test/linking_out_dir/build.rs create mode 100644 test/linking_out_dir/foo.c create mode 100644 test/linking_out_dir/main.rs diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index 5c47bf14cb..f17d63c1cf 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -54,6 +54,7 @@ DepInfo = provider( "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]", + "transitive_out_dirs": "depset[File]: The out directories of the build scripts of all transitive dependencies", "transitive_crate_outputs": "depset[File]: All transitive crate outputs.", "transitive_crates": "depset[CrateInfo]", "transitive_data": "depset[File]: Data of all transitive non-macro dependencies.", diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 3293cd9f12..aa7344d77c 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -226,6 +226,7 @@ def collect_deps( transitive_proc_macro_data = [] transitive_noncrates = [] transitive_build_infos = [] + transitive_out_dirs = [] transitive_link_search_paths = [] build_info = None linkstamps = [] @@ -321,6 +322,7 @@ def collect_deps( transitive_link_search_paths.append(dep_info.link_search_path_files) transitive_build_infos.append(dep_info.transitive_build_infos) + transitive_out_dirs.append(dep_info.transitive_out_dirs) elif cc_info or dep_build_info: if cc_info: # This dependency is a cc_library @@ -332,6 +334,8 @@ def collect_deps( "only one is allowed in the dependencies") build_info = dep_build_info transitive_build_infos.append(depset([build_info])) + if build_info.out_dir: + transitive_out_dirs.append(depset([build_info.out_dir])) if build_info.link_search_paths: transitive_link_search_paths.append(depset([build_info.link_search_paths])) transitive_data.append(build_info.compile_data) @@ -356,6 +360,7 @@ def collect_deps( transitive_crate_outputs = depset(transitive = transitive_crate_outputs), transitive_metadata_outputs = depset(transitive = transitive_metadata_outputs), transitive_build_infos = depset(transitive = transitive_build_infos), + transitive_out_dirs = depset(transitive = transitive_out_dirs), link_search_path_files = depset(transitive = transitive_link_search_paths), dep_env = build_info.dep_env if build_info else None, ), @@ -1872,7 +1877,7 @@ def _create_extra_input_args(build_info, dep_info, include_link_flags = True): out_dir_compile_inputs = depset( input_files, - transitive = [dep_info.link_search_path_files, dep_info.transitive_data] + input_depsets, + transitive = [dep_info.link_search_path_files, dep_info.transitive_data, dep_info.transitive_out_dirs] + input_depsets, ) return ( diff --git a/test/linking_out_dir/BUILD.bazel b/test/linking_out_dir/BUILD.bazel new file mode 100644 index 0000000000..cf79a69896 --- /dev/null +++ b/test/linking_out_dir/BUILD.bazel @@ -0,0 +1,43 @@ +"""Tests that cargo build scripts can create archives in OUT_DIR and link to +them. This requires that binaries or shared libraries include the out dirs of +transitive dependencies when linking.""" + +load("//cargo:defs.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_library", "rust_binary") +load("@bazel_skylib//rules:native_binary.bzl", "native_test") + +cc_library( + name = "foo", + srcs = ["foo.c"], +) + +filegroup( + name = "foo_static", + srcs = [":foo"], + output_group = "archive", +) + +cargo_build_script( + name = "build_script", + crate_root = "build.rs", + data = [":foo_static"], + srcs = ["build.rs"], + build_script_env = { "ARCHIVE_PATH": "$(execpath :foo_static)" }, +) + +rust_library( + name = "bar", + srcs = ["bar.rs"], + deps = [":build_script"], +) + +rust_binary( + name = "main", + srcs = ["main.rs"], + deps = [":bar"], +) + +native_test( + name = "test", + src = ":main", +) diff --git a/test/linking_out_dir/bar.rs b/test/linking_out_dir/bar.rs new file mode 100644 index 0000000000..e24c542625 --- /dev/null +++ b/test/linking_out_dir/bar.rs @@ -0,0 +1,7 @@ +extern "C" { + fn foo() -> i32; +} + +pub fn bar() -> i32 { + unsafe { foo() } +} diff --git a/test/linking_out_dir/build.rs b/test/linking_out_dir/build.rs new file mode 100644 index 0000000000..e047fb64fe --- /dev/null +++ b/test/linking_out_dir/build.rs @@ -0,0 +1,15 @@ +use std::env; +use std::path::PathBuf; + +fn main() { + let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + + let src = env::var_os("ARCHIVE_PATH").unwrap(); + + let dst = out_dir.join("libfoo.a"); + + std::fs::copy(&src, &dst).unwrap(); + + println!("cargo:rustc-link-lib=foo"); + println!("cargo:rustc-link-search=native={}", out_dir.display()); +} diff --git a/test/linking_out_dir/foo.c b/test/linking_out_dir/foo.c new file mode 100644 index 0000000000..cfe5524724 --- /dev/null +++ b/test/linking_out_dir/foo.c @@ -0,0 +1,3 @@ +int foo() { + return 1; +} diff --git a/test/linking_out_dir/main.rs b/test/linking_out_dir/main.rs new file mode 100644 index 0000000000..d1c0bb3f3a --- /dev/null +++ b/test/linking_out_dir/main.rs @@ -0,0 +1,5 @@ +pub fn main() { + let result = bar::bar(); + + assert_eq!(result, 1); +} From ff4a167a3e619418fe671d0329c2dc243d17cef2 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Mon, 4 Aug 2025 16:50:36 +0100 Subject: [PATCH 2/9] Add edition flag --- test/linking_out_dir/BUILD.bazel | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/linking_out_dir/BUILD.bazel b/test/linking_out_dir/BUILD.bazel index cf79a69896..043bab865e 100644 --- a/test/linking_out_dir/BUILD.bazel +++ b/test/linking_out_dir/BUILD.bazel @@ -2,9 +2,9 @@ them. This requires that binaries or shared libraries include the out dirs of transitive dependencies when linking.""" -load("//cargo:defs.bzl", "cargo_build_script") -load("//rust:defs.bzl", "rust_library", "rust_binary") load("@bazel_skylib//rules:native_binary.bzl", "native_test") +load("//cargo:defs.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_binary", "rust_library") cc_library( name = "foo", @@ -19,10 +19,11 @@ filegroup( cargo_build_script( name = "build_script", + srcs = ["build.rs"], + build_script_env = {"ARCHIVE_PATH": "$(execpath :foo_static)"}, crate_root = "build.rs", data = [":foo_static"], - srcs = ["build.rs"], - build_script_env = { "ARCHIVE_PATH": "$(execpath :foo_static)" }, + edition = "2024", ) rust_library( From b6c431b7f55ca3ed6da41260ae90410b724ed357 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 10:02:32 +0100 Subject: [PATCH 3/9] Fix linting --- rust/private/providers.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index f17d63c1cf..44c31acfa7 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -54,12 +54,12 @@ DepInfo = provider( "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]", - "transitive_out_dirs": "depset[File]: The out directories of the build scripts of all transitive dependencies", "transitive_crate_outputs": "depset[File]: All transitive crate outputs.", "transitive_crates": "depset[CrateInfo]", "transitive_data": "depset[File]: Data of all transitive non-macro dependencies.", "transitive_metadata_outputs": "depset[File]: All transitive metadata dependencies (.rmeta, for crates that provide them) and all transitive object dependencies (.rlib) for crates that don't provide metadata.", "transitive_noncrates": "depset[LinkerInput]: All transitive dependencies that aren't crates.", + "transitive_out_dirs": "depset[File]: The out directories of the build scripts of all transitive dependencies", "transitive_proc_macro_data": "depset[File]: Data of all transitive proc-macro dependencies, and non-macro dependencies of those macros.", }, ) From e3e56a9321125e9b60311c51c8d2e5ce6eb958d2 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 10:07:50 +0100 Subject: [PATCH 4/9] Add more edition flags --- test/linking_out_dir/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/linking_out_dir/BUILD.bazel b/test/linking_out_dir/BUILD.bazel index 043bab865e..976ee2f82a 100644 --- a/test/linking_out_dir/BUILD.bazel +++ b/test/linking_out_dir/BUILD.bazel @@ -29,12 +29,14 @@ cargo_build_script( rust_library( name = "bar", srcs = ["bar.rs"], + edition = "2024", deps = [":build_script"], ) rust_binary( name = "main", srcs = ["main.rs"], + edition = "2024", deps = [":bar"], ) From 24ebbac14cfccba306553a886d2e8eb0e209b335 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 10:08:51 +0100 Subject: [PATCH 5/9] Lower edition for min rust version --- test/linking_out_dir/BUILD.bazel | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/linking_out_dir/BUILD.bazel b/test/linking_out_dir/BUILD.bazel index 976ee2f82a..71ff8b53be 100644 --- a/test/linking_out_dir/BUILD.bazel +++ b/test/linking_out_dir/BUILD.bazel @@ -23,20 +23,20 @@ cargo_build_script( build_script_env = {"ARCHIVE_PATH": "$(execpath :foo_static)"}, crate_root = "build.rs", data = [":foo_static"], - edition = "2024", + edition = "2021", ) rust_library( name = "bar", srcs = ["bar.rs"], - edition = "2024", + edition = "2021", deps = [":build_script"], ) rust_binary( name = "main", srcs = ["main.rs"], - edition = "2024", + edition = "2021", deps = [":bar"], ) From c6809834a373fa49e84d5058f97d486a32f442fe Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 11:22:22 +0100 Subject: [PATCH 6/9] Attempt to fix CI --- test/linking_out_dir/BUILD.bazel | 1 + test/linking_out_dir/foo.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/linking_out_dir/BUILD.bazel b/test/linking_out_dir/BUILD.bazel index 71ff8b53be..706fccd88b 100644 --- a/test/linking_out_dir/BUILD.bazel +++ b/test/linking_out_dir/BUILD.bazel @@ -3,6 +3,7 @@ them. This requires that binaries or shared libraries include the out dirs of transitive dependencies when linking.""" load("@bazel_skylib//rules:native_binary.bzl", "native_test") +load("@rules_cc//cc:cc_library.bzl", "cc_library") load("//cargo:defs.bzl", "cargo_build_script") load("//rust:defs.bzl", "rust_binary", "rust_library") diff --git a/test/linking_out_dir/foo.c b/test/linking_out_dir/foo.c index cfe5524724..f0ffc1c0f7 100644 --- a/test/linking_out_dir/foo.c +++ b/test/linking_out_dir/foo.c @@ -1,3 +1,3 @@ -int foo() { - return 1; -} +#include + +int32_t foo() { return 1; } From 1fada2e7678eb632289893bd8be2fee2654a0819 Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 20:51:41 +0100 Subject: [PATCH 7/9] Try renaming archive in case the issue is name collisions --- test/linking_out_dir/build.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/linking_out_dir/build.rs b/test/linking_out_dir/build.rs index e047fb64fe..0a29429a26 100644 --- a/test/linking_out_dir/build.rs +++ b/test/linking_out_dir/build.rs @@ -1,15 +1,17 @@ use std::env; use std::path::PathBuf; +const ARCHIVE_NAME: &'static str = "test_linking_out_dir_foo"; + fn main() { let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); let src = env::var_os("ARCHIVE_PATH").unwrap(); - let dst = out_dir.join("libfoo.a"); + let dst = out_dir.join(format!("lib{ARCHIVE_NAME}.a")); std::fs::copy(&src, &dst).unwrap(); - println!("cargo:rustc-link-lib=foo"); + println!("cargo:rustc-link-lib={ARCHIVE_NAME}"); println!("cargo:rustc-link-search=native={}", out_dir.display()); } From 1e50dbd9488e4a9151734269825f2fa244e4813d Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 21:08:01 +0100 Subject: [PATCH 8/9] Disable this test when running coverage --- .bazelci/presubmit.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 9195249ab8..8fc64037bc 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -66,8 +66,12 @@ default_rbe_targets: &default_rbe_targets default_macos_targets: &default_macos_targets - "--" - "//..." - # Not sure why this doesn't work on bazelci, it does locally on macOS. +default_macos_coverage_targets: &default_macos_coverage_targets + - "--" + - "//..." + # Not sure why this these don't work on bazelci, it does locally on macOS. - "-//test/unit/remap_path_prefix:integration_test" + - "-//test/linking_out_dir:test" default_windows_targets: &default_windows_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 - "//..." @@ -110,7 +114,7 @@ tasks: platform: macos_arm64 build_targets: *default_macos_targets test_targets: *default_macos_targets - coverage_targets: *default_macos_targets + coverage_targets: *default_macos_coverage_targets post_shell_commands: *coverage_validation_post_shell_commands windows: build_targets: *default_windows_targets @@ -140,7 +144,7 @@ tasks: shell_commands: *no_bzlmod_shell_commands build_targets: *default_macos_targets test_targets: *default_macos_targets - coverage_targets: *default_macos_targets + coverage_targets: *default_macos_coverage_targets post_shell_commands: *coverage_validation_post_shell_commands windows_no_bzlmod: name: No Bzlmod @@ -167,7 +171,7 @@ tasks: name: Split Coverage Postprocessing platform: macos_arm64 shell_commands: *split_coverage_postprocessing_shell_commands - coverage_targets: *default_macos_targets + coverage_targets: *default_macos_coverage_targets post_shell_commands: *coverage_validation_post_shell_commands ubuntu2204_opt: name: Opt Mode @@ -230,7 +234,7 @@ tasks: build_flags: *aspects_flags build_targets: *default_macos_targets test_targets: *default_macos_targets - coverage_targets: *default_macos_targets + coverage_targets: *default_macos_coverage_targets post_shell_commands: *coverage_validation_post_shell_commands macos_rolling_with_aspects: name: "Macos Rolling Bazel Version With Aspects" @@ -238,7 +242,7 @@ tasks: build_flags: *aspects_flags build_targets: *default_macos_targets test_targets: *default_macos_targets - coverage_targets: *default_macos_targets + coverage_targets: *default_macos_coverage_targets post_shell_commands: *coverage_validation_post_shell_commands soft_fail: yes bazel: "rolling" From 65e78633bdf83cd9a73df598456615b91fcae5cf Mon Sep 17 00:00:00 2001 From: Cameron Martin Date: Tue, 5 Aug 2025 21:27:00 +0100 Subject: [PATCH 9/9] Disable previous test not just on coverage --- .bazelci/presubmit.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 8fc64037bc..025f2407fd 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -66,10 +66,12 @@ default_rbe_targets: &default_rbe_targets default_macos_targets: &default_macos_targets - "--" - "//..." + # Not sure why this doesn't work on bazelci, it does locally on macOS. + - "-//test/unit/remap_path_prefix:integration_test" default_macos_coverage_targets: &default_macos_coverage_targets - "--" - "//..." - # Not sure why this these don't work on bazelci, it does locally on macOS. + # Not sure why these don't work on bazelci, it does locally on macOS. - "-//test/unit/remap_path_prefix:integration_test" - "-//test/linking_out_dir:test" default_windows_targets: &default_windows_targets