Skip to content

Commit

Permalink
Decanonicalize labels emitted by {a,c,}query if possible
Browse files Browse the repository at this point in the history
Uses the newly added `PackageIdentifier#getDisplayForm` to turn labels
in the output of query, aquery, and cquery into the most concise
representation that allows them to be resolved from the context of the
main repository.
  • Loading branch information
fmeum committed Oct 18, 2022
1 parent e60df6b commit 30b3d98
Show file tree
Hide file tree
Showing 31 changed files with 200 additions and 55 deletions.
18 changes: 15 additions & 3 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,21 @@ public String getCanonicalForm() {
* Label.parse*(x.getUnambiguousCanonicalForm(), ...).equals(x)}).
*/
public String getUnambiguousCanonicalForm() {
return String.format(
"@@%s//%s:%s",
packageIdentifier.getRepository().getName(), packageIdentifier.getPackageFragment(), name);
return packageIdentifier.getUnambiguousCanonicalForm() + ":" + name;
}

/**
* Returns a label string that is suitable for display, i.e., it resolves to this label when
* parsed in the context of the main repository and has a repository part that is as simple as
* possible.
*
* @param mainOrOtherRepoMapping the {@link RepositoryMapping} of the main repository or, if that
* is not available to the caller, the {@link RepositoryMapping} of
* the current package. The former will yield more readable output.
* @return analogous to {@link PackageIdentifier#getDisplayForm(RepositoryMapping)}
*/
public String getDisplayForm(RepositoryMapping mainOrOtherRepoMapping) {
return packageIdentifier.getDisplayForm(mainOrOtherRepoMapping) + ":" + name;
}

/** Return the name of the repository label refers to without the leading `at` symbol. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
Expand Down Expand Up @@ -251,6 +252,11 @@ protected TargetPattern getPattern(String pattern) throws TargetParsingException
return mainRepoTargetParser.parse(pattern);
}

@Override
public RepositoryMapping getMainRepoMapping() {
return mainRepoTargetParser.getRepoMapping();
}

public ThreadSafeMutableSet<T> getFwdDeps(Iterable<T> targets) throws InterruptedException {
Map<SkyKey, T> targetsByKey = Maps.newHashMapWithExpectedSize(Iterables.size(targets));
for (T target : targets) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.ParallelVisitor.VisitTaskStatusCallback;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
Expand Down Expand Up @@ -958,6 +959,12 @@ public TargetAccessor<Target> getAccessor() {
return accessor;
}

@Override
@ThreadSafe
public RepositoryMapping getMainRepoMapping() {
return mainRepoTargetParser.getRepoMapping();
}

@ThreadSafe
private Package getPackage(PackageIdentifier packageIdentifier)
throws InterruptedException, QueryException, NoSuchPackageException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ public ConfiguredTargetValueAccessor getAccessor() {
StreamedOutputHandler.OutputType.JSON,
actionFilters),
new ActionGraphTextOutputFormatterCallback(
eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters),
eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters,
getMainRepoMapping()),
new ActionGraphSummaryOutputFormatterCallback(
eventHandler, aqueryOptions, out, skyframeExecutor, accessor, actionFilters));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
Expand All @@ -62,6 +63,7 @@ class ActionGraphTextOutputFormatterCallback extends AqueryThreadsafeCallback {

private final ActionKeyContext actionKeyContext = new ActionKeyContext();
private final AqueryActionFilter actionFilters;
private final RepositoryMapping mainRepoMapping;
private Map<String, String> paramFileNameToContentMap;

ActionGraphTextOutputFormatterCallback(
Expand All @@ -70,9 +72,11 @@ class ActionGraphTextOutputFormatterCallback extends AqueryThreadsafeCallback {
OutputStream out,
SkyframeExecutor skyframeExecutor,
TargetAccessor<KeyedConfiguredTargetValue> accessor,
AqueryActionFilter actionFilters) {
AqueryActionFilter actionFilters,
RepositoryMapping mainRepoMapping) {
super(eventHandler, options, out, skyframeExecutor, accessor);
this.actionFilters = actionFilters;
this.mainRepoMapping = mainRepoMapping;
}

@Override
Expand Down Expand Up @@ -145,15 +149,15 @@ private void writeAction(ActionAnalysisMetadata action, PrintStream printStream)

stringBuilder
.append(" Target: ")
.append(actionOwner.getLabel())
.append(actionOwner.getLabel().getDisplayForm(mainRepoMapping))
.append('\n')
.append(" Configuration: ")
.append(configProto.getMnemonic())
.append('\n');
if (actionOwner.getExecutionPlatform() != null) {
stringBuilder
.append(" Execution platform: ")
.append(actionOwner.getExecutionPlatform().label().toString())
.append(actionOwner.getExecutionPlatform().label().getDisplayForm(mainRepoMapping))
.append("\n");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
Expand Down Expand Up @@ -205,17 +206,20 @@ private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfig
cqueryOptions.aspectDeps.createResolver(packageManager, eventHandler);
return ImmutableList.of(
new LabelAndConfigurationOutputFormatterCallback(
eventHandler, cqueryOptions, out, skyframeExecutor, accessor, true),
eventHandler, cqueryOptions, out, skyframeExecutor, accessor, true,
getMainRepoMapping()),
new LabelAndConfigurationOutputFormatterCallback(
eventHandler, cqueryOptions, out, skyframeExecutor, accessor, false),
eventHandler, cqueryOptions, out, skyframeExecutor, accessor, false,
getMainRepoMapping()),
new TransitionsOutputFormatterCallback(
eventHandler,
cqueryOptions,
out,
skyframeExecutor,
accessor,
hostConfiguration,
trimmingTransitionFactory),
trimmingTransitionFactory,
getMainRepoMapping()),
new ProtoOutputFormatterCallback(
eventHandler,
cqueryOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
Expand All @@ -28,16 +29,19 @@
/** Default Output callback for cquery. Prints a label and configuration pair per result. */
public class LabelAndConfigurationOutputFormatterCallback extends CqueryThreadsafeCallback {
private final boolean showKind;
private final RepositoryMapping mainRepoMapping;

LabelAndConfigurationOutputFormatterCallback(
ExtendedEventHandler eventHandler,
CqueryOptions options,
OutputStream out,
SkyframeExecutor skyframeExecutor,
TargetAccessor<KeyedConfiguredTarget> accessor,
boolean showKind) {
boolean showKind,
RepositoryMapping mainRepoMapping) {
super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
this.showKind = showKind;
this.mainRepoMapping = mainRepoMapping;
}

@Override
Expand All @@ -55,7 +59,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult) {
}
output =
output
.append(keyedConfiguredTarget.getLabel())
.append(keyedConfiguredTarget.getLabel().getDisplayForm(mainRepoMapping))
.append(" (")
.append(shortId(getConfiguration(keyedConfiguredTarget.getConfigurationKey())))
.append(")");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.RuleTransitionData;
Expand All @@ -47,7 +48,9 @@ class TransitionsOutputFormatterCallback extends CqueryThreadsafeCallback {
protected final BuildConfigurationValue hostConfiguration;

private final HashMap<Label, Target> partialResultMap;
@Nullable private final TransitionFactory<RuleTransitionData> trimmingTransitionFactory;
@Nullable
private final TransitionFactory<RuleTransitionData> trimmingTransitionFactory;
private final RepositoryMapping mainRepoMapping;

@Override
public String getName() {
Expand All @@ -65,11 +68,13 @@ public String getName() {
SkyframeExecutor skyframeExecutor,
TargetAccessor<KeyedConfiguredTarget> accessor,
BuildConfigurationValue hostConfiguration,
@Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory) {
@Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory,
RepositoryMapping mainRepoMapping) {
super(eventHandler, options, out, skyframeExecutor, accessor, /*uniquifyResults=*/ false);
this.hostConfiguration = hostConfiguration;
this.trimmingTransitionFactory = trimmingTransitionFactory;
this.partialResultMap = Maps.newHashMap();
this.mainRepoMapping = mainRepoMapping;
}

@Override
Expand All @@ -91,8 +96,9 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
addResult(
getRuleClassTransition(keyedConfiguredTarget.getConfiguredTarget(), target)
+ String.format(
"%s (%s)",
keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel(), shortId(config)));
"%s (%s)",
keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel()
.getDisplayForm(mainRepoMapping), shortId(config)));
KnownTargetsDependencyResolver knownTargetsDependencyResolver =
new KnownTargetsDependencyResolver(partialResultMap);
ImmutableSet<ResolvedTransition> dependencies;
Expand All @@ -117,7 +123,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
" "
.concat(dep.attributeName())
.concat("#")
.concat(dep.label().toString())
.concat(dep.label().getDisplayForm(mainRepoMapping))
.concat("#")
.concat(dep.transitionName())
.concat(" -> ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.util.DetailedExitCode;
Expand Down Expand Up @@ -536,6 +537,10 @@ ThreadSafeMutableSet<T> getBuildFiles(
*/
TargetAccessor<T> getAccessor();

default RepositoryMapping getMainRepoMapping() {
return RepositoryMapping.ALWAYS_FALLBACK;
}

/**
* Whether the given setting is enabled. The code should default to return {@code false} for all
* unknown settings. The enum is used rather than a method for each setting so that adding more
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
Expand Down Expand Up @@ -500,6 +501,11 @@ public TargetAccessor<Target> getAccessor() {
return accessor;
}

@Override
public RepositoryMapping getMainRepoMapping() {
return mainRepoTargetParser.getRepoMapping();
}

/** Given a set of target nodes, returns the targets. */
private ThreadSafeMutableSet<Target> getTargetsFromNodes(Iterable<Node<Target>> input) {
ThreadSafeMutableSet<Target> result = createThreadSafeMutableSet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
Expand Down Expand Up @@ -507,4 +508,9 @@ protected void preloadOrThrow(QueryExpression caller, Collection<String> pattern
public TargetAccessor<Target> getAccessor() {
return accessor;
}

@Override
public RepositoryMapping getMainRepoMapping() {
return mainRepoTargetParser.getRepoMapping();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.query2.common.CommonQueryOptions;
import com.google.devtools.build.lib.query2.engine.OutputFormatterCallback;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver;
import com.google.devtools.build.lib.query2.query.output.QueryOptions.OrderOutput;
import java.io.IOException;
Expand All @@ -40,6 +41,7 @@ public void setEventHandler(@Nullable EventHandler eventHandler) {}

@Override
public void output(
QueryEnvironment<?> env,
QueryOptions options,
Digraph<Target> result,
OutputStream out,
Expand All @@ -50,7 +52,7 @@ public void output(
setOptions(options, aspectResolver, hashFunction);
setEventHandler(eventHandler);
OutputFormatterCallback.processAllTargets(
createPostFactoStreamCallback(out, options), getOrderedTargets(result, options));
createPostFactoStreamCallback(out, options, env), getOrderedTargets(result, options));
}

protected Iterable<Target> getOrderedTargets(Digraph<Target> result, QueryOptions options) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,15 +221,15 @@ private String reconstructSelect(Rule rule, Attribute attr) {
/** Query's implementation. */
@Override
public OutputFormatterCallback<Target> createPostFactoStreamCallback(
OutputStream out, final QueryOptions options) {
OutputStream out, final QueryOptions options, QueryEnvironment<?> env) {
return new BuildOutputFormatterCallback(out, options.getLineTerminator());
}

@Override
public ThreadSafeOutputFormatterCallback<Target> createStreamCallback(
OutputStream out, QueryOptions options, QueryEnvironment<?> env) {
return new SynchronizedDelegatingOutputFormatterCallback<>(
createPostFactoStreamCallback(out, options));
createPostFactoStreamCallback(out, options, env));
}

private static class BuildOutputFormatterCallback extends TextOutputFormatterCallback<Target> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.devtools.build.lib.graph.Digraph;
import com.google.devtools.build.lib.graph.Node;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver;
import com.google.devtools.build.lib.query2.query.output.FormatUtils.TargetOrdering;
import com.google.devtools.build.lib.query2.query.output.GraphOutputWriter.NodeReader;
Expand Down Expand Up @@ -53,6 +54,7 @@ public Comparator<Target> comparator() {

@Override
public void output(
QueryEnvironment<?> env,
QueryOptions options,
Digraph<Target> result,
OutputStream out,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public String getName() {

@Override
public OutputFormatterCallback<Target> createPostFactoStreamCallback(
OutputStream out, final QueryOptions options) {
OutputStream out, final QueryOptions options, QueryEnvironment<?> env) {
return new TextOutputFormatterCallback<Target>(out) {
@Override
public void processOutput(Iterable<Target> partialResult) throws IOException {
Expand All @@ -53,7 +53,7 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
writer.append(' ');
}
Label label = target.getLabel();
writer.append(label.getCanonicalForm()).append(lineTerm);
writer.append(label.getDisplayForm(env.getMainRepoMapping())).append(lineTerm);
}
}
};
Expand All @@ -63,6 +63,6 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
public ThreadSafeOutputFormatterCallback<Target> createStreamCallback(
OutputStream out, QueryOptions options, QueryEnvironment<?> env) {
return new SynchronizedDelegatingOutputFormatterCallback<>(
createPostFactoStreamCallback(out, options));
createPostFactoStreamCallback(out, options, env));
}
}
Loading

0 comments on commit 30b3d98

Please sign in to comment.