Skip to content

Commit a159330

Browse files
fmeumcopybara-github
authored andcommitted
Add $(rlocationpath(s) ...) expansion
The new location expansion pattern `rlocationpath` and its plural version `rlocationpaths` resolve to the path(s) suitable for the Rlocation function offered by runfiles libraries. Compared to the `rootpath` pattern, which can returns `../repo_name/pkg/file` for a file in an external repository and `pkg/file` for a file in the main repository, the path returned by `rlocationpath` is always of the form `repo_name/pkg/file`. RELNOTES: The new path variable `$(rlocationpath ...)` and its plural form `$(rlocationpaths ...)` can be used to expand labels to the paths accepted by the `Rlocation` function of runfiles libraries. This is the preferred way to access data dependencies at runtime and works on all platforms, even when runfiles are not enabled (e.g., on Windows by default). Work towards bazelbuild#16124 Fixes bazelbuild#10923 Closes bazelbuild#16428. PiperOrigin-RevId: 485930083 Change-Id: I85c0ef29c8332eeabd8473d2611ede546eb24016
1 parent b5052b8 commit a159330

File tree

7 files changed

+282
-92
lines changed

7 files changed

+282
-92
lines changed

src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm

+61-18
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,15 @@
262262
</p>
263263
<p>
264264
Output files are staged similarly, but are also prefixed with the subpath
265-
<code>bazel-out/cpu-compilation_mode/bin</code> (or for certain outputs:
266-
<code>bazel-out/cpu-compilation_mode/genfiles</code>, or for the outputs
267-
of host tools: <code>bazel-out/host/bin</code>). In the above example,
268-
<code>//testapp:app</code> is a host tool because it appears in
265+
<code>bazel-out/cpu-compilation_mode/bin</code> (or for the outputs of
266+
tools: <code>bazel-out/cpu-opt-exec-hash/bin</code>). In the above example,
267+
<code>//testapp:app</code> is a tool because it appears in
269268
<code>show_app_output</code>'s <code><a
270269
href="$expander.expandRef("genrule.tools")">tools</a></code> attribute.
271270
So its output file <code>app</code> is written to
272-
<code>bazel-myproject/bazel-out/host/bin/testapp/app</code>. The exec path
273-
is thus <code>bazel-out/host/bin/testapp/app</code>. This extra prefix
271+
<code>bazel-myproject/bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>.
272+
The exec path is thus <code>
273+
bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>. This extra prefix
274274
makes it possible to build the same target for, say, two different CPUs in
275275
the same build without the results clobbering each other.
276276
</p>
@@ -284,15 +284,57 @@
284284

285285
<li>
286286
<p>
287-
<code>rootpath</code>: Denotes the runfiles path that a built binary can
288-
use to find its dependencies at runtime.
289-
</p>
287+
<code>rootpath</code>: Denotes the path that a built binary can use to
288+
find a dependency at runtime relative to the subdirectory of its runfiles
289+
directory corresponding to the main repository.
290+
<strong>Note:</strong> This only works if <a
291+
href="/reference/command-line-reference#flag--enable_runfiles">
292+
<code>--enable_runfiles</code></a> is enabled, which is not the case on
293+
Windows by default. Use <code>rlocationpath</code> instead for
294+
cross-platform support.
290295
<p>
291-
This is the same as <code>execpath</code> but strips the output prefixes
292-
described above. In the above example this means both
296+
This is similar to <code>execpath</code> but strips the configuration
297+
prefixes described above. In the example from above this means both
293298
<code>empty.source</code> and <code>app</code> use pure workspace-relative
294299
paths: <code>testapp/empty.source</code> and <code>testapp/app</code>.
295300
</p>
301+
<p>
302+
The <code>rootpath</code> of a file in an external repository
303+
<code>repo</code> will start with <code>../repo/</code>, followed by the
304+
repository-relative path.
305+
</p>
306+
<p>
307+
This has the same "one output only" requirements as <code>execpath</code>.
308+
</p>
309+
</li>
310+
311+
<li>
312+
<p>
313+
<code>rlocationpath</code>: The path a built binary can pass to the <code>
314+
Rlocation</code> function of a runfiles library to find a dependency at
315+
runtime, either in the runfiles directory (if available) or using the
316+
runfiles manifest.
317+
</p>
318+
<p>
319+
This is similar to <code>rootpath</code> in that it does not contain
320+
configuration prefixes, but differs in that it always starts with the
321+
name of the repository. In the example from above this means that <code>
322+
empty.source</code> and <code>app</code> result in the following
323+
paths: <code>myproject/testapp/empty.source</code> and <code>
324+
myproject/testapp/app</code>.
325+
</p>
326+
<p>
327+
The <code>rlocationpath</code> of a file in an external repository
328+
<code>repo</code> will start with <code>repo/</code>, followed by the
329+
repository-relative path.
330+
</p>
331+
<p>
332+
Passing this path to a binary and resolving it to a file system path using
333+
the runfiles libraries is the preferred approach to find dependencies at
334+
runtime. Compared to <code>rootpath</code>, it has the advantage that it
335+
works on all platforms and even if the runfiles directory is not
336+
available.
337+
</p>
296338
<p>
297339
This has the same "one output only" requirements as <code>execpath</code>.
298340
</p>
@@ -303,24 +345,25 @@
303345
<code>rootpath</code>, depending on the attribute being expanded. This is
304346
legacy pre-Starlark behavior and not recommended unless you really know what
305347
it does for a particular rule. See <a
306-
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
348+
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
307349
for details.
308350
</li>
309351
</ul>
310352

311353
<p>
312-
<code>execpaths</code>, <code>rootpaths</code>, and <code>locations</code> are
313-
the plural variations of <code>execpath</code>, <code>rootpath</code>, and
314-
<code>location</code>, respectively. They support labels producing multiple
315-
outputs, in which case each output is listed separated by a space. Zero-output
316-
rules and malformed labels produce build errors.
354+
<code>execpaths</code>, <code>rootpaths</code>, <code>rlocationpaths</code>,
355+
and <code>locations</code> are the plural variations of <code>execpath</code>,
356+
<code>rootpath</code>, <code>rlocationpaths</code>, and<code>location</code>,
357+
respectively. They support labels producing multiple outputs, in which case
358+
each output is listed separated by a space. Zero-output rules and malformed
359+
labels produce build errors.
317360
</p>
318361

319362
<p>
320363
All referenced labels must appear in the consuming target's <code>srcs</code>,
321364
output files, or <code>deps</code>. Otherwise the build fails. C++ targets can
322365
also reference labels in <code><a
323-
href="$expander.expandRef("cc_binary.data")">data</a></code>.
366+
href="$expander.expandRef("cc_binary.data")">data</a></code>.
324367
</p>
325368

326369
<p>

src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java

+83-34
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static java.util.stream.Collectors.joining;
1818

1919
import com.google.common.annotations.VisibleForTesting;
20+
import com.google.common.base.Preconditions;
2021
import com.google.common.base.Supplier;
2122
import com.google.common.base.Suppliers;
2223
import com.google.common.collect.ImmutableCollection;
@@ -26,7 +27,9 @@
2627
import com.google.common.collect.Maps;
2728
import com.google.common.collect.Sets;
2829
import com.google.devtools.build.lib.actions.Artifact;
30+
import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction.PathType;
2931
import com.google.devtools.build.lib.cmdline.Label;
32+
import com.google.devtools.build.lib.cmdline.LabelConstants;
3033
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
3134
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
3235
import com.google.devtools.build.lib.packages.BuildType;
@@ -63,34 +66,35 @@ public final class LocationExpander {
6366
private static final boolean EXACTLY_ONE = false;
6467
private static final boolean ALLOW_MULTIPLE = true;
6568

66-
private static final boolean USE_LOCATION_PATHS = false;
67-
private static final boolean USE_EXEC_PATHS = true;
68-
6969
private final RuleErrorConsumer ruleErrorConsumer;
7070
private final ImmutableMap<String, LocationFunction> functions;
7171
private final RepositoryMapping repositoryMapping;
72+
private final String workspaceRunfilesDirectory;
7273

7374
@VisibleForTesting
7475
LocationExpander(
7576
RuleErrorConsumer ruleErrorConsumer,
7677
Map<String, LocationFunction> functions,
77-
RepositoryMapping repositoryMapping) {
78+
RepositoryMapping repositoryMapping,
79+
String workspaceRunfilesDirectory) {
7880
this.ruleErrorConsumer = ruleErrorConsumer;
7981
this.functions = ImmutableMap.copyOf(functions);
8082
this.repositoryMapping = repositoryMapping;
83+
this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
8184
}
8285

8386
private LocationExpander(
84-
RuleErrorConsumer ruleErrorConsumer,
87+
RuleContext ruleContext,
8588
Label root,
8689
Supplier<Map<Label, Collection<Artifact>>> locationMap,
8790
boolean execPaths,
8891
boolean legacyExternalRunfiles,
8992
RepositoryMapping repositoryMapping) {
9093
this(
91-
ruleErrorConsumer,
94+
ruleContext,
9295
allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles),
93-
repositoryMapping);
96+
repositoryMapping,
97+
ruleContext.getWorkspaceName());
9498
}
9599

96100
/**
@@ -204,7 +208,10 @@ private String expand(String value, ErrorReporter reporter) {
204208
// (2) Call appropriate function to obtain string replacement.
205209
String functionValue = value.substring(nextWhitespace + 1, end).trim();
206210
try {
207-
String replacement = functions.get(fname).apply(functionValue, repositoryMapping);
211+
String replacement =
212+
functions
213+
.get(fname)
214+
.apply(functionValue, repositoryMapping, workspaceRunfilesDirectory);
208215
result.append(replacement);
209216
} catch (IllegalStateException ise) {
210217
reporter.report(ise.getMessage());
@@ -232,23 +239,29 @@ public String expandAttribute(String attrName, String attrValue) {
232239

233240
@VisibleForTesting
234241
static final class LocationFunction {
242+
enum PathType {
243+
LOCATION,
244+
EXEC,
245+
RLOCATION,
246+
}
247+
235248
private static final int MAX_PATHS_SHOWN = 5;
236249

237250
private final Label root;
238251
private final Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier;
239-
private final boolean execPaths;
252+
private final PathType pathType;
240253
private final boolean legacyExternalRunfiles;
241254
private final boolean multiple;
242255

243256
LocationFunction(
244257
Label root,
245258
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
246-
boolean execPaths,
259+
PathType pathType,
247260
boolean legacyExternalRunfiles,
248261
boolean multiple) {
249262
this.root = root;
250263
this.locationMapSupplier = locationMapSupplier;
251-
this.execPaths = execPaths;
264+
this.pathType = Preconditions.checkNotNull(pathType);
252265
this.legacyExternalRunfiles = legacyExternalRunfiles;
253266
this.multiple = multiple;
254267
}
@@ -259,10 +272,13 @@ static final class LocationFunction {
259272
* using the {@code repositoryMapping}.
260273
*
261274
* @param arg The label-like string to be expanded, e.g. ":foo" or "//foo:bar"
262-
* @param repositoryMapping map of {@code RepositoryName}s defined in the main workspace
275+
* @param repositoryMapping map of apparent repository names to {@code RepositoryName}s
276+
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main
277+
* repository
263278
* @return The expanded value
264279
*/
265-
public String apply(String arg, RepositoryMapping repositoryMapping) {
280+
public String apply(
281+
String arg, RepositoryMapping repositoryMapping, String workspaceRunfilesDirectory) {
266282
Label label;
267283
try {
268284
label = root.getRelativeWithRemapping(arg, repositoryMapping);
@@ -271,14 +287,13 @@ public String apply(String arg, RepositoryMapping repositoryMapping) {
271287
String.format(
272288
"invalid label in %s expression: %s", functionName(), e.getMessage()), e);
273289
}
274-
Collection<String> paths = resolveLabel(label);
290+
Set<String> paths = resolveLabel(label, workspaceRunfilesDirectory);
275291
return joinPaths(paths);
276292
}
277293

278-
/**
279-
* Returns all target location(s) of the given label.
280-
*/
281-
private Collection<String> resolveLabel(Label unresolved) throws IllegalStateException {
294+
/** Returns all target location(s) of the given label. */
295+
private Set<String> resolveLabel(Label unresolved, String workspaceRunfilesDirectory)
296+
throws IllegalStateException {
282297
Collection<Artifact> artifacts = locationMapSupplier.get().get(unresolved);
283298

284299
if (artifacts == null) {
@@ -288,7 +303,7 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
288303
unresolved, functionName()));
289304
}
290305

291-
Set<String> paths = getPaths(artifacts);
306+
Set<String> paths = getPaths(artifacts, workspaceRunfilesDirectory);
292307
if (paths.isEmpty()) {
293308
throw new IllegalStateException(
294309
String.format(
@@ -313,24 +328,41 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
313328
* Extracts list of all executables associated with given collection of label artifacts.
314329
*
315330
* @param artifacts to get the paths of
331+
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main
332+
* repository
316333
* @return all associated executable paths
317334
*/
318-
private Set<String> getPaths(Collection<Artifact> artifacts) {
335+
private Set<String> getPaths(
336+
Collection<Artifact> artifacts, String workspaceRunfilesDirectory) {
319337
TreeSet<String> paths = Sets.newTreeSet();
320338
for (Artifact artifact : artifacts) {
321-
PathFragment execPath =
322-
execPaths
323-
? artifact.getExecPath()
324-
: legacyExternalRunfiles
325-
? artifact.getPathForLocationExpansion()
326-
: artifact.getRunfilesPath();
327-
if (execPath != null) { // omit middlemen etc
328-
paths.add(execPath.getCallablePathString());
339+
PathFragment path = getPath(artifact, workspaceRunfilesDirectory);
340+
if (path != null) { // omit middlemen etc
341+
paths.add(path.getCallablePathString());
329342
}
330343
}
331344
return paths;
332345
}
333346

347+
private PathFragment getPath(Artifact artifact, String workspaceRunfilesDirectory) {
348+
switch (pathType) {
349+
case LOCATION:
350+
return legacyExternalRunfiles
351+
? artifact.getPathForLocationExpansion()
352+
: artifact.getRunfilesPath();
353+
case EXEC:
354+
return artifact.getExecPath();
355+
case RLOCATION:
356+
PathFragment runfilesPath = artifact.getRunfilesPath();
357+
if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
358+
return runfilesPath.relativeTo(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX);
359+
} else {
360+
return PathFragment.create(workspaceRunfilesDirectory).getRelative(runfilesPath);
361+
}
362+
}
363+
throw new IllegalStateException("Unexpected PathType: " + pathType);
364+
}
365+
334366
private String joinPaths(Collection<String> paths) {
335367
return paths.stream().map(ShellEscaper::escapeString).collect(joining(" "));
336368
}
@@ -348,27 +380,44 @@ static ImmutableMap<String, LocationFunction> allLocationFunctions(
348380
return new ImmutableMap.Builder<String, LocationFunction>()
349381
.put(
350382
"location",
351-
new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE))
383+
new LocationFunction(
384+
root,
385+
locationMap,
386+
execPaths ? PathType.EXEC : PathType.LOCATION,
387+
legacyExternalRunfiles,
388+
EXACTLY_ONE))
352389
.put(
353390
"locations",
354391
new LocationFunction(
355-
root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE))
392+
root,
393+
locationMap,
394+
execPaths ? PathType.EXEC : PathType.LOCATION,
395+
legacyExternalRunfiles,
396+
ALLOW_MULTIPLE))
356397
.put(
357398
"rootpath",
358399
new LocationFunction(
359-
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
400+
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
360401
.put(
361402
"rootpaths",
362403
new LocationFunction(
363-
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
404+
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
364405
.put(
365406
"execpath",
366407
new LocationFunction(
367-
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
408+
root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE))
368409
.put(
369410
"execpaths",
370411
new LocationFunction(
371-
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
412+
root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE))
413+
.put(
414+
"rlocationpath",
415+
new LocationFunction(
416+
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE))
417+
.put(
418+
"rlocationpaths",
419+
new LocationFunction(
420+
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
372421
.buildOrThrow();
373422
}
374423

0 commit comments

Comments
 (0)