cpp runfiles library: compute manifest/dir paths

Use the new codepath (Runfiles::PathsFrom) to
compute the paths of the runfiles manifest file
and runfiles directory.

This method considers the RUNFILES_* envvars but
also the argv0 value, and tries hard to discover
where the runfiles manifest or directory is.

See https://github.com/bazelbuild/bazel/issues/4460

Change-Id: I041b9d12f5c4138853fac1c2bf48873af8e2bd93

Closes #5216.

Change-Id: I041b9d12f5c4138853fac1c2bf48873af8e2bd93
PiperOrigin-RevId: 197146764

3 files changed, 45 insertions, 63 deletions

diff --git a/tools/cpp/runfiles/runfiles.cc b/tools/cpp/runfiles/runfiles.cc
index 1e6f0583bc..4032ed95a1 100644
--- a/tools/cpp/runfiles/runfiles.cc
+++ b/tools/cpp/runfiles/runfiles.cc
@@ -156,38 +156,27 @@ bool IsDirectory(const string& path) {
 Runfiles* RunfilesImpl::Create(const string& argv0,
                                function<string(const string&)> env_lookup,
                                string* error) {
-  string manifest(std::move(env_lookup("RUNFILES_MANIFEST_FILE")));
-  if (!manifest.empty()) {
-    return ManifestBased::Create(manifest, error);
+  string manifest, directory;
+  if (!Runfiles::PathsFrom(
+          argv0, env_lookup("RUNFILES_MANIFEST_FILE"),
+          env_lookup("RUNFILES_DIR"),
+          [](const string& path) { return IsReadableFile(path); },
+          [](const string& path) { return IsDirectory(path); }, &manifest,
+          &directory)) {
+    if (error) {
+      std::ostringstream err;
+      err << "ERROR: " << __FILE__ << "(" << __LINE__
+          << "): cannot find runfiles (argv0=\"" << argv0 << "\")";
+      *error = err.str();
+    }
+    return nullptr;
   }
 
-  string directory(std::move(env_lookup("RUNFILES_DIR")));
-  if (!directory.empty()) {
+  if (!manifest.empty()) {
+    return ManifestBased::Create(manifest, error);
+  } else {
     return new DirectoryBased(directory);
   }
-
-  manifest = argv0 + ".runfiles_manifest";
-  if (IsReadableFile(manifest)) {
-    return CreateManifestBased(manifest, error);
-  }
-
-  manifest = argv0 + ".runfiles/MANIFEST";
-  if (IsReadableFile(manifest)) {
-    return CreateManifestBased(manifest, error);
-  }
-
-  directory = argv0 + ".runfiles";
-  if (IsDirectory(directory)) {
-    return CreateDirectoryBased(std::move(directory), error);
-  }
-
-  if (error) {
-    std::ostringstream err;
-    err << "ERROR: " << __FILE__ << "(" << __LINE__
-        << "): cannot find runfiles (argv0=\"" << argv0 << "\")";
-    *error = err.str();
-  }
-  return nullptr;
 }
 
 bool IsAbsolute(const string& path) {
@@ -348,15 +337,12 @@ Runfiles* Runfiles::CreateDirectoryBased(const string& directory_path,
   return new DirectoryBased(directory_path);
 }
 
-bool Runfiles::PathsFrom(const string& argv0,
-                         function<string(string)> env_lookup,
+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) {
   out_manifest->clear();
   out_directory->clear();
-  string mf = env_lookup("RUNFILES_MANIFEST_FILE");
-  string dir = env_lookup("RUNFILES_DIR");
 
   bool mfValid = is_runfiles_manifest(mf);
   bool dirValid = is_runfiles_directory(dir);
diff --git a/tools/cpp/runfiles/runfiles.h b/tools/cpp/runfiles/runfiles.h
index d161ef16e3..32c761ffed 100644
--- a/tools/cpp/runfiles/runfiles.h
+++ b/tools/cpp/runfiles/runfiles.h
@@ -153,8 +153,8 @@ class Runfiles {
   // 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::function<std::string(std::string)> env_lookup,
+      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);
diff --git a/tools/cpp/runfiles/runfiles_test.cc b/tools/cpp/runfiles/runfiles_test.cc
index 02135abfb3..40c405b666 100644
--- a/tools/cpp/runfiles/runfiles_test.cc
+++ b/tools/cpp/runfiles/runfiles_test.cc
@@ -315,12 +315,16 @@ TEST_F(RunfilesTest, CreatesDirectoryBasedRunfilesFromDirectoryNextToBinary) {
 }
 
 TEST_F(RunfilesTest, CreatesDirectoryBasedRunfilesFromEnvvar) {
+  // We create a directory as a side-effect of creating a mock file.
+  unique_ptr<MockFile> mf(MockFile::Create(string("foo" LINE() "/dir/dummy")));
+  string dir = mf->Path().substr(0, mf->Path().size() - 6);
+
   string error;
   unique_ptr<Runfiles> r(
       TestOnly_CreateRunfiles("ignore-argv0",
-                              [](const string& key) {
+                              [dir](const string& key) {
                                 if (key == "RUNFILES_DIR") {
-                                  return string("runfiles/dir");
+                                  return dir;
                                 } else if (key == "TEST_SRCDIR") {
                                   return string("always ignored");
                                 } else {
@@ -331,8 +335,8 @@ TEST_F(RunfilesTest, CreatesDirectoryBasedRunfilesFromEnvvar) {
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
-  EXPECT_EQ(r->Rlocation("a/b"), "runfiles/dir/a/b");
-  EXPECT_EQ(r->Rlocation("foo"), "runfiles/dir/foo");
+  EXPECT_EQ(r->Rlocation("a/b"), dir + "/a/b");
+  EXPECT_EQ(r->Rlocation("foo"), dir + "/foo");
   EXPECT_EQ(r->Rlocation("/Foo"), "/Foo");
   EXPECT_EQ(r->Rlocation("c:/Foo"), "c:/Foo");
   EXPECT_EQ(r->Rlocation("c:\\Foo"), "c:\\Foo");
@@ -359,7 +363,7 @@ TEST_F(RunfilesTest, FailsToCreateManifestBasedBecauseManifestDoesNotExist) {
 }
 
 TEST_F(RunfilesTest, FailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined) {
-  unique_ptr<MockFile> mf(MockFile::Create(string("foo" LINE())));
+  unique_ptr<MockFile> mf(MockFile::Create(string("foo" LINE() "/dir/dummy")));
   EXPECT_TRUE(mf != nullptr);
 
   string error;
@@ -380,10 +384,12 @@ TEST_F(RunfilesTest, FailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined) {
   ASSERT_NE(r, nullptr);
   EXPECT_TRUE(error.empty());
 
+  // We create a directory as a side-effect of creating a mock file.
+  string dir = mf->Path().substr(0, mf->Path().size() - 6);
   r.reset(TestOnly_CreateRunfiles("ignore-argv0",
-                                  [](const string& key) {
+                                  [dir](const string& key) {
                                     if (key == "RUNFILES_DIR") {
-                                      return string("whatever");
+                                      return dir;
                                     } else if (key == "TEST_SRCDIR") {
                                       return string("always ignored");
                                     } else {
@@ -489,21 +495,9 @@ TEST_F(RunfilesTest, IsAbsolute) {
 TEST_F(RunfilesTest, PathsFromEnvVars) {
   string mf, dir;
 
-  static const function<string(string)> kEnvVars = [](string key) {
-    if (key == "TEST_SRCDIR") {
-      return "always ignored";
-    } else if (key == "RUNFILES_MANIFEST_FILE") {
-      return "mock1/MANIFEST";
-    } else if (key == "RUNFILES_DIR") {
-      return "mock2";
-    } else {
-      return "";
-    }
-  };
-
   // Both envvars have a valid value.
   EXPECT_TRUE(Runfiles::PathsFrom(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
   EXPECT_EQ(mf, "mock1/MANIFEST");
@@ -512,7 +506,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock2/MANIFEST"; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
   EXPECT_EQ(mf, "mock2/MANIFEST");
@@ -521,7 +515,8 @@ 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(
-      "argv0", kEnvVars, [](const string& path) { return false; },
+      "argv0", "mock1/MANIFEST", "mock2",
+      [](const string& path) { return false; },
       [](const string& path) { return path == "mock2"; }, &mf, &dir));
   EXPECT_EQ(mf, "");
   EXPECT_EQ(dir, "mock2");
@@ -529,7 +524,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return path == "mock1"; }, &mf, &dir));
   EXPECT_EQ(mf, "mock1/MANIFEST");
@@ -538,7 +533,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "mock1/MANIFEST"; },
       [](const string& path) { return false; }, &mf, &dir));
   EXPECT_EQ(mf, "mock1/MANIFEST");
@@ -547,7 +542,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles/MANIFEST"; },
       [](const string& path) { return false; }, &mf, &dir));
   EXPECT_EQ(mf, "argv0.runfiles/MANIFEST");
@@ -556,7 +551,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles_manifest"; },
       [](const string& path) { return false; }, &mf, &dir));
   EXPECT_EQ(mf, "argv0.runfiles_manifest");
@@ -565,7 +560,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles_manifest"; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));
   EXPECT_EQ(mf, "argv0.runfiles_manifest");
@@ -574,7 +569,8 @@ 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(
-      "argv0", kEnvVars, [](const string& path) { return false; },
+      "argv0", "mock1/MANIFEST", "mock2",
+      [](const string& path) { return false; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));
   EXPECT_EQ(mf, "");
   EXPECT_EQ(dir, "argv0.runfiles");
@@ -582,7 +578,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(
-      "argv0", kEnvVars,
+      "argv0", "mock1/MANIFEST", "mock2",
       [](const string& path) { return path == "argv0.runfiles/MANIFEST"; },
       [](const string& path) { return path == "argv0.runfiles"; }, &mf, &dir));
   EXPECT_EQ(mf, "argv0.runfiles/MANIFEST");