Add Codec for NativeAspectClass. As a result, remove @AutoCodec from concrete subclasses. Improve debugging message on serialization failures.
Lot of test-side changes to make sure aspects are properly registered with the RuleClassProvider.
PiperOrigin-RevId: 185607202
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
index 905862a..ed6b77b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeAspectClass.java
@@ -14,6 +14,16 @@
package com.google.devtools.build.lib.packages;
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
+import com.google.devtools.build.lib.skyframe.serialization.strings.StringCodecs;
+import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
+import java.io.IOException;
+
/**
* A class of aspects that are implemented natively in Bazel.
*
@@ -21,6 +31,7 @@
* aspect factory. All wrappers of the same class are equal.
*/
public abstract class NativeAspectClass implements AspectClass {
+ public static final ObjectCodec<NativeAspectClass> CODEC = new Codec();
@Override
public String getName() {
@@ -29,4 +40,30 @@
public abstract AspectDefinition getDefinition(AspectParameters aspectParameters);
+ private static class Codec implements ObjectCodec<NativeAspectClass> {
+ @Override
+ public Class<NativeAspectClass> getEncodedClass() {
+ return NativeAspectClass.class;
+ }
+
+ @Override
+ public void serialize(
+ SerializationContext context, NativeAspectClass obj, CodedOutputStream codedOut)
+ throws SerializationException, IOException {
+ RuleClassProvider ruleClassProvider = context.getDependency(RuleClassProvider.class);
+ NativeAspectClass storedAspect = ruleClassProvider.getNativeAspectClass(obj.getKey());
+ Preconditions.checkState(
+ obj == storedAspect, "Not stored right: %s %s %s", obj, storedAspect, ruleClassProvider);
+ StringCodecs.asciiOptimized().serialize(context, obj.getKey(), codedOut);
+ }
+
+ @Override
+ public NativeAspectClass deserialize(DeserializationContext context, CodedInputStream codedIn)
+ throws SerializationException, IOException {
+ String aspectKey = StringCodecs.asciiOptimized().deserialize(context, codedIn);
+ return Preconditions.checkNotNull(
+ context.getDependency(RuleClassProvider.class).getNativeAspectClass(aspectKey),
+ aspectKey);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeAspect.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeAspect.java
index 00b8248..4048b2f 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeAspect.java
@@ -16,15 +16,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
/** A natively-defined aspect that is may be referenced by skylark attribute definitions. */
-@AutoCodec(strategy = AutoCodec.Strategy.POLYMORPHIC)
public abstract class SkylarkNativeAspect extends NativeAspectClass implements SkylarkAspect {
- public static final ObjectCodec<SkylarkNativeAspect> CODEC = new SkylarkNativeAspect_AutoCodec();
-
@Override
public void repr(SkylarkPrinter printer) {
printer.append("<native aspect>");
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
index 42858be..ff56c61 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java
@@ -28,8 +28,6 @@
import com.google.devtools.build.lib.rules.java.JavaInfo;
import com.google.devtools.build.lib.rules.java.JavaRuntimeJarProvider;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndTarget;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.ArrayList;
import java.util.List;
@@ -41,11 +39,7 @@
* <p>One would think that using the compile time classpath would be enough, but alas, those are
* ijars,
*/
-@AutoCodec
public class AndroidNeverlinkAspect extends NativeAspectClass implements ConfiguredAspectFactory {
- public static final ObjectCodec<AndroidNeverlinkAspect> CODEC =
- new AndroidNeverlinkAspect_AutoCodec();
-
public static final String NAME = "AndroidNeverlinkAspect";
private static final ImmutableList<String> ATTRIBUTES =
ImmutableList.of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index 92eaa87..e8be797 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -65,7 +65,6 @@
import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainProvider;
import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndTarget;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.HashMap;
import java.util.HashSet;
@@ -78,8 +77,6 @@
public final class DexArchiveAspect extends NativeAspectClass implements ConfiguredAspectFactory {
public static final String NAME = "DexArchiveAspect";
- public static final ObjectCodec<DexArchiveAspect> CODEC = new DexArchiveAspect_AutoCodec();
-
/**
* Function that returns a {@link Rule}'s {@code incremental_dexing} attribute for use by this
* aspect. Must be provided when attaching this aspect to a target.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 5de8b46..984d145 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -73,17 +73,13 @@
import com.google.devtools.build.lib.rules.proto.ProtoSupportDataProvider;
import com.google.devtools.build.lib.rules.proto.SupportData;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndTarget;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;
/** J2ObjC transpilation aspect for Java and proto rules. */
-@AutoCodec
public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectFactory {
public static final String NAME = "J2ObjcAspect";
- public static final ObjectCodec<J2ObjcAspect> CODEC = new J2ObjcAspect_AutoCodec();
private final String toolsRepository;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
index 9bd8e02..010f2bc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
@@ -30,19 +30,14 @@
import com.google.devtools.build.lib.packages.SkylarkNativeAspect;
import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndTarget;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
/**
* Aspect that gathers the proto dependencies of the attached rule target, and propagates the proto
* values of its dependencies through the ObjcProtoProvider.
*/
-@AutoCodec
public class ObjcProtoAspect extends SkylarkNativeAspect implements ConfiguredAspectFactory {
public static final String NAME = "ObjcProtoAspect";
- public static final ObjectCodec<ObjcProtoAspect> CODEC = new ObjcProtoAspect_AutoCodec();
-
@Override
public AspectDefinition getDefinition(AspectParameters aspectParameters) {
return new AspectDefinition.Builder(this)
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index e60bfeb..a3cb103 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -409,16 +409,25 @@
* Rule definitions to be used in emptyAspectAttributesAreAvailableInRuleContext().
*/
public static class EmptyAspectAttributesAreAvailableInRuleContext {
- public static final MockRule TEST_RULE = () ->
- MockRule.ancestor(TestAspects.BASE_RULE.getClass()).factory(DummyRuleFactory.class).define(
- "testrule",
- (builder, env) ->
- builder
- .add(attr("foo", LABEL_LIST).legacyAllowAnyFileType()
- .aspect(new AspectWithEmptyLateBoundAttribute())));
+ public static final MockRule TEST_RULE =
+ () ->
+ MockRule.ancestor(TestAspects.BASE_RULE.getClass())
+ .factory(DummyRuleFactory.class)
+ .define(
+ "testrule",
+ (builder, env) ->
+ builder.add(
+ attr("foo", LABEL_LIST)
+ .legacyAllowAnyFileType()
+ .aspect(AspectWithEmptyLateBoundAttribute.INSTANCE)));
public static class AspectWithEmptyLateBoundAttribute extends NativeAspectClass
implements ConfiguredAspectFactory {
+ static final AspectWithEmptyLateBoundAttribute INSTANCE =
+ new AspectWithEmptyLateBoundAttribute();
+
+ private AspectWithEmptyLateBoundAttribute() {}
+
@Override
public AspectDefinition getDefinition(AspectParameters params) {
return new AspectDefinition.Builder(this)
@@ -449,8 +458,13 @@
*/
@Test
public void emptyAspectAttributesAreAvailableInRuleContext() throws Exception {
- setRulesAvailableInTests(TestAspects.BASE_RULE,
- EmptyAspectAttributesAreAvailableInRuleContext.TEST_RULE);
+ setRulesAndAspectsAvailableInTests(
+ ImmutableList.of(
+ TestAspects.SIMPLE_ASPECT,
+ EmptyAspectAttributesAreAvailableInRuleContext.AspectWithEmptyLateBoundAttribute
+ .INSTANCE),
+ ImmutableList.of(
+ TestAspects.BASE_RULE, EmptyAspectAttributesAreAvailableInRuleContext.TEST_RULE));
pkg("a",
"testrule(name='a', foo=[':b'])",
"testrule(name='b')");
@@ -462,19 +476,30 @@
* Rule definitions to be used in extraActionsAreEmitted().
*/
public static class ExtraActionsAreEmitted {
- public static final MockRule TEST_RULE = () ->
- MockRule.ancestor(TestAspects.BASE_RULE.getClass()).factory(DummyRuleFactory.class).define(
- "testrule",
- (builder, env) ->
- builder
- .add(attr("foo", LABEL_LIST).legacyAllowAnyFileType()
- .aspect(new AspectThatRegistersAction()))
- .add(attr(":action_listener", LABEL_LIST)
- .cfg(HostTransition.INSTANCE)
- .value(ACTION_LISTENER)));
+ public static final MockRule TEST_RULE =
+ () ->
+ MockRule.ancestor(TestAspects.BASE_RULE.getClass())
+ .factory(DummyRuleFactory.class)
+ .define(
+ "testrule",
+ (builder, env) ->
+ builder
+ .add(
+ attr("foo", LABEL_LIST)
+ .legacyAllowAnyFileType()
+ .aspect(AspectThatRegistersAction.INSTANCE))
+ .add(
+ attr(":action_listener", LABEL_LIST)
+ .cfg(HostTransition.INSTANCE)
+ .value(ACTION_LISTENER)));
public static class AspectThatRegistersAction extends NativeAspectClass
implements ConfiguredAspectFactory {
+
+ static final AspectThatRegistersAction INSTANCE = new AspectThatRegistersAction();
+
+ private AspectThatRegistersAction() {}
+
@Override
public AspectDefinition getDefinition(AspectParameters params) {
return new AspectDefinition.Builder(this).build();
@@ -500,7 +525,10 @@
*/
@Test
public void extraActionsAreEmitted() throws Exception {
- setRulesAvailableInTests(TestAspects.BASE_RULE, ExtraActionsAreEmitted.TEST_RULE);
+ setRulesAndAspectsAvailableInTests(
+ ImmutableList.of(
+ TestAspects.SIMPLE_ASPECT, ExtraActionsAreEmitted.AspectThatRegistersAction.INSTANCE),
+ ImmutableList.of(TestAspects.BASE_RULE, ExtraActionsAreEmitted.TEST_RULE));
useConfiguration("--experimental_action_listener=//extra_actions:listener");
scratch.file(
"extra_actions/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index fc9aa99..279675f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -500,8 +500,21 @@
* Also see {@link AnalysisTestCase#setRulesAndAspectsAvailableInTests(Iterable, Iterable)}.
*/
protected void setRulesAvailableInTests(RuleDefinition... rules) throws Exception {
+ // Not all of these aspects are needed for all tests, but it makes it simple to offer them all.
setRulesAndAspectsAvailableInTests(
- ImmutableList.<NativeAspectClass>of(),
+ ImmutableList.of(
+ TestAspects.SIMPLE_ASPECT,
+ TestAspects.PARAMETRIZED_DEFINITION_ASPECT,
+ TestAspects.ASPECT_REQUIRING_PROVIDER,
+ TestAspects.FALSE_ADVERTISEMENT_ASPECT,
+ TestAspects.ALL_ATTRIBUTES_ASPECT,
+ TestAspects.ALL_ATTRIBUTES_WITH_TOOL_ASPECT,
+ TestAspects.BAR_PROVIDER_ASPECT,
+ TestAspects.EXTRA_ATTRIBUTE_ASPECT,
+ TestAspects.FOO_PROVIDER_ASPECT,
+ TestAspects.ASPECT_REQUIRING_PROVIDER_SETS,
+ TestAspects.WARNING_ASPECT,
+ TestAspects.ERROR_ASPECT),
ImmutableList.copyOf(rules));
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index 353f62f..dcea00a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -444,12 +444,11 @@
}
}
- private static final ParametrizedDefinitionAspect PARAMETRIZED_DEFINITION_ASPECT =
+ static final ParametrizedDefinitionAspect PARAMETRIZED_DEFINITION_ASPECT =
new ParametrizedDefinitionAspect();
- private static final AspectRequiringProvider ASPECT_REQUIRING_PROVIDER =
- new AspectRequiringProvider();
- private static final AspectRequiringProviderSets ASPECT_REQUIRING_PROVIDER_SETS =
+ static final AspectRequiringProvider ASPECT_REQUIRING_PROVIDER = new AspectRequiringProvider();
+ static final AspectRequiringProviderSets ASPECT_REQUIRING_PROVIDER_SETS =
new AspectRequiringProviderSets();
private static final AspectDefinition ASPECT_REQUIRING_PROVIDER_DEFINITION =
new AspectDefinition.Builder(ASPECT_REQUIRING_PROVIDER)
