Skip to content

Commit

Permalink
Automated rollback of commit bfdfa6e. (#14515)
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Due to #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
  • Loading branch information
@meteorcloudy
meteorcloudy authored Jan 7, 2022
1 parent 2b48c6b commit 7c7f102
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 180 deletions.
79 changes: 1 addition & 78 deletions src/test/py/bazel/runfiles_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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"]),
Expand Down Expand Up @@ -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()
5 changes: 0 additions & 5 deletions src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
17 changes: 0 additions & 17 deletions src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh
View file

This file was deleted.

33 changes: 17 additions & 16 deletions src/tools/launcher/launcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
80 changes: 16 additions & 64 deletions tools/cpp/runfiles/runfiles_src.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 7c7f102

Please sign in to comment.