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.

Closes #16483.

PiperOrigin-RevId: 485584469
Change-Id: I9037bf128713af75b6741eca42b25e3beeb112f7
  • Loading branch information
fmeum authored and copybara-github committed Nov 2, 2022
1 parent 8f95651 commit a757852
Show file tree
Hide file tree
Showing 34 changed files with 280 additions and 75 deletions.
16 changes: 13 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 @@ -419,9 +419,19 @@ 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 mainRepositoryMapping the {@link RepositoryMapping} of the main repository
* @return analogous to {@link PackageIdentifier#getDisplayForm(RepositoryMapping)}
*/
public String getDisplayForm(RepositoryMapping mainRepositoryMapping) {
return packageIdentifier.getDisplayForm(mainRepositoryMapping) + ":" + 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 @@ -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 @@ -965,6 +966,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,13 @@ 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 @@ -205,17 +205,30 @@ 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 Expand Up @@ -251,7 +264,8 @@ private static ImmutableMap<String, BuildConfigurationValue> getTransitiveConfig
out,
skyframeExecutor,
accessor,
kct -> getFwdDeps(ImmutableList.of(kct))),
kct -> getFwdDeps(ImmutableList.of(kct)),
getMainRepoMapping()),
new StarlarkOutputFormatterCallback(
eventHandler, cqueryOptions, out, skyframeExecutor, accessor),
new FilesOutputFormatterCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableSet;
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.graph.Digraph;
import com.google.devtools.build.lib.graph.Node;
Expand Down Expand Up @@ -65,12 +66,15 @@ Iterable<KeyedConfiguredTarget> getDirectDeps(KeyedConfiguredTarget target)
};

@Override
public String getLabel(Node<KeyedConfiguredTarget> node) {
public String getLabel(
Node<KeyedConfiguredTarget> node, RepositoryMapping mainRepositoryMapping) {
// Node payloads are ConfiguredTargets. Output node labels are target labels + config
// hashes.
KeyedConfiguredTarget kct = node.getLabel();
return String.format(
"%s (%s)", kct.getLabel(), shortId(getConfiguration(kct.getConfigurationKey())));
"%s (%s)",
kct.getLabel().getDisplayForm(mainRepositoryMapping),
shortId(getConfiguration(kct.getConfigurationKey())));
}

@Override
Expand All @@ -79,15 +83,19 @@ public Comparator<KeyedConfiguredTarget> comparator() {
}
};

private final RepositoryMapping mainRepoMapping;

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

@Override
Expand Down Expand Up @@ -117,7 +125,8 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
// select() conditions don't matter for cquery because cquery operates post-analysis
// phase, when select()s have been resolved and removed from the graph.
/*maxConditionalEdges=*/ 0,
options.graphFactored);
options.graphFactored,
mainRepoMapping);
graphWriter.write(graph, /*conditionalEdges=*/ null, outputStream);
}
}
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 @@ -48,6 +49,7 @@ class TransitionsOutputFormatterCallback extends CqueryThreadsafeCallback {

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

@Override
public String getName() {
Expand All @@ -65,11 +67,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 @@ -92,7 +96,11 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
getRuleClassTransition(keyedConfiguredTarget.getConfiguredTarget(), target)
+ String.format(
"%s (%s)",
keyedConfiguredTarget.getConfiguredTarget().getOriginalLabel(), shortId(config)));
keyedConfiguredTarget
.getConfiguredTarget()
.getOriginalLabel()
.getDisplayForm(mainRepoMapping),
shortId(config)));
KnownTargetsDependencyResolver knownTargetsDependencyResolver =
new KnownTargetsDependencyResolver(partialResultMap);
ImmutableSet<ResolvedTransition> dependencies;
Expand All @@ -117,7 +125,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 @@ -567,6 +568,13 @@ ThreadSafeMutableSet<T> getBuildFiles(
*/
TargetAccessor<T> getAccessor();

/**
* Returns the {@link RepositoryMapping} of the main repository so that output formatters can
* resolve canonical repository names in labels back to the more readable local names used by the
* main repository.
*/
RepositoryMapping getMainRepoMapping();

/**
* 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();
}
}
Loading

0 comments on commit a757852

Please sign in to comment.