Remove AspectValueKey.aspectConfigurationIsHost method. PiperOrigin-RevId: 313605453
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java index 4e7512b..d14b5a4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.AspectParameters; -import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey.KeyAndHost; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.SkyFunctionName; import javax.annotation.Nullable; @@ -45,54 +44,42 @@ public static AspectKey createAspectKey( Label label, - BuildConfiguration baseConfiguration, + @Nullable BuildConfiguration baseConfiguration, ImmutableList<AspectKey> baseKeys, AspectDescriptor aspectDescriptor, - BuildConfiguration aspectConfiguration) { - KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); + @Nullable BuildConfiguration aspectConfiguration) { return AspectKey.createAspectKey( ConfiguredTargetKey.of(label, baseConfiguration), baseKeys, aspectDescriptor, - aspectKeyAndHost.key, - aspectKeyAndHost.isHost); + aspectConfiguration == null ? null : BuildConfigurationValue.key(aspectConfiguration)); } public static AspectKey createAspectKey( Label label, - BuildConfiguration baseConfiguration, + @Nullable BuildConfiguration baseConfiguration, AspectDescriptor aspectDescriptor, - BuildConfiguration aspectConfiguration) { - KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); + @Nullable BuildConfiguration aspectConfiguration) { return AspectKey.createAspectKey( ConfiguredTargetKey.of(label, baseConfiguration), ImmutableList.of(), aspectDescriptor, - aspectKeyAndHost.key, - aspectKeyAndHost.isHost); + aspectConfiguration == null ? null : BuildConfigurationValue.key(aspectConfiguration)); } public static StarlarkAspectLoadingKey createStarlarkAspectKey( Label targetLabel, - BuildConfiguration aspectConfiguration, - BuildConfiguration targetConfiguration, + @Nullable BuildConfiguration aspectConfiguration, + @Nullable BuildConfiguration targetConfiguration, Label starlarkFileLabel, String starlarkExportName) { - KeyAndHost keyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration); StarlarkAspectLoadingKey key = - keyAndHost.isHost - ? new HostStarlarkAspectLoadingKey( - targetLabel, - keyAndHost.key, - ConfiguredTargetKey.of(targetLabel, targetConfiguration), - starlarkFileLabel, - starlarkExportName) - : new StarlarkAspectLoadingKey( - targetLabel, - keyAndHost.key, - ConfiguredTargetKey.of(targetLabel, targetConfiguration), - starlarkFileLabel, - starlarkExportName); + new StarlarkAspectLoadingKey( + targetLabel, + aspectConfiguration == null ? null : BuildConfigurationValue.key(aspectConfiguration), + ConfiguredTargetKey.of(targetLabel, targetConfiguration), + starlarkFileLabel, + starlarkExportName); return starlarkAspectKeyInterner.intern(key); } @@ -103,13 +90,13 @@ @AutoCodec public static class AspectKey extends AspectValueKey { private final ImmutableList<AspectKey> baseKeys; - private final BuildConfigurationValue.Key aspectConfigurationKey; + @Nullable private final BuildConfigurationValue.Key aspectConfigurationKey; private final ConfiguredTargetKey baseConfiguredTargetKey; private final AspectDescriptor aspectDescriptor; private int hashCode; private AspectKey( - BuildConfigurationValue.Key aspectConfigurationKey, + @Nullable BuildConfigurationValue.Key aspectConfigurationKey, ConfiguredTargetKey baseConfiguredTargetKey, ImmutableList<AspectKey> baseKeys, AspectDescriptor aspectDescriptor) { @@ -125,14 +112,10 @@ ConfiguredTargetKey baseConfiguredTargetKey, ImmutableList<AspectKey> baseKeys, AspectDescriptor aspectDescriptor, - BuildConfigurationValue.Key aspectConfigurationKey, - boolean aspectConfigurationIsHost) { + @Nullable BuildConfigurationValue.Key aspectConfigurationKey) { return aspectKeyInterner.intern( - aspectConfigurationIsHost - ? new HostAspectKey( - aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor) - : new AspectKey( - aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor)); + new AspectKey( + aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor)); } @Override @@ -175,12 +158,6 @@ } } - // Note that this does not factor into equality/hash-code computations because its value is - // already encoded in the aspectConfigurationKey, albeit in an opaque way. - protected boolean aspectConfigurationIsHost() { - return false; - } - /** * Returns the key of the configured target of the aspect; that is, the configuration in which * the aspect will be evaluated. @@ -198,6 +175,7 @@ * configuration. In untrimmed configuration mode, this configuration will be equivalent to the * base target's configuration. */ + @Nullable BuildConfigurationValue.Key getAspectConfigurationKey() { return aspectConfigurationKey; } @@ -265,11 +243,8 @@ ? "" : String.format(" (over %s)", baseKeys.toString()); return String.format( - "%s with aspect %s%s%s", - getLabel().toString(), - aspectDescriptor.getAspectClass().getName(), - (aspectConfigurationKey != null && aspectConfigurationIsHost()) ? "(host) " : "", - baseKeysString); + "%s with aspect %s%s", + getLabel(), aspectDescriptor.getAspectClass().getName(), baseKeysString); } @Override @@ -282,8 +257,7 @@ + " " + baseConfiguredTargetKey + " " - + aspectDescriptor.getParameters() - + (aspectConfigurationIsHost() ? " (host)" : ""); + + aspectDescriptor.getParameters(); } AspectKey withLabel(Label label) { @@ -296,24 +270,7 @@ ConfiguredTargetKey.of(label, baseConfiguredTargetKey.getConfigurationKey()), newBaseKeys.build(), aspectDescriptor, - aspectConfigurationKey, - aspectConfigurationIsHost()); - } - } - - /** An {@link AspectKey} for an aspect in the host configuration. */ - static class HostAspectKey extends AspectKey { - private HostAspectKey( - BuildConfigurationValue.Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - ImmutableList<AspectKey> baseKeys, - AspectDescriptor aspectDescriptor) { - super(aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor); - } - - @Override - protected boolean aspectConfigurationIsHost() { - return true; + aspectConfigurationKey); } } @@ -436,31 +393,7 @@ baseConfiguredTargetKey, ImmutableList.of(), new AspectDescriptor(aspectClass, AspectParameters.EMPTY), - aspectConfigurationKey, - isAspectConfigurationHost()); - } - } - - /** A {@link StarlarkAspectLoadingKey} for an aspect in the host configuration. */ - private static class HostStarlarkAspectLoadingKey extends StarlarkAspectLoadingKey { - - private HostStarlarkAspectLoadingKey( - Label targetLabel, - BuildConfigurationValue.Key aspectConfigurationKey, - ConfiguredTargetKey baseConfiguredTargetKey, - Label starlarkFileLabel, - String starlarkFunctionName) { - super( - targetLabel, - aspectConfigurationKey, - baseConfiguredTargetKey, - starlarkFileLabel, - starlarkFunctionName); - } - - @Override - protected boolean isAspectConfigurationHost() { - return true; + aspectConfigurationKey); } } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java index 0e69099..3e65fd6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -84,14 +84,6 @@ return interner.intern(new ConfiguredTargetKey(label, configurationKey)); } - // TODO(katre): Remove this. - static KeyAndHost keyFromConfiguration(@Nullable BuildConfiguration configuration) { - return configuration == null - ? KeyAndHost.NULL_INSTANCE - : new KeyAndHost( - BuildConfigurationValue.key(configuration), configuration.isHostConfiguration()); - } - @Override public Label getLabel() { return label; @@ -165,20 +157,4 @@ public String toString() { return String.format("%s %s", label, configurationKey); } - - /** - * Simple wrapper class for turning a {@link BuildConfiguration} into a {@link - * BuildConfigurationValue.Key} and boolean isHost. - */ - public static class KeyAndHost { - private static final KeyAndHost NULL_INSTANCE = new KeyAndHost(null, false); - - @Nullable public final BuildConfigurationValue.Key key; - final boolean isHost; - - private KeyAndHost(@Nullable BuildConfigurationValue.Key key, boolean isHost) { - this.key = key; - this.isHost = isHost; - } - } }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java index 4eb4291..1e31395 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporterTest.java
@@ -59,11 +59,7 @@ SkyKey aspectKey = AspectValueKey.AspectKey.createAspectKey( - ctKey, - ImmutableList.of(), - null, - BuildConfigurationValue.key(targetConfig), - /*aspectConfigurationIsHost=*/ false); + ctKey, ImmutableList.of(), null, BuildConfigurationValue.key(targetConfig)); assertThat(cycleReporter.getAdditionalMessageAboutCycle(reporter, aspectKey, cycle)) .contains( "The cycle is caused by a visibility edge from //foo:b to the non-package-group "