Make path attribute optional for android_sdk_repository and android_ndk_repository.

Read from ANDROID_{NDK_}HOME if not set. Note that the repository is NOT invalidated if ANDROID_{NDK_}HOME is changed. Once https://bazel.build/designs/2016/10/18/repository-invalidation.htm is implemented, that will no longer be the case.

This is one piece of https://github.com/bazelbuild/bazel/issues/2284.

RELNOTES: android_sdk_repository and android_ndk_repository now read $ANDROID_HOME and $ANDROID_NDK_HOME if the path attribute is not set.

--
PiperOrigin-RevId: 143686964
MOS_MIGRATED_REVID=143686964

4 files changed, 73 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
index f26bfc93c2..c41398338f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
@@ -26,14 +26,14 @@ import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.NdkReleas
 import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpl;
 import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpls;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.packages.AttributeMap;
-import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.FileSymlinkException;
 import com.google.devtools.build.lib.skyframe.FileValue;
 import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException;
+import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -57,7 +57,8 @@ import java.util.Map;
  */
 public class AndroidNdkRepositoryFunction extends RepositoryFunction {
 
-  private static final String TOOLCHAIN_NAME_PREFIX = "toolchain-"; 
+  private static final String TOOLCHAIN_NAME_PREFIX = "toolchain-";
+  private static final String PATH_ENV_VAR = "ANDROID_NDK_HOME";
   
   private static final class CrosstoolStlPair {
 
@@ -80,7 +81,21 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction {
       Rule rule, Path outputDirectory, BlazeDirectories directories, Environment env)
       throws InterruptedException, RepositoryFunctionException {
     prepareLocalRepositorySymlinkTree(rule, outputDirectory);
-    PathFragment pathFragment = getTargetPath(rule, directories.getWorkspace());
+    WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule);
+    PathFragment pathFragment;
+    if (attributes.isAttributeValueExplicitlySpecified("path")) {
+      pathFragment = getTargetPath(rule, directories.getWorkspace());
+    } else if (clientEnvironment.containsKey(PATH_ENV_VAR)) {
+      pathFragment = getAndroidNdkHomeEnvironmentVar(directories.getWorkspace(), clientEnvironment);
+    } else {
+      throw new RepositoryFunctionException(
+          new EvalException(
+              rule.getLocation(),
+              "Either the path attribute of android_ndk_repository or the ANDROID_NDK_HOME "
+                  + " environment variable must be set."),
+          Transience.PERSISTENT);
+    }
+
     Path ndkSymlinkTreeDirectory = outputDirectory.getRelative("ndk");
     try {
       ndkSymlinkTreeDirectory.createDirectory();
@@ -93,16 +108,20 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction {
       return null;
     }
 
-    AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
     String ruleName = rule.getName();
-    String apiLevelAttr = attributes.get("api_level", Type.INTEGER).toString();
 
     NdkRelease ndkRelease = getNdkRelease(outputDirectory, env);
     if (env.valuesMissing()) {
       return null;
     }
-    
-    ApiLevel apiLevel = ApiLevel.getApiLevel(ndkRelease, env.getListener(), ruleName, apiLevelAttr);
+
+    ApiLevel apiLevel;
+    try {
+      String apiLevelAttr = attributes.get("api_level", Type.INTEGER).toString();
+      apiLevel = ApiLevel.getApiLevel(ndkRelease, env.getListener(), ruleName, apiLevelAttr);
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
 
     if (!ndkRelease.isValid) {
       env.getListener().handle(Event.warn(String.format(
@@ -150,6 +169,11 @@ public class AndroidNdkRepositoryFunction extends RepositoryFunction {
     return AndroidNdkRepositoryRule.class;
   }
 
+  private static PathFragment getAndroidNdkHomeEnvironmentVar(
+      Path workspace, Map<String, String> env) {
+    return workspace.getRelative(new PathFragment(env.get(PATH_ENV_VAR))).asFragment();
+  }
+
   private static String createBuildFile(String ruleName, List<CrosstoolStlPair> crosstools) {
 
     String buildFileTemplate = getTemplate("android_ndk_build_file_template.txt");
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
index 2afc5b52a8..cdd9a12bc8 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
@@ -61,7 +61,7 @@ public class AndroidNdkRepositoryRule implements RuleDefinition {
         .setUndocumented()
         .setWorkspaceOnly()
         .setExternalBindingsFunction(BINDINGS_FUNCTION)
-        .add(attr("path", STRING).mandatory().nonconfigurable("WORKSPACE rule"))
+        .add(attr("path", STRING).nonconfigurable("WORKSPACE rule"))
         .add(attr("api_level", INTEGER).mandatory().nonconfigurable("WORKSPACE rule"))
         .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
index f031d69b53..28ae86cd98 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
@@ -19,11 +19,10 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
-import com.google.devtools.build.lib.packages.AttributeMap;
-import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.DirectoryListingValue;
 import com.google.devtools.build.lib.skyframe.Dirents;
 import com.google.devtools.build.lib.skyframe.FileSymlinkException;
@@ -43,6 +42,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
+import java.util.Map;
 import java.util.Properties;
 import javax.annotation.Nullable;
 
@@ -52,6 +52,7 @@ import javax.annotation.Nullable;
 public class AndroidSdkRepositoryFunction extends RepositoryFunction {
   private static final String BUILD_TOOLS_DIR_NAME = "build-tools";
   private static final Revision MIN_BUILD_TOOLS_REVISION = new Revision(24, 0, 3);
+  private static final String PATH_ENV_VAR = "ANDROID_HOME";
 
   @Override
   public boolean isLocal(Rule rule) {
@@ -64,18 +65,40 @@ public class AndroidSdkRepositoryFunction extends RepositoryFunction {
       throws SkyFunctionException, InterruptedException {
 
     prepareLocalRepositorySymlinkTree(rule, outputDirectory);
-    PathFragment pathFragment = getTargetPath(rule, directories.getWorkspace());
+    WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule);
+    PathFragment pathFragment;
+    if (attributes.isAttributeValueExplicitlySpecified("path")) {
+      pathFragment = getTargetPath(rule, directories.getWorkspace());
+    } else if (clientEnvironment.containsKey(PATH_ENV_VAR)){
+      pathFragment = getAndroidHomeEnvironmentVar(directories.getWorkspace(), clientEnvironment);
+    } else {
+      throw new RepositoryFunctionException(
+          new EvalException(
+              rule.getLocation(),
+              "Either the path attribute of android_sdk_repository or the ANDROID_HOME environment "
+                  + " variable must be set."),
+          Transience.PERSISTENT);
+    }
 
     if (!symlinkLocalRepositoryContents(
         outputDirectory, directories.getOutputBase().getFileSystem().getPath(pathFragment))) {
       return null;
     }
 
-    AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
-    Integer apiLevel = attributes.get("api_level", Type.INTEGER);
+    String apiLevel;
+    try {
+      apiLevel = attributes.get("api_level", Type.INTEGER).toString();
+    } catch (EvalException e) {
+      throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+    }
+
     String buildToolsDirectory;
     if (attributes.isAttributeValueExplicitlySpecified("build_tools_version")) {
-      buildToolsDirectory = attributes.get("build_tools_version", Type.STRING);
+      try {
+        buildToolsDirectory = attributes.get("build_tools_version", Type.STRING);
+      } catch (EvalException e) {
+        throw new RepositoryFunctionException(e, Transience.PERSISTENT);
+      }
     } else {
       // If the build_tools_version attribute is not explicitly set, we select the highest version
       // installed in the SDK.
@@ -117,10 +140,10 @@ public class AndroidSdkRepositoryFunction extends RepositoryFunction {
     String template = getStringResource("android_sdk_repository_template.txt");
 
     String buildFile = template
-        .replaceAll("%repository_name%", rule.getName())
-        .replaceAll("%build_tools_version%", buildToolsVersion)
-        .replaceAll("%build_tools_directory%", buildToolsDirectory)
-        .replaceAll("%api_level%", apiLevel.toString());
+        .replace("%repository_name%", rule.getName())
+        .replace("%build_tools_version%", buildToolsVersion)
+        .replace("%build_tools_directory%", buildToolsDirectory)
+        .replace("%api_level%", apiLevel);
 
     // All local maven repositories that are shipped in the Android SDK.
     // TODO(ajmichael): Create SkyKeys so that if the SDK changes, this function will get rerun.
@@ -136,7 +159,7 @@ public class AndroidSdkRepositoryFunction extends RepositoryFunction {
             }
           }));
       sdkExtrasRepository.writeBuildFiles(outputDirectory);
-      buildFile = buildFile.replaceAll(
+      buildFile = buildFile.replace(
           "%exported_files%", sdkExtrasRepository.getExportsFiles(outputDirectory));
     } catch (IOException e) {
       throw new RepositoryFunctionException(e, Transience.TRANSIENT);
@@ -151,6 +174,11 @@ public class AndroidSdkRepositoryFunction extends RepositoryFunction {
     return AndroidSdkRepositoryRule.class;
   }
 
+  private static PathFragment getAndroidHomeEnvironmentVar(
+      Path workspace, Map<String, String> env) {
+    return workspace.getRelative(new PathFragment(env.get(PATH_ENV_VAR))).asFragment();
+  }
+
   private static String getStringResource(String name) {
     try {
       return ResourceFileLoader.loadResource(
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
index d87f4602e9..899601a326 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
@@ -57,7 +57,7 @@ public class AndroidSdkRepositoryRule implements RuleDefinition {
         .setUndocumented()
         .setWorkspaceOnly()
         .setExternalBindingsFunction(BINDINGS_FUNCTION)
-        .add(attr("path", STRING).mandatory().nonconfigurable("WORKSPACE rule"))
+        .add(attr("path", STRING).nonconfigurable("WORKSPACE rule"))
         // This is technically the directory for the build tools in $sdk/build-tools. In particular,
         // preview SDKs are in "$sdk/build-tools/x.y.z-preview", but the version is typically
         // actually "x.y.z-rcN". E.g., for 24, the directory is "$sdk/build-tools/24.0.0-preview",