Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decanonicalize labels emitted by {a,c,}query if possible #16638

Merged
merged 2 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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,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 @@ -536,6 +537,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