Change xcode_config rule semantics to fit current usage

- require_defined_versions is deprecated and a no-op. A version must match existing defined versions if any exist
- default label must be present in versions labels if any are defined
- default label may not exist if no versions are defined
- when --xcode_version is specified on the command line, it must match a defined version if any are defined. If none are defined, this flag is a no-op

RELNOTES: None.
PiperOrigin-RevId: 167616628

2 files changed, 36 insertions, 45 deletions

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 fed295e6fc..f433d20f84 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
@@ -18,6 +18,7 @@ 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.Iterables;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RedirectChaser;
@@ -36,7 +37,6 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.syntax.Type;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -82,12 +82,8 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
     ImmutableList<XcodeVersionRuleData> versions = getAvailableVersions(env, xcodeConfigRule);
     XcodeVersionRuleData defaultVersion = getDefaultVersion(env, xcodeConfigRule);
 
-    boolean requireDefinedVersions = NonconfigurableAttributeMapper.of(xcodeConfigRule)
-        .get(XcodeConfigRule.REQUIRE_DEFINED_VERSIONS_ATTR_NAME, Type.BOOLEAN);
-
     try {
-      return resolveXcodeVersion(
-          requireDefinedVersions, appleOptions.xcodeVersion, versions, defaultVersion);
+      return resolveXcodeVersion(appleOptions.xcodeVersion, versions, defaultVersion);
     } catch (XcodeConfigException e) {
       throw new InvalidConfigurationException(e.getMessage());
     }
@@ -113,12 +109,9 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
     Iterable<XcodeVersionRuleData> availableVersions = ruleContext.getPrerequisites(
         XcodeConfigRule.VERSIONS_ATTR_NAME, RuleConfiguredTarget.Mode.TARGET,
         XcodeVersionRuleData.class);
-    boolean requireDefinedVersions = ruleContext.attributes().get(
-        XcodeConfigRule.REQUIRE_DEFINED_VERSIONS_ATTR_NAME, Type.BOOLEAN);
     XcodeVersionProperties xcodeVersionProperties;
     try {
       xcodeVersionProperties = resolveXcodeVersion(
-          requireDefinedVersions,
           appleOptions.xcodeVersion,
           availableVersions,
           defaultVersion);
@@ -166,46 +159,50 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    * @param requireDefinedVersions whether the version config requires an explicitly defined version
    * @param xcodeVersionOverrideFlag the value of the {@code --xcode_version} command line flag
    * @param xcodeVersions the Xcode versions listed in the {@code xcode_config} rule
-   * @param defaultVersion the default Xcode version in the {@code xcode_config} rule. Can be null.
+   * @param defaultVersion the default Xcode version in the {@code xcode_config} rule.
    * @throws XcodeConfigException if the options given (or configuration targets) were
    *     malformed and thus the xcode version could not be determined
    */
   static XcodeVersionProperties resolveXcodeVersion(
-      boolean requireDefinedVersions,
       String xcodeVersionOverrideFlag,
       Iterable<XcodeVersionRuleData> xcodeVersions,
-      @Nullable XcodeVersionRuleData defaultVersion)
+      XcodeVersionRuleData defaultVersion)
       throws XcodeConfigException {
-    XcodeVersionRuleData xcodeVersion = resolveExplicitlyDefinedVersion(
-        requireDefinedVersions, xcodeVersions, defaultVersion, xcodeVersionOverrideFlag);
-
-    if (xcodeVersion != null) {
-      return xcodeVersion.getXcodeVersionProperties();
+    if (defaultVersion != null
+        && Iterables.isEmpty(Iterables.filter(
+              xcodeVersions,
+              ruleData -> ruleData.getLabel().equals(defaultVersion.getLabel())))) {
+      throw new XcodeConfigException(
+          String.format("default label '%s' must be contained in versions attribute",
+              defaultVersion.getLabel()));
     }
-    // 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.
+    if (Iterables.isEmpty(xcodeVersions)) {
+      if (defaultVersion != null) {
+        throw new XcodeConfigException(
+            "default label must be contained in versions attribute");
+      }
+      return XcodeVersionProperties.unknownXcodeVersionProperties();
+    }
+    if (defaultVersion == null) {
+      throw new XcodeConfigException(
+          "if any versions are specified, a default version must be specified");
     }
-    return XcodeVersionProperties.unknownXcodeVersionProperties();
+
+    XcodeVersionRuleData xcodeVersion = resolveExplicitlyDefinedVersion(
+        xcodeVersions, defaultVersion, xcodeVersionOverrideFlag);
+
+    return xcodeVersion.getXcodeVersionProperties();
   }
 
   /**
    * Returns the {@link XcodeVersionRuleData} associated with the {@code xcode_version} target
    * explicitly defined in the {@code --xcode_version_config} build flag and selected by the {@code
    * --xcode_version} flag. If {@code --xcode_version} is unspecified, then this will return the
-   * default rule data as specified in the {@code --xcode_version_config} target. Returns null if
-   * either the {@code --xcode_version} did not match any {@code xcode_version} target, or if {@code
-   * --xcode_version} is unspecified and {@code --xcode_version_config} specified no default target.
+   * default rule data as specified in the {@code --xcode_version_config} target.
    */
-  @Nullable
   private static XcodeVersionRuleData resolveExplicitlyDefinedVersion(
-      boolean requireDefinedVersions,
       Iterable<XcodeVersionRuleData> xcodeVersionRules,
-      @Nullable XcodeVersionRuleData defaultVersion,
+      XcodeVersionRuleData defaultVersion,
       String versionOverrideFlag)
       throws XcodeConfigException {
 
@@ -218,18 +215,13 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
           aliasesToVersionMap.get(versionOverrideFlag);
       if (explicitVersion != null) {
         return explicitVersion;
+      } else {
+        throw new XcodeConfigException(
+            String.format("%s matches no alias in the config", versionOverrideFlag));
       }
-    } else if (defaultVersion != null) {
-      // No override specified. Use default.
-      return defaultVersion;
-    }
-
-    if (requireDefinedVersions) {
-      throw new XcodeConfigException(
-          "xcode version config required an explicitly defined version, but none was available");
     }
 
-    return null;
+    return defaultVersion;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigRule.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigRule.java
index 1b5a4adf40..b22e1aa6a6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigRule.java
@@ -44,7 +44,8 @@ public class XcodeConfigRule implements RuleDefinition {
         /* <!-- #BLAZE_RULE(xcode_config).ATTRIBUTE(version) -->
         The default official version of xcode to use.
         The version specified by the provided <code>xcode_version</code> target is to be used if
-        no <code>xcode_version</code> build flag is specified.
+        no <code>xcode_version</code> build flag is specified. This is required if any
+        <code>versions</code> are set.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(attr(DEFAULT_ATTR_NAME, LABEL)
             .allowedRuleClasses("xcode_version")
@@ -61,11 +62,9 @@ public class XcodeConfigRule implements RuleDefinition {
             .allowedFileTypes()
             .nonconfigurable("this rule determines configuration"))
         /* <!-- #BLAZE_RULE(xcode_config).ATTRIBUTE(version) -->
-        Whether to require the build's xcode version match one of the declared targets.
-        If true, this will raise an error if either the <code>xcode_version</code> flag value
-        or <code>default</code> attribute value do not match one of the versions declared
-        among <code>xcode_verison</code> targets.
+        Deprecated. This attribute has no effect.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
+        // TODO(b/64576392): Remove this attribute.
         .add(attr(REQUIRE_DEFINED_VERSIONS_ATTR_NAME, BOOLEAN)
             .value(false)
             .nonconfigurable("this rule determines configuration"))