Actually force loading of optional objc labels

--
MOS_MIGRATED_REVID=89697088

4 files changed, 46 insertions, 54 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index a2cb6f07e5..8789cad641 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -769,6 +769,12 @@ public final class BuildConfiguration implements Serializable {
     )
     public List<Label> targetEnvironments;
 
+    @Option(name = "objc_gcov_binary",
+        converter = LabelConverter.class,
+        defaultValue = "//third_party/gcov:gcov_for_xcode",
+        category = "undocumented")
+    public Label objcGcovBinary;
+
     @Override
     public FragmentOptions getHost(boolean fallback) {
       Options host = (Options) getDefault();
@@ -819,6 +825,9 @@ public final class BuildConfiguration implements Serializable {
       if ((runUnder != null) && (runUnder.getLabel() != null)) {
         labelMap.put("RunUnder", runUnder.getLabel());
       }
+      if (collectCodeCoverage) {
+        labelMap.put("objc_gcov", objcGcovBinary);
+      }
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
index 50290d5e48..4c790267b5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.objc;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
@@ -29,8 +30,7 @@ import java.util.List;
 /**
  * Command-line options for building Objective-C targets.
  */
-public class
-    ObjcCommandLineOptions extends FragmentOptions {
+public class ObjcCommandLineOptions extends FragmentOptions {
   @Option(name = "ios_sdk_version",
       defaultValue = DEFAULT_SDK_VERSION,
       category = "build",
@@ -109,11 +109,40 @@ public class
           "Don't set this value from the command line - it is derived from  ios_multi_cpus only.")
   public String iosSplitCpu;
 
+  @Option(name = "objc_dump_syms_binary",
+      defaultValue = "//tools/objc:dump_syms",
+      category = "undocumented",
+      converter = LabelConverter.class)
+  public Label dumpSyms;
+
+  @Option(name = "default_ios_provisiong_profile",
+      defaultValue = "//tools/objc:default_provisioning_profile",
+      category = "undocumented",
+      converter = LabelConverter.class)
+  public Label defaultProvisioningProfile;
+
   @VisibleForTesting static final String DEFAULT_MINIMUM_IOS = "7.0";
   @VisibleForTesting static final String DEFAULT_IOS_CPU = "i386";
 
   @Override
-  public void addAllLabels(Multimap<String, Label> labelMap) {}
+  public void addAllLabels(Multimap<String, Label> labelMap) {
+    if (generateDebugSymbols) {
+      labelMap.put("dump_syms", dumpSyms);
+    }
+
+    if (getPlatform() == Platform.DEVICE) {
+      labelMap.put("default_provisioning_profile", defaultProvisioningProfile);
+    }
+  }
+
+  private Platform getPlatform() {
+    for (String architecture : iosMultiCpus) {
+      if (Platform.forArch(architecture) == Platform.DEVICE) {
+        return Platform.DEVICE;
+      }
+    }
+    return Platform.forArch(iosCpu);
+  }
 
   @Override
   public List<SplitTransition<BuildOptions>> getPotentialSplitTransitions() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
index 379b5b3675..32c517ddb5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -68,12 +68,7 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
   @Nullable private final Label dumpSymsLabel;
   @Nullable private final Label defaultProvisioningProfileLabel;
 
-  ObjcConfiguration(
-      ObjcCommandLineOptions objcOptions,
-      BuildConfiguration.Options options,
-      @Nullable Label gcovLabel,
-      @Nullable Label dumpSymsLabel,
-      @Nullable Label defaultProvisioningProfileLabel) {
+  ObjcConfiguration(ObjcCommandLineOptions objcOptions, BuildConfiguration.Options options) {
     this.iosSdkVersion = Preconditions.checkNotNull(objcOptions.iosSdkVersion, "iosSdkVersion");
     this.iosMinimumOs = Preconditions.checkNotNull(objcOptions.iosMinimumOs, "iosMinimumOs");
     this.iosSimulatorDevice =
@@ -86,9 +81,9 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
     this.runMemleaks = objcOptions.runMemleaks;
     this.copts = ImmutableList.copyOf(objcOptions.copts);
     this.compilationMode = Preconditions.checkNotNull(options.compilationMode, "compilationMode");
-    this.gcovLabel = gcovLabel;
-    this.dumpSymsLabel = dumpSymsLabel;
-    this.defaultProvisioningProfileLabel = defaultProvisioningProfileLabel;
+    this.gcovLabel = options.objcGcovBinary;
+    this.dumpSymsLabel = objcOptions.dumpSyms;
+    this.defaultProvisioningProfileLabel = objcOptions.defaultProvisioningProfile;
     this.iosMultiCpus = Preconditions.checkNotNull(objcOptions.iosMultiCpus, "iosMultiCpus");
     this.iosSplitCpu = Preconditions.checkNotNull(objcOptions.iosSplitCpu, "iosSplitCpu");
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java
index b8a5996c42..e877aa63cc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java
@@ -20,9 +20,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
-import com.google.devtools.build.lib.packages.NoSuchPackageException;
-import com.google.devtools.build.lib.packages.NoSuchTargetException;
-import com.google.devtools.build.lib.syntax.Label;
 
 /**
  * A loader that creates ObjcConfiguration instances based on Objective-C configurations and
@@ -35,49 +32,11 @@ public class ObjcConfigurationLoader implements ConfigurationFragmentFactory {
     Options options = buildOptions.get(BuildConfiguration.Options.class);
     ObjcCommandLineOptions objcOptions = buildOptions.get(ObjcCommandLineOptions.class);
 
-    // TODO(danielwh): Replace these labels with something from an objc_toolchain when it exists
-    Label gcovLabel = null;
-    if (options.collectCodeCoverage) {
-      gcovLabel = forceLoad(env, "//third_party/gcov:gcov_for_xcode");
-    }
-
-    Label dumpSymsLabel = null;
-    if (objcOptions.generateDebugSymbols) {
-      dumpSymsLabel = forceLoad(env, "//tools/objc:dump_syms");
-    }
-
-    Label defaultProvisioningProfileLabel = null;
-    if (getPlatform(objcOptions) == Platform.DEVICE) {
-      defaultProvisioningProfileLabel = forceLoad(env, "//tools/objc:default_provisioning_profile");
-    }
-
-    return new ObjcConfiguration(
-        objcOptions, options, gcovLabel, dumpSymsLabel, defaultProvisioningProfileLabel);
+    return new ObjcConfiguration(objcOptions, options);
   }
 
   @Override
   public Class<? extends BuildConfiguration.Fragment> creates() {
     return ObjcConfiguration.class;
   }
-
-  private Platform getPlatform(ObjcCommandLineOptions objcOptions) {
-    for (String architecture : objcOptions.iosMultiCpus) {
-      if (Platform.forArch(architecture) == Platform.DEVICE) {
-        return Platform.DEVICE;
-      }
-    }
-    return Platform.forArch(objcOptions.iosCpu);
-  }
-
-  private static Label forceLoad(ConfigurationEnvironment env, String target)
-      throws InvalidConfigurationException {
-    try {
-      Label label = Label.parseAbsolute(target);
-      env.getTarget(label);
-      return label;
-    } catch (Label.SyntaxException | NoSuchPackageException | NoSuchTargetException e) {
-      throw new InvalidConfigurationException("Error parsing or loading " + target + ": "
-          + e.getMessage(), e);
-    }
-  }
 }