Automated rollback of commit bfdfa6ebfd21b388f1c91f512291c848e1a92a96.
*** Reason for rollback ***
Due to https://github.com/bazelbuild/bazel/issues/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
diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py
index 7682ae4..3019636 100644
--- a/src/test/py/bazel/runfiles_test.py
+++ b/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 @@
# 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 @@
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 @@
"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()
diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
index 21ce2e0..de3a5cf 100644
--- a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
+++ b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
@@ -29,8 +29,3 @@
data = ["bar-cc-data.txt"],
deps = ["@bazel_tools//tools/cpp/runfiles"],
)
-
-sh_binary(
- name = "bar-sh-run-under",
- srcs = ["bar-run-under.sh"],
-)
diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh b/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh
deleted file mode 100644
index 57fbf38..0000000
--- a/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/bash
-# Copyright 2018 The Bazel Authors. All rights reserved.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-echo "Hello Bash Bar Run under!"
-"$@"
diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc
index 205618b..2d30bb4 100644
--- a/src/tools/launcher/launcher.cc
+++ b/src/tools/launcher/launcher.cc
@@ -82,9 +82,25 @@
}
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";
+ // 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;
@@ -95,21 +111,6 @@
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 (GetEnv(L"RUNFILES_MANIFEST_FILE", result) &&
- DoesFilePathExist(result->c_str())) {
- return true;
- }
-
- if (GetEnv(L"RUNFILES_DIR", &directory)) {
- *result = directory + L"/MANIFEST";
- if (DoesFilePathExist(result->c_str())) {
- return true;
- }
- }
-
return false;
}
diff --git a/tools/cpp/runfiles/runfiles_src.cc b/tools/cpp/runfiles/runfiles_src.cc
index bc0e4a8..75d606d 100644
--- a/tools/cpp/runfiles/runfiles_src.cc
+++ b/tools/cpp/runfiles/runfiles_src.cc
@@ -93,22 +93,6 @@
#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 @@
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 @@
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;
}
