Allow non-dottedversion aliases for --xcode_version

RELNOTES: None.
PiperOrigin-RevId: 167143657

4 files changed, 22 insertions, 25 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 8cb4cace93..b6309d6866 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
@@ -67,16 +67,13 @@ public class AppleCommandLineOptions extends FragmentOptions {
     name = "xcode_version",
     defaultValue = "null",
     category = "build",
-    converter = DottedVersionConverter.class,
     documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
     effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE},
     help =
         "If specified, uses Xcode of the given version for relevant build actions. "
             + "If unspecified, uses the executor default version of Xcode."
   )
-  // TODO(bazel-team): This should be of String type, to allow referencing an alias based
-  // on an xcode_config target.
-  public DottedVersion xcodeVersion;
+  public String xcodeVersion;
 
   @Option(
     name = "ios_sdk_version",
@@ -507,7 +504,7 @@ public class AppleCommandLineOptions extends FragmentOptions {
 
   void serialize(CodedOutputStream out) throws IOException, SerializationException {
     out.writeBoolNoTag(mandatoryMinimumVersion);
-    serializeNullable(xcodeVersion, out, DottedVersion.CODEC);
+    serializeNullable(xcodeVersion, out, FastStringCodec.INSTANCE);
     serializeNullable(iosSdkVersion, out, DottedVersion.CODEC);
     serializeNullable(watchOsSdkVersion, out, DottedVersion.CODEC);
     serializeNullable(tvOsSdkVersion, out, DottedVersion.CODEC);
@@ -537,7 +534,7 @@ public class AppleCommandLineOptions extends FragmentOptions {
       throws IOException, SerializationException {
     AppleCommandLineOptions result = new AppleCommandLineOptions();
     result.mandatoryMinimumVersion = in.readBool();
-    result.xcodeVersion = deserializeNullable(in, DottedVersion.CODEC);
+    result.xcodeVersion = deserializeNullable(in, FastStringCodec.INSTANCE);
     result.iosSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
     result.watchOsSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
     result.tvOsSdkVersion = deserializeNullable(in, DottedVersion.CODEC);
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 45e23dc058..8d5ed3fdf4 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
@@ -102,7 +102,6 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
   private final ImmutableList<String> macosCpus;
   private final AppleBitcodeMode bitcodeMode;
   private final Label xcodeConfigLabel;
-  private final DottedVersion xcodeVersionCommandLineFlag;
   private final boolean enableAppleCrosstool;
   private final AppleCommandLineOptions options;
   @Nullable private final String xcodeToolchain;
@@ -157,7 +156,6 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     this.bitcodeMode = options.appleBitcodeMode;
     this.xcodeConfigLabel =
         Preconditions.checkNotNull(options.xcodeVersionConfig, "xcodeConfigLabel");
-    this.xcodeVersionCommandLineFlag = options.xcodeVersion;
     this.enableAppleCrosstool = options.enableAppleCrosstoolTransition;
     this.defaultProvisioningProfileLabel = options.defaultProvisioningProfile;
     this.xcodeToolchain = options.xcodeToolchain;
@@ -601,13 +599,6 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Returns the explicit Xcode version specified on the command line.
-   */
-  public DottedVersion getXcodeVersionCommandLineFlag() {
-    return xcodeVersionCommandLineFlag;
-  }
-
-  /**
    * Returns the unique identifier distinguishing configurations that are otherwise the same.
    *
    * <p>Use this value for situations in which two configurations create two outputs that are the
@@ -676,7 +667,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     ImmutableMap.Builder<String, Object> mapBuilder = ImmutableMap.builder();
 
     if (xcodeVersion != null) {
-      mapBuilder.put("xcode_version", xcodeVersion);
+      mapBuilder.put("xcode_version", xcodeVersion.toString());
     }
     return mapBuilder
         .put("ios_sdk_version", iosSdkVersion)
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 b0b98c5380..dbe80f1568 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
@@ -97,6 +97,9 @@ public final class DottedVersion implements Comparable<DottedVersion>, SkylarkVa
    * @throws IllegalArgumentException if the passed string is not a valid dotted version
    */
   public static DottedVersion fromString(String version) {
+    if (Strings.isNullOrEmpty(version)) {
+      throw new IllegalArgumentException(String.format(ILLEGAL_VERSION, version));
+    }
     ArrayList<Component> components = new ArrayList<>();
     for (String component : DOT_SPLITTER.split(version)) {
       components.add(toComponent(component, version));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
index d2b5859ef9..fed295e6fc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -171,7 +172,7 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    */
   static XcodeVersionProperties resolveXcodeVersion(
       boolean requireDefinedVersions,
-      DottedVersion xcodeVersionOverrideFlag,
+      String xcodeVersionOverrideFlag,
       Iterable<XcodeVersionRuleData> xcodeVersions,
       @Nullable XcodeVersionRuleData defaultVersion)
       throws XcodeConfigException {
@@ -180,11 +181,16 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
 
     if (xcodeVersion != null) {
       return xcodeVersion.getXcodeVersionProperties();
-    } else if (xcodeVersionOverrideFlag != null) {
-      return new XcodeVersionProperties(xcodeVersionOverrideFlag);
-    } else {
-      return XcodeVersionProperties.unknownXcodeVersionProperties();
     }
+    // TODO(b/64576392): Remove this fallback logic. An xcode_version target should be explicitly
+    // matched in all cases where --xcode_version is specified.
+    try {
+      DottedVersion dottedVersion = DottedVersion.fromString(xcodeVersionOverrideFlag);
+      return new XcodeVersionProperties(dottedVersion);
+    } catch (IllegalArgumentException e) {
+      // The --xcode_version flag is not a valid DottedVersion, so there is nothing to go on.
+    }
+    return XcodeVersionProperties.unknownXcodeVersionProperties();
   }
 
   /**
@@ -200,16 +206,16 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
       boolean requireDefinedVersions,
       Iterable<XcodeVersionRuleData> xcodeVersionRules,
       @Nullable XcodeVersionRuleData defaultVersion,
-      DottedVersion versionOverrideFlag)
+      String versionOverrideFlag)
       throws XcodeConfigException {
 
     Map<String, XcodeVersionRuleData> aliasesToVersionMap = aliasesToVersionMap(xcodeVersionRules);
 
-    if (versionOverrideFlag != null) {
+    if (!Strings.isNullOrEmpty(versionOverrideFlag)) {
       // The version override flag is not necessarily an actual version - it may be a version
       // alias.
       XcodeVersionRuleData explicitVersion =
-          aliasesToVersionMap.get(versionOverrideFlag.toString());
+          aliasesToVersionMap.get(versionOverrideFlag);
       if (explicitVersion != null) {
         return explicitVersion;
       }
@@ -217,7 +223,7 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
       // No override specified. Use default.
       return defaultVersion;
     }
-    
+
     if (requireDefinedVersions) {
       throw new XcodeConfigException(
           "xcode version config required an explicitly defined version, but none was available");