Use xcode-locator to locate DEVELOPER_DIR for standalone spawn strategy.

--
MOS_MIGRATED_REVID=114569255

4 files changed, 229 insertions, 126 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleHostInfo.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleHostInfo.java
index 514f5a203f..0253fe3355 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleHostInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleHostInfo.java
@@ -14,6 +14,9 @@
 
 package com.google.devtools.build.lib.rules.apple;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.shell.AbnormalTerminationException;
@@ -21,11 +24,11 @@ import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.shell.CommandException;
 import com.google.devtools.build.lib.shell.CommandResult;
 import com.google.devtools.build.lib.shell.TerminationStatus;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.Map;
 
 /**
  * Obtains information pertaining to Apple host machines required for using Apple toolkits in
@@ -34,50 +37,63 @@ import java.nio.charset.StandardCharsets;
 public class AppleHostInfo {
 
   private static final String XCRUN_CACHE_FILENAME = "__xcruncache";
+  private static final String XCODE_LOCATOR_CACHE_FILENAME = "__xcodelocatorcache";
 
   /**
-   * Returns the absolute root path of the target Apple SDK on the host system. This may spawn a
+   * Returns the absolute root path of the target Apple SDK on the host system for a given
+   * version of xcode (as defined by the given {@code developerDir}). This may spawn a
    * process and use the {@code /usr/bin/xcrun} binary to locate the target SDK. This uses a local
    * cache file under {@code bazel-out}, and will only spawn a new {@code xcrun} process in the case
    * of a cache miss.
    *
+   * @param execRoot the execution root path, used to locate the cache file
+   * @param developerDir the value of {@code DEVELOPER_DIR} for the target version of xcode
+   * @param sdkVersion the sdk version, for example, "9.1"
+   * @param appleSdkPlatform the sdk platform, for example, "iPhoneOS"
    * @throws UserExecException if there is an issue with obtaining the root from the spawned
    *     process, either because the SDK platform/version pair doesn't exist, or there was an
    *     unexpected issue finding or running the tool
    */
-  public static String getSdkRoot(Path execRoot, String sdkVersion, String appleSdkPlatform)
-      throws UserExecException {
+  public static String getSdkRoot(Path execRoot, String developerDir,
+      String sdkVersion, String appleSdkPlatform) throws UserExecException {
     try {
-      Path cacheFilePath = execRoot.getRelative(BlazeDirectories.RELATIVE_OUTPUT_PATH)
-          .getRelative(XCRUN_CACHE_FILENAME);
-      FileSystemUtils.touchFile(cacheFilePath);
-      // TODO(bazel-team): Pass DEVELOPER_DIR to the cache manager.
-      XcrunCacheManager cacheManager =
-          new XcrunCacheManager(cacheFilePath, "" /* Developer dir empty */);
+      CacheManager cacheManager =
+          new CacheManager(execRoot.getRelative(BlazeDirectories.RELATIVE_OUTPUT_PATH),
+              XCRUN_CACHE_FILENAME);
 
       String sdkString = appleSdkPlatform.toLowerCase() + sdkVersion;
-      String cacheResult = cacheManager.getSdkRoot(sdkString);
+      String cacheResult = cacheManager.getValue(developerDir, sdkString);
       if (cacheResult != null) {
         return cacheResult;
       } else {
-        // TODO(bazel-team): Pass DEVELOPER_DIR to the xcrun call.
-        CommandResult xcrunResult = new Command(new String[] {"/usr/bin/xcrun", "--sdk",
-            sdkString, "--show-sdk-path"}).execute();
-    
-        TerminationStatus xcrunStatus = xcrunResult.getTerminationStatus();
-        if (!xcrunResult.getTerminationStatus().exited()) {
-          throw new UserExecException(String.format("xcrun failed.\n%s\nStderr: %s",
-              xcrunStatus.toString(), new String(xcrunResult.getStderr(), StandardCharsets.UTF_8)));
-        }
-        
+        Map<String, String> env = Strings.isNullOrEmpty(developerDir)
+            ? ImmutableMap.<String, String>of() : ImmutableMap.of("DEVELOPER_DIR", developerDir);
+        CommandResult xcrunResult = new Command(
+            new String[] {"/usr/bin/xcrun", "--sdk", sdkString, "--show-sdk-path"},
+            env, null).execute();
+
         // calling xcrun via Command returns a value with a newline on the end.
         String sdkRoot = new String(xcrunResult.getStdout(), StandardCharsets.UTF_8).trim();
 
-        cacheManager.writeSdkRoot(sdkString, sdkRoot);
+        cacheManager.writeEntry(ImmutableList.of(developerDir, sdkString), sdkRoot);
         return sdkRoot;
       }
     } catch (AbnormalTerminationException e) {
-      String message = String.format("%s : %s",
+      TerminationStatus terminationStatus = e.getResult().getTerminationStatus();
+
+      if (terminationStatus.exited()) {
+        throw new UserExecException(
+            String.format("xcrun failed with code %s.\n"
+                + "This most likely indicates that SDK version [%s] for platform [%s] is "
+                + "unsupported for the target version of xcode.\n"
+                + "%s\n"
+                + "Stderr: %s",
+                terminationStatus.getExitCode(),
+                sdkVersion, appleSdkPlatform,
+                terminationStatus.toString(),
+                new String(e.getResult().getStderr(), StandardCharsets.UTF_8)));
+      }
+      String message = String.format("xcrun failed.\n%s\n%s",
           e.getResult().getTerminationStatus(),
           new String(e.getResult().getStderr(), StandardCharsets.UTF_8));
       throw new UserExecException(message, e);
@@ -85,4 +101,62 @@ public class AppleHostInfo {
       throw new UserExecException(e);
     }
   }
+  
+  /**
+   * Returns the absolute root path of the xcode developer directory on the host system for
+   * the given xcode version. This may spawn a process and use the {@code xcode-locator} binary.
+   * This uses a local cache file under {@code bazel-out}, and will only spawn a new process in the
+   * case of a cache miss.
+   *
+   * @param execRoot the execution root path, used to locate the cache file
+   * @param version the xcode version number to look up
+   * @throws UserExecException if there is an issue with obtaining the path from the spawned
+   *     process, either because there is no installed xcode with the given version, or
+   *     there was an unexpected issue finding or running the tool
+   */
+  public static String getDeveloperDir(Path execRoot, DottedVersion version)
+      throws UserExecException {
+    try {
+      CacheManager cacheManager =
+          new CacheManager(execRoot.getRelative(BlazeDirectories.RELATIVE_OUTPUT_PATH),
+              XCODE_LOCATOR_CACHE_FILENAME);
+
+      String cacheResult = cacheManager.getValue(version.toString());
+      if (cacheResult != null) {
+        return cacheResult;
+      } else {
+        CommandResult xcodeLocatorResult = new Command(new String[] {
+            execRoot.getRelative("_bin/xcode-locator").getPathString(), version.toString()})
+            .execute();
+
+        String developerDir =
+            new String(xcodeLocatorResult.getStdout(), StandardCharsets.UTF_8).trim();
+
+        cacheManager.writeEntry(ImmutableList.of(version.toString()), developerDir);
+        return developerDir;
+      }
+    } catch (AbnormalTerminationException e) {
+      TerminationStatus terminationStatus = e.getResult().getTerminationStatus();
+
+      String message;
+      if (e.getResult().getTerminationStatus().exited()) {
+        message = String.format("xcode-locator failed with code %s.\n"
+            + "This most likely indicates that xcode version %s is not available on the host "
+            + "machine.\n"
+            + "%s\n"
+            + "stderr: %s",
+            terminationStatus.getExitCode(),
+            version,
+            terminationStatus.toString(),
+            new String(e.getResult().getStderr(), StandardCharsets.UTF_8));
+      } else {
+        message = String.format("xcode-locator failed. %s\nstderr: %s",
+            e.getResult().getTerminationStatus(),
+            new String(e.getResult().getStderr(), StandardCharsets.UTF_8));
+      }
+      throw new UserExecException(message, e);
+    } catch (CommandException | IOException e) {
+      throw new UserExecException(e);
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/CacheManager.java b/src/main/java/com/google/devtools/build/lib/rules/apple/CacheManager.java
new file mode 100644
index 0000000000..390ef50572
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/CacheManager.java
@@ -0,0 +1,110 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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.Splitter;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+/**
+ * General cache file manager for mapping one or more keys to host-related path information.
+ *
+ * <p> Cache management has some notable restrictions:
+ * <ul>
+ * <li>Each cache entry must have the same number of (string) keys, and one value.</li>
+ * <li>An entry, once written to the cache, must be stable between builds. Clearing the cache
+ *     requires a full clean of the bazel output directory.</li>
+ * </ul>
+ *
+ * <p> Note that a single cache manager instance is not thread-safe, though multiple threads may
+ * hold cache manager instances for the same cache file. As a result, it is possible multiple
+ * threads may write the same entry to cache. This is fine, as retrieval from the cache will simply
+ * return the first found entry.
+ */
+class CacheManager {
+
+  private final Path cacheFilePath;
+  private boolean cacheFileTouched;
+
+  /**
+   * @param outputRoot path to the bazel's output root
+   * @param cacheFilename name of the cache file
+   */
+  CacheManager(Path outputRoot, String cacheFilename) {
+    cacheFilePath = outputRoot.getRelative(cacheFilename);
+  }
+  
+  private void touchCacheFile() throws IOException {
+    if (!cacheFileTouched) {
+      FileSystemUtils.touchFile(cacheFilePath);
+      cacheFileTouched = true;
+    }
+  }
+  
+  /**
+   * Returns the value associated with the given list of string keys from the cache,
+   * or null if the entry is not present in the cache. If there is more than one value for the
+   * given key, the first value is returned.
+   */
+  @Nullable
+  public String getValue(String... keys) throws IOException {
+    Preconditions.checkArgument(keys.length > 0);
+    touchCacheFile();
+
+    List<String> keyList = ImmutableList.copyOf(keys);
+    Iterable<String> cacheContents =
+        FileSystemUtils.readLines(cacheFilePath, StandardCharsets.UTF_8);
+    for (String cacheLine : cacheContents) {
+      if (cacheLine.isEmpty()) {
+        continue;
+      }
+      List<String> cacheEntry = Splitter.on(':').splitToList(cacheLine);
+      List<String> cacheKeys = cacheEntry.subList(0, cacheEntry.size() - 1);
+      String cacheValue = cacheEntry.get(cacheEntry.size() - 1);
+      if (keyList.size() != cacheKeys.size()) {
+        throw new IllegalStateException(
+            String.format("cache file %s is malformed. Expected %s keys. line: '%s'",
+                cacheFilePath, keyList.size(), cacheLine));
+      }
+      if (keyList.equals(cacheKeys)) {
+        return cacheValue;
+      }
+    }
+    return null;
+  }
+
+  /**
+   * Write an entry to the cache. An entry consists of one or more keys and a single value.
+   * No validation is made regarding whether there are redundant or conflicting entries in the
+   * cache; it is thus the responsibility of the caller to ensure that any redundant entries
+   * (entries which have the same keys) also have the same value.
+   */
+  public void writeEntry(List<String> keys, String value) throws IOException {
+    Preconditions.checkArgument(!keys.isEmpty());
+
+    touchCacheFile();
+    FileSystemUtils.appendLinesAs(cacheFilePath, StandardCharsets.UTF_8,
+        Joiner.on(":").join(ImmutableList.builder().addAll(keys).add(value).build()));
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcrunCacheManager.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcrunCacheManager.java
deleted file mode 100644
index 74ed2cc436..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcrunCacheManager.java
+++ /dev/null
@@ -1,96 +0,0 @@
-// Copyright 2015 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.rules.apple;
-
-import com.google.common.base.Splitter;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
-import com.google.devtools.build.lib.vfs.Path;
-
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.util.List;
-
-import javax.annotation.Nullable;
-
-/**
- * Manages the xcrun cache file for mapping sdk identifiers to their root absolute paths. Cache
- * entries are read with {@link #getSdkRoot} and written with {@link #writeSdkRoot}.
- * 
- * <p> Note that multiple threads may hold instances of this class and be interacting with
- * the same cache file. As a result, it is possible multiple threads may write the same entry to
- * cache. This is fine, as {@link #getSdkRoot} will simply return the first found entry. The value
- * for any given key should be stable without a full clean of the bazel output directory (and
- * consequent removal of the cache file).
- */
-class XcrunCacheManager {
-
-  private final Path cachePath;
-  private final String developerDir;
-
-  /**
-   * @param cachePath path to the cache file; this file must exist, but may be empty
-   * @param developerDir the developer directory (absolute path) from which to obtain sdk root paths
-   */
-  XcrunCacheManager(Path cachePath, String developerDir) {
-    this.cachePath = cachePath;
-    this.developerDir = developerDir;
-  }
-  
-  /**
-   * Returns the SDKROOT associated with the given sdk string as its entry exists in the file cache,
-   * or null if the entry is not present in the cache. If there is more than one value for the
-   * given key, the first value is returned.
-   *
-   * @param sdkString platform concatenated with version number, as {@code xcrun} accepts
-   *     as input, for example "iphoneos9.0"
-   */
-  @Nullable
-  String getSdkRoot(String sdkString) throws IOException {
-    Iterable<String> cacheContents = FileSystemUtils.readLines(cachePath, StandardCharsets.UTF_8);
-    for (String cacheLine : cacheContents) {
-      if (cacheLine.isEmpty()) {
-        continue;
-      }
-      List<String> items = Splitter.on(':').splitToList(cacheLine);
-      if (items.size() != 3) {
-        throw new IllegalStateException(
-            String.format("xcrun cache is malformed, line: '%s'", cacheLine));
-      }
-      String developerDir = items.get(0);
-      String sdkIdentifier = items.get(1);
-      String sdkRoot = items.get(2);
-      if (sdkIdentifier.equals(sdkString) && developerDir.equals(this.developerDir)) {
-        return sdkRoot;
-      }
-    }
-    return null;
-  }
-  
-  /**
-   * Write an entry to the cache pairing the given sdk version string with the given SDKROOT.
-   * No validation is made regarding whether there are redundant or conflicting entries in the
-   * cache; it is thus the responsibility of the caller to ensure that no entry for the given
-   * key is present in the cache, prior to calling this. (Entries added to the cache file with
-   * the same key as a prior entry will not be used.)
-   *
-   * @param sdkString platform concatenated with version number, as {@code xcrun} accepts
-   *     as input, for example "iphoneos9.0"
-   * @param sdkRoot an absolute path to the SDKROOT for the sdk
-   */
-  void writeSdkRoot(String sdkString, String sdkRoot) throws IOException {
-    FileSystemUtils.appendLinesAs(cachePath, StandardCharsets.UTF_8,
-        String.format("%s:%s:%s", this.developerDir, sdkString, sdkRoot));
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
index b2f98a36d5..7299f3555f 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
 import com.google.devtools.build.lib.rules.apple.AppleHostInfo;
+import com.google.devtools.build.lib.rules.apple.DottedVersion;
 import com.google.devtools.build.lib.shell.AbnormalTerminationException;
 import com.google.devtools.build.lib.shell.Command;
 import com.google.devtools.build.lib.shell.CommandException;
@@ -157,8 +158,17 @@ public class StandaloneSpawnStrategy implements SpawnActionContext {
    */
   private ImmutableMap<String, String> locallyDeterminedEnv(ImmutableMap<String, String> env)
       throws UserExecException {
+    // TODO(bazel-team): Remove apple-specific logic from this class.
     ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder();
     newEnvBuilder.putAll(env);
+    // Empty developer dir indicates to use the system default.
+    // TODO(bazel-team): Bazel's view of the xcode version and developer dir
+    // should be explicitly set for build hermiticity.
+    String developerDir = "";
+    if (env.containsKey(AppleConfiguration.XCODE_VERSION_ENV_NAME)) {
+      developerDir = getDeveloperDir(env.get(AppleConfiguration.XCODE_VERSION_ENV_NAME));
+      newEnvBuilder.put("DEVELOPER_DIR", developerDir);
+    }
     if (env.containsKey(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME)) {
       // The Apple platform is needed to select the appropriate SDK.
       if (!env.containsKey(AppleConfiguration.APPLE_SDK_PLATFORM_ENV_NAME)) {
@@ -166,19 +176,24 @@ public class StandaloneSpawnStrategy implements SpawnActionContext {
       }
       String iosSdkVersion = env.get(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME);
       String appleSdkPlatform = env.get(AppleConfiguration.APPLE_SDK_PLATFORM_ENV_NAME);
-      // TODO(bazel-team): Determine and set DEVELOPER_DIR.
-      addSdkRootEnv(newEnvBuilder, iosSdkVersion, appleSdkPlatform);
+      newEnvBuilder.put("SDKROOT", getSdkRootEnv(developerDir, iosSdkVersion, appleSdkPlatform));
     }
     return newEnvBuilder.build();
   }
 
-  private void addSdkRootEnv(
-      ImmutableMap.Builder<String, String> envBuilder, String iosSdkVersion,
-      String appleSdkPlatform) throws UserExecException {
-    // Sanity check, also presents a less cryptic error message.
+  private String getDeveloperDir(String xcodeVersion) throws UserExecException {
+    if (OS.getCurrent() != OS.DARWIN) {
+      throw new UserExecException(
+          "Cannot locate xcode developer directory on non-darwin operating system");
+    }
+    return AppleHostInfo.getDeveloperDir(execRoot, DottedVersion.fromString(xcodeVersion));
+  }
+
+  private String getSdkRootEnv(String developerDir,
+      String iosSdkVersion, String appleSdkPlatform) throws UserExecException {
     if (OS.getCurrent() != OS.DARWIN) {
       throw new UserExecException("Cannot locate iOS SDK on non-darwin operating system");
     }
-    envBuilder.put("SDKROOT", AppleHostInfo.getSdkRoot(execRoot, iosSdkVersion, appleSdkPlatform));
+    return AppleHostInfo.getSdkRoot(execRoot, developerDir, iosSdkVersion, appleSdkPlatform);
   }
 }