Skip to content

Commit bfdfa6e

Browse files
mai93copybara-github
mai93
authored and
copybara-github
committed
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 in `RUNFILES_MANIFEST_FILE` environment variable if the binary already has its own runfiles manifest file. While, at the same time, allow using the value in the environment variables for binaries run as data-dependency that have no runfiles manifest/directory and should use the main binary's. Fixes: bazelbuild#10598 Closes bazelbuild#12650. PiperOrigin-RevId: 347370712
1 parent c8e179f commit bfdfa6e

5 files changed

Lines changed: 180 additions & 34 deletions

File tree

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

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

1616
import os
17+
import shutil
18+
import stat
1719
import unittest
20+
1821
import six
1922
from src.test.py.bazel import test_base
2023

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

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+
242257
exit_code, stdout, stderr = self.RunProgram(
243258
[bin_path],
244259
env_remove=set(["RUNFILES_DIR"]),
@@ -313,6 +328,68 @@ def testLegacyExternalRunfilesOption(self):
313328
"host/bin/bin.runfiles_manifest")
314329
self.AssertFileContentNotContains(manifest_path, "__main__/external/A")
315330

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+
316393

317394
if __name__ == "__main__":
318395
unittest.main()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,8 @@ 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: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/bin/bash
2+
# Copyright 2018 The Bazel Authors. All rights reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
echo "Hello Bash Bar Run under!"
17+
"$@"

‎src/tools/launcher/launcher.cc‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -73,35 +73,34 @@ BinaryLauncherBase::BinaryLauncherBase(
7373
}
7474

7575
static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
76-
// If this binary X runs as the data-dependency of some other binary Y, then
77-
// X has no runfiles manifest/directory and should use Y's.
76+
// Look for the runfiles manifest of the binary in a runfiles directory next
77+
// to the binary, then look for it (the manifest) next to the binary.
78+
wstring directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
79+
*result = directory + L"/MANIFEST";
80+
if (DoesFilePathExist(result->c_str())) {
81+
return true;
82+
}
83+
84+
*result = directory + L"_manifest";
85+
if (DoesFilePathExist(result->c_str())) {
86+
return true;
87+
}
88+
89+
// If the manifest is not found then this binary (X) runs as the
90+
// data-dependency of some other binary (Y) and X has no runfiles
91+
// manifest/directory so it should use Y's.
7892
if (GetEnv(L"RUNFILES_MANIFEST_FILE", result) &&
7993
DoesFilePathExist(result->c_str())) {
8094
return true;
8195
}
8296

83-
wstring directory;
8497
if (GetEnv(L"RUNFILES_DIR", &directory)) {
8598
*result = directory + L"/MANIFEST";
8699
if (DoesFilePathExist(result->c_str())) {
87100
return true;
88101
}
89102
}
90103

91-
// If this binary X runs by itself (not as a data-dependency of another
92-
// binary), then look for the manifest in a runfiles directory next to the
93-
// main binary, then look for it (the manifest) next to the main binary.
94-
directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
95-
*result = directory + L"/MANIFEST";
96-
if (DoesFilePathExist(result->c_str())) {
97-
return true;
98-
}
99-
100-
*result = directory + L"_manifest";
101-
if (DoesFilePathExist(result->c_str())) {
102-
return true;
103-
}
104-
105104
return false;
106105
}
107106

‎tools/cpp/runfiles/runfiles_src.cc‎

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,22 @@ 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.
96112
bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
97113
std::string runfiles_dir, std::string* out_manifest,
98114
std::string* out_directory);
@@ -103,6 +119,12 @@ bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file,
103119
std::function<bool(const std::string&)> is_runfiles_directory,
104120
std::string* out_manifest, std::string* out_directory);
105121

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+
106128
bool ParseManifest(const string& path, map<string, string>* result,
107129
string* error);
108130

@@ -265,45 +287,71 @@ bool PathsFrom(const string& argv0, string mf, string dir,
265287
out_manifest->clear();
266288
out_directory->clear();
267289

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

271-
if (!argv0.empty() && !mfValid && !dirValid) {
293+
// if argv0 is not empty, try to use it to find the runfiles manifest
294+
// file/directory paths
295+
if (!argv0.empty()) {
272296
mf = argv0 + ".runfiles/MANIFEST";
273297
dir = argv0 + ".runfiles";
274-
mfValid = is_runfiles_manifest(mf);
275-
dirValid = is_runfiles_directory(dir);
276-
if (!mfValid) {
298+
if (!is_runfiles_manifest(mf)) {
277299
mf = argv0 + ".runfiles_manifest";
278-
mfValid = is_runfiles_manifest(mf);
279300
}
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);
280310
}
281311

282-
if (!mfValid && !dirValid) {
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) {
283331
return false;
284332
}
285333

286-
if (!mfValid) {
334+
if (!mf_valid) {
287335
mf = dir + "/MANIFEST";
288-
mfValid = is_runfiles_manifest(mf);
289-
if (!mfValid) {
336+
mf_valid = is_runfiles_manifest(mf);
337+
if (!mf_valid) {
290338
mf = dir + "_manifest";
291-
mfValid = is_runfiles_manifest(mf);
339+
mf_valid = is_runfiles_manifest(mf);
292340
}
293341
}
294342

295-
if (!dirValid &&
343+
if (!dir_valid &&
296344
(ends_with(mf, ".runfiles_manifest") || ends_with(mf, "/MANIFEST"))) {
297345
static const size_t kSubstrLen = 9; // "_manifest" or "/MANIFEST"
298346
dir = mf.substr(0, mf.size() - kSubstrLen);
299-
dirValid = is_runfiles_directory(dir);
347+
dir_valid = is_runfiles_directory(dir);
300348
}
301349

302-
if (mfValid) {
350+
if (mf_valid) {
303351
*out_manifest = mf;
304352
}
305353

306-
if (dirValid) {
354+
if (dir_valid) {
307355
*out_directory = dir;
308356
}
309357

0 commit comments

Comments
 (0)