Make a bunch more stuff serializable, especially anonymous classes and lambdas. Give a bit more information in JavaSerializableCodec when serialization fails on an anonymous class or lambda.

PiperOrigin-RevId: 190472245

4 files changed, 60 insertions, 36 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 5ea8584bbe..ca48a59e4b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -41,6 +41,7 @@ import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TestSize;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
@@ -48,7 +49,8 @@ import com.google.devtools.build.lib.util.FileTypeSet;
  * Rule class definitions used by (almost) every rule.
  */
 public class BaseRuleClasses {
-  private static final Attribute.ComputedDefault testonlyDefault =
+  @AutoCodec @AutoCodec.VisibleForSerialization
+  static final Attribute.ComputedDefault testonlyDefault =
       new Attribute.ComputedDefault() {
         @Override
         public Object getDefault(AttributeMap rule) {
@@ -56,7 +58,8 @@ public class BaseRuleClasses {
         }
       };
 
-  private static final Attribute.ComputedDefault deprecationDefault =
+  @AutoCodec @AutoCodec.VisibleForSerialization
+  static final Attribute.ComputedDefault deprecationDefault =
       new Attribute.ComputedDefault() {
         @Override
         public Object getDefault(AttributeMap rule) {
@@ -73,7 +76,7 @@ public class BaseRuleClasses {
    * they only run on the target configuration and should not operate on action_listeners and
    * extra_actions themselves (to avoid cycles).
    */
-  @VisibleForTesting
+  @AutoCodec @VisibleForTesting
   static final LabelListLateBoundDefault<?> ACTION_LISTENER =
       LabelListLateBoundDefault.fromTargetConfiguration(
           BuildConfiguration.class,
@@ -81,6 +84,7 @@ public class BaseRuleClasses {
 
   // TODO(b/65746853): provide a way to do this without passing the entire configuration
   /** Implementation for the :run_under attribute. */
+  @AutoCodec
   public static final LabelLateBoundDefault<?> RUN_UNDER =
       LabelLateBoundDefault.fromTargetConfiguration(
           BuildConfiguration.class,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index fb01a4a9c7..c45ec3c66d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -152,20 +152,7 @@ public class SkylarkRuleClassFunctions {
             attr("timeout", STRING)
                 .taggable()
                 .nonconfigurable("used in loading phase rule validation logic")
-                .value(
-                    new Attribute.ComputedDefault() {
-                      @Override
-                      public Object getDefault(AttributeMap rule) {
-                        TestSize size = TestSize.getTestSize(rule.get("size", Type.STRING));
-                        if (size != null) {
-                          String timeout = size.getDefaultTimeout().toString();
-                          if (timeout != null) {
-                            return timeout;
-                          }
-                        }
-                        return "illegal";
-                      }
-                    }))
+                .value(timeoutAttribute))
         .add(
             attr("flaky", BOOLEAN)
                 .value(false)
@@ -186,14 +173,16 @@ public class SkylarkRuleClassFunctions {
                 .value(
                     ImmutableList.of(
                         labelCache.getUnchecked(toolsRepository + "//tools/test:runtime"))))
-        .add(attr("$test_setup_script", LABEL)
-            .cfg(HostTransition.INSTANCE)
-            .singleArtifact()
-            .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_setup")))
-        .add(attr("$collect_coverage_script", LABEL)
-            .cfg(HostTransition.INSTANCE)
-            .singleArtifact()
-            .value(labelCache.getUnchecked(toolsRepository + "//tools/test:collect_coverage")))
+        .add(
+            attr("$test_setup_script", LABEL)
+                .cfg(HostTransition.INSTANCE)
+                .singleArtifact()
+                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_setup")))
+        .add(
+            attr("$collect_coverage_script", LABEL)
+                .cfg(HostTransition.INSTANCE)
+                .singleArtifact()
+                .value(labelCache.getUnchecked(toolsRepository + "//tools/test:collect_coverage")))
         // Input files for test actions collecting code coverage
         .add(
             attr("$coverage_support", LABEL)
@@ -205,12 +194,26 @@ public class SkylarkRuleClassFunctions {
                 .cfg(HostTransition.INSTANCE)
                 .value(labelCache.getUnchecked("//tools/defaults:coverage_report_generator"))
                 .singleArtifact())
-        .add(attr(":run_under", LABEL)
-            .cfg(lipoDataTransition)
-            .value(RUN_UNDER))
+        .add(attr(":run_under", LABEL).cfg(lipoDataTransition).value(RUN_UNDER))
         .build();
   }
 
+  @AutoCodec @AutoCodec.VisibleForSerialization
+  static final Attribute.ComputedDefault timeoutAttribute =
+      new Attribute.ComputedDefault() {
+        @Override
+        public Object getDefault(AttributeMap rule) {
+          TestSize size = TestSize.getTestSize(rule.get("size", Type.STRING));
+          if (size != null) {
+            String timeout = size.getDefaultTimeout().toString();
+            if (timeout != null) {
+              return timeout;
+            }
+          }
+          return "illegal";
+        }
+      };
+
   @SkylarkSignature(
     name = "struct",
     returnType = Info.class,
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index 67b383d533..933790f253 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -287,10 +287,12 @@ public final class Attribute implements Comparable<Attribute> {
    * Implementation of {@link SplitTransitionProvider} that returns a single {@link SplitTransition}
    * regardless of the originating rule.
    */
-  private static class BasicSplitTransitionProvider implements SplitTransitionProvider {
-
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec
+  static class BasicSplitTransitionProvider implements SplitTransitionProvider {
     private final SplitTransition splitTransition;
 
+    @AutoCodec.VisibleForSerialization
     BasicSplitTransitionProvider(SplitTransition splitTransition) {
       this.splitTransition = splitTransition;
     }
@@ -1599,10 +1601,10 @@ public final class Attribute implements Comparable<Attribute> {
     }
   }
 
-  private static class SimpleLateBoundDefault<FragmentT, ValueT>
+  @AutoCodec.VisibleForSerialization
+  static class SimpleLateBoundDefault<FragmentT, ValueT>
       extends LateBoundDefault<FragmentT, ValueT> {
-
-    private final Resolver<FragmentT, ValueT> resolver;
+    @AutoCodec.VisibleForSerialization protected final Resolver<FragmentT, ValueT> resolver;
 
     private SimpleLateBoundDefault(boolean useHostConfiguration,
         Class<FragmentT> fragmentClass,
@@ -1745,9 +1747,11 @@ public final class Attribute implements Comparable<Attribute> {
   }
 
   /** A {@link LateBoundDefault} for a {@link Label}. */
+  @AutoCodec
   public static class LabelLateBoundDefault<FragmentT>
       extends SimpleLateBoundDefault<FragmentT, Label> {
-    private LabelLateBoundDefault(
+    @AutoCodec.VisibleForSerialization
+    LabelLateBoundDefault(
         boolean useHostConfiguration,
         Class<FragmentT> fragmentClass,
         Label defaultValue,
@@ -1830,9 +1834,11 @@ public final class Attribute implements Comparable<Attribute> {
   }
 
   /** A {@link LateBoundDefault} for a {@link List} of {@link Label} objects. */
+  @AutoCodec
   public static class LabelListLateBoundDefault<FragmentT>
       extends SimpleLateBoundDefault<FragmentT, List<Label>> {
-    private LabelListLateBoundDefault(
+    @AutoCodec.VisibleForSerialization
+    LabelListLateBoundDefault(
         boolean useHostConfiguration,
         Class<FragmentT> fragmentClass,
         Resolver<FragmentT, List<Label>> resolver) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/JavaSerializableCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/JavaSerializableCodec.java
index 859b400826..487feab4a1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/JavaSerializableCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/JavaSerializableCodec.java
@@ -40,8 +40,19 @@ class JavaSerializableCodec implements ObjectCodec<Object> {
     try {
       objOut.writeObject(obj);
     } catch (NotSerializableException e) {
+      Class<?> clazz = obj.getClass();
+      Class<?> parentClass = null;
+      if (clazz.isAnonymousClass() || clazz.isSynthetic()) {
+        parentClass = clazz.getSuperclass();
+      }
       throw new SerializationException.NoCodecException(
-          "Object " + obj + " of type " + obj.getClass() + " not serializable", e);
+          "Object "
+              + obj
+              + " of type "
+              + obj.getClass()
+              + (parentClass == null ? "" : " (parent " + parentClass + ")")
+              + " not serializable",
+          e);
     } catch (NotSerializableRuntimeException e) {
       // Values that inherit from Serializable but actually aren't serializable.
       throw new SerializationException.NoCodecException(