Automated rollback of commit bfdfa6e.

*** Reason for rollback ***

Due to bazelbuild#14500

*** Original change description ***

Update find runfiles manifest file logic

This pull request updates the `FindManifestFileImpl` function to look for the runfiles manifest file of the binary first. If it is not found, it can use the manifest file specified in the environment variables `RUNFILES_MANIFEST_FILE` if it exists. The same logic is applied in `runfiles_src.cc` to find runfiles manifest file / directory of cc binaries.

This is done to avoid using the manifest file...

***

PiperOrigin-RevId: 419548921

Author: meteorcloudy

src/test/py/bazel/runfiles_test.py

@@ -14,10 +14,7 @@
 # limitations under the License.
 
 import os
-import shutil
-import stat
 import unittest
-
 import six
 from src.test.py.bazel import test_base
 
@@ -231,7 +228,7 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self):
         # runfiles tree, Bazel actually creates empty __init__.py files (again
         # on every platform). However to keep these manifest entries correct,
         # they need to have a space character.
-        # We could probably strip these lines completely, but this test doesn't
+        # We could probably strip thses lines completely, but this test doesn't
         # aim to exercise what would happen in that case.
         mock_manifest_data = [
             mock_manifest_line
@@ -242,18 +239,6 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self):
       substitute_manifest = self.ScratchFile(
           "mock-%s.runfiles/MANIFEST" % lang[0], mock_manifest_data)
 
-      # remove the original manifest file and directory so the launcher picks
-      # the one in the environment variable RUNFILES_MANIFEST_FILE
-      manifest_dir = bin_path + ".runfiles"
-      manifest_dir_file = os.path.join(manifest_dir, "MANIFEST")
-      os.chmod(manifest_dir_file, stat.S_IRWXU)
-      shutil.rmtree(manifest_dir)
-      self.assertFalse(os.path.exists(manifest_dir))
-
-      os.chmod(manifest_path, stat.S_IRWXU)
-      os.remove(manifest_path)
-      self.assertFalse(os.path.exists(manifest_path))
-
       exit_code, stdout, stderr = self.RunProgram(
           [bin_path],
           env_remove=set(["RUNFILES_DIR"]),
@@ -328,68 +313,6 @@ def testLegacyExternalRunfilesOption(self):
                                    "host/bin/bin.runfiles_manifest")
     self.AssertFileContentNotContains(manifest_path, "__main__/external/A")
 
-  def testRunUnderWithRunfiles(self):
-    for s, t, exe in [
-        ("WORKSPACE.mock", "WORKSPACE", False),
-        ("bar/BUILD.mock", "bar/BUILD", False),
-        ("bar/bar.py", "bar/bar.py", True),
-        ("bar/bar-py-data.txt", "bar/bar-py-data.txt", False),
-        ("bar/Bar.java", "bar/Bar.java", False),
-        ("bar/bar-java-data.txt", "bar/bar-java-data.txt", False),
-        ("bar/bar.sh", "bar/bar.sh", True),
-        ("bar/bar-sh-data.txt", "bar/bar-sh-data.txt", False),
-        ("bar/bar-run-under.sh", "bar/bar-run-under.sh", True),
-        ("bar/bar.cc", "bar/bar.cc", False),
-        ("bar/bar-cc-data.txt", "bar/bar-cc-data.txt", False),
-    ]:
-      self.CopyFile(
-          self.Rlocation("io_bazel/src/test/py/bazel/testdata/runfiles_test/" +
-                         s), t, exe)
-
-    exit_code, stdout, stderr = self.RunBazel(["info", "bazel-bin"])
-    self.AssertExitCode(exit_code, 0, stderr)
-    bazel_bin = stdout[0]
-
-    # build bar-sh-run-under target to run targets under it
-    exit_code, _, stderr = self.RunBazel(
-        ["build", "--verbose_failures", "//bar:bar-sh-run-under"])
-    self.AssertExitCode(exit_code, 0, stderr)
-
-    if test_base.TestBase.IsWindows():
-      run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under.exe")
-    else:
-      run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under")
-
-    self.assertTrue(os.path.exists(run_under_bin_path))
-
-    for lang in [("py", "Python", "bar.py"), ("java", "Java", "Bar.java"),
-                 ("sh", "Bash", "bar.sh"), ("cc", "C++", "bar.cc")]:
-      exit_code, stdout, stderr = self.RunBazel([
-          "run", "--verbose_failures", "--run_under=//bar:bar-sh-run-under",
-          "//bar:bar-" + lang[0]
-      ])
-      self.AssertExitCode(exit_code, 0, stderr)
-
-      if test_base.TestBase.IsWindows():
-        bin_path = os.path.join(bazel_bin, "bar/bar-%s.exe" % lang[0])
-      else:
-        bin_path = os.path.join(bazel_bin, "bar/bar-" + lang[0])
-
-      self.assertTrue(os.path.exists(bin_path))
-
-      if len(stdout) < 3:
-        self.fail("stdout(%s): %s" % (lang[0], stdout))
-
-      self.assertEqual(stdout[0], "Hello Bash Bar Run under!")
-      self.assertEqual(stdout[1], "Hello %s Bar!" % lang[1])
-      six.assertRegex(self, stdout[2], "^rloc=.*/bar/bar-%s-data.txt" % lang[0])
-
-      with open(stdout[2].split("=", 1)[1], "r") as f:
-        lines = [l.strip() for l in f.readlines()]
-      if len(lines) != 1:
-        self.fail("lines(%s): %s" % (lang[0], lines))
-      self.assertEqual(lines[0], "data for " + lang[2])
-
 
 if __name__ == "__main__":
   unittest.main()

src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock

@@ -29,8 +29,3 @@ cc_binary(
     data = ["bar-cc-data.txt"],
     deps = ["@bazel_tools//tools/cpp/runfiles"],
 )
-
-sh_binary(
-    name = "bar-sh-run-under",
-    srcs = ["bar-run-under.sh"],
-)

src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh

@@ -82,34 +82,35 @@ BinaryLauncherBase::BinaryLauncherBase(
 }
 
 static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
-  // Look for the runfiles manifest of the binary in a runfiles directory next
-  // to the binary, then look for it (the manifest) next to the binary.
-  wstring directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
-  *result = directory + L"/MANIFEST";
-  if (DoesFilePathExist(result->c_str())) {
-    return true;
-  }
-
-  *result = directory + L"_manifest";
-  if (DoesFilePathExist(result->c_str())) {
-    return true;
-  }
-
-  // If the manifest is not found then this binary (X) runs as the
-  // data-dependency of some other binary (Y) and X has no runfiles
-  // manifest/directory so it should use Y's.
+  // If this binary X runs as the data-dependency of some other binary Y, then
+  // X has no runfiles manifest/directory and should use Y's.
   if (GetEnv(L"RUNFILES_MANIFEST_FILE", result) &&
       DoesFilePathExist(result->c_str())) {
     return true;
   }
 
+  wstring directory;
   if (GetEnv(L"RUNFILES_DIR", &directory)) {
     *result = directory + L"/MANIFEST";
     if (DoesFilePathExist(result->c_str())) {
       return true;
     }
   }
 
+  // If this binary X runs by itself (not as a data-dependency of another
+  // binary), then look for the manifest in a runfiles directory next to the
+  // main binary, then look for it (the manifest) next to the main binary.
+  directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
+  *result = directory + L"/MANIFEST";
+  if (DoesFilePathExist(result->c_str())) {
+    return true;
+  }
+
+  *result = directory + L"_manifest";
+  if (DoesFilePathExist(result->c_str())) {
+    return true;
+  }
+
   return false;
 }
 

src/tools/launcher/launcher.cc

@@ -93,22 +93,6 @@ bool IsDirectory(const string& path) {
 #endif
 }
 
-// Computes the path of the runfiles manifest and the runfiles directory.
-// By searching first next to the binary location `argv0` and then falling
-// back on the values passed in `runfiles_manifest_file` and `runfiles_dir`
-//
-// 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 PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
                std::string runfiles_dir, std::string* out_manifest,
                std::string* out_directory);
@@ -119,12 +103,6 @@ bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
                std::function<bool(const std::string&)> is_runfiles_directory,
                std::string* out_manifest, std::string* out_directory);
 
-bool PathsFrom(const std::string& runfiles_manifest_file,
-               const 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);
 
@@ -287,71 +265,45 @@ bool PathsFrom(const string& argv0, string mf, string dir,
   out_manifest->clear();
   out_directory->clear();
 
-  string existing_mf = mf;
-  string existing_dir = dir;
+  bool mfValid = is_runfiles_manifest(mf);
+  bool dirValid = is_runfiles_directory(dir);
 
-  // if argv0 is not empty, try to use it to find the runfiles manifest
-  // file/directory paths
-  if (!argv0.empty()) {
+  if (!argv0.empty() && !mfValid && !dirValid) {
     mf = argv0 + ".runfiles/MANIFEST";
     dir = argv0 + ".runfiles";
-    if (!is_runfiles_manifest(mf)) {
+    mfValid = is_runfiles_manifest(mf);
+    dirValid = is_runfiles_directory(dir);
+    if (!mfValid) {
       mf = argv0 + ".runfiles_manifest";
+      mfValid = is_runfiles_manifest(mf);
     }
-    PathsFrom(mf, dir, is_runfiles_manifest, is_runfiles_directory,
-              out_manifest, out_directory);
-  }
-
-  // if the runfiles manifest file/directory paths are not found, use existing
-  // mf and dir to find the paths
-  if (out_manifest->empty() && out_directory->empty()) {
-    return PathsFrom(existing_mf, existing_dir, is_runfiles_manifest,
-                     is_runfiles_directory, out_manifest, out_directory);
   }
 
-  return true;
-}
-
-bool PathsFrom(const std::string& runfiles_manifest,
-               const std::string& runfiles_directory,
-               function<bool(const std::string&)> is_runfiles_manifest,
-               function<bool(const std::string&)> is_runfiles_directory,
-               std::string* out_manifest, std::string* out_directory) {
-  out_manifest->clear();
-  out_directory->clear();
-
-
-  std::string mf = runfiles_manifest;
-  std::string dir = runfiles_directory;
-
-  bool mf_valid = is_runfiles_manifest(mf);
-  bool dir_valid = is_runfiles_directory(dir);
-
-  if (!mf_valid && !dir_valid) {
+  if (!mfValid && !dirValid) {
     return false;
   }
 
-  if (!mf_valid) {
+  if (!mfValid) {
     mf = dir + "/MANIFEST";
-    mf_valid = is_runfiles_manifest(mf);
-    if (!mf_valid) {
+    mfValid = is_runfiles_manifest(mf);
+    if (!mfValid) {
       mf = dir + "_manifest";
-      mf_valid = is_runfiles_manifest(mf);
+      mfValid = is_runfiles_manifest(mf);
     }
   }
 
-  if (!dir_valid &&
+  if (!dirValid &&
       (ends_with(mf, ".runfiles_manifest") || ends_with(mf, "/MANIFEST"))) {
     static const size_t kSubstrLen = 9;  // "_manifest" or "/MANIFEST"
     dir = mf.substr(0, mf.size() - kSubstrLen);
-    dir_valid = is_runfiles_directory(dir);
+    dirValid = is_runfiles_directory(dir);
   }
 
-  if (mf_valid) {
+  if (mfValid) {
     *out_manifest = mf;
   }
 
-  if (dir_valid) {
+  if (dirValid) {
     *out_directory = dir;
   }