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

@@ -484,7 +484,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:
@@ -510,6 +511,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
@@ -584,8 +586,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

@@ -48,7 +48,8 @@ def rustdoc_compile_action(
         toolchain,
         crate_info,
         output = None,
-        rustdoc_flags = []):
+        rustdoc_flags = [],
+        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:
@@ -57,6 +58,7 @@ def rustdoc_compile_action(
         crate_info (CrateInfo): The provider of the crate passed to a rustdoc rule.
         output (File, optional): An optional output a `rustdoc` action is intended to produce.
         rustdoc_flags (list, optional): A list of `rustdoc` specific flags.
+        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.
@@ -117,6 +119,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):
         toolchain = find_toolchain(ctx),
         crate_info = crate_info,
         rustdoc_flags = rustdoc_flags,
+        # 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

@@ -38,8 +38,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;
@@ -79,7 +77,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 +89,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,24 +142,77 @@ 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
-      for (const System::StrType& file_arg : file_arguments) {
-        arguments.push_back(file_arg);
-      }
     } else {
       std::cerr << "process wrapper error: unknow argument \"" << ToUtf8(arg)
                 << "\"." << '\n';
       return -1;
     }
   }
 
+  // Expand tokens into the file arguments (since some paths might embed
+  // `${pwd}` that we need to resolve from the subsitution mappings).
+  for (System::StrType& file_arg : file_arguments) {
+    ReplaceTokens(file_arg, subst_mappings);
+  }
+
+  // Process the arguments, making sure that we don't have paths to param files
+  // within another param file (that is an arg composed of `@` + a path), which
+  // rustc doesn't support.
+  System::StrVecType param_file_arguments;
+  for (auto arg_it = arguments.begin(); arg_it != arguments.end(); arg_it++) {
+    System::StrType& arg = *arg_it;
+    ReplaceTokens(arg, subst_mappings);
+
+    // Check whether this is a regular argument or a path to a param file.
+    if (arg.size() > 0 && arg.substr(0, 1) == PW_SYS_STR("@")) {
+      System::StrType path = arg.substr(1);
+
+      // Read the actual params off the param file so that we can 1) extract
+      // potential other param files inside and 2) substitute mappings.
+      System::StrVecType params;
+      if (!ReadFileToVec(path, params)) {
+        std::cerr << "Cannot read param file " << ToUtf8(path) << "\n";
+        return -1;
+      }
+
+      for (auto par_it = params.begin(); par_it != params.end(); par_it++) {
+        System::StrType& param = *par_it;
+        ReplaceTokens(param, subst_mappings);
+
+        // If we find a path to a param file, move it to the top-level file
+        // arguments instead (we cannot have nested param files).
+        if (param.size() > 0 && param.substr(0, 1) == PW_SYS_STR("@")) {
+          param_file_arguments.push_back(param);
+          params.erase(par_it--);
+        }
+      }
+
+      // 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, params)) {
+        std::cerr << "Cannot write params to file " << ToUtf8(new_path) << "\n";
+        return -1;
+      }
+
+      param_file_arguments.push_back(PW_SYS_STR("@") + new_path);
+      arguments.erase(arg_it--);
+    }
+  }
+
+  // After we consume all arguments we append the param files arguments and the
+  // remaining of the files arguments
+  for (const System::StrType& file_arg : param_file_arguments) {
+    arguments.push_back(file_arg);
+  }
+  for (const System::StrType& file_arg : file_arguments) {
+    arguments.push_back(file_arg);
+  }
+
   // 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(