Skip to content

Commit c019df0

Browse files
committed
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`. Work towards #16124 Fixes #10923
1 parent a82d26f commit c019df0

File tree

7 files changed

+233
-80
lines changed

7 files changed

+233
-80
lines changed

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

Lines changed: 51 additions & 18 deletions
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,12 +284,17 @@
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>
@@ -298,29 +303,57 @@
298303
</p>
299304
</li>
300305

306+
<li>
307+
<p>
308+
<code>rlocationpath</code>: The path a built binary can pass to the <code>
309+
Rlocation</code> function of a runfiles library to find a dependency at
310+
runtime, either in the runfiles directory (if available) or using the
311+
runfiles manifest.
312+
</p>
313+
<p>
314+
This is similar to <code>rootpath</code> in that it does not contain
315+
configuration prefixes, but differs in that it always starts with the
316+
name of the repository. In the example from above this means that <code>
317+
empty.source</code> and <code>app</code> result in the following
318+
paths: <code>myproject/testapp/empty.source</code> and <code>
319+
myproject/testapp/app</code>.
320+
</p>
321+
<p>
322+
Passing this path to a binary and resolving it to a file system path using
323+
the runfiles libraries is the preferred approach to find dependencies at
324+
runtime. Compared to <code>rootpath</code>, it has the advantage that it
325+
works on all platforms and even if the runfiles directory should not be
326+
available.
327+
</p>
328+
<p>
329+
This has the same "one output only" requirements as <code>execpath</code>.
330+
</p>
331+
</li>
332+
301333
<li>
302334
<code>location</code>: A synonym for either <code>execpath</code> or
303335
<code>rootpath</code>, depending on the attribute being expanded. This is
304336
legacy pre-Starlark behavior and not recommended unless you really know what
305337
it does for a particular rule. See <a
306-
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
338+
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
307339
for details.
308340
</li>
309341
</ul>
310342

311343
<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.
344+
<code>execpaths</code>, <code>rootpaths</code>, <code>rlocationpaths</code>,
345+
and <code>locations</code> are the plural variations of <code>execpath</code>,
346+
<code>rootpath</code>, <code>rlocationpaths</code>, and<code>location</code>,
347+
respectively. They support labels producing multiple outputs, in which case
348+
each output is listed separated by a space. Zero-output rules and malformed
349+
labels produce build errors.
317350
</p>
318351

319352
<p>
320353
All referenced labels must appear in the consuming target's <code>srcs</code>,
321354
output files, or <code>deps</code>. Otherwise the build fails. C++ targets can
322355
also reference labels in <code><a
323-
href="$expander.expandRef("cc_binary.data")">data</a></code>.
356+
href="$expander.expandRef("cc_binary.data")">data</a></code>.
324357
</p>
325358

326359
<p>

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

Lines changed: 67 additions & 35 deletions
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,7 @@ 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 = functions.get(fname).apply(functionValue, repositoryMapping, workspaceRunfilesDirectory);
208212
result.append(replacement);
209213
} catch (IllegalStateException ise) {
210214
reporter.report(ise.getMessage());
@@ -232,23 +236,29 @@ public String expandAttribute(String attrName, String attrValue) {
232236

233237
@VisibleForTesting
234238
static final class LocationFunction {
239+
enum PathType {
240+
LOCATION,
241+
EXEC,
242+
RLOCATION,
243+
}
244+
235245
private static final int MAX_PATHS_SHOWN = 5;
236246

237247
private final Label root;
238248
private final Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier;
239-
private final boolean execPaths;
249+
private final PathType pathType;
240250
private final boolean legacyExternalRunfiles;
241251
private final boolean multiple;
242252

243253
LocationFunction(
244-
Label root,
245-
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
246-
boolean execPaths,
247-
boolean legacyExternalRunfiles,
248-
boolean multiple) {
254+
Label root,
255+
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
256+
PathType pathType,
257+
boolean legacyExternalRunfiles,
258+
boolean multiple) {
249259
this.root = root;
250260
this.locationMapSupplier = locationMapSupplier;
251-
this.execPaths = execPaths;
261+
this.pathType = Preconditions.checkNotNull(pathType);
252262
this.legacyExternalRunfiles = legacyExternalRunfiles;
253263
this.multiple = multiple;
254264
}
@@ -259,10 +269,11 @@ static final class LocationFunction {
259269
* using the {@code repositoryMapping}.
260270
*
261271
* @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
272+
* @param repositoryMapping map of apparent repository names to {@code RepositoryName}s
273+
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
263274
* @return The expanded value
264275
*/
265-
public String apply(String arg, RepositoryMapping repositoryMapping) {
276+
public String apply(String arg, RepositoryMapping repositoryMapping, String workspaceRunfilesDirectory) {
266277
Label label;
267278
try {
268279
label = root.getRelativeWithRemapping(arg, repositoryMapping);
@@ -271,14 +282,14 @@ public String apply(String arg, RepositoryMapping repositoryMapping) {
271282
String.format(
272283
"invalid label in %s expression: %s", functionName(), e.getMessage()), e);
273284
}
274-
Collection<String> paths = resolveLabel(label);
285+
Collection<String> paths = resolveLabel(label, workspaceRunfilesDirectory);
275286
return joinPaths(paths);
276287
}
277288

278289
/**
279290
* Returns all target location(s) of the given label.
280291
*/
281-
private Collection<String> resolveLabel(Label unresolved) throws IllegalStateException {
292+
private Collection<String> resolveLabel(Label unresolved, String workspaceRunfilesDirectory) throws IllegalStateException {
282293
Collection<Artifact> artifacts = locationMapSupplier.get().get(unresolved);
283294

284295
if (artifacts == null) {
@@ -288,7 +299,7 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
288299
unresolved, functionName()));
289300
}
290301

291-
Set<String> paths = getPaths(artifacts);
302+
Set<String> paths = getPaths(artifacts, workspaceRunfilesDirectory);
292303
if (paths.isEmpty()) {
293304
throw new IllegalStateException(
294305
String.format(
@@ -313,24 +324,37 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
313324
* Extracts list of all executables associated with given collection of label artifacts.
314325
*
315326
* @param artifacts to get the paths of
327+
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
316328
* @return all associated executable paths
317329
*/
318-
private Set<String> getPaths(Collection<Artifact> artifacts) {
330+
private Set<String> getPaths(Collection<Artifact> artifacts, String workspaceRunfilesDirectory) {
319331
TreeSet<String> paths = Sets.newTreeSet();
320332
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());
333+
PathFragment path = getPath(artifact, workspaceRunfilesDirectory);
334+
if (path != null) { // omit middlemen etc
335+
paths.add(path.getCallablePathString());
329336
}
330337
}
331338
return paths;
332339
}
333340

341+
private PathFragment getPath(Artifact artifact, String workspaceRunfilesDirectory) {
342+
switch (pathType) {
343+
case LOCATION:
344+
return legacyExternalRunfiles ? artifact.getPathForLocationExpansion() : artifact.getRunfilesPath();
345+
case EXEC:
346+
return artifact.getExecPath();
347+
case RLOCATION:
348+
PathFragment runfilesPath = artifact.getRunfilesPath();
349+
if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
350+
return runfilesPath.relativeTo(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX);
351+
} else {
352+
return PathFragment.create(workspaceRunfilesDirectory).getRelative(runfilesPath);
353+
}
354+
}
355+
throw new IllegalStateException("Unexpected PathType: " + pathType);
356+
}
357+
334358
private String joinPaths(Collection<String> paths) {
335359
return paths.stream().map(ShellEscaper::escapeString).collect(joining(" "));
336360
}
@@ -348,27 +372,35 @@ static ImmutableMap<String, LocationFunction> allLocationFunctions(
348372
return new ImmutableMap.Builder<String, LocationFunction>()
349373
.put(
350374
"location",
351-
new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE))
375+
new LocationFunction(root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
352376
.put(
353377
"locations",
354378
new LocationFunction(
355-
root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE))
379+
root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
356380
.put(
357381
"rootpath",
358382
new LocationFunction(
359-
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
383+
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
360384
.put(
361385
"rootpaths",
362386
new LocationFunction(
363-
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
387+
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
364388
.put(
365389
"execpath",
366390
new LocationFunction(
367-
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
391+
root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE))
368392
.put(
369393
"execpaths",
370394
new LocationFunction(
371-
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
395+
root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE))
396+
.put(
397+
"rlocationpath",
398+
new LocationFunction(
399+
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE))
400+
.put(
401+
"rlocationpaths",
402+
new LocationFunction(
403+
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
372404
.buildOrThrow();
373405
}
374406

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ final class LocationTemplateContext implements TemplateContext {
5050
private final ImmutableMap<String, LocationFunction> functions;
5151
private final RepositoryMapping repositoryMapping;
5252
private final boolean windowsPath;
53+
private final String workspaceRunfilesDirectory;
5354

5455
private LocationTemplateContext(
5556
TemplateContext delegate,
@@ -58,12 +59,14 @@ private LocationTemplateContext(
5859
boolean execPaths,
5960
boolean legacyExternalRunfiles,
6061
RepositoryMapping repositoryMapping,
61-
boolean windowsPath) {
62+
boolean windowsPath,
63+
String workspaceRunfilesDirectory) {
6264
this.delegate = delegate;
6365
this.functions =
6466
LocationExpander.allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles);
6567
this.repositoryMapping = repositoryMapping;
6668
this.windowsPath = windowsPath;
69+
this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
6770
}
6871

6972
public LocationTemplateContext(
@@ -83,7 +86,8 @@ public LocationTemplateContext(
8386
execPaths,
8487
ruleContext.getConfiguration().legacyExternalRunfiles(),
8588
ruleContext.getRule().getPackage().getRepositoryMapping(),
86-
windowsPath);
89+
windowsPath,
90+
ruleContext.getWorkspaceName());
8791
}
8892

8993
@Override
@@ -108,7 +112,7 @@ private String lookupFunctionImpl(String name, String param) throws ExpansionExc
108112
try {
109113
LocationFunction f = functions.get(name);
110114
if (f != null) {
111-
return f.apply(param, repositoryMapping);
115+
return f.apply(param, repositoryMapping, workspaceRunfilesDirectory);
112116
}
113117
} catch (IllegalStateException e) {
114118
throw new ExpansionException(e.getMessage(), e);

0 commit comments

Comments
 (0)