Refactor the logic to determine Xcode configuration so that getting the information from the BUILD files and determining what to do with them is separated.

This is so that we can eventually determine this information not in AppleConfiguration.Loader, but in the implementation of the xcode_config rule so that the former doesn't need to look at BUILD files anymore.

Of course, this requires changing Skylark code that uses ctx.fragments.apple.$WHATEVER, but it's a good first step.

RELNOTES: None.
PiperOrigin-RevId: 161916770

2 files changed, 100 insertions, 78 deletions

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 936a3bb6cf..4dd5b79f21 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
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -92,6 +91,7 @@ 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;
   @Nullable private final String xcodeToolchain;
   @Nullable private final Label defaultProvisioningProfileLabel;
@@ -142,6 +142,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     this.bitcodeMode = appleOptions.appleBitcodeMode;
     this.xcodeConfigLabel =
         Preconditions.checkNotNull(appleOptions.xcodeVersionConfig, "xcodeConfigLabel");
+    this.xcodeVersionCommandLineFlag = appleOptions.xcodeVersion;
     this.enableAppleCrosstool = appleOptions.enableAppleCrosstoolTransition;
     this.defaultProvisioningProfileLabel = appleOptions.defaultProvisioningProfile;
     this.xcodeToolchain = appleOptions.xcodeToolchain;
@@ -536,6 +537,13 @@ 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
@@ -617,7 +625,8 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
         throws InvalidConfigurationException, InterruptedException {
       AppleCommandLineOptions appleOptions = buildOptions.get(AppleCommandLineOptions.class);
       String cpu = buildOptions.get(BuildConfiguration.Options.class).cpu;
-      XcodeVersionProperties xcodeVersionProperties = getXcodeVersionProperties(env, appleOptions);
+      XcodeVersionProperties xcodeVersionProperties = XcodeConfig.
+          getXcodeVersionProperties(env, appleOptions);
 
       DottedVersion iosSdkVersion = (appleOptions.iosSdkVersion != null)
           ? appleOptions.iosSdkVersion : xcodeVersionProperties.getDefaultIosSdkVersion();
@@ -674,28 +683,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
       return ImmutableSet.<Class<? extends FragmentOptions>>of(AppleCommandLineOptions.class);
     }
-    
-    /**
-     * Uses the {@link AppleCommandLineOptions#xcodeVersion} and {@link
-     * AppleCommandLineOptions#xcodeVersionConfig} command line options to determine and return the
-     * effective xcode version properties. Returns absent if no explicit xcode version is declared,
-     * and host system defaults should be used.
-     *
-     * @param env the current configuration environment
-     * @param appleOptions the command line options
-     * @throws InvalidConfigurationException if the options given (or configuration targets) were
-     *     malformed and thus the xcode version could not be determined
-     */
-    private static XcodeVersionProperties getXcodeVersionProperties(
-        ConfigurationEnvironment env, AppleCommandLineOptions appleOptions)
-        throws InvalidConfigurationException, InterruptedException {
-      Optional<DottedVersion> xcodeVersionCommandLineFlag = 
-          Optional.fromNullable(appleOptions.xcodeVersion);
-      Label xcodeVersionConfigLabel = appleOptions.xcodeVersionConfig;
 
-      return XcodeConfig.resolveXcodeVersion(env, xcodeVersionConfigLabel,
-          xcodeVersionCommandLineFlag, "xcode_version_config");
-    }
   }
 
   /**
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 67a3d9e556..a486e99c56 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
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.base.Joiner;
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -43,6 +42,61 @@ import javax.annotation.Nullable;
  * Implementation for the {@code xcode_config} rule.
  */
 public class XcodeConfig implements RuleConfiguredTargetFactory {
+  private static ImmutableList<XcodeVersionRuleData> getAvailableVersions(
+      ConfigurationEnvironment env, Rule xcodeConfigTarget)
+      throws InvalidConfigurationException, InterruptedException {
+    List<Label> xcodeVersionLabels = NonconfigurableAttributeMapper.of(xcodeConfigTarget)
+        .get(XcodeConfigRule.VERSIONS_ATTR_NAME, BuildType.LABEL_LIST);
+    ImmutableList.Builder<XcodeVersionRuleData> xcodeVersionRuleListBuilder =
+        ImmutableList.builder();
+    for (Label label : xcodeVersionLabels) {
+      Rule xcodeVersionRule = getRuleForLabel(label, "xcode_version", env, "xcode_version");
+      xcodeVersionRuleListBuilder.add(new XcodeVersionRuleData(label, xcodeVersionRule));
+    }
+    return xcodeVersionRuleListBuilder.build();
+  }
+
+  /**
+   * Uses the {@link AppleCommandLineOptions#xcodeVersion} and {@link
+   * AppleCommandLineOptions#xcodeVersionConfig} command line options to determine and return the
+   * effective xcode version properties. Returns absent if no explicit xcode version is declared,
+   * and host system defaults should be used.
+   *
+   * @param env the current configuration environment
+   * @param appleOptions the command line options
+   * @throws InvalidConfigurationException if the options given (or configuration targets) were
+   *     malformed and thus the xcode version could not be determined
+   */
+  static XcodeVersionProperties getXcodeVersionProperties(
+      ConfigurationEnvironment env, AppleCommandLineOptions appleOptions)
+      throws InvalidConfigurationException, InterruptedException {
+    Label xcodeVersionConfigLabel = appleOptions.xcodeVersionConfig;
+
+    Rule xcodeConfigRule = getRuleForLabel(
+        xcodeVersionConfigLabel, "xcode_config", env, "xcode_version_config");
+
+    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);
+    } catch (XcodeConfigException e) {
+      throw new InvalidConfigurationException(e.getMessage());
+    }
+  }
+
+  /**
+   * An exception that signals that an Xcode config setup was invalid.
+   */
+  public static class XcodeConfigException extends Exception {
+    XcodeConfigException(String reason) {
+      super(reason);
+    }
+  }
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
@@ -57,31 +111,26 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    * AppleCommandLineOptions#xcodeVersionConfig} command line options to determine and return the
    * effective xcode version and its properties.
    *
-   * @param env the current configuration environment
-   * @param xcodeConfigLabel the label for the xcode_config target to parse
-   * @param xcodeVersionOverrideFlag the value of the command line flag to override the default
-   *     xcode version, absent if unspecified
-   * @param errorDescription a description of the origin of {@code #xcodeConfigLabel} for messaging
-   *     parse errors
-   * @throws InvalidConfigurationException if the options given (or configuration targets) were
+   * @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.
+   * @throws XcodeConfigException if the options given (or configuration targets) were
    *     malformed and thus the xcode version could not be determined
    */
   static XcodeVersionProperties resolveXcodeVersion(
-      ConfigurationEnvironment env,
-      Label xcodeConfigLabel,
-      Optional<DottedVersion> xcodeVersionOverrideFlag,
-      String errorDescription)
-      throws InvalidConfigurationException, InterruptedException {
-    Rule xcodeConfigRule =
-        getRuleForLabel(xcodeConfigLabel, "xcode_config", env, errorDescription);
-    
-    XcodeVersionRuleData xcodeVersion =
-        resolveExplicitlyDefinedVersion(env, xcodeConfigRule, xcodeVersionOverrideFlag);
+      boolean requireDefinedVersions,
+      DottedVersion xcodeVersionOverrideFlag,
+      ImmutableList<XcodeVersionRuleData> xcodeVersions,
+      @Nullable XcodeVersionRuleData defaultVersion)
+      throws XcodeConfigException {
+    XcodeVersionRuleData xcodeVersion = resolveExplicitlyDefinedVersion(
+        requireDefinedVersions, xcodeVersions, defaultVersion, xcodeVersionOverrideFlag);
 
     if (xcodeVersion != null) {
       return xcodeVersion.getXcodeVersionProperties();
-    } else if (xcodeVersionOverrideFlag.isPresent()) {
-      return new XcodeVersionProperties(xcodeVersionOverrideFlag.get());
+    } else if (xcodeVersionOverrideFlag != null) {
+      return new XcodeVersionProperties(xcodeVersionOverrideFlag);
     } else {
       return XcodeVersionProperties.unknownXcodeVersionProperties();
     }
@@ -97,34 +146,29 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    */
   @Nullable
   private static XcodeVersionRuleData resolveExplicitlyDefinedVersion(
-      ConfigurationEnvironment env,
-      Rule xcodeConfigTarget,
-      Optional<DottedVersion> versionOverrideFlag)
-      throws InvalidConfigurationException, InterruptedException {
+      boolean requireDefinedVersions,
+      ImmutableList<XcodeVersionRuleData> xcodeVersionRules,
+      @Nullable XcodeVersionRuleData defaultVersion,
+      DottedVersion versionOverrideFlag)
+      throws XcodeConfigException {
 
-    Map<String, XcodeVersionRuleData> aliasesToVersionMap =
-        aliasesToVersionMap(env, xcodeConfigTarget);
+    Map<String, XcodeVersionRuleData> aliasesToVersionMap = aliasesToVersionMap(xcodeVersionRules);
 
-    if (versionOverrideFlag.isPresent()) {
+    if (versionOverrideFlag != null) {
       // The version override flag is not necessarily an actual version - it may be a version
       // alias.
       XcodeVersionRuleData explicitVersion =
-          aliasesToVersionMap.get(versionOverrideFlag.get().toString());
+          aliasesToVersionMap.get(versionOverrideFlag.toString());
       if (explicitVersion != null) {
         return explicitVersion;
       }
-    } else { // No override specified. Use default.
-      XcodeVersionRuleData defaultVersion = getDefaultVersion(env, xcodeConfigTarget);
-      
-      if (defaultVersion != null) {
-        return defaultVersion;
-      }
+    } else if (defaultVersion != null) {
+      // No override specified. Use default.
+      return defaultVersion;
     }
     
-    boolean requireDefinedVersions = NonconfigurableAttributeMapper.of(xcodeConfigTarget)
-        .get(XcodeConfigRule.REQUIRE_DEFINED_VERSIONS_ATTR_NAME, Type.BOOLEAN);
     if (requireDefinedVersions) {
-      throw new InvalidConfigurationException(
+      throw new XcodeConfigException(
           "xcode version config required an explicitly defined version, but none was available");
     }
 
@@ -154,22 +198,12 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    * Returns a map where keys are "names" of xcode versions as defined by the configuration target,
    * and values are the rule data objects which contain information regarding that xcode version.
    *
-   * @throws InvalidConfigurationException if there are duplicate aliases (if two xcode versions
+   * @throws XcodeConfigException if there are duplicate aliases (if two xcode versions
    *     were registered to the same alias)
    */
   private static Map<String, XcodeVersionRuleData> aliasesToVersionMap(
-      ConfigurationEnvironment env, Rule xcodeConfigTarget)
-      throws InvalidConfigurationException, InterruptedException {
-    List<Label> xcodeVersionLabels = NonconfigurableAttributeMapper.of(xcodeConfigTarget)
-        .get(XcodeConfigRule.VERSIONS_ATTR_NAME, BuildType.LABEL_LIST);
-    ImmutableList.Builder<XcodeVersionRuleData> xcodeVersionRuleListBuilder =
-        ImmutableList.builder();
-    for (Label label : xcodeVersionLabels) {
-      Rule xcodeVersionRule = getRuleForLabel(label, "xcode_version", env, "xcode_version");
-      xcodeVersionRuleListBuilder.add(new XcodeVersionRuleData(label, xcodeVersionRule));
-    }
-    ImmutableList<XcodeVersionRuleData> xcodeVersionRules = xcodeVersionRuleListBuilder.build();
-
+      ImmutableList<XcodeVersionRuleData> xcodeVersionRules)
+      throws XcodeConfigException {
     Map<String, XcodeVersionRuleData> aliasesToXcodeRules = Maps.newLinkedHashMap();
     for (XcodeVersionRuleData xcodeVersionRule : xcodeVersionRules) {
       for (String alias : xcodeVersionRule.getAliases()) {
@@ -191,11 +225,11 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
   }
   
   /**
-   * Convenience method for throwing an {@link InvalidConfigurationException} due to presence
+   * Convenience method for throwing an {@link XcodeConfigException} due to presence
    * of duplicate aliases in an {@code xcode_config} target definition. 
    */
   private static void configErrorDuplicateAlias(String alias,
-      List<XcodeVersionRuleData> xcodeVersionRules) throws InvalidConfigurationException {
+      List<XcodeVersionRuleData> xcodeVersionRules) throws XcodeConfigException {
 
     ImmutableList.Builder<Label> labelsContainingAlias = ImmutableList.builder();
     for (XcodeVersionRuleData xcodeVersionRule : xcodeVersionRules) {
@@ -205,7 +239,7 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
       }
     }
 
-    throw new InvalidConfigurationException(
+    throw new XcodeConfigException(
         String.format("'%s' is registered to multiple labels (%s) in a single xcode_config rule",
             alias, Joiner.on(", ").join(labelsContainingAlias.build())));
   }
@@ -215,7 +249,7 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
    * returns the {@link Rule} representing that target. Otherwise, throws a {@link
    * InvalidConfigurationException}.
    */
-  private static Rule getRuleForLabel(
+  static Rule getRuleForLabel(
       Label label, String type, ConfigurationEnvironment env, String description)
       throws InvalidConfigurationException, InterruptedException {
     label = RedirectChaser.followRedirects(env, label, description);
@@ -228,7 +262,7 @@ public class XcodeConfig implements RuleConfiguredTargetFactory {
 
     try {
       Target target = env.getTarget(label);
-      
+
       if (target instanceof Rule && ((Rule) target).getRuleClass().equals(type)) {
         return (Rule) target;
       } else {