From e4f2b49d233c70de15beb4299ced67ddb1909d6e Mon Sep 17 00:00:00 2001 From: John Cater Date: Fri, 1 May 2020 10:17:29 -0400 Subject: [PATCH] Move DependencyKind subclasses into DependencyKind. Cleanup leading to toolchain transitions, #10523. --- .../google/devtools/build/lib/analysis/BUILD | 2 + .../lib/analysis/ConfiguredTargetFactory.java | 10 +-- .../build/lib/analysis/DependencyKind.java | 56 +++++++++++++++++ .../lib/analysis/DependencyResolver.java | 62 ++----------------- .../TransitionsOutputFormatterCallback.java | 2 +- .../build/lib/skyframe/AspectFunction.java | 3 +- .../skyframe/ConfiguredTargetFunction.java | 3 +- .../skyframe/SkyframeDependencyResolver.java | 2 +- .../analysis/util/BuildViewForTesting.java | 2 +- 9 files changed, 73 insertions(+), 69 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index a3a68e9103f8d5..83aca58024a7f9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -711,6 +711,8 @@ java_library( srcs = ["DependencyKind.java"], deps = [ "//src/main/java/com/google/devtools/build/lib/packages", + "//third_party:auto_value", + "//third_party:guava", "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 0a7e260bbefa6d..798e510a42343a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -154,7 +154,7 @@ private TransitiveInfoCollection findPrerequisite( Label label, BuildConfiguration config) { for (ConfiguredTargetAndData prerequisite : - prerequisiteMap.get(DependencyResolver.VISIBILITY_DEPENDENCY)) { + prerequisiteMap.get(DependencyKind.VISIBILITY_DEPENDENCY)) { if (prerequisite.getTarget().getLabel().equals(label) && Objects.equals(prerequisite.getConfiguration(), config)) { return prerequisite.getConfiguredTarget(); @@ -209,7 +209,7 @@ public final ConfiguredTarget createConfiguredTarget( analysisEnvironment, target, config, - prerequisiteMap.get(DependencyResolver.OUTPUT_FILE_RULE_DEPENDENCY), + prerequisiteMap.get(DependencyKind.OUTPUT_FILE_RULE_DEPENDENCY), visibility); if (analysisEnvironment.getSkyframeEnv().valuesMissing()) { return null; @@ -232,7 +232,7 @@ public final ConfiguredTarget createConfiguredTarget( analysisEnvironment, target, config, - prerequisiteMap.get(DependencyResolver.OUTPUT_FILE_RULE_DEPENDENCY), + prerequisiteMap.get(DependencyKind.OUTPUT_FILE_RULE_DEPENDENCY), visibility); SourceArtifact artifact = artifactFactory.getSourceArtifact( @@ -248,7 +248,7 @@ public final ConfiguredTarget createConfiguredTarget( analysisEnvironment, target, config, - prerequisiteMap.get(DependencyResolver.VISIBILITY_DEPENDENCY), + prerequisiteMap.get(DependencyKind.VISIBILITY_DEPENDENCY), visibility); return new PackageGroupConfiguredTarget(targetContext, packageGroup); } else if (target instanceof EnvironmentGroup) { @@ -576,7 +576,7 @@ public static OrderedSetMultimap transformPr OrderedSetMultimap map, Target target) { OrderedSetMultimap result = OrderedSetMultimap.create(); for (Map.Entry entry : map.entries()) { - if (entry.getKey() == DependencyResolver.TOOLCHAIN_DEPENDENCY) { + if (entry.getKey() == DependencyKind.TOOLCHAIN_DEPENDENCY) { continue; } Attribute attribute = entry.getKey().getAttribute(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java index 9543d907d06aad..eae07cdb31a01f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyKind.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.Attribute; import javax.annotation.Nullable; @@ -25,6 +27,15 @@ */ public interface DependencyKind { + /** A dependency for visibility. */ + DependencyKind VISIBILITY_DEPENDENCY = new NonAttributeDependencyKind("VISIBILITY"); + + /** The dependency on the rule that creates a given output file. */ + DependencyKind OUTPUT_FILE_RULE_DEPENDENCY = new NonAttributeDependencyKind("OUTPUT_FILE"); + + /** A dependency on a resolved toolchain. */ + DependencyKind TOOLCHAIN_DEPENDENCY = new NonAttributeDependencyKind("TOOLCHAIN"); + /** * The attribute through which a dependency arises. * @@ -41,4 +52,49 @@ public interface DependencyKind { */ @Nullable AspectClass getOwningAspect(); + + /** A dependency caused by something that's not an attribute. Special cases enumerated below. */ + final class NonAttributeDependencyKind implements DependencyKind { + private final String name; + + private NonAttributeDependencyKind(String name) { + this.name = name; + } + + @Override + public Attribute getAttribute() { + return null; + } + + @Nullable + @Override + public AspectClass getOwningAspect() { + throw new IllegalStateException(); + } + + @Override + public String toString() { + return String.format("%s(%s)", getClass().getSimpleName(), this.name); + } + } + + /** A dependency through an attribute, either that of an aspect or the rule itself. */ + @AutoValue + abstract class AttributeDependencyKind implements DependencyKind { + @Override + public abstract Attribute getAttribute(); + + @Override + @Nullable + public abstract AspectClass getOwningAspect(); + + public static AttributeDependencyKind forRule(Attribute attribute) { + return new AutoValue_DependencyKind_AttributeDependencyKind(attribute, null); + } + + public static AttributeDependencyKind forAspect(Attribute attribute, AspectClass owningAspect) { + return new AutoValue_DependencyKind_AttributeDependencyKind( + attribute, Preconditions.checkNotNull(owningAspect)); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 47c71317799a89..b8e7736264a49a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -13,6 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; +import static com.google.devtools.build.lib.analysis.DependencyKind.OUTPUT_FILE_RULE_DEPENDENCY; +import static com.google.devtools.build.lib.analysis.DependencyKind.TOOLCHAIN_DEPENDENCY; +import static com.google.devtools.build.lib.analysis.DependencyKind.VISIBILITY_DEPENDENCY; + import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -20,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException; +import com.google.devtools.build.lib.analysis.DependencyKind.AttributeDependencyKind; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; @@ -65,63 +70,6 @@ */ public abstract class DependencyResolver { - /** A dependency caused by something that's not an attribute. Special cases enumerated below. */ - private static final class NonAttributeDependencyKind implements DependencyKind { - private final String name; - - private NonAttributeDependencyKind(String name) { - this.name = name; - } - - @Override - public Attribute getAttribute() { - return null; - } - - @Nullable - @Override - public AspectClass getOwningAspect() { - throw new IllegalStateException(); - } - - @Override - public String toString() { - return String.format("%s(%s)", getClass().getSimpleName(), this.name); - } - } - - /** A dependency for visibility. */ - public static final DependencyKind VISIBILITY_DEPENDENCY = - new NonAttributeDependencyKind("VISIBILITY"); - - /** The dependency on the rule that creates a given output file. */ - public static final DependencyKind OUTPUT_FILE_RULE_DEPENDENCY = - new NonAttributeDependencyKind("OUTPUT_FILE"); - - /** A dependency on a resolved toolchain. */ - public static final DependencyKind TOOLCHAIN_DEPENDENCY = - new NonAttributeDependencyKind("TOOLCHAIN"); - - /** A dependency through an attribute, either that of an aspect or the rule itself. */ - @AutoValue - public abstract static class AttributeDependencyKind implements DependencyKind { - @Override - public abstract Attribute getAttribute(); - - @Override - @Nullable - public abstract AspectClass getOwningAspect(); - - public static AttributeDependencyKind forRule(Attribute attribute) { - return new AutoValue_DependencyResolver_AttributeDependencyKind(attribute, null); - } - - public static AttributeDependencyKind forAspect(Attribute attribute, AspectClass owningAspect) { - return new AutoValue_DependencyResolver_AttributeDependencyKind( - attribute, Preconditions.checkNotNull(owningAspect)); - } - } - /** * What we know about a dependency edge after factoring in the properties of the configured target * that the edge originates from, but not the properties of target it points to. diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java index 5cf520785ed1bb..48c98b81b37caf 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java @@ -153,7 +153,7 @@ public void processOutput(Iterable partialResult) throws Inter dep.getTransition().apply(fromOptions, eventHandler).values(); String hostConfigurationChecksum = hostConfiguration.checksum(); String dependencyName; - if (attributeAndDep.getKey() == DependencyResolver.TOOLCHAIN_DEPENDENCY) { + if (attributeAndDep.getKey() == DependencyKind.TOOLCHAIN_DEPENDENCY) { dependencyName = "[toolchain dependency]"; } else { dependencyName = attributeAndDep.getKey().getAttribute().getName(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index 85c75ac9ba1b12..53e52d0a2d675e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.DependencyKind; -import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.DuplicateException; import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException; import com.google.devtools.build.lib.analysis.ResolvedToolchainContext; @@ -454,7 +453,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) targetPkg.getRepositoryMapping(), unloadedToolchainContext, targetDescription, - depValueMap.get(DependencyResolver.TOOLCHAIN_DEPENDENCY)); + depValueMap.get(DependencyKind.TOOLCHAIN_DEPENDENCY)); } return createAspect( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 70772d6242391d..0fc70460b63415 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.DependencyKind; -import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.DuplicateException; import com.google.devtools.build.lib.analysis.EmptyConfiguredTarget; import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException; @@ -333,7 +332,7 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc target.getPackage().getRepositoryMapping(), unloadedContext.getValue(), targetDescription, - depValueMap.get(DependencyResolver.TOOLCHAIN_DEPENDENCY))); + depValueMap.get(DependencyKind.TOOLCHAIN_DEPENDENCY))); } toolchainContexts = contextsBuilder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java index e39da2e578778f..61306d726376e2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java @@ -72,7 +72,7 @@ private void missingEdgeHook( } String message; - if (dependencyKind == TOOLCHAIN_DEPENDENCY) { + if (dependencyKind == DependencyKind.TOOLCHAIN_DEPENDENCY) { message = String.format( "Target '%s' depends on toolchain '%s', which cannot be found: %s'", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java index 92fa67063d623e..60c85e791ea806 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java @@ -556,7 +556,7 @@ public RuleContext getRuleContextForTesting( target.getPackage().getRepositoryMapping(), unloadedToolchainContext.getValue(), targetDescription, - prerequisiteMap.get(DependencyResolver.TOOLCHAIN_DEPENDENCY)); + prerequisiteMap.get(DependencyKind.TOOLCHAIN_DEPENDENCY)); resolvedToolchainContext.addContext(unloadedToolchainContext.getKey(), toolchainContext); }