From 9d3303dda76503af42987c322eac00a17888d2fd Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 14 Oct 2021 15:07:37 +0100 Subject: [PATCH 1/4] Re-use tags for rust_binary and build script (#974) --- cargo/cargo_build_script.bzl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cargo/cargo_build_script.bzl b/cargo/cargo_build_script.bzl index e86d4c8e63..dfc1c779df 100644 --- a/cargo/cargo_build_script.bzl +++ b/cargo/cargo_build_script.bzl @@ -336,6 +336,10 @@ def cargo_build_script( if "CARGO_CRATE_NAME" not in rustc_env: rustc_env["CARGO_CRATE_NAME"] = name_to_crate_name(_name_to_pkg_name(name)) + binary_tags = [tag for tag in tags or []] + if "manual" not in binary_tags: + binary_tags.append("manual") + rust_binary( name = name + "_script_", crate_features = crate_features, @@ -343,7 +347,7 @@ def cargo_build_script( deps = deps, data = data, rustc_env = rustc_env, - tags = ["manual"], + tags = binary_tags, **kwargs ) _build_script_run( From 89d207bae700497dc37b2a66a8f338b88c83ddaa Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Fri, 15 Oct 2021 11:04:35 +0200 Subject: [PATCH 2/4] Also propagate linkstamps through rust_libraries (#975) Before this PR we were only propagating linkstamps from C++ dependency to Rust target, but we did not propagate from Rust to Rust. This PR fixes it by moving the logic to collect linkstamps from dependencies from the if branch that only covers C++ deps to a place that affects all deps. --- rust/private/rustc.bzl | 5 ++- test/unit/linkstamps/linkstamps_test.bzl | 51 ++++++++++++++++-------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index df54795d95..617c0227a6 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -147,6 +147,9 @@ def collect_deps(label, deps, proc_macro_deps, aliases, are_linkstamps_supported cc_info = _get_cc_info(dep) dep_build_info = _get_build_info(dep) + if cc_info and are_linkstamps_supported: + linkstamps.append(cc_info.linking_context.linkstamps()) + if crate_info: # This dependency is a rust_library @@ -174,8 +177,6 @@ def collect_deps(label, deps, proc_macro_deps, aliases, are_linkstamps_supported libs = [get_preferred_artifact(lib) for li in linker_inputs for lib in li.libraries] transitive_noncrate_libs.append(depset(libs)) transitive_noncrates.append(cc_info.linking_context.linker_inputs) - if are_linkstamps_supported: - linkstamps.append(cc_info.linking_context.linkstamps()) elif dep_build_info: if build_info: fail("Several deps are providing build information, " + diff --git a/test/unit/linkstamps/linkstamps_test.bzl b/test/unit/linkstamps/linkstamps_test.bzl index a8d6a7fb78..d7423f3b6d 100644 --- a/test/unit/linkstamps/linkstamps_test.bzl +++ b/test/unit/linkstamps/linkstamps_test.bzl @@ -2,7 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("@rules_cc//cc:defs.bzl", "cc_library") -load("//rust:defs.bzl", "rust_binary", "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): @@ -64,38 +64,43 @@ def _linkstamps_test(): }), ) - cc_library( - name = "cc_lib_with_linkstamp_transitively", - deps = [":cc_lib_with_linkstamp"], - ) - rust_binary( name = "some_rust_binary", srcs = ["foo.rs"], deps = [":cc_lib_with_linkstamp"], ) - rust_binary( - name = "some_rust_binary_with_multiple_paths_to_a_linkstamp", - srcs = ["foo.rs"], - deps = [":cc_lib_with_linkstamp", ":cc_lib_with_linkstamp_transitively"], + supports_linkstamps_test( + name = "rust_binary_supports_linkstamps_test", + target_under_test = ":some_rust_binary", ) - rust_test( - name = "some_rust_test1", + rust_library( + name = "some_rust_library_with_linkstamp_transitively", srcs = ["foo.rs"], deps = [":cc_lib_with_linkstamp"], ) - rust_test( - name = "some_rust_test2", + rust_binary( + name = "some_rust_binary_with_linkstamp_transitively", srcs = ["foo.rs"], - deps = [":cc_lib_with_linkstamp"], + deps = [":some_rust_library_with_linkstamp_transitively"], ) supports_linkstamps_test( - name = "rust_binary_supports_linkstamps_test", - target_under_test = ":some_rust_binary", + name = "rust_binary_with_linkstamp_transitively", + target_under_test = ":some_rust_binary_with_linkstamp_transitively", + ) + + cc_library( + name = "cc_lib_with_linkstamp_transitively", + deps = [":cc_lib_with_linkstamp"], + ) + + rust_binary( + name = "some_rust_binary_with_multiple_paths_to_a_linkstamp", + srcs = ["foo.rs"], + deps = [":cc_lib_with_linkstamp", ":cc_lib_with_linkstamp_transitively"], ) supports_linkstamps_test( @@ -103,11 +108,23 @@ def _linkstamps_test(): target_under_test = ":some_rust_binary_with_multiple_paths_to_a_linkstamp", ) + rust_test( + name = "some_rust_test1", + srcs = ["foo.rs"], + deps = [":cc_lib_with_linkstamp"], + ) + supports_linkstamps_test( name = "rust_test_supports_linkstamps_test1", target_under_test = ":some_rust_test1", ) + rust_test( + name = "some_rust_test2", + srcs = ["foo.rs"], + deps = [":cc_lib_with_linkstamp"], + ) + supports_linkstamps_test( name = "rust_test_supports_linkstamps_test2", target_under_test = ":some_rust_test2", From 6f79458dee68d691d6a5aee67b06a620bdf9293f Mon Sep 17 00:00:00 2001 From: scentini Date: Fri, 15 Oct 2021 13:06:32 +0200 Subject: [PATCH 3/4] Fix `cargo_build_script` breakage when `SYSROOT` is specified (#976) https://github.com/bazelbuild/rules_rust/pull/973 changed the type of `cc_path` from `std::ffi::OsString` to `std::path::PathBuf`, which breaks builds that specify `SYSROOT`. As `std::path::PathBuf::push(x)` replaces the current path when x is absolute, the [`cc_path.push(&exec_root.join(sysroot_path));`](https://github.com/bazelbuild/rules_rust/blob/89d207bae700497dc37b2a66a8f338b88c83ddaa/cargo/cargo_build_script_runner/bin.rs#L95) line ends up dropping everything that was previously added to `cc_path`. This PR fixes the issue. --- cargo/cargo_build_script_runner/bin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index 04d89a34d9..baee4ff724 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -89,7 +89,7 @@ fn run_buildrs() -> Result<(), String> { } if let Some(cc_path) = env::var_os("CC") { - let mut cc_path = exec_root.join(cc_path); + let mut cc_path = exec_root.join(cc_path).into_os_string(); if let Some(sysroot_path) = env::var_os("SYSROOT") { cc_path.push(" --sysroot="); cc_path.push(&exec_root.join(sysroot_path)); From cdca6ed5eb9609cbc3f6b5d8f760e46e3b0596a2 Mon Sep 17 00:00:00 2001 From: RS Date: Sat, 16 Oct 2021 13:00:06 -0700 Subject: [PATCH 4/4] Add test case for OUT_DIR in tests. (#954) * Add test case for OUT_DIR in tests. * Add some env printing, and make the test fail on purpose. * Passing when OUT_DIR is used in a macro. * Cleanup * Remove Cargo.lock * Buildifier warnings=all * Remove Cargo.toml --- test/out_dir_in_tests/BUILD.bazel | 34 +++++++++++++++++++ test/out_dir_in_tests/build.rs | 11 ++++++ test/out_dir_in_tests/src/lib.rs | 18 ++++++++++ test/out_dir_in_tests/tests/out_dir_reader.rs | 15 ++++++++ 4 files changed, 78 insertions(+) create mode 100644 test/out_dir_in_tests/BUILD.bazel create mode 100644 test/out_dir_in_tests/build.rs create mode 100644 test/out_dir_in_tests/src/lib.rs create mode 100644 test/out_dir_in_tests/tests/out_dir_reader.rs diff --git a/test/out_dir_in_tests/BUILD.bazel b/test/out_dir_in_tests/BUILD.bazel new file mode 100644 index 0000000000..67c4140725 --- /dev/null +++ b/test/out_dir_in_tests/BUILD.bazel @@ -0,0 +1,34 @@ +load("//cargo:cargo_build_script.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_test_suite") +load( + "//rust:rust.bzl", + "rust_library", + "rust_test", +) + +cargo_build_script( + name = "build_script", + srcs = ["build.rs"], +) + +rust_library( + name = "demo_lib", + srcs = [ + "src/lib.rs", + ], + deps = [":build_script"], +) + +rust_test( + name = "demo_lib_test", + crate = ":demo_lib", +) + +rust_test_suite( + name = "suite", + srcs = glob(["tests/**"]), + # Add the 'crate' argument, which will be passed as a kwarg + # to the underlying rust_test rules. This will make OUT_DIR + # available when compiling integration tests. + crate = ":demo_lib", +) diff --git a/test/out_dir_in_tests/build.rs b/test/out_dir_in_tests/build.rs new file mode 100644 index 0000000000..f13327afc3 --- /dev/null +++ b/test/out_dir_in_tests/build.rs @@ -0,0 +1,11 @@ +use std::env; +use std::fs::File; +use std::io::prelude::*; +use std::path::PathBuf; + +fn main() -> std::io::Result<()> { + let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); + let mut file = File::create(out_path.join("test_content.txt"))?; + file.write_all(b"Test content")?; + Ok(()) +} diff --git a/test/out_dir_in_tests/src/lib.rs b/test/out_dir_in_tests/src/lib.rs new file mode 100644 index 0000000000..4eaefdbef9 --- /dev/null +++ b/test/out_dir_in_tests/src/lib.rs @@ -0,0 +1,18 @@ +#[cfg(test)] +mod tests { + use std::env; + + #[test] + fn can_find_the_out_dir_file() { + // The file contents must be included via a macro. + let contents = include_str!(concat!(env!("OUT_DIR"), "/test_content.txt")); + assert_eq!("Test content", contents); + } + + #[test] + fn no_out_dir_at_runtime() { + // Cargo seems to set this at runtime as well, although the documentation + // says it's only available at compile time. + assert!(env::var("OUT_DIR").is_err()); + } +} diff --git a/test/out_dir_in_tests/tests/out_dir_reader.rs b/test/out_dir_in_tests/tests/out_dir_reader.rs new file mode 100644 index 0000000000..9081f0f9e7 --- /dev/null +++ b/test/out_dir_in_tests/tests/out_dir_reader.rs @@ -0,0 +1,15 @@ +use std::env; + +#[test] +fn can_find_the_out_dir_file() { + // The file contents must be included via a macro. + let contents = include_str!(concat!(env!("OUT_DIR"), "/test_content.txt")); + assert_eq!("Test content", contents); +} + +#[test] +fn no_out_dir_at_runtime() { + // Cargo seems to set this at runtime as well, although the documentation + // says it's only available at compile time. + assert!(env::var("OUT_DIR").is_err()); +}