process_wrapper: Apply substitutions to param files (#1565)

Author: goffrie

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,

test/process_wrapper/process_wrapper_tester.cc

@@ -14,6 +14,7 @@
 
 #include <cstdlib>
 #include <iostream>
+#include <fstream>
 #include <string>
 
 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);
   }

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<Options, OptionError> {
     );
     // 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<String>) -> Result<HashMap<String, String>, OptionE
     Ok(env_vars)
 }
 
-fn prepare_args(mut args: Vec<String>, subst_mappings: &[(String, String)]) -> Vec<String> {
+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<String, OptionError> {
+    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<File>,
+        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<String>,
+    subst_mappings: &[(String, String)],
+) -> Result<Vec<String>, 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(