Always use a param file when calling into rustc.

The argument list seems to constantly overflow on Windows (for reasons
that I don't really understand since `use_param_file` should dynamically
switch to using a param file in these cases), so always generate a param
file.

Author: ddeville

rust/private/rustc.bzl

@@ -474,7 +474,8 @@ def construct_arguments(
         force_all_deps_direct = False,
         force_link = False,
         stamp = False,
-        remap_path_prefix = "."):
+        remap_path_prefix = ".",
+        use_param_file_for_rustc_args = True):
     """Builds an Args object containing common rustc flags
 
     Args:
@@ -500,6 +501,7 @@ def construct_arguments(
         stamp (bool, optional): Whether or not workspace status stamping is enabled. For more details see
             https://docs.bazel.build/versions/main/user-manual.html#flag--stamp
         remap_path_prefix (str, optional): A value used to remap `${pwd}` to. If set to a falsey value, no prefix will be set.
+        use_param_file_for_rustc_args(bool, optional): Whether Bazel should use a param file to pass the arguments to rustc.
 
     Returns:
         tuple: A tuple of the following items
@@ -574,8 +576,11 @@ def construct_arguments(
 
     # Rustc arguments
     rustc_flags = ctx.actions.args()
-    rustc_flags.set_param_file_format("multiline")
-    rustc_flags.use_param_file("@%s", use_always = False)
+
+    if use_param_file_for_rustc_args:
+        rustc_flags.set_param_file_format("multiline")
+        rustc_flags.use_param_file("@%s", use_always = True)
+
     rustc_flags.add(crate_info.root)
     rustc_flags.add("--crate-name=" + crate_info.name)
     rustc_flags.add("--crate-type=" + crate_info.type)

rust/private/rustdoc.bzl

@@ -49,7 +49,8 @@ def rustdoc_compile_action(
         crate_info,
         output = None,
         rustdoc_flags = [],
-        is_test = False):
+        is_test = False,
+        use_param_file_for_rustc_args = True):
     """Create a struct of information needed for a `rustdoc` compile action based on crate passed to the rustdoc rule.
 
     Args:
@@ -59,6 +60,7 @@ def rustdoc_compile_action(
         output (File, optional): An optional output a `rustdoc` action is intended to produce.
         rustdoc_flags (list, optional): A list of `rustdoc` specific flags.
         is_test (bool, optional): If True, the action will be configured for `rust_doc_test` targets
+        use_param_file_for_rustc_args(bool, optional): Whether Bazel should use a param file to pass the arguments to rustc.
 
     Returns:
         struct: A struct of some `ctx.actions.run` arguments.
@@ -118,6 +120,7 @@ def rustdoc_compile_action(
         emit = [],
         remap_path_prefix = None,
         force_link = True,
+        use_param_file_for_rustc_args = use_param_file_for_rustc_args,
     )
 
     return struct(

rust/private/rustdoc_test.bzl

@@ -117,6 +117,9 @@ def _rust_doc_test_impl(ctx):
         crate_info = crate_info,
         rustdoc_flags = rustdoc_flags,
         is_test = True,
+        # The arguments are written to a script file that is used to run the
+        # tests and the args point to a param file outside of the `runfiles`.
+        use_param_file_for_rustc_args = False,
     )
 
     tools = action.tools + [ctx.executable._process_wrapper]

util/process_wrapper/process_wrapper.cc

@@ -21,14 +21,41 @@
 #include "util/process_wrapper/system.h"
 #include "util/process_wrapper/utils.h"
 
-using CharType = process_wrapper::System::StrType::value_type;
+using namespace process_wrapper;
+using CharType = System::StrType::value_type;
+
+static int process_param_file(System::StrType& path,
+                              System::Arguments& collected_args,
+                              const std::vector<Subst>& subst_mappings) {
+  System::StrVecType args;
+  if (!ReadFileToVec(path, args)) {
+    std::cerr << "Cannot read param file " << ToUtf8(path) << "\n";
+    return -1;
+  }
+  for (System::StrType& arg : args) {
+    // Expand tokens into each argument (since some paths might embed `${pwd}`
+    // that we need to resolve from the subsitution mappings).
+    ReplaceTokens(arg, subst_mappings);
+
+    // If this is a param file, expand it into the top-level arguments by
+    // recursing into its content.
+    if (arg.size() > 0 && arg.substr(0, 1) == PW_SYS_STR("@")) {
+      System::StrType path = arg.substr(1);
+      int res = process_param_file(path, collected_args, subst_mappings);
+      if (res != 0) {
+        return res;
+      }
+    } else {
+      collected_args.push_back(arg);
+    }
+  }
+  return 0;
+}
 
 // Simple process wrapper allowing us to not depend on the shell to run a
 // process to perform basic operations like capturing the output or having
 // the $pwd used in command line arguments or environment variables
 int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
-  using namespace process_wrapper;
-
   System::EnvironmentBlock environment_block;
   // Taking all environment variables from the current process
   // and sending them down to the child process
@@ -38,8 +65,6 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
 
   System::EnvironmentBlock environment_file_block;
 
-  using Subst = std::pair<System::StrType, System::StrType>;
-
   System::StrType exec_path;
   std::vector<Subst> subst_mappings;
   std::vector<Subst> stamp_mappings;
@@ -53,7 +78,7 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
   System::Arguments file_arguments;
 
   // Processing current process argument list until -- is encountered
-  // everthing after gets sent down to the child process
+  // everything after gets sent down to the child process
   for (int i = 1; i < argc; ++i) {
     System::StrType arg = argv[i];
     if (++i == argc) {
@@ -79,7 +104,8 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
       if (value == PW_SYS_STR("${pwd}")) {
         value = System::GetWorkingDirectory();
       }
-      subst_mappings.push_back({std::move(key), std::move(value)});
+      subst_mappings.push_back({PW_SYS_STR("${") + key + PW_SYS_STR("}"),
+                                std::move(value)});
     } else if (arg == PW_SYS_STR("--volatile-status-file")) {
       if (!volatile_status_file.empty()) {
         std::cerr << "process wrapper error: \"--volatile-status-file\" can "
@@ -90,11 +116,11 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
         return -1;
       }
     } else if (arg == PW_SYS_STR("--env-file")) {
-      if (!ReadFileToArray(argv[i], environment_file_block)) {
+      if (!ReadFileToVec(argv[i], environment_file_block)) {
         return -1;
       }
     } else if (arg == PW_SYS_STR("--arg-file")) {
-      if (!ReadFileToArray(argv[i], file_arguments)) {
+      if (!ReadFileToVec(argv[i], file_arguments)) {
         return -1;
       }
     } else if (arg == PW_SYS_STR("--touch-file")) {
@@ -143,7 +169,7 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
       for (++i; i < argc; ++i) {
         arguments.push_back(argv[i]);
       }
-      // after we consume all arguments we append the files arguments
+      // After we have consumed all arguments add the files arguments the them.
       for (const System::StrType& file_arg : file_arguments) {
         arguments.push_back(file_arg);
       }
@@ -154,13 +180,39 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
     }
   }
 
+  // Go through the arguments and make sure that we process param files so that
+  // we can flatten potential recursive param files within them.
+  for (System::StrType& arg : arguments) {
+    if (arg.size() > 0 && arg.substr(0, 1) == PW_SYS_STR("@")) {
+      // This is a param file, process it separately.
+      System::StrType path = arg.substr(1);
+      System::Arguments processed_args;
+      if (process_param_file(path, processed_args, subst_mappings) != 0) {
+        std::cerr << "Failure to process arguments list\n";
+        return -1;
+      }
+
+      // Create a new param file so that we don't need to edit the original one
+      // that Bazel created.
+      System::StrType new_path = path + PW_SYS_STR(".expanded");
+      if (!WriteVecToFile(new_path, processed_args)) {
+        std::cerr << "Cannot write params to file " << ToUtf8(new_path) << "\n";
+        return -1;
+      }
+
+      // Replace the params file arg with the new one.
+      arg = PW_SYS_STR("@") + new_path;
+    } else {
+      // Expand tokens into each argument (since some paths might embed `${pwd}`
+      // that we need to resolve from the subsitution mappings).
+      ReplaceTokens(arg, subst_mappings);
+    }
+  }
+
   // Stamp any format string in an environment variable block
-  for (const Subst& stamp : stamp_mappings) {
-    System::StrType token = PW_SYS_STR("{");
-    token += stamp.first;
-    token.push_back('}');
+  if (stamp_mappings.size()) {
     for (System::StrType& env : environment_file_block) {
-      ReplaceToken(env, token, stamp.second);
+      ReplaceTokens(env, stamp_mappings);
     }
   }
 
@@ -171,19 +223,9 @@ int PW_MAIN(int argc, const CharType* argv[], const CharType* envp[]) {
                            environment_file_block.begin(),
                            environment_file_block.end());
 
-  if (subst_mappings.size()) {
-    for (const Subst& subst : subst_mappings) {
-      System::StrType token = PW_SYS_STR("${");
-      token += subst.first;
-      token.push_back('}');
-      for (System::StrType& arg : arguments) {
-        ReplaceToken(arg, token, subst.second);
-      }
-
-      for (System::StrType& env : environment_block) {
-        ReplaceToken(env, token, subst.second);
-      }
-    }
+  // Expand tokens in environment
+  for (System::StrType& env : environment_block) {
+    ReplaceTokens(env, subst_mappings);
   }
 
   // Have the last values added take precedence over the first.

util/process_wrapper/utils.cc

@@ -41,16 +41,17 @@ std::string ToUtf8(const System::StrType& string) {
 #endif  // defined(PW_WIN_UNICODE)
 }
 
-void ReplaceToken(System::StrType& str, const System::StrType& token,
-                  const System::StrType& replacement) {
-  std::size_t pos = str.find(token);
-  if (pos != std::string::npos) {
-    str.replace(pos, token.size(), replacement);
+void ReplaceTokens(System::StrType& str, const std::vector<Subst>& substs) {
+  for (const Subst& subst : substs) {
+    std::size_t pos = str.find(subst.first);
+    if (pos != std::string::npos) {
+      str.replace(pos, subst.first.size(), subst.second);
+    }
   }
 }
 
-bool ReadFileToArray(const System::StrType& file_path,
-                     System::StrVecType& vec) {
+bool ReadFileToVec(const System::StrType& file_path,
+                   System::StrVecType& vec) {
   std::ifstream file(file_path);
   if (file.fail()) {
     std::cerr << "process wrapper error: failed to open file: "
@@ -101,12 +102,25 @@ bool ReadFileToArray(const System::StrType& file_path,
   return true;
 }
 
+bool WriteVecToFile(const System::StrType& file_path, const System::StrVecType& vec) {
+  std::ofstream file(file_path, std::ofstream::out | std::ofstream::trunc);
+  if (file.fail()) {
+    std::cerr << "process wrapper error: failed to open file: "
+              << ToUtf8(file_path) << '\n';
+    return false;
+  }
+  for (const System::StrType& line : vec) {
+    file << ToUtf8(line) << std::endl;
+  }
+  return true;
+}
+
 bool ReadStampStatusToArray(
     const System::StrType& stamp_path,
     std::vector<std::pair<System::StrType, System::StrType>>& vec) {
   // Read each line of the stamp file and split on the first space
   System::StrVecType stamp_block;
-  if (!ReadFileToArray(stamp_path, stamp_block)) {
+  if (!ReadFileToVec(stamp_path, stamp_block)) {
     return false;
   }
 
@@ -118,7 +132,8 @@ bool ReadStampStatusToArray(
                 << ToUtf8(stamp_block[i]) << "\".\n";
       return false;
     }
-    System::StrType key = stamp_block[i].substr(0, space_pos);
+    System::StrType key =
+        PW_SYS_STR("{") + stamp_block[i].substr(0, space_pos) + PW_SYS_STR("}");
     System::StrType value =
         stamp_block[i].substr(space_pos + 1, stamp_block[i].size());
     vec.push_back({std::move(key), std::move(value)});

util/process_wrapper/utils.h

@@ -21,16 +21,20 @@
 
 namespace process_wrapper {
 
+using Subst = std::pair<System::StrType, System::StrType>;
+
 // Converts to and frin the system string format
 System::StrType FromUtf8(const std::string& string);
 std::string ToUtf8(const System::StrType& string);
 
-// Replaces a token in str by replacement
-void ReplaceToken(System::StrType& str, const System::StrType& token,
-                  const System::StrType& replacement);
+// Replaces tokens in str by replacement
+void ReplaceTokens(System::StrType& str, const std::vector<Subst>& substs);
 
 // Reads a file in text mode and feeds each line to item in the vec output
-bool ReadFileToArray(const System::StrType& file_path, System::StrVecType& vec);
+bool ReadFileToVec(const System::StrType& file_path, System::StrVecType& vec);
+
+// Write the content of an array file as lines in a file
+bool WriteVecToFile(const System::StrType& file_path, const System::StrVecType& vec);
 
 // Reads a workspace status stamp file to an array of key value pairs
 bool ReadStampStatusToArray(