runfiles,cc: merge strategies into Runfiles

Merge the manifest- and directory-based runfiles
strategies into the Runfiles class. The resulting
Runfiles object can look up runfiles from the
manifest or from the filesystem, as needed.

Change-Id: Ifff4b76849ce67248bb9152901024e241aaea800

Closes #5251.

Change-Id: Ifff4b76849ce67248bb9152901024e241aaea800
PiperOrigin-RevId: 197863872

3 files changed, 146 insertions, 274 deletions

diff --git a/tools/cpp/runfiles/runfiles.cc b/tools/cpp/runfiles/runfiles.cc
index 0cdc2a0c21..79ca943ca7 100644
--- a/tools/cpp/runfiles/runfiles.cc
+++ b/tools/cpp/runfiles/runfiles.cc
@@ -44,8 +44,8 @@ using std::vector;
 
 namespace {
 
-bool starts_with(const string& s, const string& prefix) {
-  if (prefix.empty()) {
+bool starts_with(const string& s, const char* prefix) {
+  if (!prefix || !*prefix) {
     return true;
   }
   if (s.empty()) {
@@ -54,8 +54,8 @@ bool starts_with(const string& s, const string& prefix) {
   return s.find(prefix) == 0;
 }
 
-bool contains(const string& s, const string& substr) {
-  if (substr.empty()) {
+bool contains(const string& s, const char* substr) {
+  if (!substr || !*substr) {
     return true;
   }
   if (s.empty()) {
@@ -74,36 +74,6 @@ bool ends_with(const string& s, const string& suffix) {
   return s.rfind(suffix) == s.size() - suffix.size();
 }
 
-class RunfilesImpl : public Runfiles {
- public:
-  static Runfiles* Create(const string& argv0,
-                          function<string(const string&)> env_lookup,
-                          string* error);
-
-  string Rlocation(const string& path) const override;
-
-  const vector<pair<string, string> >& EnvVars() const override {
-    return envvars_;
-  }
-
- protected:
-  RunfilesImpl(const map<string, string>&& runfiles_map,
-               const string&& directory,
-               const vector<pair<string, string> >&& envvars)
-      : runfiles_map_(std::move(runfiles_map)),
-        directory_(std::move(directory)),
-        envvars_(std::move(envvars)) {}
-  virtual ~RunfilesImpl() {}
-
- private:
-  static bool ParseManifest(const string& path, map<string, string>* result,
-                            string* error);
-
-  const map<string, string> runfiles_map_;
-  const string directory_;
-  const vector<pair<string, string> > envvars_;
-};
-
 bool IsReadableFile(const string& path) {
   return std::ifstream(path).is_open();
 }
@@ -119,22 +89,31 @@ bool IsDirectory(const string& path) {
 #endif
 }
 
-Runfiles* RunfilesImpl::Create(const string& argv0,
-                               function<string(const string&)> env_lookup,
-                               string* error) {
+bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
+               std::string runfiles_dir,
+               std::function<bool(const std::string&)> is_runfiles_manifest,
+               std::function<bool(const std::string&)> is_runfiles_directory,
+               std::string* out_manifest, std::string* out_directory);
+
+bool ParseManifest(const string& path, map<string, string>* result,
+                   string* error);
+
+}  // namespace
+
+Runfiles* Runfiles::Create(const string& argv0,
+                           const string& runfiles_manifest_file,
+                           const string& runfiles_dir, string* error) {
   string manifest, directory;
-  if (!Runfiles::PathsFrom(argv0, env_lookup("RUNFILES_MANIFEST_FILE"),
-                           env_lookup("RUNFILES_DIR"),
-                           [](const string& path) {
-                             return (ends_with(path, "MANIFEST") ||
-                                     ends_with(path, ".runfiles_manifest")) &&
-                                    IsReadableFile(path);
-                           },
-                           [](const string& path) {
-                             return ends_with(path, ".runfiles") &&
-                                    IsDirectory(path);
-                           },
-                           &manifest, &directory)) {
+  if (!PathsFrom(argv0, runfiles_manifest_file, runfiles_dir,
+                 [](const string& path) {
+                   return (ends_with(path, "MANIFEST") ||
+                           ends_with(path, ".runfiles_manifest")) &&
+                          IsReadableFile(path);
+                 },
+                 [](const string& path) {
+                   return ends_with(path, ".runfiles") && IsDirectory(path);
+                 },
+                 &manifest, &directory)) {
     if (error) {
       std::ostringstream err;
       err << "ERROR: " << __FILE__ << "(" << __LINE__
@@ -158,8 +137,8 @@ Runfiles* RunfilesImpl::Create(const string& argv0,
     }
   }
 
-  return new RunfilesImpl(std::move(runfiles), std::move(directory),
-                          std::move(envvars));
+  return new Runfiles(std::move(runfiles), std::move(directory),
+                      std::move(envvars));
 }
 
 bool IsAbsolute(const string& path) {
@@ -181,18 +160,18 @@ string GetEnv(const string& key) {
   }
   std::unique_ptr<char[]> value(new char[size]);
   ::GetEnvironmentVariableA(key.c_str(), value.get(), size);
-  return move(string(value.get()));
+  return std::move(string(value.get()));
 #else
   char* result = getenv(key.c_str());
   return std::move((result == NULL) ? string() : string(result));
 #endif
 }
 
-string RunfilesImpl::Rlocation(const string& path) const {
+string Runfiles::Rlocation(const string& path) const {
   if (path.empty() || starts_with(path, "../") || contains(path, "/..") ||
       starts_with(path, "./") || contains(path, "/./") ||
       ends_with(path, "/.") || contains(path, "//")) {
-    return std::move(string());
+    return string();
   }
   if (IsAbsolute(path)) {
     return path;
@@ -207,8 +186,10 @@ string RunfilesImpl::Rlocation(const string& path) const {
   return "";
 }
 
-bool RunfilesImpl::ParseManifest(const string& path,
-                                 map<string, string>* result, string* error) {
+namespace {
+
+bool ParseManifest(const string& path, map<string, string>* result,
+                   string* error) {
   std::ifstream stm(path);
   if (!stm.is_open()) {
     if (error) {
@@ -245,10 +226,12 @@ bool RunfilesImpl::ParseManifest(const string& path,
 
 namespace testing {
 
-Runfiles* TestOnly_CreateRunfiles(const std::string& argv0,
-                                  function<string(const string&)> env_lookup,
-                                  string* error) {
-  return RunfilesImpl::Create(argv0, env_lookup, error);
+bool TestOnly_PathsFrom(const string& argv0, string mf, string dir,
+                        function<bool(const string&)> is_runfiles_manifest,
+                        function<bool(const string&)> is_runfiles_directory,
+                        string* out_manifest, string* out_directory) {
+  return PathsFrom(argv0, mf, dir, is_runfiles_manifest, is_runfiles_directory,
+                   out_manifest, out_directory);
 }
 
 bool TestOnly_IsAbsolute(const string& path) { return IsAbsolute(path); }
@@ -256,23 +239,16 @@ bool TestOnly_IsAbsolute(const string& path) { return IsAbsolute(path); }
 }  // namespace testing
 
 Runfiles* Runfiles::Create(const string& argv0, string* error) {
-  return RunfilesImpl::Create(
-      argv0,
-      [](const string& key) {
-        if (key == "RUNFILES_MANIFEST_FILE" || key == "RUNFILES_DIR") {
-          string val(GetEnv(key));
-          return std::move(val);
-        } else {
-          return std::move(string());
-        }
-      },
-      error);
+  return Runfiles::Create(argv0, GetEnv("RUNFILES_MANIFEST_FILE"),
+                          GetEnv("RUNFILES_DIR"), error);
 }
 
-bool Runfiles::PathsFrom(const string& argv0, string mf, string dir,
-                         function<bool(const string&)> is_runfiles_manifest,
-                         function<bool(const string&)> is_runfiles_directory,
-                         string* out_manifest, string* out_directory) {
+namespace {
+
+bool PathsFrom(const string& argv0, string mf, string dir,
+               function<bool(const string&)> is_runfiles_manifest,
+               function<bool(const string&)> is_runfiles_directory,
+               string* out_manifest, string* out_directory) {
   out_manifest->clear();
   out_directory->clear();
 
@@ -320,6 +296,8 @@ bool Runfiles::PathsFrom(const string& argv0, string mf, string dir,
   return true;
 }
 
+}  // namespace
+
 }  // namespace runfiles
 }  // namespace cpp
 }  // namespace tools
diff --git a/tools/cpp/runfiles/runfiles.h b/tools/cpp/runfiles/runfiles.h
index 5e9db580bf..8c016a3030 100644
--- a/tools/cpp/runfiles/runfiles.h
+++ b/tools/cpp/runfiles/runfiles.h
@@ -26,25 +26,28 @@
 //     if (runfiles == nullptr) {
 //       ...  // error handling
 //     }
-//     std::string path(runfiles->Rlocation("io_bazel/src/bazel"));
-//     std::ifstream data(path);
-//     if (data.is_open()) {
-//       ...  // use the runfile
+//     std::string path = runfiles->Rlocation("io_bazel/src/bazel");
+//     if (!path.empty()) {
+//       std::ifstream data(path);
+//       if (data.is_open()) {
+//         ...  // use the runfile
 //
-// The code above creates a manifest- or directory-based implementations
-// depending on it finding a runfiles manifest or -directory near argv[0] or
-// finding appropriate environment variables that tell it where to find the
-// manifest or directory. See `Runfiles::Create` for more info.
+// The code above creates a Runfiles object and retrieves a runfile path.
 //
-// If you want to start child processes that also need runfiles, you need to set
-// the right environment variables for them:
+// The Runfiles::Create function uses the runfiles manifest and the runfiles
+// directory from the RUNFILES_MANIFEST_FILE and RUNFILES_DIR environment
+// variables. If not present, the function looks for the manifest and directory
+// near argv[0], the path of the main program.
+//
+// To start child processes that also need runfiles, you need to set the right
+// environment variables for them:
 //
 //   std::unique_ptr<Runfiles> runfiles(Runfiles::Create(argv[0], &error));
 //
 //   for (const auto i : runfiles->EnvVars()) {
 //     setenv(i.first, i.second, 1);
 //   }
-//   std::string path(runfiles->Rlocation("path/to/binary"));
+//   std::string path = runfiles->Rlocation("path/to/binary"));
 //   if (!path.empty()) {
 //     pid_t child = fork();
 //     ...
@@ -53,6 +56,7 @@
 #define TOOLS_CPP_RUNFILES_RUNFILES_H_ 1
 
 #include <functional>
+#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -68,39 +72,33 @@ class Runfiles {
 
   // Returns a new `Runfiles` instance.
   //
-  // The returned object is either:
-  // - manifest-based, meaning it looks up runfile paths from a manifest file,
-  //   or
-  // - directory-based, meaning it looks up runfile paths under a given
-  //   directory path
-  //
-  // This method:
-  // 1. checks the RUNFILES_MANIFEST_FILE or RUNFILES_DIR environment variables;
-  //    if either is non-empty, returns a manifest- or directory-based Runfiles
-  //    object; otherwise
-  // 2. checks if there's a runfiles manifest (argv0 + ".runfiles_manifest") or
-  //    runfiles directory (argv0 + ".runfiles") next to this binary; if so,
-  //    returns a manifest- or directory-based Runfiles object; otherwise
-  // 3. returns nullptr.
-  //
-  // The manifest-based Runfiles object eagerly reads and caches the whole
-  // manifest file upon instantiation; this may be relevant for performance
-  // consideration.
-  //
   // Returns nullptr on error. If `error` is provided, the method prints an
   // error message into it.
+  //
+  // This method looks at the RUNFILES_MANIFEST_FILE and RUNFILES_DIR
+  // environment variables. If either is empty, the method looks for the
+  // manifest or directory using the other environment variable, or using argv0.
+  static Runfiles* Create(const std::string& argv0,
+                          std::string* error = nullptr);
+
+  // Returns a new `Runfiles` instance.
+  //
+  // Same as `Create(argv0, error)`, except it uses `runfiles_manifest_file` and
+  // `runfiles_dir` as the corresponding environment variable values, instead of
+  // looking up the actual environment variables.
   static Runfiles* Create(const std::string& argv0,
+                          const std::string& runfiles_manifest_file,
+                          const std::string& runfiles_dir,
                           std::string* error = nullptr);
 
   // Returns the runtime path of a runfile.
   //
   // Runfiles are data-dependencies of Bazel-built binaries and tests.
   //
-  // The returned path may not be valid. The caller should check the path's
-  // validity and that the path exists.
+  // The returned path may not exist. The caller should verify the path's
+  // existence.
   //
-  // The function may return an empty string. In that case the caller can be
-  // sure that the Runfiles object does not know about this data-dependency.
+  // The function may return an empty string if it cannot find a runfile.
   //
   // Args:
   //   path: runfiles-root-relative path of the runfile; must not be empty and
@@ -108,45 +106,32 @@ class Runfiles {
   // Returns:
   //   the path to the runfile, which the caller should check for existence, or
   //   an empty string if the method doesn't know about this runfile
-  virtual std::string Rlocation(const std::string& path) const = 0;
+  std::string Rlocation(const std::string& path) const;
 
   // Returns environment variables for subprocesses.
   //
   // The caller should set the returned key-value pairs in the environment of
-  // subprocesses in case those subprocesses are also Bazel-built binaries that
-  // need to use runfiles.
-  virtual const std::vector<std::pair<std::string, std::string> >& EnvVars()
-      const = 0;
-
-  // Computes the path of the runfiles manifest and the runfiles directory.
-  //
-  // If the method finds both a valid manifest and valid directory according to
-  // `is_runfiles_manifest` and `is_runfiles_directory`, then the method sets
-  // the corresponding values to `out_manifest` and `out_directory` and returns
-  // true.
-  //
-  // If the method only finds a valid manifest or a valid directory, but not
-  // both, then it sets the corresponding output variable (`out_manifest` or
-  // `out_directory`) to the value while clearing the other output variable. The
-  // method still returns true in this case.
-  //
-  // If the method cannot find either a valid manifest or valid directory, it
-  // clears both output variables and returns false.
-  static bool PathsFrom(
-      const std::string& argv0, std::string runfiles_manifest_file,
-      std::string runfiles_dir,
-      std::function<bool(const std::string&)> is_runfiles_manifest,
-      std::function<bool(const std::string&)> is_runfiles_directory,
-      std::string* out_manifest, std::string* out_directory);
-
- protected:
-  Runfiles() {}
+  // subprocesses, so that those subprocesses can also access runfiles (in case
+  // they are also Bazel-built binaries).
+  const std::vector<std::pair<std::string, std::string> >& EnvVars() const {
+    return envvars_;
+  }
 
  private:
+  Runfiles(const std::map<std::string, std::string>&& runfiles_map,
+           const std::string&& directory,
+           const std::vector<std::pair<std::string, std::string> >&& envvars)
+      : runfiles_map_(std::move(runfiles_map)),
+        directory_(std::move(directory)),
+        envvars_(std::move(envvars)) {}
   Runfiles(const Runfiles&) = delete;
   Runfiles(Runfiles&&) = delete;
   Runfiles& operator=(const Runfiles&) = delete;
   Runfiles& operator=(Runfiles&&) = delete;
+
+  const std::map<std::string, std::string> runfiles_map_;
+  const std::string directory_;
+  const std::vector<std::pair<std::string, std::string> > envvars_;
 };
 
 // The "testing" namespace contains functions that allow unit testing the code.
@@ -157,18 +142,26 @@ namespace testing {
 
 // For testing only.
 //
-// Create a new Runfiles instance, looking up environment variables using
-// `env_lookup`.
-//
-// Args:
-//   argv0: name of the binary; if this string is not empty, then the function
-//     looks for a runfiles manifest or directory next to this
-//   env_lookup: a function that returns envvar values if an envvar is known, or
-//     empty string otherwise
-Runfiles* TestOnly_CreateRunfiles(
-    const std::string& argv0,
-    std::function<std::string(const std::string&)> env_lookup,
-    std::string* error);
+// Computes the path of the runfiles manifest and the runfiles directory.
+//
+// If the method finds both a valid manifest and valid directory according to
+// `is_runfiles_manifest` and `is_runfiles_directory`, then the method sets
+// the corresponding values to `out_manifest` and `out_directory` and returns
+// true.
+//
+// If the method only finds a valid manifest or a valid directory, but not
+// both, then it sets the corresponding output variable (`out_manifest` or
+// `out_directory`) to the value while clearing the other output variable. The
+// method still returns true in this case.
+//
+// If the method cannot find either a valid manifest or valid directory, it
+// clears both output variables and returns false.
+bool TestOnly_PathsFrom(
+    const std::string& argv0, std::string runfiles_manifest_file,
+    std::string runfiles_dir,
+    std::function<bool(const std::string&)> is_runfiles_manifest,
+    std::function<bool(const std::string&)> is_runfiles_directory,
+    std::string* out_manifest, std::string* out_directory);
 
 // For testing only.
 // Returns true if `path` is an absolute Unix or Windows path.
diff --git a/tools/cpp/runfiles/runfiles_test.cc b/tools/cpp/runfiles/runfiles_test.cc
index 6b84fa51e0..4d2b1eafca 100644
--- a/tools/cpp/runfiles/runfiles_test.cc
+++ b/tools/cpp/runfiles/runfiles_test.cc
@@ -36,8 +36,8 @@ namespace cpp {
 namespace runfiles {
 namespace {
 
-using bazel::tools::cpp::runfiles::testing::TestOnly_CreateRunfiles;
 using bazel::tools::cpp::runfiles::testing::TestOnly_IsAbsolute;
+using bazel::tools::cpp::runfiles::testing::TestOnly_PathsFrom;
 using std::cerr;
 using std::endl;
 using std::function;
@@ -82,19 +82,8 @@ class RunfilesTest : public ::testing::Test {
                      const string& expected_directory);
 
   static string GetTemp();
-
-  static function<string(const string&)> kEnvWithTestSrcdir;
 };
 
-function<string(const string&)> RunfilesTest::kEnvWithTestSrcdir =
-    [](const string& key) {
-      if (key == "TEST_SRCDIR") {
-        return string("always ignored");
-      } else {
-        return string();
-      }
-    };
-
 void RunfilesTest::AssertEnvvars(const Runfiles& runfiles,
                                  const string& expected_manifest_file,
                                  const string& expected_directory) {
@@ -163,8 +152,7 @@ TEST_F(RunfilesTest, CreatesManifestBasedRunfilesFromManifestNextToBinary) {
       0, mf->Path().size() - string(".runfiles_manifest").size()));
 
   string error;
-  unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles(argv0, kEnvWithTestSrcdir, &error));
+  unique_ptr<Runfiles> r(Runfiles::Create(argv0, "", "", &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
   EXPECT_EQ(r->Rlocation("a/b"), "c/d");
@@ -183,8 +171,7 @@ TEST_F(RunfilesTest,
       0, mf->Path().size() - string(".runfiles/MANIFEST").size()));
 
   string error;
-  unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles(argv0, kEnvWithTestSrcdir, &error));
+  unique_ptr<Runfiles> r(Runfiles::Create(argv0, "", "", &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
   EXPECT_EQ(r->Rlocation("a/b"), "c/d");
@@ -198,20 +185,8 @@ TEST_F(RunfilesTest, CreatesManifestBasedRunfilesFromEnvvar) {
   EXPECT_TRUE(mf != nullptr);
 
   string error;
-  unique_ptr<Runfiles> r(TestOnly_CreateRunfiles(
-      "ignore-argv0",
-      [&mf](const string& key) {
-        if (key == "RUNFILES_MANIFEST_FILE") {
-          return mf->Path();
-        } else if (key == "RUNFILES_DIR") {
-          return string("non-existent");
-        } else if (key == "TEST_SRCDIR") {
-          return string("always ignored");
-        } else {
-          return string();
-        }
-      },
-      &error));
+  unique_ptr<Runfiles> r(Runfiles::Create("ignore-argv0", mf->Path(),
+                                          "non-existent-runfiles_dir", &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
   EXPECT_EQ(r->Rlocation("a/b"), "c/d");
@@ -228,15 +203,7 @@ TEST_F(RunfilesTest, CannotCreateManifestBasedRunfilesDueToBadManifest) {
 
   string error;
   unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles("ignore-argv0",
-                              [&mf](const string& key) {
-                                if (key == "RUNFILES_MANIFEST_FILE") {
-                                  return mf->Path();
-                                } else {
-                                  return string();
-                                }
-                              },
-                              &error));
+      Runfiles::Create("ignore-argv0", mf->Path(), "", &error));
   ASSERT_EQ(r, nullptr);
   EXPECT_NE(error.find("bad runfiles manifest entry"), string::npos);
   EXPECT_NE(error.find("line #2: \"nospace\""), string::npos);
@@ -249,15 +216,7 @@ TEST_F(RunfilesTest, ManifestBasedRunfilesRlocationAndEnvVars) {
 
   string error;
   unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles("ignore-argv0",
-                              [&mf](const string& key) {
-                                if (key == "RUNFILES_MANIFEST_FILE") {
-                                  return mf->Path();
-                                } else {
-                                  return string();
-                                }
-                              },
-                              &error));
+      Runfiles::Create("ignore-argv0", mf->Path(), "", &error));
 
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
@@ -288,15 +247,7 @@ TEST_F(RunfilesTest, DirectoryBasedRunfilesRlocationAndEnvVars) {
   string dir = dummy->DirName();
 
   string error;
-  unique_ptr<Runfiles> r(TestOnly_CreateRunfiles("ignore-argv0",
-                                                 [dir](const string& key) {
-                                                   if (key == "RUNFILES_DIR") {
-                                                     return dir;
-                                                   } else {
-                                                     return string();
-                                                   }
-                                                 },
-                                                 &error));
+  unique_ptr<Runfiles> r(Runfiles::Create("ignore-argv0", "", dir, &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
@@ -329,15 +280,7 @@ TEST_F(RunfilesTest, ManifestAndDirectoryBasedRunfilesRlocationAndEnvVars) {
 
   string error;
   unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles("ignore-argv0",
-                              [&mf](const string& key) {
-                                if (key == "RUNFILES_MANIFEST_FILE") {
-                                  return mf->Path();
-                                } else {
-                                  return string();
-                                }
-                              },
-                              &error));
+      Runfiles::Create("ignore-argv0", mf->Path(), "", &error));
 
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
@@ -373,15 +316,7 @@ TEST_F(RunfilesTest, ManifestBasedRunfilesEnvVars) {
 
     string error;
     unique_ptr<Runfiles> r(
-        TestOnly_CreateRunfiles("ignore-argv0",
-                                [&mf](const string& key) {
-                                  if (key == "RUNFILES_MANIFEST_FILE") {
-                                    return mf->Path();
-                                  } else {
-                                    return string();
-                                  }
-                                },
-                                &error));
+        Runfiles::Create("ignore-argv0", mf->Path(), "", &error));
     if (i < 2) {
       ASSERT_NE(r, nullptr) << " (suffix=\"" << suffixes[i] << "\")";
       EXPECT_TRUE(error.empty());
@@ -404,8 +339,7 @@ TEST_F(RunfilesTest, CreatesDirectoryBasedRunfilesFromDirectoryNextToBinary) {
       0, mf->Path().size() - string(".runfiles/dummy").size()));
 
   string error;
-  unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles(argv0, kEnvWithTestSrcdir, &error));
+  unique_ptr<Runfiles> r(Runfiles::Create(argv0, "", "", &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
@@ -423,18 +357,7 @@ TEST_F(RunfilesTest, CreatesDirectoryBasedRunfilesFromEnvvar) {
   string dir = mf->DirName();
 
   string error;
-  unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles("ignore-argv0",
-                              [dir](const string& key) {
-                                if (key == "RUNFILES_DIR") {
-                                  return dir;
-                                } else if (key == "TEST_SRCDIR") {
-                                  return string("always ignored");
-                                } else {
-                                  return string();
-                                }
-                              },
-                              &error));
+  unique_ptr<Runfiles> r(Runfiles::Create("ignore-argv0", "", dir, &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
@@ -453,39 +376,17 @@ TEST_F(RunfilesTest, FailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined) {
 
   string error;
   unique_ptr<Runfiles> r(
-      TestOnly_CreateRunfiles("ignore-argv0",
-                              [&mf](const string& key) {
-                                if (key == "RUNFILES_MANIFEST_FILE") {
-                                  return mf->Path();
-                                } else if (key == "RUNFILES_DIR") {
-                                  return string("whatever");
-                                } else if (key == "TEST_SRCDIR") {
-                                  return string("always ignored");
-                                } else {
-                                  return string();
-                                }
-                              },
-                              &error));
+      Runfiles::Create("ignore-argv0", mf->Path(), "whatever", &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
   // We create a directory as a side-effect of creating a mock file.
   mf.reset(MockFile::Create(string("foo" LINE() ".runfiles/dummy")));
-  r.reset(TestOnly_CreateRunfiles("ignore-argv0",
-                                  [&mf](const string& key) {
-                                    if (key == "RUNFILES_DIR") {
-                                      return mf->DirName();
-                                    } else if (key == "TEST_SRCDIR") {
-                                      return string("always ignored");
-                                    } else {
-                                      return string();
-                                    }
-                                  },
-                                  &error));
+  r.reset(Runfiles::Create("ignore-argv0", "", mf->DirName(), &error));
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
-  r.reset(TestOnly_CreateRunfiles("ignore-argv0", kEnvWithTestSrcdir, &error));
+  r.reset(Runfiles::Create("ignore-argv0", "", "", &error));
   ASSERT_EQ(r, nullptr);
   EXPECT_NE(error.find("cannot find runfiles"), string::npos);
 }
@@ -581,7 +482,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
   string mf, dir;
 
   // Both envvars have a valid value.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
@@ -590,7 +491,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // RUNFILES_MANIFEST_FILE is invalid but RUNFILES_DIR is good and there's a
   // runfiles manifest in the runfiles directory.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock2/MANIFEST"; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
@@ -599,7 +500,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // RUNFILES_MANIFEST_FILE is invalid but RUNFILES_DIR is good, but there's no
   // runfiles manifest in the runfiles directory.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return false; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
@@ -608,7 +509,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // RUNFILES_DIR is invalid but RUNFILES_MANIFEST_FILE is good, and it is in
   // a valid-looking runfiles directory.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return path == "mock1"; }, &mf, &dir));
@@ -617,7 +518,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // RUNFILES_DIR is invalid but RUNFILES_MANIFEST_FILE is good, but it is not
   // in any valid-looking runfiles directory.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return false; }, &mf, &dir));
@@ -626,7 +527,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // Both envvars are invalid, but there's a manifest in a runfiles directory
   // next to argv0, however there's no other content in the runfiles directory.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles/MANIFEST"; },
       [](const string& path) { return false; }, &mf, &dir));
@@ -635,7 +536,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // Both envvars are invalid, but there's a manifest next to argv0. There's
   // no runfiles tree anywhere.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles_manifest"; },
       [](const string& path) { return false; }, &mf, &dir));
@@ -644,7 +545,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // Both envvars are invalid, but there's a valid manifest next to argv0, and a
   // valid runfiles directory (without a manifest in it).
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles_manifest"; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));
@@ -653,7 +554,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // Both envvars are invalid, but there's a valid runfiles directory next to
   // argv0, though no manifest in it.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return false; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));
@@ -662,7 +563,7 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
 
   // Both envvars are invalid, but there's a valid runfiles directory next to
   // argv0 with a valid manifest in it.
-  EXPECT_TRUE(Runfiles::PathsFrom(
+  EXPECT_TRUE(TestOnly_PathsFrom(
       "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles/MANIFEST"; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));