From 58dcafb8e7ff6ae909abc53662f4482df7ce6e74 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 30 Sep 2021 21:50:45 +0200 Subject: [PATCH 1/3] Do not use test launcher when not needed --- rust/private/rust.bzl | 36 +++++++++++++++++++---------- test/test_env/BUILD.bazel | 1 - test/test_env/tests/run.rs | 6 ----- test/test_env_launcher/BUILD.bazel | 31 +++++++++++++++++++++++++ test/test_env_launcher/src/main.rs | 3 +++ test/test_env_launcher/tests/run.rs | 35 ++++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 test/test_env_launcher/BUILD.bazel create mode 100644 test/test_env_launcher/src/main.rs create mode 100644 test/test_env_launcher/tests/run.rs diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index f1d24ef55d..de68f9e61b 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -306,13 +306,14 @@ def _rust_binary_impl(ctx): ), ) -def _create_test_launcher(ctx, toolchain, output, providers): +def _create_test_launcher(ctx, toolchain, output, env, providers): """Create a process wrapper to ensure runtime environment variables are defined for the test binary Args: ctx (ctx): The rule's context object toolchain (rust_toolchain): The current rust toolchain output (File): The output File that will be produced, depends on crate type. + env (dict): Dict of environment variables providers (list): Providers from a rust compile action. See `rustc_compile_action` Returns: @@ -337,20 +338,12 @@ def _create_test_launcher(ctx, toolchain, output, providers): is_executable = True, ) - # Get data attribute - data = getattr(ctx.attr, "data", []) - # Expand the environment variables and write them to a file environ_file = ctx.actions.declare_file(launcher_filename + ".launchfiles/env") - environ = expand_dict_value_locations( - ctx, - getattr(ctx.attr, "env", {}), - data, - ) # Convert the environment variables into a list to be written into a file. environ_list = [] - for key, value in sorted(environ.items()): + for key, value in sorted(env.items()): environ_list.extend([key, value]) ctx.actions.write( @@ -454,8 +447,23 @@ def _rust_test_common(ctx, toolchain, output): crate_info = crate_info, rust_flags = ["--test"] if ctx.attr.use_libtest_harness else ["--cfg", "test"], ) + data = getattr(ctx.attr, "data", []) + + env = expand_dict_value_locations( + ctx, + getattr(ctx.attr, "env", {}), + data, + ) + providers.append(testing.TestEnvironment(env)) + + if any(["{pwd}" in v for v in env.values()]): + # Some of the environment variables require expanding {pwd} placeholder at runtime, + # we need a launcher for that. + return _create_test_launcher(ctx, toolchain, output, env, providers) + else: + return providers + - return _create_test_launcher(ctx, toolchain, output, providers) def _rust_test_impl(ctx): """The implementation of the `rust_test` rule @@ -630,8 +638,12 @@ _rust_test_attrs = { mandatory = False, doc = dedent("""\ Specifies additional environment variables to set when the test is executed by bazel test. - Values are subject to `$(execpath)` and + Values are subject to `$(rootpath)`, `$(execpath)`, location, and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution. + + Execpath returns absolute path, and in order to be able to construct the absolute path we + need to wrap the test binary in a launcher. Using a launcher comes with complications, such as + more complicated debugger attachment. """), ), "use_libtest_harness": attr.bool( diff --git a/test/test_env/BUILD.bazel b/test/test_env/BUILD.bazel index ba7ba26075..f3f34ff5b8 100644 --- a/test/test_env/BUILD.bazel +++ b/test/test_env/BUILD.bazel @@ -26,6 +26,5 @@ rust_test( env = { "FERRIS_SAYS": "Hello fellow Rustaceans!", "HELLO_WORLD_BIN_ROOTPATH": "$(rootpath :hello-world)", - "HELLO_WORLD_SRC_EXECPATH": "$(execpath :hello_world_main)", }, ) diff --git a/test/test_env/tests/run.rs b/test/test_env/tests/run.rs index 0ee58ecf92..1c78412ee5 100644 --- a/test/test_env/tests/run.rs +++ b/test/test_env/tests/run.rs @@ -26,10 +26,4 @@ fn run() { ); assert!(!hello_world_bin.is_absolute()); assert!(hello_world_bin.exists()); - - // Ensure `execpath` expanded variables map to real files and have absolute paths - let hello_world_src = - std::path::PathBuf::from(std::env::var("HELLO_WORLD_SRC_EXECPATH").unwrap()); - assert!(hello_world_src.is_absolute()); - assert!(hello_world_src.exists()); } diff --git a/test/test_env_launcher/BUILD.bazel b/test/test_env_launcher/BUILD.bazel new file mode 100644 index 0000000000..ba7ba26075 --- /dev/null +++ b/test/test_env_launcher/BUILD.bazel @@ -0,0 +1,31 @@ +load( + "//rust:rust.bzl", + "rust_binary", + "rust_test", +) + +rust_binary( + name = "hello-world", + srcs = ["src/main.rs"], + edition = "2018", +) + +filegroup( + name = "hello_world_main", + srcs = ["src/main.rs"], +) + +rust_test( + name = "test", + srcs = ["tests/run.rs"], + data = [ + ":hello-world", + ":hello_world_main", + ], + edition = "2018", + env = { + "FERRIS_SAYS": "Hello fellow Rustaceans!", + "HELLO_WORLD_BIN_ROOTPATH": "$(rootpath :hello-world)", + "HELLO_WORLD_SRC_EXECPATH": "$(execpath :hello_world_main)", + }, +) diff --git a/test/test_env_launcher/src/main.rs b/test/test_env_launcher/src/main.rs new file mode 100644 index 0000000000..5bf256ea97 --- /dev/null +++ b/test/test_env_launcher/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("Hello world"); +} diff --git a/test/test_env_launcher/tests/run.rs b/test/test_env_launcher/tests/run.rs new file mode 100644 index 0000000000..01903cd2aa --- /dev/null +++ b/test/test_env_launcher/tests/run.rs @@ -0,0 +1,35 @@ +#[test] +fn run() { + let path = env!("CARGO_BIN_EXE_hello-world"); + let output = std::process::Command::new(path) + .output() + .expect("Failed to run process"); + assert_eq!(&b"Hello world\n"[..], output.stdout.as_slice()); + + // Test the `env` attribute of `rust_test` at run time + assert_eq!( + std::env::var("FERRIS_SAYS").unwrap(), + "Hello fellow Rustaceans!" + ); + + // Test the behavior of `rootpath` and that a binary can be found relative to current_dir + let hello_world_bin = + std::path::PathBuf::from(std::env::var_os("HELLO_WORLD_BIN_ROOTPATH").unwrap()); + + assert_eq!( + hello_world_bin.as_path(), + std::path::Path::new(if std::env::consts::OS == "windows" { + "test/test_env_launcher/hello-world.exe" + } else { + "test/test_env_launcher/hello-world" + }) + ); + assert!(!hello_world_bin.is_absolute()); + assert!(hello_world_bin.exists()); + + // Ensure `execpath` expanded variables map to real files and have absolute paths + let hello_world_src = + std::path::PathBuf::from(std::env::var("HELLO_WORLD_SRC_EXECPATH").unwrap()); + assert!(hello_world_src.is_absolute()); + assert!(hello_world_src.exists()); +} From a75b52a927548254710c4ad0abf3494f3b0ec4de Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 30 Sep 2021 22:00:43 +0200 Subject: [PATCH 2/3] Regenerate documentation --- docs/defs.md | 2 +- docs/flatten.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/defs.md b/docs/defs.md index 3d0136a60c..5f5a0df19a 100644 --- a/docs/defs.md +++ b/docs/defs.md @@ -550,7 +550,7 @@ Run the test with `bazel build //hello_lib:hello_lib_test`. | data | List of files used by this rule at compile time and runtime.

If including data at compile time with include_str!() and similar, prefer compile_data over data, to prevent the data also being included in the runfiles. | List of labels | optional | [] | | deps | List of other libraries to be linked to this library target.

These can be either other rust_library targets or cc_library targets if linking a native library. | List of labels | optional | [] | | edition | The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain. | String | optional | "" | -| env | Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to $(execpath) and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution. | Dictionary: String -> String | optional | {} | +| env | Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to $(rootpath), $(execpath), location, and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.

Execpath returns absolute path, and in order to be able to construct the absolute path we need to wrap the test binary in a launcher. Using a launcher comes with complications, such as more complicated debugger attachment. | Dictionary: String -> String | optional | {} | | proc_macro_deps | List of rust_library targets with kind proc-macro used to help build this library target. | List of labels | optional | [] | | rustc_env | Dictionary of additional "key": "value" environment variables to set for rustc.

rust_test()/rust_binary() rules can use $(rootpath //package:target) to pass in the location of a generated file or external tool. Cargo build scripts that wish to expand locations should use cargo_build_script()'s build_script_env argument instead, as build scripts are run in a different environment - see cargo_build_script()'s documentation for more. | Dictionary: String -> String | optional | {} | | rustc_env_files | Files containing additional environment variables to set for rustc.

These files should contain a single variable per line, of format NAME=value, and newlines may be included in a value by ending a line with a trailing back-slash (\).

The order that these files will be processed is unspecified, so multiple definitions of a particular variable are discouraged. | List of labels | optional | [] | diff --git a/docs/flatten.md b/docs/flatten.md index ffeb8fd92f..159965433c 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -1094,7 +1094,7 @@ Run the test with `bazel build //hello_lib:hello_lib_test`. | data | List of files used by this rule at compile time and runtime.

If including data at compile time with include_str!() and similar, prefer compile_data over data, to prevent the data also being included in the runfiles. | List of labels | optional | [] | | deps | List of other libraries to be linked to this library target.

These can be either other rust_library targets or cc_library targets if linking a native library. | List of labels | optional | [] | | edition | The rust edition to use for this crate. Defaults to the edition specified in the rust_toolchain. | String | optional | "" | -| env | Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to $(execpath) and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution. | Dictionary: String -> String | optional | {} | +| env | Specifies additional environment variables to set when the test is executed by bazel test. Values are subject to $(rootpath), $(execpath), location, and ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution.

Execpath returns absolute path, and in order to be able to construct the absolute path we need to wrap the test binary in a launcher. Using a launcher comes with complications, such as more complicated debugger attachment. | Dictionary: String -> String | optional | {} | | proc_macro_deps | List of rust_library targets with kind proc-macro used to help build this library target. | List of labels | optional | [] | | rustc_env | Dictionary of additional "key": "value" environment variables to set for rustc.

rust_test()/rust_binary() rules can use $(rootpath //package:target) to pass in the location of a generated file or external tool. Cargo build scripts that wish to expand locations should use cargo_build_script()'s build_script_env argument instead, as build scripts are run in a different environment - see cargo_build_script()'s documentation for more. | Dictionary: String -> String | optional | {} | | rustc_env_files | Files containing additional environment variables to set for rustc.

These files should contain a single variable per line, of format NAME=value, and newlines may be included in a value by ending a line with a trailing back-slash (\).

The order that these files will be processed is unspecified, so multiple definitions of a particular variable are discouraged. | List of labels | optional | [] | From 1f50988aab9b1fb9e3c4be5b4696cc81d8d2e8e2 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 30 Sep 2021 22:01:22 +0200 Subject: [PATCH 3/3] Format --- rust/private/rust.bzl | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index de68f9e61b..89d95e6c7c 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -463,8 +463,6 @@ def _rust_test_common(ctx, toolchain, output): else: return providers - - def _rust_test_impl(ctx): """The implementation of the `rust_test` rule