Skip to content

Commit

Permalink
Add $(rlocationpath(s) ...) expansion
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@fmeum
fmeum committed Oct 26, 2022
1 parent a82d26f commit c019df0
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 80 deletions.
69 changes: 51 additions & 18 deletions src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,15 @@
</p>
<p>
Output files are staged similarly, but are also prefixed with the subpath
<code>bazel-out/cpu-compilation_mode/bin</code> (or for certain outputs:
<code>bazel-out/cpu-compilation_mode/genfiles</code>, or for the outputs
of host tools: <code>bazel-out/host/bin</code>). In the above example,
<code>//testapp:app</code> is a host tool because it appears in
<code>bazel-out/cpu-compilation_mode/bin</code> (or for the outputs of
tools: <code>bazel-out/cpu-opt-exec-hash/bin</code>). In the above example,
<code>//testapp:app</code> is a tool because it appears in
<code>show_app_output</code>'s <code><a
href="$expander.expandRef("genrule.tools")">tools</a></code> attribute.
So its output file <code>app</code> is written to
<code>bazel-myproject/bazel-out/host/bin/testapp/app</code>. The exec path
is thus <code>bazel-out/host/bin/testapp/app</code>. This extra prefix
<code>bazel-myproject/bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>.
The exec path is thus <code>
bazel-out/cpu-opt-exec-hash/bin/testapp/app</code>. This extra prefix
makes it possible to build the same target for, say, two different CPUs in
the same build without the results clobbering each other.
</p>
Expand All @@ -284,12 +284,17 @@

<li>
<p>
<code>rootpath</code>: Denotes the runfiles path that a built binary can
use to find its dependencies at runtime.
</p>
<code>rootpath</code>: Denotes the path that a built binary can use to
find a dependency at runtime relative to the subdirectory of its runfiles
directory corresponding to the main repository.
<strong>Note:</strong> This only works if <a
href="/reference/command-line-reference#flag--enable_runfiles">
<code>--enable_runfiles</code></a> is enabled, which is not the case on
Windows by default. Use <code>rlocationpath</code> instead for
cross-platform support.
<p>
This is the same as <code>execpath</code> but strips the output prefixes
described above. In the above example this means both
This is similar to <code>execpath</code> but strips the configuration
prefixes described above. In the example from above this means both
<code>empty.source</code> and <code>app</code> use pure workspace-relative
paths: <code>testapp/empty.source</code> and <code>testapp/app</code>.
</p>
Expand All @@ -298,29 +303,57 @@
</p>
</li>

<li>
<p>
<code>rlocationpath</code>: The path a built binary can pass to the <code>
Rlocation</code> function of a runfiles library to find a dependency at
runtime, either in the runfiles directory (if available) or using the
runfiles manifest.
</p>
<p>
This is similar to <code>rootpath</code> in that it does not contain
configuration prefixes, but differs in that it always starts with the
name of the repository. In the example from above this means that <code>
empty.source</code> and <code>app</code> result in the following
paths: <code>myproject/testapp/empty.source</code> and <code>
myproject/testapp/app</code>.
</p>
<p>
Passing this path to a binary and resolving it to a file system path using
the runfiles libraries is the preferred approach to find dependencies at
runtime. Compared to <code>rootpath</code>, it has the advantage that it
works on all platforms and even if the runfiles directory should not be
available.
</p>
<p>
This has the same "one output only" requirements as <code>execpath</code>.
</p>
</li>

<li>
<code>location</code>: A synonym for either <code>execpath</code> or
<code>rootpath</code>, depending on the attribute being expanded. This is
legacy pre-Starlark behavior and not recommended unless you really know what
it does for a particular rule. See <a
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475</a>
for details.
</li>
</ul>

<p>
<code>execpaths</code>, <code>rootpaths</code>, and <code>locations</code> are
the plural variations of <code>execpath</code>, <code>rootpath</code>, and
<code>location</code>, respectively. They support labels producing multiple
outputs, in which case each output is listed separated by a space. Zero-output
rules and malformed labels produce build errors.
<code>execpaths</code>, <code>rootpaths</code>, <code>rlocationpaths</code>,
and <code>locations</code> are the plural variations of <code>execpath</code>,
<code>rootpath</code>, <code>rlocationpaths</code>, and<code>location</code>,
respectively. They support labels producing multiple outputs, in which case
each output is listed separated by a space. Zero-output rules and malformed
labels produce build errors.
</p>

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

<p>
Expand Down
102 changes: 67 additions & 35 deletions src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static java.util.stream.Collectors.joining;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableCollection;
Expand All @@ -26,7 +27,9 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction.PathType;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.packages.BuildType;
Expand Down Expand Up @@ -63,34 +66,35 @@ public final class LocationExpander {
private static final boolean EXACTLY_ONE = false;
private static final boolean ALLOW_MULTIPLE = true;

private static final boolean USE_LOCATION_PATHS = false;
private static final boolean USE_EXEC_PATHS = true;

private final RuleErrorConsumer ruleErrorConsumer;
private final ImmutableMap<String, LocationFunction> functions;
private final RepositoryMapping repositoryMapping;
private final String workspaceRunfilesDirectory;

@VisibleForTesting
LocationExpander(
RuleErrorConsumer ruleErrorConsumer,
Map<String, LocationFunction> functions,
RepositoryMapping repositoryMapping) {
RepositoryMapping repositoryMapping,
String workspaceRunfilesDirectory) {
this.ruleErrorConsumer = ruleErrorConsumer;
this.functions = ImmutableMap.copyOf(functions);
this.repositoryMapping = repositoryMapping;
this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
}

private LocationExpander(
RuleErrorConsumer ruleErrorConsumer,
RuleContext ruleContext,
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMap,
boolean execPaths,
boolean legacyExternalRunfiles,
RepositoryMapping repositoryMapping) {
this(
ruleErrorConsumer,
ruleContext,
allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles),
repositoryMapping);
repositoryMapping,
ruleContext.getWorkspaceName());
}

/**
Expand Down Expand Up @@ -204,7 +208,7 @@ private String expand(String value, ErrorReporter reporter) {
// (2) Call appropriate function to obtain string replacement.
String functionValue = value.substring(nextWhitespace + 1, end).trim();
try {
String replacement = functions.get(fname).apply(functionValue, repositoryMapping);
String replacement = functions.get(fname).apply(functionValue, repositoryMapping, workspaceRunfilesDirectory);
result.append(replacement);
} catch (IllegalStateException ise) {
reporter.report(ise.getMessage());
Expand Down Expand Up @@ -232,23 +236,29 @@ public String expandAttribute(String attrName, String attrValue) {

@VisibleForTesting
static final class LocationFunction {
enum PathType {
LOCATION,
EXEC,
RLOCATION,
}

private static final int MAX_PATHS_SHOWN = 5;

private final Label root;
private final Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier;
private final boolean execPaths;
private final PathType pathType;
private final boolean legacyExternalRunfiles;
private final boolean multiple;

LocationFunction(
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
boolean execPaths,
boolean legacyExternalRunfiles,
boolean multiple) {
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
PathType pathType,
boolean legacyExternalRunfiles,
boolean multiple) {
this.root = root;
this.locationMapSupplier = locationMapSupplier;
this.execPaths = execPaths;
this.pathType = Preconditions.checkNotNull(pathType);
this.legacyExternalRunfiles = legacyExternalRunfiles;
this.multiple = multiple;
}
Expand All @@ -259,10 +269,11 @@ static final class LocationFunction {
* using the {@code repositoryMapping}.
*
* @param arg The label-like string to be expanded, e.g. ":foo" or "//foo:bar"
* @param repositoryMapping map of {@code RepositoryName}s defined in the main workspace
* @param repositoryMapping map of apparent repository names to {@code RepositoryName}s
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
* @return The expanded value
*/
public String apply(String arg, RepositoryMapping repositoryMapping) {
public String apply(String arg, RepositoryMapping repositoryMapping, String workspaceRunfilesDirectory) {
Label label;
try {
label = root.getRelativeWithRemapping(arg, repositoryMapping);
Expand All @@ -271,14 +282,14 @@ public String apply(String arg, RepositoryMapping repositoryMapping) {
String.format(
"invalid label in %s expression: %s", functionName(), e.getMessage()), e);
}
Collection<String> paths = resolveLabel(label);
Collection<String> paths = resolveLabel(label, workspaceRunfilesDirectory);
return joinPaths(paths);
}

/**
* Returns all target location(s) of the given label.
*/
private Collection<String> resolveLabel(Label unresolved) throws IllegalStateException {
private Collection<String> resolveLabel(Label unresolved, String workspaceRunfilesDirectory) throws IllegalStateException {
Collection<Artifact> artifacts = locationMapSupplier.get().get(unresolved);

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

Set<String> paths = getPaths(artifacts);
Set<String> paths = getPaths(artifacts, workspaceRunfilesDirectory);
if (paths.isEmpty()) {
throw new IllegalStateException(
String.format(
Expand All @@ -313,24 +324,37 @@ private Collection<String> resolveLabel(Label unresolved) throws IllegalStateExc
* Extracts list of all executables associated with given collection of label artifacts.
*
* @param artifacts to get the paths of
* @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository
* @return all associated executable paths
*/
private Set<String> getPaths(Collection<Artifact> artifacts) {
private Set<String> getPaths(Collection<Artifact> artifacts, String workspaceRunfilesDirectory) {
TreeSet<String> paths = Sets.newTreeSet();
for (Artifact artifact : artifacts) {
PathFragment execPath =
execPaths
? artifact.getExecPath()
: legacyExternalRunfiles
? artifact.getPathForLocationExpansion()
: artifact.getRunfilesPath();
if (execPath != null) { // omit middlemen etc
paths.add(execPath.getCallablePathString());
PathFragment path = getPath(artifact, workspaceRunfilesDirectory);
if (path != null) { // omit middlemen etc
paths.add(path.getCallablePathString());
}
}
return paths;
}

private PathFragment getPath(Artifact artifact, String workspaceRunfilesDirectory) {
switch (pathType) {
case LOCATION:
return legacyExternalRunfiles ? artifact.getPathForLocationExpansion() : artifact.getRunfilesPath();
case EXEC:
return artifact.getExecPath();
case RLOCATION:
PathFragment runfilesPath = artifact.getRunfilesPath();
if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
return runfilesPath.relativeTo(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX);
} else {
return PathFragment.create(workspaceRunfilesDirectory).getRelative(runfilesPath);
}
}
throw new IllegalStateException("Unexpected PathType: " + pathType);
}

private String joinPaths(Collection<String> paths) {
return paths.stream().map(ShellEscaper::escapeString).collect(joining(" "));
}
Expand All @@ -348,27 +372,35 @@ static ImmutableMap<String, LocationFunction> allLocationFunctions(
return new ImmutableMap.Builder<String, LocationFunction>()
.put(
"location",
new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE))
new LocationFunction(root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
.put(
"locations",
new LocationFunction(
root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE))
root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
.put(
"rootpath",
new LocationFunction(
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
.put(
"rootpaths",
new LocationFunction(
root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
.put(
"execpath",
new LocationFunction(
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE))
.put(
"execpaths",
new LocationFunction(
root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE))
.put(
"rlocationpath",
new LocationFunction(
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE))
.put(
"rlocationpaths",
new LocationFunction(
root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
.buildOrThrow();
}

Expand Down
10 changes: 7 additions & 3 deletions src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ final class LocationTemplateContext implements TemplateContext {
private final ImmutableMap<String, LocationFunction> functions;
private final RepositoryMapping repositoryMapping;
private final boolean windowsPath;
private final String workspaceRunfilesDirectory;

private LocationTemplateContext(
TemplateContext delegate,
Expand All @@ -58,12 +59,14 @@ private LocationTemplateContext(
boolean execPaths,
boolean legacyExternalRunfiles,
RepositoryMapping repositoryMapping,
boolean windowsPath) {
boolean windowsPath,
String workspaceRunfilesDirectory) {
this.delegate = delegate;
this.functions =
LocationExpander.allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles);
this.repositoryMapping = repositoryMapping;
this.windowsPath = windowsPath;
this.workspaceRunfilesDirectory = workspaceRunfilesDirectory;
}

public LocationTemplateContext(
Expand All @@ -83,7 +86,8 @@ public LocationTemplateContext(
execPaths,
ruleContext.getConfiguration().legacyExternalRunfiles(),
ruleContext.getRule().getPackage().getRepositoryMapping(),
windowsPath);
windowsPath,
ruleContext.getWorkspaceName());
}

@Override
Expand All @@ -108,7 +112,7 @@ private String lookupFunctionImpl(String name, String param) throws ExpansionExc
try {
LocationFunction f = functions.get(name);
if (f != null) {
return f.apply(param, repositoryMapping);
return f.apply(param, repositoryMapping, workspaceRunfilesDirectory);
}
} catch (IllegalStateException e) {
throw new ExpansionException(e.getMessage(), e);
Expand Down
Loading

0 comments on commit c019df0

Please sign in to comment.