Skip to content

Commit

Permalink
Enable android_sdk_repository to read the $ANDROID_SDK_ROOT environment
Browse files Browse the repository at this point in the history 
variable.

FIXES #13612
RELNOTES: android_sdk_repository read $ANDROID_SDK_ROOT in addition to
$ANDROID_HOME.
PiperOrigin-RevId: 435745046
  • Loading branch information
Googler authored and Copybara-Service committed Mar 18, 2022
1 parent 822d823 commit 314b090
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 13 deletions.
48 changes: 35 additions & 13 deletions .../java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ public String toString() {
private static final PathFragment PLATFORMS_DIR = PathFragment.create("platforms");
private static final PathFragment SYSTEM_IMAGES_DIR = PathFragment.create("system-images");
private static final AndroidRevision MIN_BUILD_TOOLS_REVISION = AndroidRevision.parse("30.0.0");
private static final String PATH_ENV_VAR = "ANDROID_HOME";
private static final ImmutableList<String> PATH_ENV_VAR_AS_LIST = ImmutableList.of(PATH_ENV_VAR);
private static final ImmutableList<String> PATH_ENV_VAR_CANDIDATES =
ImmutableList.of("ANDROID_HOME", "ANDROID_SDK_ROOT");
private static final ImmutableList<String> LOCAL_MAVEN_REPOSITORIES =
ImmutableList.of(
"extras/android/m2repository",
Expand All @@ -181,7 +181,7 @@ public boolean verifyMarkerData(Rule rule, Map<String, String> markerData, Envir
if (attributes.isAttributeValueExplicitlySpecified("path")) {
return true;
}
return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_LIST);
return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_CANDIDATES);
}

@Override
Expand All @@ -194,7 +194,7 @@ public RepositoryDirectoryValue.Builder fetch(
SkyKey key)
throws RepositoryFunctionException, InterruptedException {
Map<String, String> environ =
declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_LIST);
declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_CANDIDATES);
if (environ == null) {
return null;
}
Expand All @@ -203,13 +203,16 @@ public RepositoryDirectoryValue.Builder fetch(
FileSystem fs = directories.getOutputBase().getFileSystem();
Path androidSdkPath;
String userDefinedPath = null;
String validAndroidHomeEnvironmentVar = disambiguateAndroidHomeEnvironmentVar(environ);
if (attributes.isAttributeValueExplicitlySpecified("path")) {
userDefinedPath = getPathAttr(rule);
androidSdkPath = fs.getPath(getTargetPath(userDefinedPath, directories.getWorkspace()));
} else if (environ.get(PATH_ENV_VAR) != null) {
userDefinedPath = environ.get(PATH_ENV_VAR);
} else if (validAndroidHomeEnvironmentVar != null) {
userDefinedPath = environ.get(validAndroidHomeEnvironmentVar);
androidSdkPath =
fs.getPath(getAndroidHomeEnvironmentVar(directories.getWorkspace(), environ));
fs.getPath(
getAndroidHomeEnvironmentVar(
directories.getWorkspace(), environ, validAndroidHomeEnvironmentVar));
} else {
// Write an empty BUILD file that declares errors when referred to.
String buildFile = getStringResource("android_sdk_repository_empty_template.txt");
Expand Down Expand Up @@ -347,9 +350,27 @@ public Class<? extends RuleDefinition> getRuleDefinition() {
return AndroidSdkRepositoryRule.class;
}

private static String disambiguateAndroidHomeEnvironmentVar(Map<String, String> env) {
// Returns empty string if none of the environment variable candidates
// were in the environment. Otherwise, returns the environment variable
// pointing to an SDK installation.
String disambiguatedAndroidHomeVariable = null;
for (String candidate : PATH_ENV_VAR_CANDIDATES) {
// Iterates through the environment variable candidates in order.
// Bails out once a single valid environment variable is found; earlier list entries have
// precedence over sequentially later ones.
if (env.get(candidate) != null) {
disambiguatedAndroidHomeVariable = candidate;
break;
}
}

return disambiguatedAndroidHomeVariable;
}

private static PathFragment getAndroidHomeEnvironmentVar(
Path workspace, Map<String, String> env) {
return workspace.getRelative(PathFragment.create(env.get(PATH_ENV_VAR))).asFragment();
Path workspace, Map<String, String> env, String pathEnvVar) {
return workspace.getRelative(PathFragment.create(env.get(pathEnvVar))).asFragment();
}

private static String getStringResource(String name) {
Expand Down Expand Up @@ -531,10 +552,11 @@ protected void throwInvalidPathException(Path path, Exception e)
throw new RepositoryFunctionException(
new IOException(
String.format(
"%s Unable to read the Android SDK at %s, the path may be invalid. Is "
+ "the path in android_sdk_repository() or %s set correctly? If the path is "
+ "correct, the contents in the Android SDK directory may have been modified.",
e.getMessage(), path, PATH_ENV_VAR),
"%s Unable to read the Android SDK at %s, the path may be invalid. Is the path in"
+ " android_sdk_repository() or $ANDROID_SDK_ROOT (or the deprecated"
+ " $ANDROID_HOME) set correctly? If the path is correct, the contents in the"
+ " Android SDK directory may have been modified.",
e.getMessage(), path),
e),
Transience.PERSISTENT);
}
Expand Down
20 changes: 20 additions & 0 deletions src/test/shell/bazel/local_repository_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ eof

ANDROID_HOME=./fake bazel query @foo3b//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo3b.* path is \"./fake\" (absolute: \"[^\"]*workspace/fake\").*symlink could not be created"
ANDROID_SDK_ROOT=./fake bazel query @foo3b//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo3b.* path is \"./fake\" (absolute: \"[^\"]*workspace/fake\").*symlink could not be created"

bazel query @foo4a//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo4a.* path is \"./missing\" (absolute: \"[^\"]*workspace/missing\").*symlink could not be created"
Expand Down Expand Up @@ -554,6 +556,9 @@ eof
# will actually be expanded to the full path of the home directory.
HOME=/fake_home ANDROID_HOME=~/fake bazel query @foo3b//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo3b.* path is \"/fake_home/fake\" (absolute: \"/fake_home/fake\").*symlink could not be created"
# Run the same test for ANDROID_SDK_ROOT
HOME=/fake_home ANDROID_SDK_ROOT=~/fake bazel query @foo3b//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo3b.* path is \"/fake_home/fake\" (absolute: \"/fake_home/fake\").*symlink could not be created"

bazel query @foo4a//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo4a.* path is \"~/missing\" (absolute: \"[^\"]*workspace/~/missing\").*symlink could not be created"
Expand All @@ -564,6 +569,21 @@ eof
expect_log "@foo4b.* path is \"/fake_home/fake\" (absolute: \"/fake_home/fake\").*symlink could not be created"
}

function test_android_home_env_var_precedence() {
cat >WORKSPACE <<eof
android_sdk_repository(name = "foo")
eof
# Only ANDROID_HOME is set.
ANDROID_HOME=./fake bazel query @foo//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo.* path is \"./fake\" (absolute: \"[^\"]*workspace/fake\").*symlink could not be created"
# Only ANDROID_SDK_ROOT is set.
ANDROID_SDK_ROOT=./fake bazel query @foo//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo.* path is \"./fake\" (absolute: \"[^\"]*workspace/fake\").*symlink could not be created"
# Both are set. ANDROID_HOME should have priority over ANDROID_SDK_ROOT.
ANDROID_SDK_ROOT=./wrongfake ANDROID_HOME=./fake bazel query @foo//:* >& "$TEST_log" && fail "Expected failure" || true
expect_log "@foo.* path is \"./fake\" (absolute: \"[^\"]*workspace/fake\").*symlink could not be created"
}

function test_overlaid_build_file() {
local mutant=$TEST_TMPDIR/mutant
mkdir $mutant
Expand Down

0 comments on commit 314b090

Please sign in to comment.