search

bazel-contrib / rules_jvm_external

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

Revert "Update rules_jvm_external to use the Starlark version of aar_import after the native version was removed from Bazel (#1149)" #1215

Closed shs96c closed 3 months ago

shs96c commented 3 months ago

This reverts commit ba7310ce4a1d8fb14434597fbc7440a4074f7695.

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.

jin commented 3 months ago

cc @ahumesky

jin commented 3 months ago

This looks like https://github.com/bazelbuild/rules_android/issues/219, and we probably cannot roll forward on this until we drop the dependency on that flag.

shs96c commented 3 months ago

To show this in action:

git clone https://github.com/bazelbuild/rules_jvm_external.git
git clone https://github.com/bazel-contrib/rules_jvm.git
cd rules_jvm
bazel build //... --override_module=$(pwd)/../rules_jvm_external

With the rules_android update in place, this will fail. Without it, the build will succeed.

ahumesky commented 3 months ago

Sorry for the trouble, @rules_android//rules/reexport_providers.bzl is actually a part of removing the need for --experimental_google_legacy_api and was added after starting on https://github.com/bazelbuild/rules_jvm_external/commit/ba7310ce4a1d8fb14434597fbc7440a4074f7695, didn't realize this would require projects not using the android rules to add the flag.

shs96c commented 3 months ago

It happens, and I'm surprised it managed to slip through all the tests and checks. We know now, and we can fix things. Genuinely looking forwards to the time we can roll this forward again. Thank you for all your work making that happen!

ahumesky commented 2 months ago

We've made significant progress on https://github.com/bazelbuild/rules_android/issues/256 / https://github.com/bazelbuild/rules_android/issues/219 Maybe this week or next we can try again with an updated version of https://github.com/bazelbuild/rules_jvm_external/commit/ba7310ce4a1d8fb14434597fbc7440a4074f7695 + updated version of rules_android

shs96c commented 2 months ago

I'd be happy to try it. Would be great to land this properly :)

ahumesky commented 1 month ago

With bazel 7.4.0rc1 created today, I tested reapplying ba7310ce4a1d8fb14434597fbc7440a4074f7695 and using the latest rules_android, and this all seems to work now without either --experimental_google_legacy_api nor --experimental_enable_android_migration_apis:

~/rules_jvm$ USE_BAZEL_VERSION=7.4.0rc1 bazelisk build //... --override_module=rules_jvm_external=$(pwd)/../rules_jvm_external --override_module=rules_android=$(pwd)/../rules_android

......

INFO: Found 256 targets...
INFO: Elapsed time: 45.499s, Critical Path: 41.73s
INFO: 2318 processes: 1013 internal, 1237 linux-sandbox, 68 worker.
INFO: Build completed successfully, 2318 total actions
ahumesky commented 3 weeks ago

We've been working to create rules_android version 0.6.0 this week and last, when that's out I'll resume getting all this committed again