Skip to content

Commit 486d153

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Find runfiles in directories that are themselves runfiles
When a target has a runfile that is contained in a directory that is itself one of its runfiles, the runfile will be shadowed by the `SymlinkEntry` for the directory. While this still allows to resolve the file in the runfiles symlink tree, a manifest-based lookup will fail. Since the manifest is used to create the symlink tree for which it is important that no path is a prefix of another, this can only be fixed in the runfiles libraries. This PR extends the lookup logic of the Bash, C++, Java, and Python runfiles libraries to also find runfiles contained within directories that are themselves runfiles. It does so by searching the manifest not only for the exact provided rlocation path, but also for all path prefixes. If a prefix is looked up successfully, the corresponding suffix is resolved relative to the looked up path. Fixes #14336. Closes #14335. PiperOrigin-RevId: 426894517
1 parent c23410c commit 486d153

File tree

8 files changed

+145
-36
lines changed

8 files changed

+145
-36
lines changed

‎tools/bash/runfiles/runfiles.bash

+41-6
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,51 @@ function rlocation() {
122122
echo >&2 "INFO[runfiles.bash]: rlocation($1): looking in RUNFILES_MANIFEST_FILE ($RUNFILES_MANIFEST_FILE)"
123123
fi
124124
local -r result=$(grep -m1 "^$1 " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
125-
if [[ -e "${result:-/dev/null}" ]]; then
126-
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
127-
echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($result)"
128-
fi
129-
echo "$result"
130-
else
125+
if [[ -z "$result" ]]; then
126+
# If path references a runfile that lies under a directory that itself
127+
# is a runfile, then only the directory is listed in the manifest. Look
128+
# up all prefixes of path in the manifest and append the relative path
129+
# from the prefix if there is a match.
130+
local prefix="$1"
131+
local prefix_result=
132+
local new_prefix=
133+
while true; do
134+
new_prefix="${prefix%/*}"
135+
[[ "$new_prefix" == "$prefix" ]] && break
136+
prefix="$new_prefix"
137+
prefix_result=$(grep -m1 "^$prefix " "${RUNFILES_MANIFEST_FILE}" | cut -d ' ' -f 2-)
138+
[[ -z "$prefix_result" ]] && continue
139+
local -r candidate="${prefix_result}${1#"${prefix}"}"
140+
if [[ -e "$candidate" ]]; then
141+
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
142+
echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($candidate) via prefix ($prefix)"
143+
fi
144+
echo "$candidate"
145+
return 0
146+
fi
147+
# At this point, the manifest lookup of prefix has been successful,
148+
# but the file at the relative path given by the suffix does not
149+
# exist. We do not continue the lookup with a shorter prefix for two
150+
# reasons:
151+
# 1. Manifests generated by Bazel never contain a path that is a
152+
# prefix of another path.
153+
# 2. Runfiles libraries for other languages do not check for file
154+
# existence and would have returned the non-existent path. It seems
155+
# better to return no path rather than a potentially different,
156+
# non-empty path.
157+
break
158+
done
131159
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
132160
echo >&2 "INFO[runfiles.bash]: rlocation($1): not found in manifest"
133161
fi
134162
echo ""
163+
else
164+
if [[ -e "$result" ]]; then
165+
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then
166+
echo >&2 "INFO[runfiles.bash]: rlocation($1): found in manifest as ($result)"
167+
fi
168+
echo "$result"
169+
fi
135170
fi
136171
else
137172
if [[ "${RUNFILES_LIB_DEBUG:-}" == 1 ]]; then

‎tools/bash/runfiles/runfiles_test.bash

+13-1
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,14 @@ function test_init_manifest_based_runfiles() {
139139
a/b $tmpdir/c/d
140140
e/f $tmpdir/g h
141141
y $tmpdir/y
142+
c/dir $tmpdir/dir
142143
EOF
143144
mkdir "${tmpdir}/c"
144145
mkdir "${tmpdir}/y"
146+
mkdir -p "${tmpdir}/dir/deeply/nested"
145147
touch "${tmpdir}/c/d" "${tmpdir}/g h"
148+
touch "${tmpdir}/dir/file"
149+
touch "${tmpdir}/dir/deeply/nested/file"
146150

147151
export RUNFILES_DIR=
148152
export RUNFILES_MANIFEST_FILE=$tmpdir/foo.runfiles_manifest
@@ -153,10 +157,18 @@ EOF
153157
[[ "$(rlocation a/b)" == "$tmpdir/c/d" ]] || fail
154158
[[ "$(rlocation e/f)" == "$tmpdir/g h" ]] || fail
155159
[[ "$(rlocation y)" == "$tmpdir/y" ]] || fail
156-
rm -r "$tmpdir/c/d" "$tmpdir/g h" "$tmpdir/y"
160+
[[ "$(rlocation c)" == "" ]] || fail
161+
[[ "$(rlocation c/di)" == "" ]] || fail
162+
[[ "$(rlocation c/dir)" == "$tmpdir/dir" ]] || fail
163+
[[ "$(rlocation c/dir/file)" == "$tmpdir/dir/file" ]] || fail
164+
[[ "$(rlocation c/dir/deeply/nested/file)" == "$tmpdir/dir/deeply/nested/file" ]] || fail
165+
rm -r "$tmpdir/c/d" "$tmpdir/g h" "$tmpdir/y" "$tmpdir/dir"
157166
[[ -z "$(rlocation a/b)" ]] || fail
158167
[[ -z "$(rlocation e/f)" ]] || fail
159168
[[ -z "$(rlocation y)" ]] || fail
169+
[[ -z "$(rlocation c/dir)" ]] || fail
170+
[[ -z "$(rlocation c/dir/file)" ]] || fail
171+
[[ -z "$(rlocation c/dir/deeply/nested/file)" ]] || fail
160172
}
161173

162174
function test_manifest_based_envvars() {

‎tools/cpp/runfiles/runfiles_src.cc

+19-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <unistd.h>
2626
#endif // _WIN32
2727

28+
#include <algorithm>
2829
#include <fstream>
2930
#include <functional>
3031
#include <map>
@@ -176,9 +177,24 @@ string Runfiles::Rlocation(const string& path) const {
176177
if (IsAbsolute(path)) {
177178
return path;
178179
}
179-
const auto value = runfiles_map_.find(path);
180-
if (value != runfiles_map_.end()) {
181-
return value->second;
180+
const auto exact_match = runfiles_map_.find(path);
181+
if (exact_match != runfiles_map_.end()) {
182+
return exact_match->second;
183+
}
184+
if (!runfiles_map_.empty()) {
185+
// If path references a runfile that lies under a directory that itself is a
186+
// runfile, then only the directory is listed in the manifest. Look up all
187+
// prefixes of path in the manifest and append the relative path from the
188+
// prefix to the looked up path.
189+
std::size_t prefix_end = path.size();
190+
while ((prefix_end = path.find_last_of('/', prefix_end - 1)) !=
191+
string::npos) {
192+
const string prefix = path.substr(0, prefix_end);
193+
const auto prefix_match = runfiles_map_.find(prefix);
194+
if (prefix_match != runfiles_map_.end()) {
195+
return prefix_match->second + "/" + path.substr(prefix_end + 1);
196+
}
197+
}
182198
}
183199
if (!directory_.empty()) {
184200
return directory_ + "/" + path;

‎tools/cpp/runfiles/runfiles_test.cc

+4
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ TEST_F(RunfilesTest, ManifestBasedRunfilesRlocationAndEnvVars) {
252252
EXPECT_EQ(r->Rlocation("/Foo"), "/Foo");
253253
EXPECT_EQ(r->Rlocation("c:/Foo"), "c:/Foo");
254254
EXPECT_EQ(r->Rlocation("c:\\Foo"), "c:\\Foo");
255+
EXPECT_EQ(r->Rlocation("a/b/file"), "c/d/file");
256+
EXPECT_EQ(r->Rlocation("a/b/deeply/nested/file"), "c/d/deeply/nested/file");
255257
}
256258

257259
TEST_F(RunfilesTest, DirectoryBasedRunfilesRlocationAndEnvVars) {
@@ -316,6 +318,8 @@ TEST_F(RunfilesTest, ManifestAndDirectoryBasedRunfilesRlocationAndEnvVars) {
316318
EXPECT_EQ(r->Rlocation("/Foo"), "/Foo");
317319
EXPECT_EQ(r->Rlocation("c:/Foo"), "c:/Foo");
318320
EXPECT_EQ(r->Rlocation("c:\\Foo"), "c:\\Foo");
321+
EXPECT_EQ(r->Rlocation("a/b/file"), "c/d/file");
322+
EXPECT_EQ(r->Rlocation("a/b/deeply/nested/file"), "c/d/deeply/nested/file");
319323
AssertEnvvars(*r, mf->Path(), dir);
320324
}
321325

‎tools/java/runfiles/Runfiles.java

+15-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,21 @@ private static String findRunfilesDir(String manifest) {
227227

228228
@Override
229229
public String rlocationChecked(String path) {
230-
return runfiles.get(path);
230+
String exactMatch = runfiles.get(path);
231+
if (exactMatch != null) {
232+
return exactMatch;
233+
}
234+
// If path references a runfile that lies under a directory that itself is a runfile, then
235+
// only the directory is listed in the manifest. Look up all prefixes of path in the manifest
236+
// and append the relative path from the prefix if there is a match.
237+
int prefixEnd = path.length();
238+
while ((prefixEnd = path.lastIndexOf('/', prefixEnd - 1)) != -1) {
239+
String prefixMatch = runfiles.get(path.substring(0, prefixEnd));
240+
if (prefixMatch != null) {
241+
return prefixMatch + '/' + path.substring(prefixEnd + 1);
242+
}
243+
}
244+
return null;
231245
}
232246

233247
@Override

‎tools/java/runfiles/testing/RunfilesTest.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,15 @@ public void testManifestBasedRlocation() throws Exception {
249249
new MockFile(
250250
ImmutableList.of(
251251
"Foo/runfile1 C:/Actual Path\\runfile1",
252-
"Foo/Bar/runfile2 D:\\the path\\run file 2.txt"))) {
252+
"Foo/Bar/runfile2 D:\\the path\\run file 2.txt",
253+
"Foo/Bar/Dir E:\\Actual Path\\Directory"))) {
253254
Runfiles r = Runfiles.createManifestBasedForTesting(mf.path.toString());
254255
assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1");
255256
assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt");
257+
assertThat(r.rlocation("Foo/Bar/Dir")).isEqualTo("E:\\Actual Path\\Directory");
258+
assertThat(r.rlocation("Foo/Bar/Dir/File")).isEqualTo("E:\\Actual Path\\Directory/File");
259+
assertThat(r.rlocation("Foo/Bar/Dir/Deeply/Nested/File"))
260+
.isEqualTo("E:\\Actual Path\\Directory/Deeply/Nested/File");
256261
assertThat(r.rlocation("unknown")).isNull();
257262
}
258263
}

‎tools/python/runfiles/runfiles.py

+16-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,22 @@ def __init__(self, path):
174174
self._runfiles = _ManifestBased._LoadRunfiles(path)
175175

176176
def RlocationChecked(self, path):
177-
return self._runfiles.get(path)
177+
"""Returns the runtime path of a runfile."""
178+
exact_match = self._runfiles.get(path)
179+
if exact_match:
180+
return exact_match
181+
# If path references a runfile that lies under a directory that itself is a
182+
# runfile, then only the directory is listed in the manifest. Look up all
183+
# prefixes of path in the manifest and append the relative path from the
184+
# prefix to the looked up path.
185+
prefix_end = len(path)
186+
while True:
187+
prefix_end = path.rfind("/", 0, prefix_end - 1)
188+
if prefix_end == -1:
189+
return None
190+
prefix_match = self._runfiles.get(path[0:prefix_end])
191+
if prefix_match:
192+
return prefix_match + "/" + path[prefix_end + 1:]
178193

179194
@staticmethod
180195
def _LoadRunfiles(path):

‎tools/python/runfiles/runfiles_test.py

+31-23
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,26 @@ def testRlocationArgumentValidation(self):
2828
self.assertRaises(ValueError, lambda: r.Rlocation(None))
2929
self.assertRaises(ValueError, lambda: r.Rlocation(""))
3030
self.assertRaises(TypeError, lambda: r.Rlocation(1))
31-
self.assertRaisesRegexp(ValueError, "is not normalized",
32-
lambda: r.Rlocation("../foo"))
33-
self.assertRaisesRegexp(ValueError, "is not normalized",
34-
lambda: r.Rlocation("foo/.."))
35-
self.assertRaisesRegexp(ValueError, "is not normalized",
36-
lambda: r.Rlocation("foo/../bar"))
37-
self.assertRaisesRegexp(ValueError, "is not normalized",
38-
lambda: r.Rlocation("./foo"))
39-
self.assertRaisesRegexp(ValueError, "is not normalized",
40-
lambda: r.Rlocation("foo/."))
41-
self.assertRaisesRegexp(ValueError, "is not normalized",
42-
lambda: r.Rlocation("foo/./bar"))
43-
self.assertRaisesRegexp(ValueError, "is not normalized",
44-
lambda: r.Rlocation("//foobar"))
45-
self.assertRaisesRegexp(ValueError, "is not normalized",
46-
lambda: r.Rlocation("foo//"))
47-
self.assertRaisesRegexp(ValueError, "is not normalized",
48-
lambda: r.Rlocation("foo//bar"))
49-
self.assertRaisesRegexp(ValueError, "is absolute without a drive letter",
50-
lambda: r.Rlocation("\\foo"))
31+
self.assertRaisesRegex(ValueError, "is not normalized",
32+
lambda: r.Rlocation("../foo"))
33+
self.assertRaisesRegex(ValueError, "is not normalized",
34+
lambda: r.Rlocation("foo/.."))
35+
self.assertRaisesRegex(ValueError, "is not normalized",
36+
lambda: r.Rlocation("foo/../bar"))
37+
self.assertRaisesRegex(ValueError, "is not normalized",
38+
lambda: r.Rlocation("./foo"))
39+
self.assertRaisesRegex(ValueError, "is not normalized",
40+
lambda: r.Rlocation("foo/."))
41+
self.assertRaisesRegex(ValueError, "is not normalized",
42+
lambda: r.Rlocation("foo/./bar"))
43+
self.assertRaisesRegex(ValueError, "is not normalized",
44+
lambda: r.Rlocation("//foobar"))
45+
self.assertRaisesRegex(ValueError, "is not normalized",
46+
lambda: r.Rlocation("foo//"))
47+
self.assertRaisesRegex(ValueError, "is not normalized",
48+
lambda: r.Rlocation("foo//bar"))
49+
self.assertRaisesRegex(ValueError, "is absolute without a drive letter",
50+
lambda: r.Rlocation("\\foo"))
5151

5252
def testCreatesManifestBasedRunfiles(self):
5353
with _MockFile(contents=["a/b c/d"]) as mf:
@@ -122,7 +122,7 @@ def testFailsToCreateManifestBasedBecauseManifestDoesNotExist(self):
122122
def _Run():
123123
runfiles.Create({"RUNFILES_MANIFEST_FILE": "non-existing path"})
124124

125-
self.assertRaisesRegexp(IOError, "non-existing path", _Run)
125+
self.assertRaisesRegex(IOError, "non-existing path", _Run)
126126

127127
def testFailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined(self):
128128
with _MockFile(contents=["a b"]) as mf:
@@ -140,14 +140,22 @@ def testFailsToCreateAnyRunfilesBecauseEnvvarsAreNotDefined(self):
140140

141141
def testManifestBasedRlocation(self):
142142
with _MockFile(contents=[
143-
"Foo/runfile1", "Foo/runfile2 C:/Actual Path\\runfile2",
144-
"Foo/Bar/runfile3 D:\\the path\\run file 3.txt"
143+
"Foo/runfile1",
144+
"Foo/runfile2 C:/Actual Path\\runfile2",
145+
"Foo/Bar/runfile3 D:\\the path\\run file 3.txt",
146+
"Foo/Bar/Dir E:\\Actual Path\\Directory",
145147
]) as mf:
146148
r = runfiles.CreateManifestBased(mf.Path())
147149
self.assertEqual(r.Rlocation("Foo/runfile1"), "Foo/runfile1")
148150
self.assertEqual(r.Rlocation("Foo/runfile2"), "C:/Actual Path\\runfile2")
149151
self.assertEqual(
150152
r.Rlocation("Foo/Bar/runfile3"), "D:\\the path\\run file 3.txt")
153+
self.assertEqual(
154+
r.Rlocation("Foo/Bar/Dir/runfile4"),
155+
"E:\\Actual Path\\Directory/runfile4")
156+
self.assertEqual(
157+
r.Rlocation("Foo/Bar/Dir/Deeply/Nested/runfile4"),
158+
"E:\\Actual Path\\Directory/Deeply/Nested/runfile4")
151159
self.assertIsNone(r.Rlocation("unknown"))
152160
if RunfilesTest.IsWindows():
153161
self.assertEqual(r.Rlocation("c:/foo"), "c:/foo")

0 commit comments

Comments
 (0)