Properly handle some values that can be null in AppleCommandLineOptions.

PiperOrigin-RevId: 165622047

4 files changed, 152 insertions, 107 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
index 268c47d4e7..8cb4cace93 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java
@@ -15,6 +15,8 @@
 package com.google.devtools.build.lib.rules.apple;
 
 import static com.google.devtools.build.lib.skyframe.serialization.SerializationCommonUtils.STRING_LIST_CODEC;
+import static com.google.devtools.build.lib.skyframe.serialization.SerializationCommonUtils.deserializeNullable;
+import static com.google.devtools.build.lib.skyframe.serialization.SerializationCommonUtils.serializeNullable;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
@@ -505,15 +507,15 @@ public class AppleCommandLineOptions extends FragmentOptions {
 
   void serialize(CodedOutputStream out) throws IOException, SerializationException {
     out.writeBoolNoTag(mandatoryMinimumVersion);
-    xcodeVersion.serialize(out);
-    iosSdkVersion.serialize(out);
-    watchOsSdkVersion.serialize(out);
-    tvOsSdkVersion.serialize(out);
-    macOsSdkVersion.serialize(out);
-    iosMinimumOs.serialize(out);
-    watchosMinimumOs.serialize(out);
-    tvosMinimumOs.serialize(out);
-    macosMinimumOs.serialize(out);
+    serializeNullable(xcodeVersion, out, DottedVersion.CODEC);
+    serializeNullable(iosSdkVersion, out, DottedVersion.CODEC);
+    serializeNullable(watchOsSdkVersion, out, DottedVersion.CODEC);
+    serializeNullable(tvOsSdkVersion, out, DottedVersion.CODEC);
+    serializeNullable(macOsSdkVersion, out, DottedVersion.CODEC);
+    serializeNullable(iosMinimumOs, out, DottedVersion.CODEC);
+    serializeNullable(watchosMinimumOs, out, DottedVersion.CODEC);
+    serializeNullable(tvosMinimumOs, out, DottedVersion.CODEC);
+    serializeNullable(macosMinimumOs, out, DottedVersion.CODEC);
     FastStringCodec.INSTANCE.serialize(iosCpu, out);
     LabelCodec.INSTANCE.serialize(appleCrosstoolTop, out);
     PlatformType.CODEC.serialize(applePlatformType, out);
@@ -525,7 +527,7 @@ public class AppleCommandLineOptions extends FragmentOptions {
     STRING_LIST_CODEC.serialize((ImmutableList<String>) macosCpus, out);
     LabelCodec.INSTANCE.serialize(defaultProvisioningProfile, out);
     LabelCodec.INSTANCE.serialize(xcodeVersionConfig, out);
-    FastStringCodec.INSTANCE.serialize(xcodeToolchain, out);
+    serializeNullable(xcodeToolchain, out, FastStringCodec.INSTANCE);
     AppleBitcodeMode.CODEC.serialize(appleBitcodeMode, out);
     out.writeBoolNoTag(enableAppleCrosstoolTransition);
     out.writeBoolNoTag(targetUsesAppleCrosstool);
@@ -535,15 +537,15 @@ public class AppleCommandLineOptions extends FragmentOptions {
       throws IOException, SerializationException {
     AppleCommandLineOptions result = new AppleCommandLineOptions();
     result.mandatoryMinimumVersion = in.readBool();
-    result.xcodeVersion = DottedVersion.deserialize(in);
-    result.iosSdkVersion = DottedVersion.deserialize(in);
-    result.watchOsSdkVersion = DottedVersion.deserialize(in);
-    result.tvOsSdkVersion = DottedVersion.deserialize(in);
-    result.macOsSdkVersion = DottedVersion.deserialize(in);
-    result.iosMinimumOs = DottedVersion.deserialize(in);
-    result.watchosMinimumOs = DottedVersion.deserialize(in);
-    result.tvosMinimumOs = DottedVersion.deserialize(in);
-    result.macosMinimumOs = DottedVersion.deserialize(in);
+    result.xcodeVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.iosSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.watchOsSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.tvOsSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.macOsSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.iosMinimumOs = deserializeNullable(in, DottedVersion.CODEC);
+    result.watchosMinimumOs = deserializeNullable(in, DottedVersion.CODEC);
+    result.tvosMinimumOs = deserializeNullable(in, DottedVersion.CODEC);
+    result.macosMinimumOs = deserializeNullable(in, DottedVersion.CODEC);
     result.iosCpu = FastStringCodec.INSTANCE.deserialize(in);
     result.appleCrosstoolTop = LabelCodec.INSTANCE.deserialize(in);
     result.applePlatformType = PlatformType.CODEC.deserialize(in);
@@ -555,7 +557,7 @@ public class AppleCommandLineOptions extends FragmentOptions {
     result.macosCpus = STRING_LIST_CODEC.deserialize(in);
     result.defaultProvisioningProfile = LabelCodec.INSTANCE.deserialize(in);
     result.xcodeVersionConfig = LabelCodec.INSTANCE.deserialize(in);
-    result.xcodeToolchain = FastStringCodec.INSTANCE.deserialize(in);
+    result.xcodeToolchain = deserializeNullable(in, FastStringCodec.INSTANCE);
     result.appleBitcodeMode = AppleBitcodeMode.CODEC.deserialize(in);
     result.enableAppleCrosstoolTransition = in.readBool();
     result.targetUsesAppleCrosstool = in.readBool();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
index b2a1a98604..fed83662a7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -14,6 +14,9 @@
 
 package com.google.devtools.build.lib.rules.apple;
 
+import static com.google.devtools.build.lib.skyframe.serialization.SerializationCommonUtils.deserializeNullable;
+import static com.google.devtools.build.lib.skyframe.serialization.SerializationCommonUtils.serializeNullable;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
@@ -717,40 +720,94 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
   void serialize(CodedOutputStream out) throws IOException, SerializationException {
     options.serialize(out);
     out.writeStringNoTag(iosCpu);
-    if (xcodeVersion == null) {
-      out.writeBoolNoTag(false);
-    } else {
-      out.writeBoolNoTag(true);
-      xcodeVersion.serialize(out);
-    }
-    iosSdkVersion.serialize(out);
-    iosMinimumOs.serialize(out);
-    watchosSdkVersion.serialize(out);
-    watchosMinimumOs.serialize(out);
-    tvosSdkVersion.serialize(out);
-    tvosMinimumOs.serialize(out);
-    macosSdkVersion.serialize(out);
-    macosMinimumOs.serialize(out);
+    serializeNullable(xcodeVersion, out, DottedVersion.CODEC);
+    DottedVersion.CODEC.serialize(iosSdkVersion, out);
+    DottedVersion.CODEC.serialize(iosMinimumOs, out);
+    DottedVersion.CODEC.serialize(watchosSdkVersion, out);
+    DottedVersion.CODEC.serialize(watchosMinimumOs, out);
+    DottedVersion.CODEC.serialize(tvosSdkVersion, out);
+    DottedVersion.CODEC.serialize(tvosMinimumOs, out);
+    DottedVersion.CODEC.serialize(macosSdkVersion, out);
+    DottedVersion.CODEC.serialize(macosMinimumOs, out);
   }
 
   static AppleConfiguration deserialize(CodedInputStream in)
       throws IOException, SerializationException {
     AppleCommandLineOptions options = AppleCommandLineOptions.deserialize(in);
     String iosCpu = FastStringCodec.INSTANCE.deserialize(in);
-    boolean hasXcodeVersion = in.readBool();
-    DottedVersion xcodeVersion = hasXcodeVersion ? DottedVersion.deserialize(in) : null;
+    DottedVersion xcodeVersion = deserializeNullable(in, DottedVersion.CODEC);
     return new AppleConfiguration(
         options,
         iosCpu,
         xcodeVersion,
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in),
-        DottedVersion.deserialize(in));
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in),
+        DottedVersion.CODEC.deserialize(in));
+  }
+
+  @VisibleForTesting
+  static AppleConfiguration create(
+      AppleCommandLineOptions appleOptions,
+      String cpu,
+      XcodeVersionProperties xcodeVersionProperties)
+      throws InvalidConfigurationException {
+    DottedVersion iosSdkVersion =
+        (appleOptions.iosSdkVersion != null)
+            ? appleOptions.iosSdkVersion
+            : xcodeVersionProperties.getDefaultIosSdkVersion();
+    DottedVersion iosMinimumOsVersion =
+        (appleOptions.iosMinimumOs != null) ? appleOptions.iosMinimumOs : iosSdkVersion;
+    DottedVersion watchosSdkVersion =
+        (appleOptions.watchOsSdkVersion != null)
+            ? appleOptions.watchOsSdkVersion
+            : xcodeVersionProperties.getDefaultWatchosSdkVersion();
+    DottedVersion watchosMinimumOsVersion =
+        (appleOptions.watchosMinimumOs != null) ? appleOptions.watchosMinimumOs : watchosSdkVersion;
+    DottedVersion tvosSdkVersion =
+        (appleOptions.tvOsSdkVersion != null)
+            ? appleOptions.tvOsSdkVersion
+            : xcodeVersionProperties.getDefaultTvosSdkVersion();
+    DottedVersion tvosMinimumOsVersion =
+        (appleOptions.tvosMinimumOs != null) ? appleOptions.tvosMinimumOs : tvosSdkVersion;
+    DottedVersion macosSdkVersion =
+        (appleOptions.macOsSdkVersion != null)
+            ? appleOptions.macOsSdkVersion
+            : xcodeVersionProperties.getDefaultMacosSdkVersion();
+    DottedVersion macosMinimumOsVersion =
+        (appleOptions.macosMinimumOs != null) ? appleOptions.macosMinimumOs : macosSdkVersion;
+    AppleConfiguration configuration =
+        new AppleConfiguration(
+            appleOptions,
+            iosCpuFromCpu(cpu),
+            xcodeVersionProperties.getXcodeVersion().orNull(),
+            iosSdkVersion,
+            iosMinimumOsVersion,
+            watchosSdkVersion,
+            watchosMinimumOsVersion,
+            tvosSdkVersion,
+            tvosMinimumOsVersion,
+            macosSdkVersion,
+            macosMinimumOsVersion);
+
+    validate(configuration);
+    return configuration;
+  }
+
+  private static void validate(AppleConfiguration config) throws InvalidConfigurationException {
+    DottedVersion xcodeVersion = config.getXcodeVersion();
+    if (config.getBitcodeMode() != AppleBitcodeMode.NONE
+        && xcodeVersion != null
+        && xcodeVersion.compareTo(MINIMUM_BITCODE_XCODE_VERSION) < 0) {
+      throw new InvalidConfigurationException(
+          String.format(
+              "apple_bitcode mode '%s' is unsupported for xcode version '%s'",
+              config.getBitcodeMode(), xcodeVersion));
+    }
   }
 
   /**
@@ -764,51 +821,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
       String cpu = buildOptions.get(BuildConfiguration.Options.class).cpu;
       XcodeVersionProperties xcodeVersionProperties = XcodeConfig.
           getXcodeVersionProperties(env, appleOptions);
-
-      DottedVersion iosSdkVersion = (appleOptions.iosSdkVersion != null)
-          ? appleOptions.iosSdkVersion : xcodeVersionProperties.getDefaultIosSdkVersion();
-      DottedVersion iosMinimumOsVersion = (appleOptions.iosMinimumOs != null)
-          ? appleOptions.iosMinimumOs : iosSdkVersion;
-      DottedVersion watchosSdkVersion = (appleOptions.watchOsSdkVersion != null)
-          ? appleOptions.watchOsSdkVersion : xcodeVersionProperties.getDefaultWatchosSdkVersion();
-      DottedVersion watchosMinimumOsVersion = (appleOptions.watchosMinimumOs != null)
-          ? appleOptions.watchosMinimumOs : watchosSdkVersion;
-      DottedVersion tvosSdkVersion = (appleOptions.tvOsSdkVersion != null)
-          ? appleOptions.tvOsSdkVersion : xcodeVersionProperties.getDefaultTvosSdkVersion();
-      DottedVersion tvosMinimumOsVersion = (appleOptions.tvosMinimumOs != null)
-          ? appleOptions.tvosMinimumOs : tvosSdkVersion;
-      DottedVersion macosSdkVersion = (appleOptions.macOsSdkVersion != null)
-          ? appleOptions.macOsSdkVersion : xcodeVersionProperties.getDefaultMacosSdkVersion();
-      DottedVersion macosMinimumOsVersion = (appleOptions.macosMinimumOs != null)
-          ? appleOptions.macosMinimumOs : macosSdkVersion;
-      AppleConfiguration configuration =
-          new AppleConfiguration(
-              appleOptions,
-              iosCpuFromCpu(cpu),
-              xcodeVersionProperties.getXcodeVersion().orNull(),
-              iosSdkVersion,
-              iosMinimumOsVersion,
-              watchosSdkVersion,
-              watchosMinimumOsVersion,
-              tvosSdkVersion,
-              tvosMinimumOsVersion,
-              macosSdkVersion,
-              macosMinimumOsVersion);
-
-      validate(configuration);
-      return configuration;
-    }
-
-    private void validate(AppleConfiguration config)
-        throws InvalidConfigurationException {
-      DottedVersion xcodeVersion = config.getXcodeVersion();
-      if (config.getBitcodeMode() != AppleBitcodeMode.NONE
-          && xcodeVersion != null
-          && xcodeVersion.compareTo(MINIMUM_BITCODE_XCODE_VERSION) < 0) {
-        throw new InvalidConfigurationException(
-            String.format("apple_bitcode mode '%s' is unsupported for xcode version '%s'",
-                config.getBitcodeMode(), xcodeVersion));
-      }
+      return AppleConfiguration.create(appleOptions, cpu, xcodeVersionProperties);
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
index 5fbbef561c..b0b98c5380 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Ordering;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
@@ -253,24 +254,35 @@ public final class DottedVersion implements Comparable<DottedVersion>, SkylarkVa
     printer.append(stringRepresentation);
   }
 
-  void serialize(CodedOutputStream out) throws IOException {
-    out.writeInt32NoTag(components.size());
-    for (Component component : components) {
-      component.serialize(out);
-    }
-    out.writeStringNoTag(stringRepresentation);
-    out.writeInt32NoTag(numOriginalComponents);
-  }
-
-  static DottedVersion deserialize(CodedInputStream in) throws IOException {
-    int numComponents = in.readInt32();
-    // TODO(janakr: Presize this if/when https://github.com/google/guava/issues/196 is resolved.
-    ImmutableList.Builder<Component> components = ImmutableList.builder();
-    for (int i = 0; i < numComponents; i++) {
-      components.add(Component.deserialize(in));
-    }
-    return new DottedVersion(components.build(), in.readString(), in.readInt32());
-  }
+  static final ObjectCodec<DottedVersion> CODEC =
+      new ObjectCodec<DottedVersion>() {
+        @Override
+        public void serialize(DottedVersion obj, CodedOutputStream codedOut) throws IOException {
+          codedOut.writeInt32NoTag(obj.components.size());
+          for (Component component : obj.components) {
+            component.serialize(codedOut);
+          }
+          codedOut.writeStringNoTag(obj.stringRepresentation);
+          codedOut.writeInt32NoTag(obj.numOriginalComponents);
+        }
+
+        @Override
+        public DottedVersion deserialize(CodedInputStream codedIn) throws IOException {
+          int numComponents = codedIn.readInt32();
+          // TODO(janakr: Presize this if/when https://github.com/google/guava/issues/196 is
+          // resolved.
+          ImmutableList.Builder<Component> components = ImmutableList.builder();
+          for (int i = 0; i < numComponents; i++) {
+            components.add(Component.deserialize(codedIn));
+          }
+          return new DottedVersion(components.build(), codedIn.readString(), codedIn.readInt32());
+        }
+
+        @Override
+        public Class<DottedVersion> getEncodedClass() {
+          return DottedVersion.class;
+        }
+      };
 
   private static final class Component implements Comparable<Component> {
     private final int firstNumber;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationCommonUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationCommonUtils.java
index 50d81b43e7..5efea4b5cd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationCommonUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationCommonUtils.java
@@ -17,6 +17,9 @@ package com.google.devtools.build.lib.skyframe.serialization;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.protobuf.ByteString;
+import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
+import java.io.IOException;
 
 /** Common utilities for serialization. */
 public class SerializationCommonUtils {
@@ -38,4 +41,19 @@ public class SerializationCommonUtils {
       return RepositoryName.create(repoNameBytes.toStringUtf8());
     }
   }
+
+  public static <T> void serializeNullable(T obj, CodedOutputStream out, ObjectCodec<T> codec)
+      throws IOException, SerializationException {
+    if (obj == null) {
+      out.writeBoolNoTag(false);
+    } else {
+      out.writeBoolNoTag(true);
+      codec.serialize(obj, out);
+    }
+  }
+
+  public static <T> T deserializeNullable(CodedInputStream in, ObjectCodec<T> codec)
+      throws IOException, SerializationException {
+    return in.readBool() ? codec.deserialize(in) : null;
+  }
 }