search

bazelbuild / rules_jvm_external

Bazel rules to resolve, fetch and export Maven artifacts
Apache License 2.0
316 stars 236 forks source link

Update rules_jvm_external to use the Starlark version of aar_import #1149

Open ahumesky opened 1 month ago

ahumesky commented 1 month ago

  1. Update rules_jvm_external to use the Starlark version of aar_import automatically if the version of Bazel used does not contain the native version of aar_import.

  2. This also updates the version of rules_android to the latest commit, because version 0.1.1 is basically just wrappers around the native versions of the rules.

  3. This also updates the default label to import aar_import from, from @build_bazel_rules_android//android:rules.bzl to @rules_android//rules:rules.bzl, both to match the bzlmod style name of the repo ("rules_android") and to use the latest label inside the repo (//android:rules.bzl exists, but only for backwards compatibility).

  4. This also disbles --experimental_sibling_repository_layout added in https://github.com/bazelbuild/rules_jvm_external/commit/b6631f90a46c23be32000bf14b8e817f44017cc4 because the latest rules_android uses protos in go, and there is a problem compiling protos in go with --experimental_sibling_repository_layout: https://github.com/bazelbuild/rules_go/issues/3947

Fixes #1139.

ahumesky commented 1 month ago

Oh, I just noticed the tests run on bazel as far back as bazel 5.4.0: https://github.com/bazelbuild/rules_jvm_external/blob/71e08c82709b24d12e2d1040d86caa05bf6a8399/.bazelci/presubmit.yml#L40

The starlark Android rules will only work with 7.2.0 (releasing soon). Might need to segment the tests so that the tests with the starlark version of aar_import are only run with 7.2.0

ahumesky commented 1 month ago

The windows tests will also be a problem because windows support for starlark rules_android is low priority

shs96c commented 1 month ago

We still need to support Windows users, so those tests will need to pass properly before we can land this PR.

ahumesky commented 1 month ago

https://github.com/bazelbuild/rules_android/pull/235 should fix up the windows problems

jin commented 1 month ago

Error looks legit:


(19:43:56) ERROR: C:/b/rxn2cr7v/external/rules_android~/tools/android/BUILD:273:6: in alias rule @@rules_android~//tools/android:dexsharder: target '@@bazel_tools//src/tools/android/java/com/google/devtools/build/android/dexer:DexFileSplitter' is not visible from target '@@rules_android~//tools/android:dexsharder'. Check the visibility declaration of the former target if you think the dependency is legitimate
--
  | (19:43:56) ERROR: C:/b/rxn2cr7v/external/rules_android~/tools/android/BUILD:273:6: Analysis of target '@@rules_android~//tools/android:dexsharder' failed
  | (19:43:57) ERROR: Analysis of target '//tests/unit/aar_import:starlark_aar_import_test' failed; build aborted: Analysis failed

from https://buildkite.com/bazel/rules-jvm-external/builds/4289#018fc5e0-bc05-4701-80b8-55fdf1f6ddb0 (and others)

ahumesky commented 1 month ago

That's fixed in 7.2.0, specifically https://github.com/bazelbuild/bazel/pull/22416, but there are other problems revealed after https://github.com/bazelbuild/rules_android/pull/235

I'll make this a draft until CI is passing