Push down Package serialization io logic

Rearrange Package serialization logic to hide ProtocolBuffer implementation
details and make it easier to transition to a more "streamed" approach.

Also moved the public members of PackageSerializer to the top of the class
for visibility.

--
MOS_MIGRATED_REVID=94905360

3 files changed, 226 insertions, 212 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 91c328d8c7..1f5dc2a429 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -233,25 +233,19 @@ public class Package implements Serializable {
   }
 
   private void writeObject(ObjectOutputStream out) {
-    com.google.devtools.build.lib.query2.proto.proto2api.Build.Package pb =
-        PackageSerializer.serializePackage(this);
     try {
-      pb.writeDelimitedTo(out);
-    } catch (IOException e) {
-      throw new IllegalStateException(e);
+      PackageSerializer.serializePackage(this, out);
+    } catch (IOException ioe) {
+      throw new IllegalStateException(ioe);
     }
   }
 
   private void readObject(ObjectInputStream in) throws IOException {
-    com.google.devtools.build.lib.query2.proto.proto2api.Build.Package pb =
-        com.google.devtools.build.lib.query2.proto.proto2api.Build.Package.parseDelimitedFrom(in);
-    Package pkg;
     try {
-      pkg = new PackageDeserializer(null, null).deserialize(pb);
+      deserializedPkg = new PackageDeserializer(null, null).deserialize(in);
     } catch (PackageDeserializationException e) {
       throw new IllegalStateException(e);
     }
-    deserializedPkg = pkg;
   }
 
   protected Object readResolve() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
index bb6c8905e5..d9a55e045a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageDeserializer.java
@@ -38,6 +38,8 @@ import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -135,7 +137,7 @@ public class PackageDeserializer {
             ruleLabel, packageBuilder, ruleLocation, attributeValues,
             NullEventHandler.INSTANCE);
         packageBuilder.addRule(rule);
-        
+
         Preconditions.checkState(!rule.containsErrors());
       } catch (NameConflictException | SyntaxException e) {
         throw new PackageDeserializationException(e);
@@ -378,11 +380,19 @@ public class PackageDeserializer {
   }
 
   /**
-   * Deserialize a protocol message to a package. The inverse of
+   * Deserializes a {@link Package} from {@code in}. The inverse of
    * {@link PackageSerializer#serializePackage}.
+   *
+   * <p>Expects {@code in} to contain a single
+   * {@link com.google.devtools.build.lib.query2.proto.proto2api.Build.Package} message.
+   *
+   * @param in stream to read from
+   * @return a new {@link Package} as read from {@code in}
+   * @throws PackageDeserializationException on failures deserializing the input
+   * @throws IOException on failures reading from {@code in}
    */
-  public Package deserialize(Build.Package packagePb)
-      throws PackageDeserializationException {
+  public Package deserialize(InputStream in) throws PackageDeserializationException, IOException {
+    Build.Package packagePb = Build.Package.parseDelimitedFrom(in);
     Package.Builder builder;
     try {
       builder = new Package.Builder(
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageSerializer.java b/src/main/java/com/google/devtools/build/lib/packages/PackageSerializer.java
index 9d40bb4aca..0542e8eaf8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageSerializer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageSerializer.java
@@ -45,6 +45,8 @@ import com.google.devtools.build.lib.syntax.GlobCriteria;
 import com.google.devtools.build.lib.syntax.GlobList;
 import com.google.devtools.build.lib.syntax.Label;
 
+import java.io.IOException;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedList;
@@ -55,209 +57,22 @@ import java.util.Map;
  * Functionality to serialize loaded packages.
  */
 public class PackageSerializer {
-  private static Build.SourceFile serializeInputFile(InputFile inputFile) {
-    Build.SourceFile.Builder result = Build.SourceFile.newBuilder();
-    result.setName(inputFile.getLabel().toString());
-    if (inputFile.isVisibilitySpecified()) {
-      for (Label visibilityLabel : inputFile.getVisibility().getDeclaredLabels()) {
-        result.addVisibilityLabel(visibilityLabel.toString());
-      }
-    }
-    if (inputFile.isLicenseSpecified()) {
-      result.setLicense(serializeLicense(inputFile.getLicense()));
-    }
-
-    result.setParseableLocation(serializeLocation(inputFile.getLocation()));
-    return result.build();
-  }
-
-  private static Build.Location serializeLocation(Location location) {
-    Build.Location.Builder result = Build.Location.newBuilder();
-
-    result.setStartOffset(location.getStartOffset());
-    if (location.getStartLineAndColumn() != null) {
-      result.setStartLine(location.getStartLineAndColumn().getLine());
-      result.setStartColumn(location.getStartLineAndColumn().getColumn());
-    }
-
-    result.setEndOffset(location.getEndOffset());
-    if (location.getEndLineAndColumn() != null) {
-      result.setEndLine(location.getEndLineAndColumn().getLine());
-      result.setEndColumn(location.getEndLineAndColumn().getColumn());
-    }
-
-    return result.build();
-  }
-
-  private static Build.PackageGroup serializePackageGroup(PackageGroup packageGroup) {
-    Build.PackageGroup.Builder result = Build.PackageGroup.newBuilder();
-
-    result.setName(packageGroup.getLabel().toString());
-    result.setParseableLocation(serializeLocation(packageGroup.getLocation()));
-
-    for (PackageSpecification packageSpecification : packageGroup.getPackageSpecifications()) {
-      result.addContainedPackage(packageSpecification.toString());
-    }
-
-    for (Label include : packageGroup.getIncludes()) {
-      result.addIncludedPackageGroup(include.toString());
-    }
-
-    return result.build();
-  }
-
-  private static Build.Rule serializeRule(Rule rule) {
-    Build.Rule.Builder result = Build.Rule.newBuilder();
-    result.setName(rule.getLabel().toString());
-    result.setRuleClass(rule.getRuleClass());
-    result.setParseableLocation(serializeLocation(rule.getLocation()));
-    for (Attribute attribute : rule.getAttributes()) {
-      PackageSerializer.addAttributeToProto(result, attribute,
-          getAttributeValues(rule, attribute), rule.getAttributeLocation(attribute.getName()),
-          rule.isAttributeValueExplicitlySpecified(attribute), true);
-    }
-
-    return result.build();
-  }
-
-  private static List<Build.MakeVar> serializeMakeEnvironment(MakeEnvironment makeEnv) {
-    List<Build.MakeVar> result = new ArrayList<>();
-
-    for (Map.Entry<String, ImmutableList<Binding>> var : makeEnv.getBindings().entrySet()) {
-      Build.MakeVar.Builder varPb = Build.MakeVar.newBuilder();
-      varPb.setName(var.getKey());
-      for (Binding binding : var.getValue()) {
-        Build.MakeVarBinding.Builder bindingPb = Build.MakeVarBinding.newBuilder();
-        bindingPb.setValue(binding.getValue());
-        bindingPb.setPlatformSetRegexp(binding.getPlatformSetRegexp());
-        varPb.addBinding(bindingPb);
-      }
-
-      result.add(varPb.build());
-    }
-
-    return result;
-  }
-
-  private static Build.License serializeLicense(License license) {
-    Build.License.Builder result = Build.License.newBuilder();
-
-    for (License.LicenseType licenseType : license.getLicenseTypes()) {
-      result.addLicenseType(licenseType.toString());
-    }
-
-    for (Label exception : license.getExceptions()) {
-      result.addException(exception.toString());
-    }
-    return result.build();
-  }
-
-  private static Build.Event serializeEvent(Event event) {
-    Build.Event.Builder result = Build.Event.newBuilder();
-    result.setMessage(event.getMessage());
-    if (event.getLocation() != null) {
-      result.setLocation(serializeLocation(event.getLocation()));
-    }
-
-    Build.Event.EventKind kind;
-
-    switch (event.getKind()) {
-      case ERROR:
-        kind = Build.Event.EventKind.ERROR;
-        break;
-      case WARNING:
-        kind = Build.Event.EventKind.WARNING;
-        break;
-      case INFO:
-        kind = Build.Event.EventKind.INFO;
-        break;
-      case PROGRESS:
-        kind = Build.Event.EventKind.PROGRESS;
-        break;
-      default: throw new IllegalArgumentException("unexpected event type: " + event.getKind());
-    }
-
-    result.setKind(kind);
-    return result.build();
-  }
-
-  private static void serializePackageInternal(Package pkg, Build.Package.Builder builder) {
-    builder.setName(pkg.getName());
-    builder.setRepository(pkg.getPackageIdentifier().getRepository().toString());
-    builder.setBuildFilePath(pkg.getFilename().getPathString());
-    // The extra bit is needed to handle the corner case when the default visibility is [], i.e.
-    // zero labels.
-    builder.setDefaultVisibilitySet(pkg.isDefaultVisibilitySet());
-    if (pkg.isDefaultVisibilitySet()) {
-      for (Label visibilityLabel : pkg.getDefaultVisibility().getDeclaredLabels()) {
-        builder.addDefaultVisibilityLabel(visibilityLabel.toString());
-      }
-    }
-
-    builder.setDefaultTestonly(pkg.getDefaultTestOnly());
-    if (pkg.getDefaultDeprecation() != null) {
-      builder.setDefaultDeprecation(pkg.getDefaultDeprecation());
-    }
-
-    for (String defaultCopt : pkg.getDefaultCopts()) {
-      builder.addDefaultCopt(defaultCopt);
-    }
-
-    if (pkg.isDefaultHdrsCheckSet()) {
-      builder.setDefaultHdrsCheck(pkg.getDefaultHdrsCheck());
-    }
-
-    builder.setDefaultLicense(serializeLicense(pkg.getDefaultLicense()));
-
-    for (DistributionType distributionType : pkg.getDefaultDistribs()) {
-      builder.addDefaultDistrib(distributionType.toString());
-    }
-
-    for (String feature : pkg.getFeatures()) {
-      builder.addDefaultSetting(feature);
-    }
-
-    for (Label subincludeLabel : pkg.getSubincludeLabels()) {
-      builder.addSubincludeLabel(subincludeLabel.toString());
-    }
-
-    for (Label skylarkLabel : pkg.getSkylarkFileDependencies()) {
-      builder.addSkylarkLabel(skylarkLabel.toString());
-    }
-
-    for (Build.MakeVar makeVar :
-         serializeMakeEnvironment(pkg.getMakeEnvironment())) {
-      builder.addMakeVariable(makeVar);
-    }
-
-    for (Target target : pkg.getTargets()) {
-      if (target instanceof InputFile) {
-        builder.addSourceFile(serializeInputFile((InputFile) target));
-      } else if (target instanceof OutputFile) {
-        // Output files are ignored; they are recorded in rules.
-      } else if (target instanceof PackageGroup) {
-        builder.addPackageGroup(serializePackageGroup((PackageGroup) target));
-      } else if (target instanceof Rule) {
-        builder.addRule(serializeRule((Rule) target));
-      }
-    }
-
-    for (Event event : pkg.getEvents()) {
-      builder.addEvent(serializeEvent(event));
-    }
-
-    builder.setContainsErrors(pkg.containsErrors());
-    builder.setContainsTemporaryErrors(pkg.containsTemporaryErrors());
-  }
 
   /**
-   * Serialize a package to a protocol message. The inverse of
-   * {@link PackageDeserializer#deserialize}.
+   * Serialize a package to {@code out}. The inverse of {@link PackageDeserializer#deserialize}.
+   *
+   * <p>Writes pkg as a single
+   * {@link com.google.devtools.build.lib.query2.proto.proto2api.Build.Package} protocol buffer
+   * message.
+   *
+   * @param pkg the {@link Package} to be serialized
+   * @param out the stream to pkg's serialized representation to
+   * @throws IOException on failure writing to {@code out}
    */
-  public static Build.Package serializePackage(Package pkg) {
+  public static void serializePackage(Package pkg, OutputStream out) throws IOException {
     Build.Package.Builder builder = Build.Package.newBuilder();
     serializePackageInternal(pkg, builder);
-    return builder.build();
+    builder.build().writeDelimitedTo(out);
   }
 
   /**
@@ -503,6 +318,201 @@ public class PackageSerializer {
     rulePb.addAttribute(attrPb);
   }
 
+  private static Build.SourceFile serializeInputFile(InputFile inputFile) {
+    Build.SourceFile.Builder result = Build.SourceFile.newBuilder();
+    result.setName(inputFile.getLabel().toString());
+    if (inputFile.isVisibilitySpecified()) {
+      for (Label visibilityLabel : inputFile.getVisibility().getDeclaredLabels()) {
+        result.addVisibilityLabel(visibilityLabel.toString());
+      }
+    }
+    if (inputFile.isLicenseSpecified()) {
+      result.setLicense(serializeLicense(inputFile.getLicense()));
+    }
+
+    result.setParseableLocation(serializeLocation(inputFile.getLocation()));
+    return result.build();
+  }
+
+  private static Build.Location serializeLocation(Location location) {
+    Build.Location.Builder result = Build.Location.newBuilder();
+
+    result.setStartOffset(location.getStartOffset());
+    if (location.getStartLineAndColumn() != null) {
+      result.setStartLine(location.getStartLineAndColumn().getLine());
+      result.setStartColumn(location.getStartLineAndColumn().getColumn());
+    }
+
+    result.setEndOffset(location.getEndOffset());
+    if (location.getEndLineAndColumn() != null) {
+      result.setEndLine(location.getEndLineAndColumn().getLine());
+      result.setEndColumn(location.getEndLineAndColumn().getColumn());
+    }
+
+    return result.build();
+  }
+
+  private static Build.PackageGroup serializePackageGroup(PackageGroup packageGroup) {
+    Build.PackageGroup.Builder result = Build.PackageGroup.newBuilder();
+
+    result.setName(packageGroup.getLabel().toString());
+    result.setParseableLocation(serializeLocation(packageGroup.getLocation()));
+
+    for (PackageSpecification packageSpecification : packageGroup.getPackageSpecifications()) {
+      result.addContainedPackage(packageSpecification.toString());
+    }
+
+    for (Label include : packageGroup.getIncludes()) {
+      result.addIncludedPackageGroup(include.toString());
+    }
+
+    return result.build();
+  }
+
+  private static Build.Rule serializeRule(Rule rule) {
+    Build.Rule.Builder result = Build.Rule.newBuilder();
+    result.setName(rule.getLabel().toString());
+    result.setRuleClass(rule.getRuleClass());
+    result.setParseableLocation(serializeLocation(rule.getLocation()));
+    for (Attribute attribute : rule.getAttributes()) {
+      PackageSerializer.addAttributeToProto(result, attribute,
+          getAttributeValues(rule, attribute), rule.getAttributeLocation(attribute.getName()),
+          rule.isAttributeValueExplicitlySpecified(attribute), true);
+    }
+
+    return result.build();
+  }
+
+  private static List<Build.MakeVar> serializeMakeEnvironment(MakeEnvironment makeEnv) {
+    List<Build.MakeVar> result = new ArrayList<>();
+
+    for (Map.Entry<String, ImmutableList<Binding>> var : makeEnv.getBindings().entrySet()) {
+      Build.MakeVar.Builder varPb = Build.MakeVar.newBuilder();
+      varPb.setName(var.getKey());
+      for (Binding binding : var.getValue()) {
+        Build.MakeVarBinding.Builder bindingPb = Build.MakeVarBinding.newBuilder();
+        bindingPb.setValue(binding.getValue());
+        bindingPb.setPlatformSetRegexp(binding.getPlatformSetRegexp());
+        varPb.addBinding(bindingPb);
+      }
+
+      result.add(varPb.build());
+    }
+
+    return result;
+  }
+
+  private static Build.License serializeLicense(License license) {
+    Build.License.Builder result = Build.License.newBuilder();
+
+    for (License.LicenseType licenseType : license.getLicenseTypes()) {
+      result.addLicenseType(licenseType.toString());
+    }
+
+    for (Label exception : license.getExceptions()) {
+      result.addException(exception.toString());
+    }
+    return result.build();
+  }
+
+  private static Build.Event serializeEvent(Event event) {
+    Build.Event.Builder result = Build.Event.newBuilder();
+    result.setMessage(event.getMessage());
+    if (event.getLocation() != null) {
+      result.setLocation(serializeLocation(event.getLocation()));
+    }
+
+    Build.Event.EventKind kind;
+
+    switch (event.getKind()) {
+      case ERROR:
+        kind = Build.Event.EventKind.ERROR;
+        break;
+      case WARNING:
+        kind = Build.Event.EventKind.WARNING;
+        break;
+      case INFO:
+        kind = Build.Event.EventKind.INFO;
+        break;
+      case PROGRESS:
+        kind = Build.Event.EventKind.PROGRESS;
+        break;
+      default: throw new IllegalArgumentException("unexpected event type: " + event.getKind());
+    }
+
+    result.setKind(kind);
+    return result.build();
+  }
+
+  private static void serializePackageInternal(Package pkg, Build.Package.Builder builder) {
+    builder.setName(pkg.getName());
+    builder.setRepository(pkg.getPackageIdentifier().getRepository().toString());
+    builder.setBuildFilePath(pkg.getFilename().getPathString());
+    // The extra bit is needed to handle the corner case when the default visibility is [], i.e.
+    // zero labels.
+    builder.setDefaultVisibilitySet(pkg.isDefaultVisibilitySet());
+    if (pkg.isDefaultVisibilitySet()) {
+      for (Label visibilityLabel : pkg.getDefaultVisibility().getDeclaredLabels()) {
+        builder.addDefaultVisibilityLabel(visibilityLabel.toString());
+      }
+    }
+
+    builder.setDefaultTestonly(pkg.getDefaultTestOnly());
+    if (pkg.getDefaultDeprecation() != null) {
+      builder.setDefaultDeprecation(pkg.getDefaultDeprecation());
+    }
+
+    for (String defaultCopt : pkg.getDefaultCopts()) {
+      builder.addDefaultCopt(defaultCopt);
+    }
+
+    if (pkg.isDefaultHdrsCheckSet()) {
+      builder.setDefaultHdrsCheck(pkg.getDefaultHdrsCheck());
+    }
+
+    builder.setDefaultLicense(serializeLicense(pkg.getDefaultLicense()));
+
+    for (DistributionType distributionType : pkg.getDefaultDistribs()) {
+      builder.addDefaultDistrib(distributionType.toString());
+    }
+
+    for (String feature : pkg.getFeatures()) {
+      builder.addDefaultSetting(feature);
+    }
+
+    for (Label subincludeLabel : pkg.getSubincludeLabels()) {
+      builder.addSubincludeLabel(subincludeLabel.toString());
+    }
+
+    for (Label skylarkLabel : pkg.getSkylarkFileDependencies()) {
+      builder.addSkylarkLabel(skylarkLabel.toString());
+    }
+
+    for (Build.MakeVar makeVar :
+         serializeMakeEnvironment(pkg.getMakeEnvironment())) {
+      builder.addMakeVariable(makeVar);
+    }
+
+    for (Target target : pkg.getTargets()) {
+      if (target instanceof InputFile) {
+        builder.addSourceFile(serializeInputFile((InputFile) target));
+      } else if (target instanceof OutputFile) {
+        // Output files are ignored; they are recorded in rules.
+      } else if (target instanceof PackageGroup) {
+        builder.addPackageGroup(serializePackageGroup((PackageGroup) target));
+      } else if (target instanceof Rule) {
+        builder.addRule(serializeRule((Rule) target));
+      }
+    }
+
+    for (Event event : pkg.getEvents()) {
+      builder.addEvent(serializeEvent(event));
+    }
+
+    builder.setContainsErrors(pkg.containsErrors());
+    builder.setContainsTemporaryErrors(pkg.containsTemporaryErrors());
+  }
+
   // This is needed because I do not want to use the SymlinkBehavior from the
   // protocol buffer all over the place, so there are two classes that do
   // essentially the same thing.