5 files changed, 151 insertions, 116 deletions

diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 158ecb99f3..fdfd18070b 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -129,52 +129,45 @@ std::unique_ptr<CommandLine> OptionProcessor::SplitCommandLine(
       new CommandLine(path_to_binary, startup_args, command, command_args));
 }
 
-// Return the path to the user's rc file.  If cmdLineRcFile != NULL,
-// use it, dying if it is not readable.  Otherwise, return the first
-// readable file called rc_basename from [workspace, $HOME]
-//
-// If no readable .blazerc file is found, return the empty string.
 blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
-    const char* cmdLineRcFile,
-    const string& workspace,
-    string* blaze_rc_file,
-    string* error) {
+    const char* cmd_line_rc_file, const string& workspace,
+    string* user_blazerc_file, string* error) const {
   const string rc_basename =
       "." + parsed_startup_options_->GetLowercaseProductName() + "rc";
 
-  if (cmdLineRcFile != NULL) {
-    string rcFile = MakeAbsolute(cmdLineRcFile);
+  if (cmd_line_rc_file != nullptr) {
+    string rcFile = MakeAbsolute(cmd_line_rc_file);
     if (!blaze_util::CanReadFile(rcFile)) {
       blaze_util::StringPrintf(error,
           "Error: Unable to read %s file '%s'.", rc_basename.c_str(),
           rcFile.c_str());
       return blaze_exit_code::BAD_ARGV;
     }
-    *blaze_rc_file = rcFile;
+    *user_blazerc_file = rcFile;
     return blaze_exit_code::SUCCESS;
   }
 
   string workspaceRcFile = blaze_util::JoinPath(workspace, rc_basename);
   if (blaze_util::CanReadFile(workspaceRcFile)) {
-    *blaze_rc_file = workspaceRcFile;
+    *user_blazerc_file = workspaceRcFile;
     return blaze_exit_code::SUCCESS;
   }
 
   string home = blaze::GetHomeDir();
-  if (home.empty()) {
-    *blaze_rc_file = "";
-    return blaze_exit_code::SUCCESS;
+  if (!home.empty()) {
+    string userRcFile = blaze_util::JoinPath(home, rc_basename);
+    if (blaze_util::CanReadFile(userRcFile)) {
+      *user_blazerc_file = userRcFile;
+      return blaze_exit_code::SUCCESS;
+    }
   }
 
-  string userRcFile = blaze_util::JoinPath(home, rc_basename);
-  if (blaze_util::CanReadFile(userRcFile)) {
-    *blaze_rc_file = userRcFile;
-    return blaze_exit_code::SUCCESS;
-  }
-  *blaze_rc_file = "";
+  BAZEL_LOG(INFO) << "User provided no rc file.";
+  *user_blazerc_file = "";
   return blaze_exit_code::SUCCESS;
 }
 
+// TODO(#4502 related cleanup) This should be an anonymous namespace.
 namespace internal {
 
 vector<string> DedupeBlazercPaths(const vector<string>& paths) {
@@ -193,6 +186,18 @@ vector<string> DedupeBlazercPaths(const vector<string>& paths) {
   return result;
 }
 
+string FindRcAlongsideBinary(const string& cwd, const string& path_to_binary) {
+  const string path = blaze_util::IsAbsolute(path_to_binary)
+                          ? path_to_binary
+                          : blaze_util::JoinPath(cwd, path_to_binary);
+  const string base = blaze_util::Basename(path_to_binary);
+  const string binary_blazerc_path = path + "." + base + "rc";
+  if (blaze_util::CanReadFile(binary_blazerc_path)) {
+    return binary_blazerc_path;
+  }
+  return "";
+}
+
 blaze_exit_code::ExitCode ParseErrorToExitCode(RcFile::ParseError parse_error) {
   switch (parse_error) {
     case RcFile::ParseError::NONE:
@@ -210,73 +215,107 @@ blaze_exit_code::ExitCode ParseErrorToExitCode(RcFile::ParseError parse_error) {
 
 }  // namespace internal
 
-// Parses the arguments provided in args using the workspace path and the
-// current working directory (cwd) and stores the results.
-blaze_exit_code::ExitCode OptionProcessor::ParseOptions(
-    const vector<string>& args,
-    const string& workspace,
-    const string& cwd,
-    string* error) {
+// TODO(#4502) Consider simplifying result_rc_files to a vector of RcFiles, no
+// unique_ptrs.
+blaze_exit_code::ExitCode OptionProcessor::GetRcFiles(
+    const WorkspaceLayout* workspace_layout, const std::string& workspace,
+    const std::string& cwd, const CommandLine* cmd_line,
+    std::vector<std::unique_ptr<RcFile>>* result_rc_files,
+    std::string* error) const {
+  assert(cmd_line != nullptr);
+  assert(result_rc_files != nullptr);
+
+  // Find the master bazelrcs if requested. This list may contain duplicates.
+  vector<string> candidate_bazelrc_paths;
+  if (SearchNullaryOption(cmd_line->startup_args, "master_blazerc", true) &&
+      SearchNullaryOption(cmd_line->startup_args, "master_bazelrc", true)) {
+    const string workspace_rc =
+        workspace_layout->GetWorkspaceRcPath(workspace, cmd_line->startup_args);
+    const string binary_rc =
+        internal::FindRcAlongsideBinary(cwd, cmd_line->path_to_binary);
+    const string system_rc = FindSystemWideBlazerc();
+    BAZEL_LOG(INFO)
+        << "Looking for master bazelrcs in the following three paths: "
+        << workspace_rc << ", " << binary_rc << ", " << system_rc;
+    candidate_bazelrc_paths = {workspace_rc, binary_rc, system_rc};
+  }
 
-  cmd_line_ = SplitCommandLine(args, error);
-  if (cmd_line_ == nullptr) {
-    return blaze_exit_code::BAD_ARGV;
+  const char* blazerc = SearchUnaryOption(cmd_line->startup_args, "--blazerc");
+  if (blazerc == nullptr) {
+    blazerc = SearchUnaryOption(cmd_line->startup_args, "--bazelrc");
   }
-  const char* blazerc = SearchUnaryOption(cmd_line_->startup_args, "--blazerc");
-  if (blazerc == NULL) {
-    blazerc = SearchUnaryOption(cmd_line_->startup_args, "--bazelrc");
+  string user_bazelrc_path;
+  blaze_exit_code::ExitCode find_bazelrc_exit_code =
+      FindUserBlazerc(blazerc, workspace, &user_bazelrc_path, error);
+  if (find_bazelrc_exit_code != blaze_exit_code::SUCCESS) {
+    return find_bazelrc_exit_code;
   }
 
-  bool use_master_blazerc = true;
-  if (!SearchNullaryOption(cmd_line_->startup_args, "master_blazerc", true) ||
-      !SearchNullaryOption(cmd_line_->startup_args, "master_bazelrc", true)) {
-    use_master_blazerc = false;
+  vector<string> deduped_blazerc_paths =
+      internal::DedupeBlazercPaths(candidate_bazelrc_paths);
+  deduped_blazerc_paths.push_back(user_bazelrc_path);
+
+  for (const auto& bazelrc_path : deduped_blazerc_paths) {
+    if (bazelrc_path.empty()) {
+      continue;
+    }
+    std::unique_ptr<RcFile> parsed_rc;
+    blaze_exit_code::ExitCode parse_rcfile_exit_code = ParseRcFile(
+        workspace_layout, workspace, bazelrc_path, &parsed_rc, error);
+    if (parse_rcfile_exit_code != blaze_exit_code::SUCCESS) {
+      return parse_rcfile_exit_code;
+    }
+    result_rc_files->push_back(std::move(parsed_rc));
   }
 
-  // Use the workspace path, the current working directory, the path to the
-  // blaze binary and the startup args to determine the list of possible
-  // paths to the rc files. This list may contain duplicates.
-  vector<string> candidate_blazerc_paths;
-  if (use_master_blazerc) {
-    candidate_blazerc_paths =
-        workspace_layout_->FindCandidateBlazercPaths(
-            workspace, cwd, cmd_line_->path_to_binary, cmd_line_->startup_args);
+  return blaze_exit_code::SUCCESS;
+}
+
+blaze_exit_code::ExitCode ParseRcFile(const WorkspaceLayout* workspace_layout,
+                                      const std::string& workspace,
+                                      const std::string& rc_file_path,
+                                      std::unique_ptr<RcFile>* result_rc_file,
+                                      std::string* error) {
+  assert(!rc_file_path.empty());
+  assert(result_rc_file != nullptr);
+
+  RcFile::ParseError parse_error;
+  std::unique_ptr<RcFile> parsed_file = RcFile::Parse(
+      rc_file_path, workspace_layout, workspace, &parse_error, error);
+  if (parsed_file == nullptr) {
+    return internal::ParseErrorToExitCode(parse_error);
   }
+  *result_rc_file = std::move(parsed_file);
+  return blaze_exit_code::SUCCESS;
+}
 
-  string user_blazerc_path;
-  blaze_exit_code::ExitCode find_blazerc_exit_code = FindUserBlazerc(
-      blazerc, workspace, &user_blazerc_path, error);
-  if (find_blazerc_exit_code != blaze_exit_code::SUCCESS) {
-    return find_blazerc_exit_code;
+blaze_exit_code::ExitCode OptionProcessor::ParseOptions(
+    const vector<string>& args, const string& workspace, const string& cwd,
+    string* error) {
+  cmd_line_ = SplitCommandLine(args, error);
+  if (cmd_line_ == nullptr) {
+    return blaze_exit_code::BAD_ARGV;
   }
 
-  vector<string> deduped_blazerc_paths =
-      internal::DedupeBlazercPaths(candidate_blazerc_paths);
-  // TODO(b/37731193): Decide whether the user blazerc should be included in
-  // the deduplication process. If so then we need to handle all cases
-  // (e.g. user rc coming from process substitution).
-  deduped_blazerc_paths.push_back(user_blazerc_path);
-
-  for (const auto& blazerc_path : deduped_blazerc_paths) {
-    if (!blazerc_path.empty()) {
-      RcFile::ParseError parse_error;
-      auto rcfile = RcFile::Parse(blazerc_path, workspace_layout_, workspace,
-                                  &parse_error, error);
-      if (rcfile == nullptr) {
-        return internal::ParseErrorToExitCode(parse_error);
-      }
-      blazercs_.push_back(std::move(rcfile));
-    }
+  // Populate rc_files_. This depends on the startup options in argv since these
+  // may contain rc-modifying options. For all other options, the precedence of
+  // options will be rc first, then command line options, though, despite this
+  // exception.
+  const blaze_exit_code::ExitCode rc_parsing_exit_code = GetRcFiles(
+      workspace_layout_, workspace, cwd, cmd_line_.get(), &rc_files_, error);
+  if (rc_parsing_exit_code != blaze_exit_code::SUCCESS) {
+    return rc_parsing_exit_code;
   }
 
-  blaze_exit_code::ExitCode parse_startup_options_exit_code =
+  // Parse the startup options in the correct priority order.
+  const blaze_exit_code::ExitCode parse_startup_options_exit_code =
       ParseStartupOptions(error);
   if (parse_startup_options_exit_code != blaze_exit_code::SUCCESS) {
     return parse_startup_options_exit_code;
   }
 
   blazerc_and_env_command_args_ =
-      GetBlazercAndEnvCommandArgs(cwd, blazercs_, GetProcessedEnv());
+      GetBlazercAndEnvCommandArgs(cwd, rc_files_, GetProcessedEnv());
   return blaze_exit_code::SUCCESS;
 }
 
@@ -326,7 +365,7 @@ blaze_exit_code::ExitCode OptionProcessor::ParseStartupOptions(
   // option_sources tracking and for sending to the server.
   std::vector<RcStartupFlag> rcstartup_flags;
 
-  for (const auto& blazerc : blazercs_) {
+  for (const auto& blazerc : rc_files_) {
     const auto iter = blazerc->options().find("startup");
     if (iter == blazerc->options().end()) continue;
 
diff --git a/src/main/cpp/option_processor.h b/src/main/cpp/option_processor.h
index d2053017ef..736c004903 100644
--- a/src/main/cpp/option_processor.h
+++ b/src/main/cpp/option_processor.h
@@ -82,8 +82,8 @@ class OptionProcessor {
       const std::vector<std::string>& args,
       std::string* error) const;
 
-  // Parse a command line and the appropriate blazerc files. This should be
-  // invoked only once per OptionProcessor object.
+  // Parse a command line and the appropriate blazerc files and stores the
+  // results. This should be invoked only once per OptionProcessor object.
   blaze_exit_code::ExitCode ParseOptions(const std::vector<std::string>& args,
                                          const std::string& workspace,
                                          const std::string& cwd,
@@ -104,18 +104,11 @@ class OptionProcessor {
 
   virtual StartupOptions* GetParsedStartupOptions() const;
 
-  virtual blaze_exit_code::ExitCode FindUserBlazerc(
-      const char* cmdLineRcFile,
-      const std::string& workspace, std::string* user_blazerc_file,
-      std::string* error);
-
   // Prints a message about the origin of startup options. This should be called
-  // if the server is not started or called, in case the options are related
-  // to the failure. Otherwise, the server will handle any required logging.
+  // if the server is not started or called, in case the options are related to
+  // the failure. Otherwise, the server will handle any required logging.
   void PrintStartupOptionsProvenanceMessage() const;
 
-  blaze_exit_code::ExitCode ParseStartupOptions(std::string* error);
-
   // Constructs all synthetic command args that should be passed to the
   // server to configure blazerc options and client environment.
   static std::vector<std::string> GetBlazercAndEnvCommandArgs(
@@ -123,8 +116,29 @@ class OptionProcessor {
       const std::vector<std::unique_ptr<RcFile>>& blazercs,
       const std::vector<std::string>& env);
 
+  // Finds and parses the appropriate RcFiles.
+  // TODO(#4502) Change where the bazelrcs are read from.
+  virtual blaze_exit_code::ExitCode GetRcFiles(
+      const WorkspaceLayout* workspace_layout, const std::string& workspace,
+      const std::string& cwd, const CommandLine* cmd_line,
+      std::vector<std::unique_ptr<RcFile>>* result_rc_files,
+      std::string* error) const;
+
+ protected:
+  // Return the path to the user's rc file.  If cmd_line_rc_file != NULL,
+  // use it, dying if it is not readable.  Otherwise, return the first
+  // readable file called rc_basename from [workspace, $HOME]
+  //
+  // If no readable .blazerc file is found, return the empty string.
+  virtual blaze_exit_code::ExitCode FindUserBlazerc(
+      const char* cmd_line_rc_file, const std::string& workspace,
+      std::string* user_blazerc_file, std::string* error) const;
+
+ private:
+  blaze_exit_code::ExitCode ParseStartupOptions(std::string* error);
+
   // The list of parsed rc files, this field is initialized by ParseOptions.
-  std::vector<std::unique_ptr<RcFile>> blazercs_;
+  std::vector<std::unique_ptr<RcFile>> rc_files_;
 
   // A map representing the flags parsed from the bazelrc files.
   // A key is a command (e.g. 'build', 'startup') and its value is an ordered
@@ -145,6 +159,13 @@ class OptionProcessor {
   std::unique_ptr<StartupOptions> parsed_startup_options_;
 };
 
+// Parses and returns the contents of the rc file.
+blaze_exit_code::ExitCode ParseRcFile(const WorkspaceLayout* workspace_layout,
+                                      const std::string& workspace,
+                                      const std::string& rc_file_path,
+                                      std::unique_ptr<RcFile>* result_rc_file,
+                                      std::string* error);
+
 }  // namespace blaze
 
 #endif  // BAZEL_SRC_MAIN_CPP_OPTION_PROCESSOR_H_
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index 4dc289a66d..1830cc5246 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -77,7 +77,7 @@ struct RcStartupFlag {
 // This class holds the parsed startup options for Blaze.
 // These options and their defaults must be kept in sync with those in
 // src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java.
-// The latter are purely decorative (they affect the help message,
+// The latter are (usually) purely decorative (they affect the help message,
 // which displays the defaults).  The actual defaults are defined
 // in the constructor.
 //
diff --git a/src/main/cpp/workspace_layout.cc b/src/main/cpp/workspace_layout.cc
index 9748cf4309..64f4a6c06d 100644
--- a/src/main/cpp/workspace_layout.cc
+++ b/src/main/cpp/workspace_layout.cc
@@ -56,31 +56,10 @@ string WorkspaceLayout::GetPrettyWorkspaceName(
   return blaze_util::Basename(workspace);
 }
 
-static string FindAlongsideBinaryBlazerc(const string& cwd,
-                                         const string& path_to_binary) {
-  // TODO(b/32115171): This doesn't work on Windows. Fix this together with the
-  // associated bug.
-  const string path = blaze_util::IsAbsolute(path_to_binary)
-                          ? path_to_binary
-                          : blaze_util::JoinPath(cwd, path_to_binary);
-  const string base = blaze_util::Basename(path_to_binary);
-  const string binary_blazerc_path = path + "." + base + "rc";
-  if (blaze_util::CanReadFile(binary_blazerc_path)) {
-    return binary_blazerc_path;
-  }
-  return "";
-}
-
-vector<string> WorkspaceLayout::FindCandidateBlazercPaths(
-    const string& workspace,
-    const string& cwd,
-    const string& path_to_binary,
-    const vector<string>& startup_args) const {
-  return {
-    blaze_util::JoinPath(workspace, "tools/bazel.rc"),
-    FindAlongsideBinaryBlazerc(cwd, path_to_binary),
-    FindSystemWideBlazerc(),
-  };
+std::string WorkspaceLayout::GetWorkspaceRcPath(
+    const std::string &workspace,
+    const std::vector<std::string> &startup_args) const {
+  return blaze_util::JoinPath(workspace, "tools/bazel.rc");
 }
 
 bool WorkspaceLayout::WorkspaceRelativizeRcFilePath(const string &workspace,
diff --git a/src/main/cpp/workspace_layout.h b/src/main/cpp/workspace_layout.h
index edb2c292e1..3827b87406 100644
--- a/src/main/cpp/workspace_layout.h
+++ b/src/main/cpp/workspace_layout.h
@@ -48,13 +48,9 @@ class WorkspaceLayout {
   // Returns if workspace is a valid build workspace.
   virtual bool InWorkspace(const std::string& workspace) const;
 
-  // Returns the candidate master RC file absolute paths.
-  // All readable files from the result will be used. Empty or nonexistant files
-  // will be ignored. It is ok if no usable candidate exists.
-  virtual std::vector<std::string> FindCandidateBlazercPaths(
+  // Returns the path of the workspace rc file.
+  virtual std::string GetWorkspaceRcPath(
       const std::string& workspace,
-      const std::string& cwd,
-      const std::string& path_to_binary,
       const std::vector<std::string>& startup_args) const;
 
   // Turn a %workspace%-relative import into its true name in the filesystem.