Skip to content

Commit

Permalink
Revert "Update rules_jvm_external to use the Starlark version of aar_…
Browse files Browse the repository at this point in the history
…import after the native version was removed from Bazel (#1149)" (#1215)

This reverts commit ba7310c.

The default label for AAR imports is `@rules_android//android:rules.bzl`. This
imports `@rules_android//rules:providers.bzl`, which calls
`@rules_android//rules/reexport_providers.bzl`. This means that any downstream
user of `rules_jvm_external` with a sufficiently recent version of Bazel needs
to add `build --experimental_google_legacy_api` to their `.bazelrc`

That's a significantly breaking change, and one that it's not reasonable to ask
our users to do, especially since the vast majority of projects don't use the
Android rules.
  • Loading branch information
shs96c authored Aug 12, 2024
1 parent 9a10cae commit 7b4512b
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 70 deletions.
6 changes: 1 addition & 5 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ build --tool_java_runtime_version=remotejdk_11
build --experimental_strict_java_deps=strict
build --explicit_java_test_deps

# Re-enable once https://github.com/bazelbuild/rules_go/issues/3947 is addressed.
#build --experimental_sibling_repository_layout

# Remove once https://github.com/bazelbuild/rules_android/issues/219 is fixed
build --experimental_google_legacy_api
build --experimental_sibling_repository_layout

# Make sure we get something helpful when tests fail
test --verbose_failures
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.2.1
7.1.0
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ bazel_dep(
)
bazel_dep(
name = "rules_android",
version = "0.5.1",
version = "0.1.1",
)
bazel_dep(
name = "stardoc",
Expand Down
23 changes: 8 additions & 15 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -519,10 +519,9 @@ maven_install(

maven_install(
name = "starlark_aar_import_with_sources_test",
# The default is "@rules_android//rules:rules.bzl" but use
# "@rules_android//android:rules.bzl" with the older 0.1.1 release
# to use the native rules.
aar_import_bzl_label = "@rules_android//android:rules.bzl",
# Not actually necessary since this is the default value, but useful for
# testing.
aar_import_bzl_label = "@build_bazel_rules_android//android:rules.bzl",
artifacts = [
"androidx.work:work-runtime:2.6.0",
],
Expand All @@ -536,10 +535,9 @@ maven_install(

maven_install(
name = "starlark_aar_import_test",
# The default is "@rules_android//rules:rules.bzl" but use
# "@rules_android//android:rules.bzl" with the older 0.1.1 release
# to use the native rules.
aar_import_bzl_label = "@rules_android//android:rules.bzl",
# Not actually necessary since this is the default value, but useful for
# testing.
aar_import_bzl_label = "@build_bazel_rules_android//android:rules.bzl",
artifacts = [
"com.android.support:appcompat-v7:28.0.0",
],
Expand All @@ -552,14 +550,9 @@ maven_install(
)

# for the above "starlark_aar_import_test" maven_install with
# use_starlark_android_rules = True.
# Note that this version is different from the version in MODULE.bazel
# because the latest versions of rules_android do not support Bazel 5 or 6,
# which rules_jvm_external supports and uses in CI tests. So use
# rules_android 0.1.1, which are wrappers around the native Android rules,
# since the tests with Bazel 5 and 6 do no use bzlmod.
# use_starlark_android_rules = True
http_archive(
name = "rules_android",
name = "build_bazel_rules_android",
sha256 = "cd06d15dd8bb59926e4d65f9003bfc20f9da4b2519985c27e190cddc8b7a7806",
strip_prefix = "rules_android-0.1.1",
urls = ["https://github.com/bazelbuild/rules_android/archive/v0.1.1.zip"],
Expand Down
6 changes: 2 additions & 4 deletions private/rules/coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ bzl_library(
)
"""

DEFAULT_AAR_IMPORT_LABEL = "@rules_android//rules:rules.bzl"
DEFAULT_AAR_IMPORT_LABEL = "@build_bazel_rules_android//android:rules.bzl"

_AAR_IMPORT_STATEMENT = """\
load("%s", "aar_import")
Expand Down Expand Up @@ -245,9 +245,7 @@ def _relativize_and_symlink_file_in_maven_local(repository_ctx, absolute_path):
return artifact_relative_path

def _get_aar_import_statement_or_empty_str(repository_ctx):
# Use the Starlark version of aar_import if requested, or if this version of Bazel
# does not have native aar_import.
if repository_ctx.attr.use_starlark_android_rules or not hasattr(native, "aar_import"):
if repository_ctx.attr.use_starlark_android_rules:
# parse the label to validate it
_ = Label(repository_ctx.attr.aar_import_bzl_label)
return _AAR_IMPORT_STATEMENT % repository_ctx.attr.aar_import_bzl_label
Expand Down
12 changes: 3 additions & 9 deletions private/rules/maven_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,12 @@ def maven_install(
use_credentials_from_home_netrc_file: Whether to pass machine login credentials from the ~/.netrc file to coursier.
fail_if_repin_required: Whether to fail the build if the required maven artifacts have been changed but not repinned. Requires the `maven_install_json` to have been set.
use_starlark_android_rules: Whether to use the native or Starlark version
of the Android rules. Default is False if the running version of Bazel supports native aar_import.
If the running version of Bazel does not support native aar_import, this parameter is ignored and the
Starlark Android rules is used.
of the Android rules. Default is False.
aar_import_bzl_label: The label (as a string) to use to import aar_import
from. This is usually needed only if the top-level workspace file does
not use the typical default repository name to import the Android
Starlark rules. Default is "@rules_android//rules:rules.bzl".
Starlark rules. Default is
"@build_bazel_rules_android//rules:rules.bzl".
duplicate_version_warning: What to do if an artifact is specified multiple times. If "error" then
fail the build, if "warn" then print a message and continue, if "none" then do nothing. The default
is "warn".
Expand Down Expand Up @@ -107,11 +106,6 @@ def maven_install(
if additional_netrc_lines and maven_install_json == None:
fail("`additional_netrc_lines` is only supported with `maven_install_json` specified", "additional_netrc_lines")

if not hasattr(native, "aar_import"):
# If this version of bazel does not have the native version of
# aar_import, then the Starlark version of aar_import must be used.
use_starlark_android_rules = True

# The first coursier_fetch generates the @unpinned_maven
# repository, which executes Coursier.
#
Expand Down
10 changes: 0 additions & 10 deletions tests/integration/override_targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ aar_import(
],
visibility = ["@regression_testing_coursier//:__subpackages__"],
deps = [
# This is a hack to get past ImportDepsChecker because
# apparently the Load class is nowhere to be found in any
# sceneform AARs or deps.
":fake_loader",
# Add the missing dependencies
"@regression_testing_coursier//:com_android_support_support_annotations",
"@regression_testing_coursier//:com_google_ar_core",
"@regression_testing_coursier//:com_google_ar_sceneform_sceneform_base",
"@regression_testing_coursier//:com_google_ar_sceneform_filament_android",
Expand Down Expand Up @@ -73,8 +68,3 @@ sh_test(
"@bazel_tools//tools/bash/runfiles",
],
)

java_library(
name = "fake_loader",
srcs = ["Loader.java"],
)
14 changes: 0 additions & 14 deletions tests/integration/override_targets/Loader.java

This file was deleted.

11 changes: 0 additions & 11 deletions tests/unit/aar_import/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,12 @@
# limitations under the License.

load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@rules_android//android:rules.bzl", "aar_import")
load(":aar_import_test.bzl", "aar_import_test_suite")

aar_import(
name = "aar_import_that_consumes_the_downloaded_file_directly",
# Will produce an error if the downloaded file does not have the `.aar` file extension
aar = "@com_android_support_appcompat_v7_aar_28_0_0//file:file",
deps = [
"@starlark_aar_import_test//:com_android_support_animated_vector_drawable",
"@starlark_aar_import_test//:com_android_support_collections",
"@starlark_aar_import_test//:com_android_support_cursoradapter",
"@starlark_aar_import_test//:com_android_support_support_annotations",
"@starlark_aar_import_test//:com_android_support_support_compat",
"@starlark_aar_import_test//:com_android_support_support_core_utils",
"@starlark_aar_import_test//:com_android_support_support_fragment",
"@starlark_aar_import_test//:com_android_support_support_vector_drawable",
],
)

build_test(
Expand Down

0 comments on commit 7b4512b

Please sign in to comment.