Skip to content

Commit

Permalink
Get rid of LabelAndConfiguration class: ConfiguredTargetKey contains …
Browse files Browse the repository at this point in the history
…the same information and is more useful, since it's practically a SkyKey.

PiperOrigin-RevId: 179727105
  • Loading branch information
janakdr authored and Copybara-Service committed Dec 20, 2017
1 parent 8a56db3 commit ac2cd35
Show file tree
Hide file tree
Showing 18 changed files with 116 additions and 195 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,11 @@ public final String getExtension() {
}

/**
* Get the {@code LabelAndConfiguration} of the {@code ConfiguredTarget} that owns this artifact,
* if it was set. Otherwise, this should be a dummy value -- either {@link
* ArtifactOwner#NULL_OWNER} or a dummy owner set in tests. Such a dummy value should only occur
* for source artifacts if created without specifying the owner, or for special derived artifacts,
* such as target completion middleman artifacts, build info artifacts, and the like.
* Gets the {@code ActionLookupKey} of the {@code ConfiguredTarget} that owns this artifact, if it
* was set. Otherwise, this should be a dummy value -- either {@link ArtifactOwner#NULL_OWNER} or
* a dummy owner set in tests. Such a dummy value should only occur for source artifacts if
* created without specifying the owner, or for special derived artifacts, such as target
* completion middleman artifacts, build info artifacts, and the like.
*/
public final ArtifactOwner getArtifactOwner() {
return owner;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,24 @@
import com.google.devtools.build.lib.cmdline.Label;

/**
* An interface for {@code LabelAndConfiguration}, or at least for a {@link Label}. Only tests and
* internal {@link Artifact}-generators should implement this interface -- otherwise,
* {@code LabelAndConfiguration} should be the only implementation.
* An interface for {@code ActionLookupKey}, or at least for a {@link Label}. Only tests and
* internal {@link Artifact}-generators should implement this interface -- otherwise, {@code
* ActionLookupKey} and its subclasses should be the only implementation.
*/
public interface ArtifactOwner {
Label getLabel();

@VisibleForTesting
public static final ArtifactOwner NULL_OWNER = new ArtifactOwner() {
@Override
public Label getLabel() {
return null;
}
ArtifactOwner NULL_OWNER =
new ArtifactOwner() {
@Override
public Label getLabel() {
return null;
}

@Override
public String toString() {
return "NULL_OWNER";
}
};
@Override
public String toString() {
return "NULL_OWNER";
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,23 @@
package com.google.devtools.build.lib.analysis;

import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;

/**
* This event is fired during the build, when it becomes known that the analysis
* of a target cannot be completed because of an error in one of its
* dependencies.
*/
public class AnalysisFailureEvent {
private final LabelAndConfiguration failedTarget;
private final ConfiguredTargetKey failedTarget;
private final Label failureReason;

public AnalysisFailureEvent(LabelAndConfiguration failedTarget, Label failureReason) {
public AnalysisFailureEvent(ConfiguredTargetKey failedTarget, Label failureReason) {
this.failedTarget = failedTarget;
this.failureReason = failureReason;
}

public LabelAndConfiguration getFailedTarget() {
public ConfiguredTargetKey getFailedTarget() {
return failedTarget;
}

Expand Down

This file was deleted.

17 changes: 9 additions & 8 deletions src/main/java/com/google/devtools/build/lib/analysis/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -77,10 +78,10 @@ public static boolean containsHyphen(PathFragment path) {
* build file and all toolchain deps.
* note: nodes that are depended on both implicitly and explicitly are considered explicit.
*/
public static ImmutableSet<LabelAndConfiguration> findImplicitDeps(RuleContext ruleContext) {
public static ImmutableSet<ConfiguredTargetKey> findImplicitDeps(RuleContext ruleContext) {
// (1) Consider rule attribute dependencies.
Set<LabelAndConfiguration> maybeImplicitDeps = CompactHashSet.create();
Set<LabelAndConfiguration> explicitDeps = CompactHashSet.create();
Set<ConfiguredTargetKey> maybeImplicitDeps = CompactHashSet.create();
Set<ConfiguredTargetKey> explicitDeps = CompactHashSet.create();
AttributeMap attributes = ruleContext.attributes();
ListMultimap<String, ? extends TransitiveInfoCollection> targetMap =
ruleContext.getConfiguredTargetMap();
Expand All @@ -99,19 +100,19 @@ public static ImmutableSet<LabelAndConfiguration> findImplicitDeps(RuleContext r
if (toolchainContext != null) {
BuildConfiguration config = ruleContext.getConfiguration();
for (Label toolchain : toolchainContext.getResolvedToolchainLabels()) {
maybeImplicitDeps.add(LabelAndConfiguration.of(toolchain, config));
maybeImplicitDeps.add(ConfiguredTargetKey.of(toolchain, config));
}
maybeImplicitDeps.add(
LabelAndConfiguration.of(toolchainContext.getExecutionPlatform().label(), config));
ConfiguredTargetKey.of(toolchainContext.getExecutionPlatform().label(), config));
maybeImplicitDeps.add(
LabelAndConfiguration.of(toolchainContext.getTargetPlatform().label(), config));
ConfiguredTargetKey.of(toolchainContext.getTargetPlatform().label(), config));
}
return ImmutableSet.copyOf(Sets.difference(maybeImplicitDeps, explicitDeps));
}

private static void addLabelsAndConfigs(
Set<LabelAndConfiguration> set, List<? extends TransitiveInfoCollection> deps) {
Set<ConfiguredTargetKey> set, List<? extends TransitiveInfoCollection> deps) {
deps.forEach(
target -> set.add(LabelAndConfiguration.of(target.getLabel(), target.getConfiguration())));
target -> set.add(ConfiguredTargetKey.of(target.getLabel(), target.getConfiguration())));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.LabelAndConfiguration;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
Expand All @@ -38,6 +37,7 @@
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.syntax.Printer;
import java.util.function.Consumer;
Expand All @@ -63,14 +63,14 @@ public enum Mode {
DONT_CHECK
}
/** A set of this target's implicitDeps. */
private final ImmutableSet<LabelAndConfiguration> implicitDeps;
private final ImmutableSet<ConfiguredTargetKey> implicitDeps;

/*
* An interner for the implicitDeps set. {@link Util.findImplicitDeps} is called upon every
* construction of a RuleConfiguredTarget and we expect many of these targets to contain the same
* set of implicit deps so this reduces the memory load per build.
*/
private static final Interner<ImmutableSet<LabelAndConfiguration>> IMPLICIT_DEPS_INTERNER =
private static final Interner<ImmutableSet<ConfiguredTargetKey>> IMPLICIT_DEPS_INTERNER =
BlazeInterners.newWeakInterner();

private final TransitiveInfoProviderMap providers;
Expand Down Expand Up @@ -124,7 +124,7 @@ public ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
return configConditions;
}

public ImmutableSet<LabelAndConfiguration> getImplicitDeps() {
public ImmutableSet<ConfiguredTargetKey> getImplicitDeps() {
return implicitDeps;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.LabelAndConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -104,14 +103,14 @@ public class ConfiguredTargetQueryEnvironment

private static final Function<ConfiguredTarget, SkyKey> CT_TO_SKYKEY =
target -> ConfiguredTargetValue.key(target.getLabel(), target.getConfiguration());
private static final Function<SkyKey, LabelAndConfiguration> SKYKEY_TO_LANDC =
private static final Function<SkyKey, ConfiguredTargetKey> SKYKEY_TO_CTKEY =
skyKey -> {
ConfiguredTargetKey key = (ConfiguredTargetKey) skyKey.argument();
return LabelAndConfiguration.of(key.getLabel(), key.getConfiguration());
return ConfiguredTargetKey.of(key.getLabel(), key.getConfiguration());
};
private static final ImmutableList<TargetPatternKey> ALL_PATTERNS;
private static final KeyExtractor<ConfiguredTarget, LabelAndConfiguration>
CONFIGURED_TARGET_KEY_EXTRACTOR = LabelAndConfiguration::of;
private static final KeyExtractor<ConfiguredTarget, ConfiguredTargetKey>
CONFIGURED_TARGET_KEY_EXTRACTOR = ConfiguredTargetKey::of;

static {
TargetPattern targetPattern;
Expand Down Expand Up @@ -310,13 +309,13 @@ private Collection<ConfiguredTarget> getAllowedDeps(
}
}
if (settings.contains(Setting.NO_IMPLICIT_DEPS) && target instanceof RuleConfiguredTarget) {
Set<LabelAndConfiguration> implicitDeps = ((RuleConfiguredTarget) target).getImplicitDeps();
Set<ConfiguredTargetKey> implicitDeps = ((RuleConfiguredTarget) target).getImplicitDeps();
deps =
deps.stream()
.filter(
dep ->
!implicitDeps.contains(
LabelAndConfiguration.of(
ConfiguredTargetKey.of(
dep.getTarget().getLabel(), dep.getConfiguration())))
.collect(Collectors.toList());
}
Expand All @@ -333,10 +332,10 @@ public ThreadSafeMutableSet<ConfiguredTarget> getFwdDeps(Iterable<ConfiguredTarg
Map<SkyKey, Collection<ConfiguredTarget>> directDeps =
targetifyValues(graph.getDirectDeps(targetsByKey.keySet()));
if (targetsByKey.keySet().size() != directDeps.keySet().size()) {
Iterable<LabelAndConfiguration> missingTargets =
Iterable<ConfiguredTargetKey> missingTargets =
Sets.difference(targetsByKey.keySet(), directDeps.keySet())
.stream()
.map(SKYKEY_TO_LANDC)
.map(SKYKEY_TO_CTKEY)
.collect(Collectors.toList());
eventHandler.handle(Event.warn("Targets were missing from graph: " + missingTargets));
}
Expand Down Expand Up @@ -376,10 +375,10 @@ public Collection<ConfiguredTarget> getReverseDeps(Iterable<ConfiguredTarget> ta
Map<SkyKey, Collection<ConfiguredTarget>> reverseDepsByKey =
targetifyValues(graph.getReverseDeps(targetsByKey.keySet()));
if (targetsByKey.size() != reverseDepsByKey.size()) {
Iterable<LabelAndConfiguration> missingTargets =
Iterable<ConfiguredTargetKey> missingTargets =
Sets.difference(targetsByKey.keySet(), reverseDepsByKey.keySet())
.stream()
.map(SKYKEY_TO_LANDC)
.map(SKYKEY_TO_CTKEY)
.collect(Collectors.toList());
eventHandler.handle(Event.warn("Targets were missing from graph: " + missingTargets));
}
Expand Down Expand Up @@ -407,7 +406,7 @@ public void buildTransitiveClosure(
@Override
public ImmutableList<ConfiguredTarget> getNodesOnPath(ConfiguredTarget from, ConfiguredTarget to)
throws InterruptedException {
return SkyQueryUtils.getNodesOnPath(from, to, this::getFwdDeps, LabelAndConfiguration::of);
return SkyQueryUtils.getNodesOnPath(from, to, this::getFwdDeps, ConfiguredTargetKey::of);
}

@Override
Expand Down
Loading

0 comments on commit ac2cd35

Please sign in to comment.