Skip to content

Commit

Permalink
WIP: Decanonicalize repo names in query output if possible
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Oct 16, 2022
1 parent 95bda29 commit cb269bb
Show file tree
Hide file tree
Showing 33 changed files with 140 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ public String getUnambiguousCanonicalForm() {
return packageIdentifier.getUnambiguousCanonicalForm() + ":" + name;
}

public String getDisplayForm(RepositoryMapping mainRepoMapping) {
return packageIdentifier.getDisplayForm(mainRepoMapping) + ":" + name;
}

/** Return the name of the repository label refers to without the leading `at` symbol. */
@StarlarkMethod(
name = "workspace_name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ public String getUnambiguousCanonicalForm() {
return String.format("@@%s//%s", getRepository().getName(), getPackageFragment());
}

public String getDisplayForm(RepositoryMapping mainRepoMapping) {
if (repository.isMain()) {
return getCanonicalForm();
}
return mainRepoMapping.getLocalNameFromMainRepo(repository)
.map(localName -> String.format("@%s//%s", localName, getPackageFragment()))
.orElseGet(this::getUnambiguousCanonicalForm);
}

/**
* Returns the package path, possibly qualified with a repository name.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -93,4 +95,17 @@ public RepositoryName get(String preMappingName) {
return RepositoryName.createUnvalidated(preMappingName).toNonVisible(ownerRepo());
}
}

public Optional<String> getLocalNameFromMainRepo(RepositoryName postMappingName) {
if (ownerRepo() == null) {
// This can only happen if Bzlmod is not enabled. In this case, assume no repo mappings are
// in use, so that canonical and local names can be used interchangeably.
return Optional.of(postMappingName.getName());
}
Preconditions.checkArgument(ownerRepo().isMain());
return repositoryMapping().entrySet().stream()
.filter(e -> e.getValue().equals(postMappingName))
.map(Entry::getKey)
.findFirst();
}
}
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
Loading

0 comments on commit cb269bb

Please sign in to comment.