Skip to content

Commit c63d9ec

Browse files
meteorcloudycopybara-github
meteorcloudy
authored and
copybara-github
committed
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
1 parent 2edab73 commit c63d9ec

File tree

5 files changed

+34
-180
lines changed

5 files changed

+34
-180
lines changed

‎src/test/py/bazel/runfiles_test.py‎

Lines changed: 1 addition & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@
1414
# limitations under the License.
1515

1616
import os
17-
import shutil
18-
import stat
1917
import unittest
20-
2118
import six
2219
from src.test.py.bazel import test_base
2320

@@ -231,7 +228,7 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self):
231228
# runfiles tree, Bazel actually creates empty __init__.py files (again
232229
# on every platform). However to keep these manifest entries correct,
233230
# they need to have a space character.
234-
# We could probably strip these lines completely, but this test doesn't
231+
# We could probably strip thses lines completely, but this test doesn't
235232
# aim to exercise what would happen in that case.
236233
mock_manifest_data = [
237234
mock_manifest_line
@@ -242,18 +239,6 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self):
242239
substitute_manifest = self.ScratchFile(
243240
"mock-%s.runfiles/MANIFEST" % lang[0], mock_manifest_data)
244241

245-
# remove the original manifest file and directory so the launcher picks
246-
# the one in the environment variable RUNFILES_MANIFEST_FILE
247-
manifest_dir = bin_path + ".runfiles"
248-
manifest_dir_file = os.path.join(manifest_dir, "MANIFEST")
249-
os.chmod(manifest_dir_file, stat.S_IRWXU)
250-
shutil.rmtree(manifest_dir)
251-
self.assertFalse(os.path.exists(manifest_dir))
252-
253-
os.chmod(manifest_path, stat.S_IRWXU)
254-
os.remove(manifest_path)
255-
self.assertFalse(os.path.exists(manifest_path))
256-
257242
exit_code, stdout, stderr = self.RunProgram(
258243
[bin_path],
259244
env_remove=set(["RUNFILES_DIR"]),
@@ -328,68 +313,6 @@ def testLegacyExternalRunfilesOption(self):
328313
"host/bin/bin.runfiles_manifest")
329314
self.AssertFileContentNotContains(manifest_path, "__main__/external/A")
330315

331-
def testRunUnderWithRunfiles(self):
332-
for s, t, exe in [
333-
("WORKSPACE.mock", "WORKSPACE", False),
334-
("bar/BUILD.mock", "bar/BUILD", False),
335-
("bar/bar.py", "bar/bar.py", True),
336-
("bar/bar-py-data.txt", "bar/bar-py-data.txt", False),
337-
("bar/Bar.java", "bar/Bar.java", False),
338-
("bar/bar-java-data.txt", "bar/bar-java-data.txt", False),
339-
("bar/bar.sh", "bar/bar.sh", True),
340-
("bar/bar-sh-data.txt", "bar/bar-sh-data.txt", False),
341-
("bar/bar-run-under.sh", "bar/bar-run-under.sh", True),
342-
("bar/bar.cc", "bar/bar.cc", False),
343-
("bar/bar-cc-data.txt", "bar/bar-cc-data.txt", False),
344-
]:
345-
self.CopyFile(
346-
self.Rlocation("io_bazel/src/test/py/bazel/testdata/runfiles_test/" +
347-
s), t, exe)
348-
349-
exit_code, stdout, stderr = self.RunBazel(["info", "bazel-bin"])
350-
self.AssertExitCode(exit_code, 0, stderr)
351-
bazel_bin = stdout[0]
352-
353-
# build bar-sh-run-under target to run targets under it
354-
exit_code, _, stderr = self.RunBazel(
355-
["build", "--verbose_failures", "//bar:bar-sh-run-under"])
356-
self.AssertExitCode(exit_code, 0, stderr)
357-
358-
if test_base.TestBase.IsWindows():
359-
run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under.exe")
360-
else:
361-
run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under")
362-
363-
self.assertTrue(os.path.exists(run_under_bin_path))
364-
365-
for lang in [("py", "Python", "bar.py"), ("java", "Java", "Bar.java"),
366-
("sh", "Bash", "bar.sh"), ("cc", "C++", "bar.cc")]:
367-
exit_code, stdout, stderr = self.RunBazel([
368-
"run", "--verbose_failures", "--run_under=//bar:bar-sh-run-under",
369-
"//bar:bar-" + lang[0]
370-
])
371-
self.AssertExitCode(exit_code, 0, stderr)
372-
373-
if test_base.TestBase.IsWindows():
374-
bin_path = os.path.join(bazel_bin, "bar/bar-%s.exe" % lang[0])
375-
else:
376-
bin_path = os.path.join(bazel_bin, "bar/bar-" + lang[0])
377-
378-
self.assertTrue(os.path.exists(bin_path))
379-
380-
if len(stdout) < 3:
381-
self.fail("stdout(%s): %s" % (lang[0], stdout))
382-
383-
self.assertEqual(stdout[0], "Hello Bash Bar Run under!")
384-
self.assertEqual(stdout[1], "Hello %s Bar!" % lang[1])
385-
six.assertRegex(self, stdout[2], "^rloc=.*/bar/bar-%s-data.txt" % lang[0])
386-
387-
with open(stdout[2].split("=", 1)[1], "r") as f:
388-
lines = [l.strip() for l in f.readlines()]
389-
if len(lines) != 1:
390-
self.fail("lines(%s): %s" % (lang[0], lines))
391-
self.assertEqual(lines[0], "data for " + lang[2])
392-
393316

394317
if __name__ == "__main__":
395318
unittest.main()

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,3 @@ cc_binary(
2929
data = ["bar-cc-data.txt"],
3030
deps = ["@bazel_tools//tools/cpp/runfiles"],
3131
)
32-
33-
sh_binary(
34-
name = "bar-sh-run-under",
35-
srcs = ["bar-run-under.sh"],
36-
)

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

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎src/tools/launcher/launcher.cc‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -82,34 +82,35 @@ BinaryLauncherBase::BinaryLauncherBase(
8282
}
8383

8484
static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
85-
// Look for the runfiles manifest of the binary in a runfiles directory next
86-
// to the binary, then look for it (the manifest) next to the binary.
87-
wstring directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
88-
*result = directory + L"/MANIFEST";
89-
if (DoesFilePathExist(result->c_str())) {
90-
return true;
91-
}
92-
93-
*result = directory + L"_manifest";
94-
if (DoesFilePathExist(result->c_str())) {
95-
return true;
96-
}
97-
98-
// If the manifest is not found then this binary (X) runs as the
99-
// data-dependency of some other binary (Y) and X has no runfiles
100-
// manifest/directory so it should use Y's.
85+
// If this binary X runs as the data-dependency of some other binary Y, then
86+
// X has no runfiles manifest/directory and should use Y's.
10187
if (GetEnv(L"RUNFILES_MANIFEST_FILE", result) &&
10288
DoesFilePathExist(result->c_str())) {
10389
return true;
10490
}
10591

92+
wstring directory;
10693
if (GetEnv(L"RUNFILES_DIR", &directory)) {
10794
*result = directory + L"/MANIFEST";
10895
if (DoesFilePathExist(result->c_str())) {
10996
return true;
11097
}
11198
}
11299

100+
// If this binary X runs by itself (not as a data-dependency of another
101+
// binary), then look for the manifest in a runfiles directory next to the
102+
// main binary, then look for it (the manifest) next to the main binary.
103+
directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
104+
*result = directory + L"/MANIFEST";
105+
if (DoesFilePathExist(result->c_str())) {
106+
return true;
107+
}
108+
109+
*result = directory + L"_manifest";
110+
if (DoesFilePathExist(result->c_str())) {
111+
return true;
112+
}
113+
113114
return false;
114115
}
115116

‎tools/cpp/runfiles/runfiles_src.cc‎

Lines changed: 16 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -93,22 +93,6 @@ bool IsDirectory(const string& path) {
9393
#endif
9494
}
9595

96-
// Computes the path of the runfiles manifest and the runfiles directory.
97-
// By searching first next to the binary location `argv0` and then falling
98-
// back on the values passed in `runfiles_manifest_file` and `runfiles_dir`
99-
//
100-
// If the method finds both a valid manifest and valid directory according to
101-
// `is_runfiles_manifest` and `is_runfiles_directory`, then the method sets
102-
// the corresponding values to `out_manifest` and `out_directory` and returns
103-
// true.
104-
//
105-
// If the method only finds a valid manifest or a valid directory, but not
106-
// both, then it sets the corresponding output variable (`out_manifest` or
107-
// `out_directory`) to the value while clearing the other output variable. The
108-
// method still returns true in this case.
109-
//
110-
// If the method cannot find either a valid manifest or valid directory, it
111-
// clears both output variables and returns false.
11296
bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
11397
std::string runfiles_dir, std::string* out_manifest,
11498
std::string* out_directory);
@@ -119,12 +103,6 @@ bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
119103
std::function<bool(const std::string&)> is_runfiles_directory,
120104
std::string* out_manifest, std::string* out_directory);
121105

122-
bool PathsFrom(const std::string& runfiles_manifest_file,
123-
const std::string& runfiles_dir,
124-
std::function<bool(const std::string&)> is_runfiles_manifest,
125-
std::function<bool(const std::string&)> is_runfiles_directory,
126-
std::string* out_manifest, std::string* out_directory);
127-
128106
bool ParseManifest(const string& path, map<string, string>* result,
129107
string* error);
130108

@@ -287,71 +265,45 @@ bool PathsFrom(const string& argv0, string mf, string dir,
287265
out_manifest->clear();
288266
out_directory->clear();
289267

290-
string existing_mf = mf;
291-
string existing_dir = dir;
268+
bool mfValid = is_runfiles_manifest(mf);
269+
bool dirValid = is_runfiles_directory(dir);
292270

293-
// if argv0 is not empty, try to use it to find the runfiles manifest
294-
// file/directory paths
295-
if (!argv0.empty()) {
271+
if (!argv0.empty() && !mfValid && !dirValid) {
296272
mf = argv0 + ".runfiles/MANIFEST";
297273
dir = argv0 + ".runfiles";
298-
if (!is_runfiles_manifest(mf)) {
274+
mfValid = is_runfiles_manifest(mf);
275+
dirValid = is_runfiles_directory(dir);
276+
if (!mfValid) {
299277
mf = argv0 + ".runfiles_manifest";
278+
mfValid = is_runfiles_manifest(mf);
300279
}
301-
PathsFrom(mf, dir, is_runfiles_manifest, is_runfiles_directory,
302-
out_manifest, out_directory);
303-
}
304-
305-
// if the runfiles manifest file/directory paths are not found, use existing
306-
// mf and dir to find the paths
307-
if (out_manifest->empty() && out_directory->empty()) {
308-
return PathsFrom(existing_mf, existing_dir, is_runfiles_manifest,
309-
is_runfiles_directory, out_manifest, out_directory);
310280
}
311281

312-
return true;
313-
}
314-
315-
bool PathsFrom(const std::string& runfiles_manifest,
316-
const std::string& runfiles_directory,
317-
function<bool(const std::string&)> is_runfiles_manifest,
318-
function<bool(const std::string&)> is_runfiles_directory,
319-
std::string* out_manifest, std::string* out_directory) {
320-
out_manifest->clear();
321-
out_directory->clear();
322-
323-
324-
std::string mf = runfiles_manifest;
325-
std::string dir = runfiles_directory;
326-
327-
bool mf_valid = is_runfiles_manifest(mf);
328-
bool dir_valid = is_runfiles_directory(dir);
329-
330-
if (!mf_valid && !dir_valid) {
282+
if (!mfValid && !dirValid) {
331283
return false;
332284
}
333285

334-
if (!mf_valid) {
286+
if (!mfValid) {
335287
mf = dir + "/MANIFEST";
336-
mf_valid = is_runfiles_manifest(mf);
337-
if (!mf_valid) {
288+
mfValid = is_runfiles_manifest(mf);
289+
if (!mfValid) {
338290
mf = dir + "_manifest";
339-
mf_valid = is_runfiles_manifest(mf);
291+
mfValid = is_runfiles_manifest(mf);
340292
}
341293
}
342294

343-
if (!dir_valid &&
295+
if (!dirValid &&
344296
(ends_with(mf, ".runfiles_manifest") || ends_with(mf, "/MANIFEST"))) {
345297
static const size_t kSubstrLen = 9; // "_manifest" or "/MANIFEST"
346298
dir = mf.substr(0, mf.size() - kSubstrLen);
347-
dir_valid = is_runfiles_directory(dir);
299+
dirValid = is_runfiles_directory(dir);
348300
}
349301

350-
if (mf_valid) {
302+
if (mfValid) {
351303
*out_manifest = mf;
352304
}
353305

354-
if (dir_valid) {
306+
if (dirValid) {
355307
*out_directory = dir;
356308
}
357309

0 commit comments

Comments
 (0)