From fd436df9e2d4ac1b234ca5e969e34a4cb5891910 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Sun, 16 Jan 2022 23:44:05 -0800 Subject: [PATCH] Deleted `incompatible_disable_custom_test_launcher` and related code (#1095) --- rust/private/rust.bzl | 108 +---------------- rust/settings/BUILD.bazel | 6 - rust/toolchain.bzl | 5 - .../disable_custom_test_launcher/BUILD.bazel | 5 - .../disable_custom_test_launcher_test.bzl | 83 ------------- util/launcher/BUILD.bazel | 12 -- util/launcher/launcher_main.rs | 114 ------------------ 7 files changed, 1 insertion(+), 332 deletions(-) delete mode 100644 test/unit/disable_custom_test_launcher/BUILD.bazel delete mode 100644 test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl delete mode 100644 util/launcher/BUILD.bazel delete mode 100644 util/launcher/launcher_main.rs diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index a5e824df0b..8f0877e348 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -326,98 +326,6 @@ def _rust_binary_impl(ctx): ), ) -def _create_test_launcher(ctx, toolchain, output, env, providers): - """Create a process wrapper to ensure runtime environment variables are defined for the test binary - - WARNING: This function is subject to deletion with the removal of - incompatible_disable_custom_test_launcher - - 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: - list: A list of providers similar to `rustc_compile_action` but with modified default info - """ - - # 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_filename = ctx.label.name + ".launcher.exe" - else: - launcher_filename = ctx.label.name + ".launcher" - - launcher = ctx.actions.declare_file(launcher_filename) - - # Because returned executables must be created from the same rule, the - # launcher target is simply symlinked and exposed. - ctx.actions.symlink( - output = launcher, - target_file = ctx.executable._launcher, - is_executable = True, - ) - - # Expand the environment variables and write them to a file - environ_file = ctx.actions.declare_file(launcher_filename + ".launchfiles/env") - - # Convert the environment variables into a list to be written into a file. - environ_list = [] - for key, value in sorted(env.items()): - environ_list.extend([key, value]) - - ctx.actions.write( - output = environ_file, - content = "\n".join(environ_list), - ) - - launcher_files = [environ_file] - - # 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`") - - output_group_info = OutputGroupInfo( - launcher_files = depset(launcher_files), - output = depset([output]), - ) - - # Binaries on Windows might provide a pdb file via an `OutputGroupInfo` that we need to merge - if toolchain.os == "windows": - for i in range(len(providers)): - if type(providers[i]) == "OutputGroupInfo": - output_group_info = OutputGroupInfo( - launcher_files = output_group_info.launcher_files, - output = output_group_info.output, - pdb_file = providers[i].pdb_file, - ) - providers.pop(i) - break - - providers.extend([ - DefaultInfo( - files = default_info.files, - runfiles = default_info.default_runfiles.merge( - # The output is now also considered a runfile - ctx.runfiles(files = launcher_files + [output]), - ), - executable = launcher, - ), - output_group_info, - ]) - - return providers - def _rust_test_common(ctx, toolchain, output): """Builds a Rust test binary. @@ -499,12 +407,7 @@ def _rust_test_common(ctx, toolchain, output): ) providers.append(testing.TestEnvironment(env)) - if not toolchain._incompatible_disable_custom_test_launcher and 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 providers def _rust_test_impl(ctx): """The implementation of the `rust_test` rule @@ -734,15 +637,6 @@ _rust_test_attrs = { default = Label("@bazel_tools//tools/cpp:grep-includes"), executable = True, ), - "_launcher": attr.label( - executable = True, - default = Label("//util/launcher:launcher"), - cfg = "exec", - doc = dedent("""\ - 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. - """), - ), } _common_providers = [ diff --git a/rust/settings/BUILD.bazel b/rust/settings/BUILD.bazel index 19c5b29306..e34fa92ea0 100644 --- a/rust/settings/BUILD.bazel +++ b/rust/settings/BUILD.bazel @@ -10,12 +10,6 @@ incompatible_flag( issue = "https://github.com/bazelbuild/rules_rust/issues/1051", ) -incompatible_flag( - name = "incompatible_disable_custom_test_launcher", - build_setting_default = True, - issue = "https://github.com/bazelbuild/rules_rust/issues/1069", -) - # A flag controlling whether to rename first-party crates such that their names # encode the Bazel package and target name, instead of just the target name. # diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index 834382ecf3..868f28d526 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -233,7 +233,6 @@ def _rust_toolchain_impl(ctx): fail("Do not specify both target_triple and target_json, either use a builtin triple or provide a custom specification file.") remove_transitive_libs_from_dep_info = ctx.attr._incompatible_remove_transitive_libs_from_dep_info[IncompatibleFlagInfo] - disable_custom_test_launcher = ctx.attr._incompatible_disable_custom_test_launcher[IncompatibleFlagInfo] rename_first_party_crates = ctx.attr._rename_first_party_crates[BuildSettingInfo].value third_party_dir = ctx.attr._third_party_dir[BuildSettingInfo].value @@ -289,7 +288,6 @@ def _rust_toolchain_impl(ctx): crosstool_files = ctx.files._crosstool, libstd_and_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, ctx.attr.rust_lib, ctx.attr.allocator_library), _incompatible_remove_transitive_libs_from_dep_info = remove_transitive_libs_from_dep_info.enabled, - _incompatible_disable_custom_test_launcher = disable_custom_test_launcher.enabled, _rename_first_party_crates = rename_first_party_crates, _third_party_dir = third_party_dir, ) @@ -404,9 +402,6 @@ rust_toolchain = rule( "_crosstool": attr.label( default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"), ), - "_incompatible_disable_custom_test_launcher": attr.label( - default = Label("@rules_rust//rust/settings:incompatible_disable_custom_test_launcher"), - ), "_incompatible_remove_transitive_libs_from_dep_info": attr.label( default = "@rules_rust//rust/settings:incompatible_remove_transitive_libs_from_dep_info", ), diff --git a/test/unit/disable_custom_test_launcher/BUILD.bazel b/test/unit/disable_custom_test_launcher/BUILD.bazel deleted file mode 100644 index b6fd3e9ce5..0000000000 --- a/test/unit/disable_custom_test_launcher/BUILD.bazel +++ /dev/null @@ -1,5 +0,0 @@ -load(":disable_custom_test_launcher_test.bzl", "disable_custom_test_launcher_test_suite") - -disable_custom_test_launcher_test_suite( - name = "disable_custom_test_launcher_test_suite", -) diff --git a/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl b/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl deleted file mode 100644 index f4dbb0c81b..0000000000 --- a/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl +++ /dev/null @@ -1,83 +0,0 @@ -"""Unittests for rust rules.""" - -load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") -load("@bazel_skylib//rules:write_file.bzl", "write_file") -load("//rust:defs.bzl", "rust_test") - -def _incompatible_enable_custom_test_launcher_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - executable = tut.files_to_run.executable - asserts.true(env, executable.basename.endswith(".launcher") or executable.basename.endswith(".launcher.exe")) - - return analysistest.end(env) - -def _incompatible_disable_custom_test_launcher_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - executable = tut.files_to_run.executable - asserts.false(env, executable.basename.endswith(".launcher") or executable.basename.endswith(".launcher.exe")) - - return analysistest.end(env) - -incompatible_enable_custom_test_launcher_test = analysistest.make( - _incompatible_enable_custom_test_launcher_test_impl, - config_settings = { - "@//rust/settings:incompatible_disable_custom_test_launcher": False, - }, -) - -incompatible_disable_custom_test_launcher_test = analysistest.make( - _incompatible_disable_custom_test_launcher_test_impl, - config_settings = { - "@//rust/settings:incompatible_disable_custom_test_launcher": True, - }, -) - -def _disable_custom_test_launcher_test(): - write_file( - name = "src", - out = "lib.rs", - content = [], - ) - - write_file( - name = "data", - out = "data.txt", - content = [], - ) - - rust_test( - name = "disable_custom_test_launcher_test", - srcs = [":lib.rs"], - env = {"CUSTOM_TEST_ENV": "$(execpath :data)"}, - data = [":data"], - ) - - incompatible_enable_custom_test_launcher_test( - name = "incompatible_enable_custom_test_launcher_test", - target_under_test = ":disable_custom_test_launcher_test", - ) - - incompatible_disable_custom_test_launcher_test( - name = "incompatible_disable_custom_test_launcher_test", - target_under_test = ":disable_custom_test_launcher_test", - ) - -def disable_custom_test_launcher_test_suite(name): - """Entry-point macro called from the BUILD file. - - Args: - name: Name of the macro. - """ - _disable_custom_test_launcher_test() - - native.test_suite( - name = name, - tests = [ - ":incompatible_disable_custom_test_launcher_test", - ":incompatible_enable_custom_test_launcher_test", - ], - ) diff --git a/util/launcher/BUILD.bazel b/util/launcher/BUILD.bazel deleted file mode 100644 index ca8fb72439..0000000000 --- a/util/launcher/BUILD.bazel +++ /dev/null @@ -1,12 +0,0 @@ -# WARNING: This package is subject to deletion with the removal of -# incompatible_disable_custom_test_launcher - -load("//rust:defs.bzl", "rust_binary") - -package(default_visibility = ["//visibility:public"]) - -rust_binary( - name = "launcher", - srcs = ["launcher_main.rs"], - edition = "2018", -) diff --git a/util/launcher/launcher_main.rs b/util/launcher/launcher_main.rs deleted file mode 100644 index 006d051911..0000000000 --- a/util/launcher/launcher_main.rs +++ /dev/null @@ -1,114 +0,0 @@ -use std::collections::BTreeMap; -use std::fs::File; -use std::io::{BufRead, BufReader}; -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: &str = ".launchfiles/env"; - -/// Load environment variables from a uniquly formatted -fn environ() -> BTreeMap { - let mut environ = BTreeMap::new(); - - let mut key: Option = None; - - // Consume the first argument (argv[0]) - let exe_path = std::env::args().next().expect("arg 0 was not set"); - - // Load the environment file into a map - let env_path = exe_path + LAUNCHFILES_ENV_PATH; - let file = File::open(env_path).expect("Failed to load the environment file"); - - // Variables will have the `${pwd}` variable replaced which is rendered by - // `@rules_rust//rust/private:util.bzl::expand_dict_value_locations` - let pwd = std::env::current_dir().expect("Failed to get current working directory"); - let pwd_str = pwd.to_string_lossy(); - - // 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") - .replace("${pwd}", &pwd_str), - ); - - key = None; - } - - environ -} - -/// Locate the executable based on the name of the launcher executable -fn executable() -> PathBuf { - // Consume the first argument (argv[0]) - let mut exec_path = std::env::args().next().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"); -}