From 7f043a47fb301c61a8bcd2d0836322fa95eb6faf Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Sun, 7 Feb 2021 09:31:33 -0800 Subject: [PATCH 01/15] Rust test targets now create and run a test runner allow for setting env vars --- rust/private/rust.bzl | 58 ++++++++++++++++++++++++++++++++++++-- rust/private/rustc.bzl | 7 ----- test/test_env/tests/run.rs | 6 ++-- tools/runfiles/runfiles.rs | 24 ++++++++++++---- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index f2055fe714..af16ec038c 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -15,7 +15,7 @@ # buildifier: disable=module-docstring load("//rust/private:common.bzl", "rust_common") load("//rust/private:rustc.bzl", "rustc_compile_action") -load("//rust/private:utils.bzl", "determine_output_hash", "find_toolchain") +load("//rust/private:utils.bzl", "determine_output_hash", "expand_locations", "find_toolchain") # TODO(marco): Separate each rule into its own file. @@ -267,15 +267,67 @@ def _rust_test_common(ctx, toolchain, output): is_test = True, ) - return rustc_compile_action( + environ = expand_locations( + ctx, + getattr(ctx.attr, "env", {}), + getattr(ctx.attr, "data", []), + ) + + providers = rustc_compile_action( ctx = ctx, toolchain = toolchain, crate_type = crate_type, crate_info = target, rust_flags = ["--test"], - environ = ctx.attr.env, + environ = environ, + ) + + # Create a process wrapper to ensure runtime environment + # variables are defined for the test binary + runner = ctx.actions.declare_file(ctx.label.name + ".runner") + runner_content = [ + "#!/bin/bash", + "", + "pwd=$(pwd)", + ] + + for key, value in environ.items(): + runner_content.append("export {}=\"{}\"".format(key, value)) + + runner_content.extend([ + "", + "exec {}".format(output.short_path), + "", + ]) + + ctx.actions.write( + runner, + content = "\n".join(runner_content), + is_executable = True, ) + # Replace the `DefaultInfo` provider in the returned list + default_info = None + for i in range(len(providers)): + if type(providers[i]) == "DefaultInfo": + default_info = providers[i] + providers.pop(i) + break + + if not default_info: + fail("No DefaultInfo provider returned from `rustc_compile_action`") + + providers.append(DefaultInfo( + files = default_info.files, + runfiles = default_info.default_runfiles.merge( + # The output is now also considered a runfile + ctx.runfiles(files = [output]), + ), + executable = runner, + )) + + return providers + def _rust_test_impl(ctx): """The implementation of the `rust_test` rule diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 1949eb7a62..7318f034ff 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -586,13 +586,6 @@ def rustc_compile_action( build_flags_files, ) - # Make the user defined enviroment variables available to the action - expanded_env = dict() - data = getattr(ctx.attr, "data", []) - for key in environ: - expanded_env[key] = ctx.expand_location(environ[key], data) - env.update(expanded_env) - if hasattr(ctx.attr, "version") and ctx.attr.version != "0.0.0": formatted_version = " v{}".format(ctx.attr.version) else: diff --git a/test/test_env/tests/run.rs b/test/test_env/tests/run.rs index 85bbccd984..9c1760e0c8 100644 --- a/test/test_env/tests/run.rs +++ b/test/test_env/tests/run.rs @@ -4,7 +4,7 @@ fn run() { 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` - assert_eq!(env!("FERRIS_SAYS"), "Hello fellow Rustaceans!"); - assert_eq!(env!("HELLO_WORLD_BIN"), "test/test_env/hello-world"); + // Test the `env` attribute of `rust_test` at run time + assert_eq!(std::env::var("FERRIS_SAYS").unwrap(), "Hello fellow Rustaceans!"); + assert_eq!(std::env::var("HELLO_WORLD_BIN").unwrap(), "test/test_env/hello-world"); } diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index a2e2d1c8e8..439bf1d79f 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -72,12 +72,26 @@ fn find_runfiles_dir() -> io::Result { let mut binary_path = PathBuf::from(&exec_path); loop { // Check for our neighboring $binary.runfiles directory. - let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(".runfiles"); + { + let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); + runfiles_name.push(".runfiles"); + + let runfiles_path = binary_path.with_file_name(&runfiles_name); + if runfiles_path.is_dir() { + return Ok(runfiles_path); + } + } + + // Test binaries are executed by a $binary.runner, so check for + // $binary.runner.runfiles + { + let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); + runfiles_name.push(".runner.runfiles"); - let runfiles_path = binary_path.with_file_name(&runfiles_name); - if runfiles_path.is_dir() { - return Ok(runfiles_path); + let runfiles_path = binary_path.with_file_name(&runfiles_name); + if runfiles_path.is_dir() { + return Ok(runfiles_path); + } } // Check if we're already under a *.runfiles directory. From e29d204ee439df127b6eb712002fa21ec4c1a7e6 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 8 Feb 2021 08:43:29 -0800 Subject: [PATCH 02/15] Rewrote test runner in rust --- rust/private/BUILD | 2 + rust/private/rust.bzl | 152 ++++++++++++++++++--------- rust/private/test_runner_template.rs | 51 +++++++++ tools/runfiles/runfiles.rs | 26 ++++- 4 files changed, 174 insertions(+), 57 deletions(-) create mode 100644 rust/private/test_runner_template.rs diff --git a/rust/private/BUILD b/rust/private/BUILD index 119af07117..3ef4da80be 100644 --- a/rust/private/BUILD +++ b/rust/private/BUILD @@ -1,5 +1,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +exports_files(["test_runner_template.rs"]) + bzl_library( name = "rules", srcs = glob( diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index af16ec038c..5cfe2cb690 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -219,6 +219,97 @@ def _rust_binary_impl(ctx): ), ) +def _create_rust_test_runner(ctx, toolchain, output, 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. + providers (list): Providers from a rust compile action. See `rustc_compile_action` + + Returns: + list: A list of providers similar to `rustc_compile_action` but with modified default info + """ + + # Expand the environment variables + environ = expand_locations( + ctx, + getattr(ctx.attr, "env", {}), + getattr(ctx.attr, "data", []), + ) + + # Generate strings for inlining them in the Rust source file + environ_defines = [] + if environ: + environ_defines.append(" let mut environ = BTreeMap::new();") + for env, value in environ.items(): + environ_defines.append(" environ.insert(r#####\"{}\"#####, r#####\"{}\"#####);".format( + env, + value, + )) + environ_defines.append(" environ") + else: + environ_defines.append(" BTreeMap::new()") + + # Generate the test runner's source file + runner_src = ctx.actions.declare_file(ctx.label.name + ".runner_main.rs") + ctx.actions.expand_template( + template = ctx.file._test_runner_template, + output = runner_src, + substitutions = { + "// {environ}": "\n".join(environ_defines), + "{executable}": output.short_path, + } + ) + + # Compile the test runner + runner = ctx.actions.declare_file(ctx.label.name + ".runner" + toolchain.binary_ext) + crate_name = name_to_crate_name(ctx.label.name) + "_runner" + crate_type = "bin" + runner_target = rust_common.crate_info( + name = crate_name, + type = crate_type, + root = runner_src, + srcs = [runner_src], + deps = [], + proc_macro_deps = [], + aliases = {}, + output = runner, + edition = "2018", + rustc_env = ctx.attr.rustc_env, + is_test = False, + ) + + runner_providers = rustc_compile_action( + ctx = ctx, + toolchain = toolchain, + crate_type = crate_type, + crate_info = runner_target, + ) + + # Replace the `DefaultInfo` provider in the returned list + default_info = None + for i in range(len(providers)): + if type(providers[i]) == "DefaultInfo": + default_info = providers[i] + providers.pop(i) + break + + if not default_info: + fail("No DefaultInfo provider returned from `rustc_compile_action`") + + providers.append(DefaultInfo( + files = default_info.files, + runfiles = default_info.default_runfiles.merge( + # The output is now also considered a runfile + ctx.runfiles(files = [output]), + ), + executable = runner, + )) + + return providers + def _rust_test_common(ctx, toolchain, output): """Builds a Rust test binary. @@ -267,66 +358,15 @@ def _rust_test_common(ctx, toolchain, output): is_test = True, ) - environ = expand_locations( - ctx, - getattr(ctx.attr, "env", {}), - getattr(ctx.attr, "data", []), - ) - providers = rustc_compile_action( ctx = ctx, toolchain = toolchain, crate_type = crate_type, crate_info = target, rust_flags = ["--test"], - environ = environ, ) - # Create a process wrapper to ensure runtime environment - # variables are defined for the test binary - runner = ctx.actions.declare_file(ctx.label.name + ".runner") - runner_content = [ - "#!/bin/bash", - "", - "pwd=$(pwd)", - ] - - for key, value in environ.items(): - runner_content.append("export {}=\"{}\"".format(key, value)) - - runner_content.extend([ - "", - "exec {}".format(output.short_path), - "", - ]) - - ctx.actions.write( - runner, - content = "\n".join(runner_content), - is_executable = True, - ) - - # Replace the `DefaultInfo` provider in the returned list - default_info = None - for i in range(len(providers)): - if type(providers[i]) == "DefaultInfo": - default_info = providers[i] - providers.pop(i) - break - - if not default_info: - fail("No DefaultInfo provider returned from `rustc_compile_action`") - - providers.append(DefaultInfo( - files = default_info.files, - runfiles = default_info.default_runfiles.merge( - # The output is now also considered a runfile - ctx.runfiles(files = [output]), - ), - executable = runner, - )) - - return providers + return _create_rust_test_runner(ctx, toolchain, output, providers) def _rust_test_impl(ctx): """The implementation of the `rust_test` rule @@ -563,6 +603,14 @@ _rust_test_attrs = { ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution. """), ), + "_test_runner_template": attr.label( + allow_single_file = True, + default = Label("//rust/private:test_runner_template.rs"), + doc = _tidy(""" + A templated rust source file used for inlining environment variables into a test + runner/launcher executable. + """), + ), } rust_library = rule( diff --git a/rust/private/test_runner_template.rs b/rust/private/test_runner_template.rs new file mode 100644 index 0000000000..337933963e --- /dev/null +++ b/rust/private/test_runner_template.rs @@ -0,0 +1,51 @@ +use std::vec::Vec; +use std::collections::BTreeMap; +use std::process::Command; + +#[cfg(target_family = "unix")] +use std::os::unix::process::CommandExt; + +/// This is a templated function for defining a map of environment +/// variables. The comment in this function is replaced by the +/// definition of this map. +fn environ() -> BTreeMap<&'static str, &'static str> { +// {environ} +} + +/// Parse the command line arguments but skip the first element which +/// is the path to the test runner executable. +fn args() -> Vec { + std::env::args().skip(1).collect() +} + +/// Simply replace the current process with our test +#[cfg(target_family = "unix")] +fn exec(environ: BTreeMap<&'static str, &'static str>) { + Command::new("{executable}") + .envs(environ.iter()) + .args(args()) + .exec(); +} + +/// On windows, there is no way to replace the current process +/// so instead we allow the command to run in a subprocess. +#[cfg(target_family = "windows")] +fn exec(environ: BTreeMap<&'static str, &'static str>) { + let output = Command::new(r#####"{executable}"#####) + .envs(environ.iter()) + .args(args()) + .output() + .expect("Failed to run process"); + + std::process::exit(output.status.code().unwrap_or(1)); +} + +fn main() { + // Gather environment variables + let environ = environ(); + + // Replace the current process with the test target + exec(environ); + + panic!("Process did not exit"); +} diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 439bf1d79f..3fc651b870 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -37,6 +37,17 @@ use std::io; use std::path::Path; use std::path::PathBuf; + +/// The common name of Bazel runfiles directory suffix +const RUNFILES_SUFFIX: &'static str = ".runfiles"; + +/// Tests are wrapped in a `.runner` executable so we need to +/// also know to look for this directory as well +#[cfg(target_family = "unix")] +const TEST_RUNFILES_SUFFIX: &'static str = ".runner.runfiles"; +#[cfg(target_family = "windows")] +const TEST_RUNFILES_SUFFIX: &'static str = ".runner.exe.runfiles"; + pub struct Runfiles { runfiles_dir: PathBuf, } @@ -69,12 +80,14 @@ impl Runfiles { fn find_runfiles_dir() -> io::Result { let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); - let mut binary_path = PathBuf::from(&exec_path); + let mut binary_path = PathBuf::from(&exec_path) + .canonicalize() + .expect("Failed to locate path to executable"); loop { // Check for our neighboring $binary.runfiles directory. { let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(".runfiles"); + runfiles_name.push(RUNFILES_SUFFIX); let runfiles_path = binary_path.with_file_name(&runfiles_name); if runfiles_path.is_dir() { @@ -86,8 +99,8 @@ fn find_runfiles_dir() -> io::Result { // $binary.runner.runfiles { let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(".runner.runfiles"); - + runfiles_name.push(TEST_RUNFILES_SUFFIX); + let runfiles_path = binary_path.with_file_name(&runfiles_name); if runfiles_path.is_dir() { return Ok(runfiles_path); @@ -101,7 +114,10 @@ fn find_runfiles_dir() -> io::Result { while let Some(ancestor) = next { if ancestor .file_name() - .map_or(false, |f| f.to_string_lossy().ends_with(".runfiles")) + .map_or(false, |f| { + let f_str = f.to_string_lossy(); + f_str.ends_with(RUNFILES_SUFFIX) || f_str.ends_with(TEST_RUNFILES_SUFFIX) + }) { return Ok(ancestor.to_path_buf()); } From 3d31d1a0173b44c609691dd73c8f19580fe84373 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 8 Feb 2021 11:57:53 -0800 Subject: [PATCH 03/15] Fixed resolving symlinks --- tools/runfiles/runfiles.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 3fc651b870..b2b78d6eb1 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -80,9 +80,12 @@ impl Runfiles { fn find_runfiles_dir() -> io::Result { let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); - let mut binary_path = PathBuf::from(&exec_path) - .canonicalize() - .expect("Failed to locate path to executable"); + let mut binary_path = PathBuf::from(&exec_path); + if !binary_path.is_absolute() { + binary_path = std::env::current_dir() + .expect("Failed to get current directory") + .join(binary_path); + } loop { // Check for our neighboring $binary.runfiles directory. { From eb6c46af858574ac632584411ca312e9ef64f266 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 8 Feb 2021 12:22:50 -0800 Subject: [PATCH 04/15] Added OutputGroupInfo to rules that use the rust test runner --- rust/private/rust.bzl | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 5cfe2cb690..4d26925b03 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -260,7 +260,7 @@ def _create_rust_test_runner(ctx, toolchain, output, providers): substitutions = { "// {environ}": "\n".join(environ_defines), "{executable}": output.short_path, - } + }, ) # Compile the test runner @@ -299,14 +299,21 @@ def _create_rust_test_runner(ctx, toolchain, output, providers): if not default_info: fail("No DefaultInfo provider returned from `rustc_compile_action`") - providers.append(DefaultInfo( - files = default_info.files, - runfiles = default_info.default_runfiles.merge( - # The output is now also considered a runfile - ctx.runfiles(files = [output]), + providers.extend([ + DefaultInfo( + files = default_info.files, + runfiles = default_info.default_runfiles.merge( + # The output is now also considered a runfile + ctx.runfiles(files = [output]), + ), + executable = runner, + ), + OutputGroupInfo( + test_runner = depset([runner]), + test_runner_source = depset([runner_src]), + output = depset([output]), ), - executable = runner, - )) + ]) return providers From ee32e420e637964e806f27f8670ad8c5cc12c99f Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 8 Feb 2021 12:27:56 -0800 Subject: [PATCH 05/15] Fixed inconsistent template arg --- rust/private/test_runner_template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/private/test_runner_template.rs b/rust/private/test_runner_template.rs index 337933963e..1764d1aa07 100644 --- a/rust/private/test_runner_template.rs +++ b/rust/private/test_runner_template.rs @@ -21,7 +21,7 @@ fn args() -> Vec { /// Simply replace the current process with our test #[cfg(target_family = "unix")] fn exec(environ: BTreeMap<&'static str, &'static str>) { - Command::new("{executable}") + Command::new(r#####"{executable}"#####) .envs(environ.iter()) .args(args()) .exec(); From 249dbdd0730d3282aaf35aaf93e656bbeec90e2e Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 8 Feb 2021 12:33:41 -0800 Subject: [PATCH 06/15] Slight cleanup --- rust/private/test_runner_template.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rust/private/test_runner_template.rs b/rust/private/test_runner_template.rs index 1764d1aa07..b70cbe8c55 100644 --- a/rust/private/test_runner_template.rs +++ b/rust/private/test_runner_template.rs @@ -5,6 +5,9 @@ use std::process::Command; #[cfg(target_family = "unix")] use std::os::unix::process::CommandExt; +/// The executable to launch from this test runner +const EXECUTABLE: &'static str = r#####"{executable}"#####; + /// This is a templated function for defining a map of environment /// variables. The comment in this function is replaced by the /// definition of this map. @@ -21,7 +24,7 @@ fn args() -> Vec { /// Simply replace the current process with our test #[cfg(target_family = "unix")] fn exec(environ: BTreeMap<&'static str, &'static str>) { - Command::new(r#####"{executable}"#####) + Command::new(EXECUTABLE) .envs(environ.iter()) .args(args()) .exec(); @@ -31,7 +34,7 @@ fn exec(environ: BTreeMap<&'static str, &'static str>) { /// so instead we allow the command to run in a subprocess. #[cfg(target_family = "windows")] fn exec(environ: BTreeMap<&'static str, &'static str>) { - let output = Command::new(r#####"{executable}"#####) + let output = Command::new(EXECUTABLE) .envs(environ.iter()) .args(args()) .output() From a14c27c197afa3659c0bb314932be8d71b71d559 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Tue, 9 Feb 2021 08:31:38 -0800 Subject: [PATCH 07/15] More slight cleanup --- rust/private/rust.bzl | 18 +++++++----------- rust/private/test_runner_template.rs | 7 ++++++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 4d26925b03..671c4d2b3f 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -240,17 +240,13 @@ def _create_rust_test_runner(ctx, toolchain, output, providers): ) # Generate strings for inlining them in the Rust source file - environ_defines = [] - if environ: - environ_defines.append(" let mut environ = BTreeMap::new();") - for env, value in environ.items(): - environ_defines.append(" environ.insert(r#####\"{}\"#####, r#####\"{}\"#####);".format( - env, - value, - )) - environ_defines.append(" environ") - else: - environ_defines.append(" BTreeMap::new()") + environ_defines = [ + " environ.insert(r#####\"{}\"#####, r#####\"{}\"#####);".format( + env, + value, + ) + for (env, value) in environ.items() + ] # Generate the test runner's source file runner_src = ctx.actions.declare_file(ctx.label.name + ".runner_main.rs") diff --git a/rust/private/test_runner_template.rs b/rust/private/test_runner_template.rs index b70cbe8c55..5a12c18f79 100644 --- a/rust/private/test_runner_template.rs +++ b/rust/private/test_runner_template.rs @@ -1,6 +1,6 @@ -use std::vec::Vec; use std::collections::BTreeMap; use std::process::Command; +use std::vec::Vec; #[cfg(target_family = "unix")] use std::os::unix::process::CommandExt; @@ -11,8 +11,11 @@ const EXECUTABLE: &'static str = r#####"{executable}"#####; /// This is a templated function for defining a map of environment /// variables. The comment in this function is replaced by the /// definition of this map. +#[allow(unused_mut)] fn environ() -> BTreeMap<&'static str, &'static str> { + let mut environ = BTreeMap::new(); // {environ} + environ } /// Parse the command line arguments but skip the first element which @@ -50,5 +53,7 @@ fn main() { // Replace the current process with the test target exec(environ); + // The call to exec should have exited the application. + // This code should be unreachable. panic!("Process did not exit"); } From 6d656211ae4e72755cea7e52d555359f11cb89b2 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Wed, 10 Feb 2021 23:47:53 -0800 Subject: [PATCH 08/15] Made the test launcher a separate binary --- rust/private/rust.bzl | 117 +++++++++++++------------ rust/private/test_runner_template.rs | 59 ------------- util/launcher/BUILD.bazel | 24 ++++++ util/launcher/installer.bzl | 36 ++++++++ util/launcher/launcher_installer.bat | 5 ++ util/launcher/launcher_installer.sh | 5 ++ util/launcher/launcher_main.rs | 122 +++++++++++++++++++++++++++ 7 files changed, 255 insertions(+), 113 deletions(-) delete mode 100644 rust/private/test_runner_template.rs create mode 100644 util/launcher/BUILD.bazel create mode 100644 util/launcher/installer.bzl create mode 100755 util/launcher/launcher_installer.bat create mode 100755 util/launcher/launcher_installer.sh create mode 100644 util/launcher/launcher_main.rs diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 671c4d2b3f..c04f2947e0 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -219,7 +219,7 @@ def _rust_binary_impl(ctx): ), ) -def _create_rust_test_runner(ctx, toolchain, output, providers): +def _create_test_launcher(ctx, toolchain, output, providers): """Create a process wrapper to ensure runtime environment variables are defined for the test binary Args: @@ -232,57 +232,58 @@ def _create_rust_test_runner(ctx, toolchain, output, providers): list: A list of providers similar to `rustc_compile_action` but with modified default info """ - # Expand the environment variables + args = ctx.actions.args() + + # Because returned executables must be created from the same rule, the + # launcher target is simply copied and exposed. + + # TODO: Find a better way to detect the configuration this executable + # was built in. Here we detect whether or not the executable is built + # for windows based on it's extension. + if ctx.executable._launcher.basename.endswith(".exe"): + launcher = ctx.actions.declare_file(name_to_crate_name(ctx.label.name + ".launcher.exe")) + # Because the windows target + args.add_all([ + ctx.executable._launcher.path.replace("/", "\\"), + launcher.path.replace("/", "\\"), + ]) + else: + launcher = ctx.actions.declare_file(name_to_crate_name(ctx.label.name + ".launcher")) + args.add_all([ + ctx.executable._launcher, + launcher, + ]) + + ctx.actions.run( + outputs = [launcher], + tools = [ctx.executable._launcher], + mnemonic = "GeneratingLauncher", + executable = ctx.executable._launcher_installer, + arguments = [args], + ) + + # 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.basename + ".launchfiles/env") environ = expand_locations( ctx, getattr(ctx.attr, "env", {}), - getattr(ctx.attr, "data", []), + data, ) - # Generate strings for inlining them in the Rust source file - environ_defines = [ - " environ.insert(r#####\"{}\"#####, r#####\"{}\"#####);".format( - env, - value, - ) - for (env, value) in environ.items() - ] + # Convert the environment variables into a list to be written into a file. + environ_list = [] + for key, value in environ.items(): + environ_list.extend([key, value]) - # Generate the test runner's source file - runner_src = ctx.actions.declare_file(ctx.label.name + ".runner_main.rs") - ctx.actions.expand_template( - template = ctx.file._test_runner_template, - output = runner_src, - substitutions = { - "// {environ}": "\n".join(environ_defines), - "{executable}": output.short_path, - }, + ctx.actions.write( + output = environ_file, + content = "\n".join(environ_list) ) - # Compile the test runner - runner = ctx.actions.declare_file(ctx.label.name + ".runner" + toolchain.binary_ext) - crate_name = name_to_crate_name(ctx.label.name) + "_runner" - crate_type = "bin" - runner_target = rust_common.crate_info( - name = crate_name, - type = crate_type, - root = runner_src, - srcs = [runner_src], - deps = [], - proc_macro_deps = [], - aliases = {}, - output = runner, - edition = "2018", - rustc_env = ctx.attr.rustc_env, - is_test = False, - ) - - runner_providers = rustc_compile_action( - ctx = ctx, - toolchain = toolchain, - crate_type = crate_type, - crate_info = runner_target, - ) + launcher_files = [environ_file] # Replace the `DefaultInfo` provider in the returned list default_info = None @@ -300,13 +301,12 @@ def _create_rust_test_runner(ctx, toolchain, output, providers): files = default_info.files, runfiles = default_info.default_runfiles.merge( # The output is now also considered a runfile - ctx.runfiles(files = [output]), + ctx.runfiles(files = launcher_files + [output]), ), - executable = runner, + executable = launcher, ), OutputGroupInfo( - test_runner = depset([runner]), - test_runner_source = depset([runner_src]), + launcher_files = depset(launcher_files), output = depset([output]), ), ]) @@ -369,7 +369,7 @@ def _rust_test_common(ctx, toolchain, output): rust_flags = ["--test"], ) - return _create_rust_test_runner(ctx, toolchain, output, providers) + return _create_test_launcher(ctx, toolchain, output, providers) def _rust_test_impl(ctx): """The implementation of the `rust_test` rule @@ -606,12 +606,21 @@ _rust_test_attrs = { ["Make variable"](https://docs.bazel.build/versions/master/be/make-variables.html) substitution. """), ), - "_test_runner_template": attr.label( - allow_single_file = True, - default = Label("//rust/private:test_runner_template.rs"), + "_launcher": attr.label( + executable = True, + default = Label("//util/launcher:launcher"), + cfg = "exec", + doc = _tidy(""" + A launcher executable for loading environment and argument files passed in via the `env` attribute + and ensuring the variables are set for the underlying test executable. + """), + ), + "_launcher_installer": attr.label( + executable = True, + default = Label("//util/launcher:installer"), + cfg = "exec", doc = _tidy(""" - A templated rust source file used for inlining environment variables into a test - runner/launcher executable. + A helper script for creating an installer within the test rule. """), ), } diff --git a/rust/private/test_runner_template.rs b/rust/private/test_runner_template.rs deleted file mode 100644 index 5a12c18f79..0000000000 --- a/rust/private/test_runner_template.rs +++ /dev/null @@ -1,59 +0,0 @@ -use std::collections::BTreeMap; -use std::process::Command; -use std::vec::Vec; - -#[cfg(target_family = "unix")] -use std::os::unix::process::CommandExt; - -/// The executable to launch from this test runner -const EXECUTABLE: &'static str = r#####"{executable}"#####; - -/// This is a templated function for defining a map of environment -/// variables. The comment in this function is replaced by the -/// definition of this map. -#[allow(unused_mut)] -fn environ() -> BTreeMap<&'static str, &'static str> { - let mut environ = BTreeMap::new(); -// {environ} - environ -} - -/// Parse the command line arguments but skip the first element which -/// is the path to the test runner executable. -fn args() -> Vec { - std::env::args().skip(1).collect() -} - -/// Simply replace the current process with our test -#[cfg(target_family = "unix")] -fn exec(environ: BTreeMap<&'static str, &'static str>) { - Command::new(EXECUTABLE) - .envs(environ.iter()) - .args(args()) - .exec(); -} - -/// On windows, there is no way to replace the current process -/// so instead we allow the command to run in a subprocess. -#[cfg(target_family = "windows")] -fn exec(environ: BTreeMap<&'static str, &'static str>) { - let output = Command::new(EXECUTABLE) - .envs(environ.iter()) - .args(args()) - .output() - .expect("Failed to run process"); - - std::process::exit(output.status.code().unwrap_or(1)); -} - -fn main() { - // Gather environment variables - let environ = environ(); - - // Replace the current process with the test target - exec(environ); - - // The call to exec should have exited the application. - // This code should be unreachable. - panic!("Process did not exit"); -} diff --git a/util/launcher/BUILD.bazel b/util/launcher/BUILD.bazel new file mode 100644 index 0000000000..c8d375dd2d --- /dev/null +++ b/util/launcher/BUILD.bazel @@ -0,0 +1,24 @@ +load("//rust:rust.bzl", "rust_binary", "rust_test") +load(":installer.bzl", "installer") + +package(default_visibility = ["//visibility:public"]) + +rust_binary( + name = "launcher", + edition = "2018", + srcs = ["launcher_main.rs"] +) + +rust_test( + name = "launcher_test", + edition = "2018", + crate = ":launcher", +) + +installer( + name = "installer", + src = select({ + "//rust/platform:windows": "launcher_installer.bat", + "//conditions:default": "launcher_installer.sh", + }), +) diff --git a/util/launcher/installer.bzl b/util/launcher/installer.bzl new file mode 100644 index 0000000000..7035cdbafb --- /dev/null +++ b/util/launcher/installer.bzl @@ -0,0 +1,36 @@ +"""A module defining the installer rule for the rules_rust test launcher""" + +def _installer_impl(ctx): + """The `installer` rule's implementation + + Args: + ctx (ctx): The rule's context object + + Returns: + list: A list a DefaultInfo provider + """ + + installer = ctx.actions.declare_file(ctx.file.src.basename) + + ctx.actions.expand_template( + template = ctx.file.src, + output = installer, + substitutions = {}, + is_executable = True, + ) + + return [DefaultInfo( + files = depset([installer]), + executable = installer, + )] + +installer = rule( + doc = "A rule which makes a native executable script avaialble to other rules", + implementation = _installer_impl, + attrs = { + "src": attr.label( + allow_single_file = [".sh", ".bat"], + ), + }, + executable = True, +) diff --git a/util/launcher/launcher_installer.bat b/util/launcher/launcher_installer.bat new file mode 100755 index 0000000000..d8b43a1532 --- /dev/null +++ b/util/launcher/launcher_installer.bat @@ -0,0 +1,5 @@ +@ECHO OFF +@REM A native windows script for creating a `run` action output of the +@REM the rules_rust launcher binary + +copy /v /y /b "%1" "%2" diff --git a/util/launcher/launcher_installer.sh b/util/launcher/launcher_installer.sh new file mode 100755 index 0000000000..4113cb8aac --- /dev/null +++ b/util/launcher/launcher_installer.sh @@ -0,0 +1,5 @@ +#!/bin/bash +# A simple script for creating a `run` action output of the +# the rules_rust launcher binary + +cp "$1" "$2" diff --git a/util/launcher/launcher_main.rs b/util/launcher/launcher_main.rs new file mode 100644 index 0000000000..26e296ce47 --- /dev/null +++ b/util/launcher/launcher_main.rs @@ -0,0 +1,122 @@ +use std::collections::BTreeMap; +use std::fs::File; +use std::io::{BufReader, BufRead}; +use std::path::PathBuf; +use std::process::Command; +use std::vec::Vec; + +#[cfg(target_family = "unix")] +use std::os::unix::process::CommandExt; + +/// This string must match the one found in `_create_test_launcher` +const LAUNCHFILES_ENV_PATH: &'static str = ".launchfiles/env"; + +/// Load environment variables from a uniquly formatted +fn environ() -> BTreeMap { + let mut environ = BTreeMap::new(); + + let mut key: Option = None; + + // Load the environment file into a map + let env_path = std::env::args().nth(0).expect("arg 0 was not set") + LAUNCHFILES_ENV_PATH; + let file = File::open(env_path).expect("Failed to load the environment file"); + + // Find all environment variables by reading pairs of lines as key/value pairs + for line in BufReader::new(file).lines() { + if key.is_none() { + key = Some(line.expect("Failed to read line")); + continue; + } + + environ.insert( + key.expect("Key is not set"), + line.expect("Failed to read line"), + ); + + key = None; + } + + environ +} + +/// Locate the executable based on the name of the launcher executable +fn executable() -> PathBuf { + let mut exec_path = std::env::args().nth(0).expect("arg 0 was not set"); + let stem_index = exec_path.rfind(".launcher").expect("This executable should always contain `.launcher`"); + + // Remove the substring from the exec path + for _char in ".launcher".chars() { + exec_path.remove(stem_index); + } + + PathBuf::from(exec_path) +} + +/// Parse the command line arguments but skip the first element which +/// is the path to the test runner executable. +fn args() -> Vec { + std::env::args().skip(1).collect() +} + +/// Simply replace the current process with our test +#[cfg(target_family = "unix")] +fn exec(environ: BTreeMap, executable: PathBuf, args: Vec) { + let error = Command::new(&executable) + .envs(environ.iter()) + .args(args) + .exec(); + + panic!("Process failed to start: {:?} with {:?}", executable, error) +} + +/// On windows, there is no way to replace the current process +/// so instead we allow the command to run in a subprocess. +#[cfg(target_family = "windows")] +fn exec(environ: BTreeMap, executable: PathBuf, args: Vec) { + let output = Command::new(executable) + .envs(environ.iter()) + .args(args) + .output() + .expect("Failed to run process"); + + std::process::exit(output.status.code().unwrap_or(1)); +} + +/// Main entrypoint +fn main() { + // Gather environment variables + let environ = environ(); + + // Gather arguments + let args = args(); + + // Find executable + let executable = executable(); + + // Replace the current process with the test target + exec(environ, executable, args); + + // The call to exec should have exited the application. + // This code should be unreachable. + panic!("Process did not exit"); +} + + +#[cfg(test)] +mod test { + + #[test] + fn test_environ() { + + } + + #[test] + fn test_executable() { + + } + + #[test] + fn test_args() { + + } +} \ No newline at end of file From aa41b581ebecfb52361f0d378ed47bb08c925ff6 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 11 Feb 2021 08:44:27 -0800 Subject: [PATCH 09/15] Cleaned up runfiles util library --- tools/runfiles/runfiles.rs | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index b2b78d6eb1..451a91666e 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -41,13 +41,6 @@ use std::path::PathBuf; /// The common name of Bazel runfiles directory suffix const RUNFILES_SUFFIX: &'static str = ".runfiles"; -/// Tests are wrapped in a `.runner` executable so we need to -/// also know to look for this directory as well -#[cfg(target_family = "unix")] -const TEST_RUNFILES_SUFFIX: &'static str = ".runner.runfiles"; -#[cfg(target_family = "windows")] -const TEST_RUNFILES_SUFFIX: &'static str = ".runner.exe.runfiles"; - pub struct Runfiles { runfiles_dir: PathBuf, } @@ -88,26 +81,12 @@ fn find_runfiles_dir() -> io::Result { } loop { // Check for our neighboring $binary.runfiles directory. - { - let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(RUNFILES_SUFFIX); + let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); + runfiles_name.push(RUNFILES_SUFFIX); - let runfiles_path = binary_path.with_file_name(&runfiles_name); - if runfiles_path.is_dir() { - return Ok(runfiles_path); - } - } - - // Test binaries are executed by a $binary.runner, so check for - // $binary.runner.runfiles - { - let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(TEST_RUNFILES_SUFFIX); - - let runfiles_path = binary_path.with_file_name(&runfiles_name); - if runfiles_path.is_dir() { - return Ok(runfiles_path); - } + let runfiles_path = binary_path.with_file_name(&runfiles_name); + if runfiles_path.is_dir() { + return Ok(runfiles_path); } // Check if we're already under a *.runfiles directory. @@ -118,8 +97,7 @@ fn find_runfiles_dir() -> io::Result { if ancestor .file_name() .map_or(false, |f| { - let f_str = f.to_string_lossy(); - f_str.ends_with(RUNFILES_SUFFIX) || f_str.ends_with(TEST_RUNFILES_SUFFIX) + f.to_string_lossy().ends_with(RUNFILES_SUFFIX) }) { return Ok(ancestor.to_path_buf()); From 195828d6f9eaf0c9370951f73bf9471a8cf61522 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 11 Feb 2021 08:47:01 -0800 Subject: [PATCH 10/15] Cleanup from previous changes --- rust/private/BUILD | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust/private/BUILD b/rust/private/BUILD index 3ef4da80be..119af07117 100644 --- a/rust/private/BUILD +++ b/rust/private/BUILD @@ -1,7 +1,5 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") -exports_files(["test_runner_template.rs"]) - bzl_library( name = "rules", srcs = glob( From 90c8c38b3d9b495f241f682487d2ca3ad0c3c17c Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 11 Feb 2021 09:08:41 -0800 Subject: [PATCH 11/15] Reverted all util/runfiles changes --- tools/runfiles/runfiles.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 451a91666e..a2e2d1c8e8 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -37,10 +37,6 @@ use std::io; use std::path::Path; use std::path::PathBuf; - -/// The common name of Bazel runfiles directory suffix -const RUNFILES_SUFFIX: &'static str = ".runfiles"; - pub struct Runfiles { runfiles_dir: PathBuf, } @@ -74,15 +70,10 @@ fn find_runfiles_dir() -> io::Result { let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); let mut binary_path = PathBuf::from(&exec_path); - if !binary_path.is_absolute() { - binary_path = std::env::current_dir() - .expect("Failed to get current directory") - .join(binary_path); - } loop { // Check for our neighboring $binary.runfiles directory. let mut runfiles_name = binary_path.file_name().unwrap().to_owned(); - runfiles_name.push(RUNFILES_SUFFIX); + runfiles_name.push(".runfiles"); let runfiles_path = binary_path.with_file_name(&runfiles_name); if runfiles_path.is_dir() { @@ -96,9 +87,7 @@ fn find_runfiles_dir() -> io::Result { while let Some(ancestor) = next { if ancestor .file_name() - .map_or(false, |f| { - f.to_string_lossy().ends_with(RUNFILES_SUFFIX) - }) + .map_or(false, |f| f.to_string_lossy().ends_with(".runfiles")) { return Ok(ancestor.to_path_buf()); } From 9dc8038d7c80c674b2df50c618334923be5a3289 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 11 Feb 2021 09:09:57 -0800 Subject: [PATCH 12/15] The launcher's tests are sufficiently covered by virtually all other tests --- util/launcher/BUILD.bazel | 8 +------- util/launcher/launcher_main.rs | 20 -------------------- 2 files changed, 1 insertion(+), 27 deletions(-) diff --git a/util/launcher/BUILD.bazel b/util/launcher/BUILD.bazel index c8d375dd2d..15949c4072 100644 --- a/util/launcher/BUILD.bazel +++ b/util/launcher/BUILD.bazel @@ -1,4 +1,4 @@ -load("//rust:rust.bzl", "rust_binary", "rust_test") +load("//rust:rust.bzl", "rust_binary") load(":installer.bzl", "installer") package(default_visibility = ["//visibility:public"]) @@ -9,12 +9,6 @@ rust_binary( srcs = ["launcher_main.rs"] ) -rust_test( - name = "launcher_test", - edition = "2018", - crate = ":launcher", -) - installer( name = "installer", src = select({ diff --git a/util/launcher/launcher_main.rs b/util/launcher/launcher_main.rs index 26e296ce47..168453a016 100644 --- a/util/launcher/launcher_main.rs +++ b/util/launcher/launcher_main.rs @@ -100,23 +100,3 @@ fn main() { // This code should be unreachable. panic!("Process did not exit"); } - - -#[cfg(test)] -mod test { - - #[test] - fn test_environ() { - - } - - #[test] - fn test_executable() { - - } - - #[test] - fn test_args() { - - } -} \ No newline at end of file From 069bee24b248510191290892f1895b6a1db8badd Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Fri, 12 Feb 2021 10:27:44 -0800 Subject: [PATCH 13/15] Update util/launcher/installer.bzl Co-authored-by: Daniel Wagner-Hall --- util/launcher/installer.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/launcher/installer.bzl b/util/launcher/installer.bzl index 7035cdbafb..6f264ae98f 100644 --- a/util/launcher/installer.bzl +++ b/util/launcher/installer.bzl @@ -25,7 +25,7 @@ def _installer_impl(ctx): )] installer = rule( - doc = "A rule which makes a native executable script avaialble to other rules", + doc = "A rule which makes a native executable script available to other rules", implementation = _installer_impl, attrs = { "src": attr.label( From 7e384f3b240ee06ed557147a610799b20887105d Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Fri, 12 Feb 2021 10:27:52 -0800 Subject: [PATCH 14/15] Update rust/private/rust.bzl Co-authored-by: Daniel Wagner-Hall --- rust/private/rust.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index c04f2947e0..12ca4d1ad4 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -275,7 +275,7 @@ def _create_test_launcher(ctx, toolchain, output, providers): # Convert the environment variables into a list to be written into a file. environ_list = [] - for key, value in environ.items(): + for key, value in sorted(environ.items()): environ_list.extend([key, value]) ctx.actions.write( From a644c8379314172174abbb36aa192c4f49ec0fce Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Fri, 12 Feb 2021 10:36:14 -0800 Subject: [PATCH 15/15] Use toolcahin to detect os --- rust/private/rust.bzl | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 12ca4d1ad4..56c7476faa 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -234,15 +234,12 @@ def _create_test_launcher(ctx, toolchain, output, providers): args = ctx.actions.args() - # Because returned executables must be created from the same rule, the - # launcher target is simply copied and exposed. - - # TODO: Find a better way to detect the configuration this executable - # was built in. Here we detect whether or not the executable is built - # for windows based on it's extension. - if ctx.executable._launcher.basename.endswith(".exe"): + # TODO: It's unclear if the toolchain is in the same configuration as the `_launcher` attribute + # This should be investigated but for now, we generally assume if the target environment is windows, + # the execution environment is windows. + if toolchain.os == "windows": launcher = ctx.actions.declare_file(name_to_crate_name(ctx.label.name + ".launcher.exe")) - # Because the windows target + # Because the windows target is a batch file, it expects native windows paths (with backslashes) args.add_all([ ctx.executable._launcher.path.replace("/", "\\"), launcher.path.replace("/", "\\"), @@ -254,6 +251,8 @@ def _create_test_launcher(ctx, toolchain, output, providers): launcher, ]) + # Because returned executables must be created from the same rule, the + # launcher target is simply copied and exposed. ctx.actions.run( outputs = [launcher], tools = [ctx.executable._launcher],