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

Use a more recent rules_android #1185

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ted-xie
Copy link
Contributor

@ted-xie ted-xie commented Jun 27, 2024

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

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

shs96c commented Jun 28, 2024

@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
Copy link
Contributor Author

ted-xie commented Jun 28, 2024

@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
Copy link
Collaborator

shs96c commented Jul 1, 2024

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
Copy link
Contributor Author

ted-xie commented Jul 1, 2024

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
Copy link
Collaborator

shs96c commented Jul 1, 2024

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
Copy link
Contributor Author

ted-xie commented Jul 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants