Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update rules_jvm_external to use the Starlark version of aar_import after the native version was removed from Bazel #1149

Merged
merged 15 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ build --tool_java_runtime_version=remotejdk_11
build --experimental_strict_java_deps=strict
build --explicit_java_test_deps

build --experimental_sibling_repository_layout
# 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

# 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.1.0
7.2.1
6 changes: 6 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ bazel_dep(
name = "rules_android",
version = "0.1.1",
)
# Remove this once rules_android has an updated BCR release.
jin marked this conversation as resolved.
Show resolved Hide resolved
git_override(
module_name = "rules_android",
remote = "https://github.com/bazelbuild/rules_android",
commit = "f4b6edb93e13ba0bd17ecf7f99c80f4d3d6ca767",
)
bazel_dep(
name = "stardoc",
version = "0.7.0",
Expand Down
23 changes: 15 additions & 8 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,10 @@ maven_install(

maven_install(
name = "starlark_aar_import_with_sources_test",
# Not actually necessary since this is the default value, but useful for
# testing.
aar_import_bzl_label = "@build_bazel_rules_android//android:rules.bzl",
# 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",
artifacts = [
"androidx.work:work-runtime:2.6.0",
],
Expand All @@ -535,9 +536,10 @@ maven_install(

maven_install(
name = "starlark_aar_import_test",
# Not actually necessary since this is the default value, but useful for
# testing.
aar_import_bzl_label = "@build_bazel_rules_android//android:rules.bzl",
# 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",
artifacts = [
"com.android.support:appcompat-v7:28.0.0",
],
Expand All @@ -550,9 +552,14 @@ maven_install(
)

# for the above "starlark_aar_import_test" maven_install with
# use_starlark_android_rules = True
# 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.
http_archive(
name = "build_bazel_rules_android",
name = "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: 4 additions & 2 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 = "@build_bazel_rules_android//android:rules.bzl"
DEFAULT_AAR_IMPORT_LABEL = "@rules_android//rules:rules.bzl"

_AAR_IMPORT_STATEMENT = """\
load("%s", "aar_import")
Expand Down Expand Up @@ -242,7 +242,9 @@ 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):
if repository_ctx.attr.use_starlark_android_rules:
# Use the Starlark version of aar_import is requested, or if this version of Bazel
# does not have native aar_import.
if repository_ctx.attr.use_starlark_android_rules or getattr(native, "aar_import", None) == None:
jin marked this conversation as resolved.
Show resolved Hide resolved
# 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: 9 additions & 3 deletions private/rules/maven_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,13 @@ 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.
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.
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
"@build_bazel_rules_android//rules:rules.bzl".
Starlark rules. Default is "@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 @@ -106,6 +107,11 @@ 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 getattr(native, "aar_import", None) == None:
# If this version of bazel does not have the native version of
jin marked this conversation as resolved.
Show resolved Hide resolved
# 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: 10 additions & 0 deletions tests/integration/override_targets/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ 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 @@ -68,3 +73,8 @@ sh_test(
"@bazel_tools//tools/bash/runfiles",
],
)

java_library(
name = "fake_loader",
srcs = ["Loader.java"],
)
14 changes: 14 additions & 0 deletions tests/integration/override_targets/Loader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.google.ar.sceneform.assets;

/**
* Some class in com.google.ar.sceneform:rendering:aar:1.10.0 loads
* this class, but there is no class Loader in
* com.google.ar.sceneform:assets:1.10.0 or any other aar in sceneform.
* This is only to satisfy the ImportDepsChecker check since none of
jin marked this conversation as resolved.
Show resolved Hide resolved
* this code is actually run.
*/
public class Loader {
public static boolean loadUnifiedJni() {
return false;
}
}
11 changes: 11 additions & 0 deletions tests/unit/aar_import/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,23 @@
# 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