Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x] Automated rollback of commit bfdfa6e. #14515

Merged
merged 1 commit into from
Jan 7, 2022
Merged

[5.x] Automated rollback of commit bfdfa6e. #14515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 1 addition & 78 deletions src/test/py/bazel/runfiles_test.py
View file
Open in desktop
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
Open in desktop
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
Open in desktop

This file was deleted.

33 changes: 17 additions & 16 deletions src/tools/launcher/launcher.cc
View file
Open in desktop
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
Open in desktop
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