Skip to content

Commit

Permalink
Update android_sdk_repository to create a valid, but useless, repository
Browse files Browse the repository at this point in the history
if the SDK doesn't actually exist.

This allows non-Android builds to proceed but will fail building android
code.

Fixes #12069.

Closes #12070.
RELNOTES: Non-android targets can again be built when android_sdk_repository is present but invalid.
PiperOrigin-RevId: 330832321
  • Loading branch information
katre authored and laurentlb committed Oct 1, 2020
1 parent 8abe603 commit 4f05123
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,10 @@ public RepositoryDirectoryValue.Builder fetch(
androidSdkPath =
fs.getPath(getAndroidHomeEnvironmentVar(directories.getWorkspace(), environ));
} 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);
// Write an empty BUILD file that declares errors when referred to.
String buildFile = getStringResource("android_sdk_repository_empty_template.txt");
writeBuildFile(outputDirectory, buildFile);
return RepositoryDirectoryValue.builder().setPath(outputDirectory);
}

if (!symlinkLocalRepositoryContents(outputDirectory, androidSdkPath, userDefinedPath)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ private static final ImmutableMap<String, Label> calculateBindings(Rule rule) {
builder.put("android/d8_jar_import", Label.parseAbsoluteUnchecked(prefix + "d8_jar_import"));
builder.put("android/dx_jar_import", Label.parseAbsoluteUnchecked(prefix + "dx_jar_import"));
builder.put("android_sdk_for_testing", Label.parseAbsoluteUnchecked(prefix + "files"));
builder.put(
"has_androidsdk", Label.parseAbsoluteUnchecked("@bazel_tools//tools/android:always_true"));
builder.put("has_androidsdk", Label.parseAbsoluteUnchecked(prefix + "has_androidsdk"));
return builder.build();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package(default_visibility = ["//visibility:public"])

# android_sdk_repository was used without a valid Android SDK being set.
# Either the path attribute of android_sdk_repository or the ANDROID_HOME
# environment variable must be set.
# This is a minimal BUILD file to allow non-Android builds to continue.

alias(
name = "has_androidsdk",
actual = "@bazel_tools//tools/android:always_false",
)

filegroup(
name = "files",
srcs = [":error_message"],
)

filegroup(
name = "sdk",
srcs = [":error_message"],
)

filegroup(
name = "d8_jar_import",
srcs = [":error_message"],
)

filegroup(
name = "dx_jar_import",
srcs = [":error_message"],
)

genrule(
name = "invalid_android_sdk_repository_error",
outs = [
"error_message",
"error_message.jar",
],
cmd = """echo \
android_sdk_repository was used without a valid Android SDK being set. \
Either the path attribute of android_sdk_repository or the ANDROID_HOME \
environment variable must be set. ; \
exit 1 """,
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ load(
"create_android_sdk_rules",
"create_system_images_filegroups")

alias(
name = "has_androidsdk",
actual = "@bazel_tools//tools/android:always_true",
)

create_android_sdk_rules(
name = "%repository_name%",
build_tools_version = "%build_tools_version%",
Expand Down
36 changes: 34 additions & 2 deletions src/test/shell/bazel/android/android_sdk_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ android_sdk_repository(
api_level = 25,
)
EOF
bazel build @androidsdk//:files >& $TEST_log && fail "Should have failed"
bazel build @androidsdk//:files >& $TEST_log && fail "Should have failed" || true
expect_log "Either the path attribute of android_sdk_repository"
}

Expand All @@ -71,7 +71,7 @@ android_sdk_repository(
path = "$TEST_SRCDIR/some_dir",
)
EOF
bazel build @androidsdk//:files >& $TEST_log && fail "Should have failed"
bazel build @androidsdk//:files >& $TEST_log && fail "Should have failed" || true
expect_log "Unable to read the Android SDK at $TEST_SRCDIR/some_dir, the path may be invalid." \
" Is the path in android_sdk_repository() or \$ANDROID_SDK_HOME set correctly?"
}
Expand Down Expand Up @@ -102,4 +102,36 @@ function test_android_sdk_repository_returns_null_if_env_vars_missing() {
ANDROID_HOME=$ANDROID_SDK bazel build @androidsdk//:files || "Build failed"
}

# Regression test for https://github.com/bazelbuild/bazel/issues/12069
function test_android_sdk_repository_present_not_set() {
create_new_workspace
cat >> WORKSPACE <<EOF
android_sdk_repository(
name = "androidsdk",
)
EOF

mkdir -p a
cat > a/BUILD <<EOF
package(default_visibility = ["//visibility:public"])
sh_test(
name = 'helper',
srcs = [ 'helper.sh' ],
)
EOF

cat > a/helper.sh <<EOF
#!/bin/sh
exit 0
EOF
chmod +x a/helper.sh

unset ANDROID_HOME
# We should be able to build non-android targets without a valid SDK.
bazel build //a:helper || fail "Build failed"
# Trying to actually build android code repo should still fail.
create_android_binary
bazel build //java/bazel:bin && fail "Build should have failed" || true
}

run_suite "Android integration tests for SDK"
5 changes: 3 additions & 2 deletions tools/android/BUILD.tools
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,9 @@ genrule(
# //external:has_androidsdk is bound to either
# @bazel_tools//tools/android:always_true or
# @bazel_tools//tools/android:always_false depending on whether
# android_sdk_repository has run. This allows targets to depend on targets from
# @androidsdk if and only if the user has an android_sdk_repository set up.
# android_sdk_repository has run and is valid. This allows targets to depend on
# targets from @androidsdk if and only if the user has an
# android_sdk_repository set up.

config_feature_flag(
name = "true",
Expand Down

0 comments on commit 4f05123

Please sign in to comment.