search

bazelbuild / rules_jvm_external

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

Use a more recent rules_android #1185

Open ted-xie opened 5 days ago

ted-xie commented 5 days ago

Necessary to avoid loading the @bazel_tools//tools/android package, which relies on bind()s to work, which rules_android HEAD has deleted.

shs96c commented 4 days ago

@ted-xie I see this isn't ready for review yet, but it'd be nice to do the update :) Thank you for picking this up!

ted-xie commented 4 days ago

@shs96c My pleasure. There's some kind of issue with bzlmod registries not understanding the fake 0.2.0 version I added in this PR (work on my machine and in other projects...) so that will require a little more investigation.

shs96c commented 2 days ago

We should probably just wait until rules_android ship the 0.2.0 release. Since we're used as a library rather than a root project, we're not going to be able to apply the git-override in most cases.

ted-xie commented 1 day ago

Ah, here's part of the complication: this PR is actually a minor blocker for the next rules_android release, since it fixes broken toolchain issues with bazel query 'deps(...)' originating from rules_jvm_external. For a 0.2 (or 0.5... we are jumping a few minor generations) release, this is probably OK, but generally undesirable.

Since we're used as a library rather than a root project, we're not going to be able to apply the git-override in most cases.

I was not aware of this bzlmod behavior. So any git_override() in a library project essentially gets ignored by the root project?

shs96c commented 1 day ago

I was not aware of this bzlmod behavior. So any git_override() in a library project essentially gets ignored by the root project?

From the docs:

This directive only takes effect in the root module.

Would it be possible to override rules_jvm_external in rules_android for the purposes of the test you're working on? If the issue is to do with toolchains, than you're likely in need of rules_jvm_external 6.0 or 6.1, or limit your test to using Bazel 6 only.

ted-xie commented 1 day ago

If the issue is to do with toolchains, than you're likely in need of rules_jvm_external 6.0 or 6.1, or limit your test to using Bazel 6 only.

We're striving for compatibility Bazel 7.2.1 for the next rules_android release. We're already on 6.1 (link).

Would it be possible to override rules_jvm_external in rules_android for the purposes of the test you're working on?

To clarify, I'm trying to fix a fundamental toolchain issue that will be exposed in rules_android @ HEAD; IOW, a bunch of BazelCI tests will fail without this fix eventually. We can do a single_version_override for rules_jvm_external, but according to the docs, that also only takes effect in the root module.

To clarify even further: at rules_android HEAD, I've landed a bunch of commits removing (transitive) references to legacy bind()s in native Bazel, some of which were related to the Android SDK toolchain. One such transitive reference in rules_jvm_external is here. When evaluated, this single line will cause Bazel to throw an error like this:

$ bazel query 'deps(...)'
[...]/external/bazel_tools/tools/android/BUILD:9:6: no such target '//external:android/sdk': target 'android/sdk' not declared in package 'external' defined by [...]/WORKSPACE (did you mean androidndk?) and referenced by '@@bazel_tools//tools/android:sdk'

One possible solution to this is that on my side, we can release rules_android v0.5 first with a suggestion to users to apply a single_version_override patch with the @bazel_tools//tools/android fix here, then land this PR, then later release rules_android v1.0 with all fixes baked in.