From 798dc00aa17c701c5273e5f3bf956e9404daaa5c Mon Sep 17 00:00:00 2001 From: Geoffry Song Date: Mon, 26 Sep 2022 15:25:27 -0700 Subject: [PATCH] process_wrapper: Apply substitutions to param files --- .../process_wrapper_tester.bzl | 10 ++- .../process_wrapper/process_wrapper_tester.cc | 24 ++++++- util/process_wrapper/options.rs | 67 +++++++++++++++++-- 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/test/process_wrapper/process_wrapper_tester.bzl b/test/process_wrapper/process_wrapper_tester.bzl index cf7e51979e..27ac349550 100644 --- a/test/process_wrapper/process_wrapper_tester.bzl +++ b/test/process_wrapper/process_wrapper_tester.bzl @@ -57,13 +57,20 @@ def _impl(ctx): args.add(ctx.attr.test_config) args.add("--current-dir", "${pwd}") args.add("--test-subst", "subst key to ${key}") + + extra_args = ctx.actions.args() + if combined or ctx.attr.test_config == "subst-pwd": + extra_args.set_param_file_format("multiline") + extra_args.use_param_file("@%s", use_always = True) + extra_args.add("${pwd}") + env = {"CURRENT_DIR": "${pwd}/test_path"} ctx.actions.run( executable = ctx.executable._process_wrapper, inputs = ctx.files.env_files + ctx.files.arg_files, outputs = outputs, - arguments = [args], + arguments = [args, extra_args], env = env, tools = [ctx.executable._process_wrapper_tester], ) @@ -76,6 +83,7 @@ process_wrapper_tester = rule( "arg_files": attr.label_list(), "env_files": attr.label_list(), "test_config": attr.string(mandatory = True), + "use_param_file": attr.bool(default = False), "_process_wrapper": attr.label( default = Label("//util/process_wrapper"), executable = True, diff --git a/test/process_wrapper/process_wrapper_tester.cc b/test/process_wrapper/process_wrapper_tester.cc index ab4e96edcb..0dc8837aff 100644 --- a/test/process_wrapper/process_wrapper_tester.cc +++ b/test/process_wrapper/process_wrapper_tester.cc @@ -14,6 +14,7 @@ #include #include +#include #include void basic_part1_test(std::string current_dir_arg) { @@ -42,11 +43,30 @@ void basic_part2_test(std::string current_dir, const char* envp[]) { } } -void subst_pwd_test(std::string current_dir, const char* envp[]) { +void subst_pwd_test(int argc, const char* argv[], const char* envp[]) { + std::string current_dir = argv[3]; if (current_dir.find("${pwd}") != std::string::npos) { std::cerr << "error: argument ${pwd} substitution failed.\n"; std::exit(1); } + // find the param file using its "@" prefix + std::string param_file; + for (int i = 1; i < argc; ++i) { + if (argv[i][0] == '@') { + param_file = std::string(argv[i]+1); + break; + } + } + if (param_file.empty()) { + std::cerr << "error: no param file.\n"; + std::exit(1); + } + std::string param_file_line; + getline(std::ifstream(param_file), param_file_line); + if (param_file_line != current_dir) { + std::cerr << "error: param file " << param_file << " should contain " << current_dir << ", found " << param_file_line << ".\n"; + std::exit(1); + } bool found = false; for (int i = 0; envp[i] != nullptr; ++i) { const std::string env = envp[i]; @@ -147,7 +167,7 @@ int main(int argc, const char* argv[], const char* envp[]) { } if (combined || test_config == "subst-pwd") { - subst_pwd_test(argv[3], envp); + subst_pwd_test(argc, argv, envp); } else if (test_config == "basic") { basic_part2_test(argv[3], envp); } diff --git a/util/process_wrapper/options.rs b/util/process_wrapper/options.rs index 869b5c32b5..841962d3ac 100644 --- a/util/process_wrapper/options.rs +++ b/util/process_wrapper/options.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; use std::env; use std::fmt; +use std::fs::File; +use std::io::{self, Write}; use std::process::exit; use crate::flags::{FlagParseError, Flags, ParseOutcome}; @@ -185,7 +187,7 @@ pub(crate) fn options() -> Result { ); // Append all the arguments fetched from files to those provided via command line. child_args.append(&mut file_arguments); - let child_args = prepare_args(child_args, &subst_mappings); + let child_args = prepare_args(child_args, &subst_mappings)?; // Split the executable path from the rest of the arguments. let (exec_path, args) = child_args.split_first().ok_or_else(|| { OptionError::Generic( @@ -234,15 +236,66 @@ fn env_from_files(paths: Vec) -> Result, OptionE Ok(env_vars) } -fn prepare_args(mut args: Vec, subst_mappings: &[(String, String)]) -> Vec { +fn prepare_arg(mut arg: String, subst_mappings: &[(String, String)]) -> String { for (f, replace_with) in subst_mappings { - for arg in args.iter_mut() { - let from = format!("${{{}}}", f); - let new = arg.replace(from.as_str(), replace_with); - *arg = new; + let from = format!("${{{}}}", f); + arg = arg.replace(&from, replace_with); + } + arg +} + +/// Apply substitutions to the given param file. Returns the new filename. +fn prepare_param_file( + filename: &str, + subst_mappings: &[(String, String)], +) -> Result { + let expanded_file = format!("{}.expanded", filename); + let format_err = |err: io::Error| { + OptionError::Generic(format!( + "{} writing path: {:?}, current directory: {:?}", + err, + expanded_file, + std::env::current_dir() + )) + }; + let mut out = io::BufWriter::new(File::create(&expanded_file).map_err(&format_err)?); + fn process_file( + filename: &str, + out: &mut io::BufWriter, + subst_mappings: &[(String, String)], + format_err: &impl Fn(io::Error) -> OptionError, + ) -> Result<(), OptionError> { + for arg in read_file_to_array(filename).map_err(OptionError::Generic)? { + let arg = prepare_arg(arg, subst_mappings); + if let Some(arg_file) = arg.strip_prefix('@') { + process_file(arg_file, out, subst_mappings, format_err)?; + } else { + writeln!(out, "{}", arg).map_err(format_err)?; + } } + Ok(()) } - args + process_file(filename, &mut out, subst_mappings, &format_err)?; + Ok(expanded_file) +} + +/// Apply substitutions to the provided arguments, recursing into param files. +fn prepare_args( + args: Vec, + subst_mappings: &[(String, String)], +) -> Result, OptionError> { + args.into_iter() + .map(|arg| { + let arg = prepare_arg(arg, subst_mappings); + if let Some(param_file) = arg.strip_prefix('@') { + // Note that substitutions may also apply to the param file path! + prepare_param_file(param_file, subst_mappings) + .map(|filename| format!("@{}", filename)) + } else { + Ok(arg) + } + }) + .collect() } fn environment_block(