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 jetpack-compose example #455

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ted-xie
Copy link

@ted-xie ted-xie commented May 16, 2024

  • Use a more recent rules_kotlin version (1.9.5)
  • Use a more recent compose compiler version (1.9.22)
  • Add android_ndk_repository dependency

Fixes bazelbuild/bazel#20970

* Use a more recent rules_kotlin version (1.9.5)
* Use a more recent compose compiler version (1.9.22)
* Add android_ndk_repository dependency

Fixes bazelbuild/bazel#20970
@ted-xie ted-xie requested review from comius and ahumesky May 16, 2024 22:23
@ted-xie ted-xie requested a review from jin as a code owner May 16, 2024 22:23
@ted-xie
Copy link
Author

ted-xie commented May 17, 2024

All of the failures in this PR were caused by missing native aar_import in the latest version of Bazel.

@alexeagle
Copy link
Collaborator

those failures are blocking other PRs like #453 - @meteorcloudy do you know if anyone is buildcop for this repo?

@meteorcloudy
Copy link
Member

Unfortunately, there is no buildcop for this repo, we basically try to identify the owner for specific example code.

However, @sgowroji is helping reporting breakages: #454

This looks like to be caused by some changes at Bazel HEAD. Like bazelbuild/bazel@947c72d, @ahumesky can you help fix the example in this repo?

@ted-xie
Copy link
Author

ted-xie commented May 21, 2024

The root cause of the problem is that we deleted the native aar_import() rule in Bazel, but rules_jvm_external instantiates a native aar_import rule in all previous versions. In the latest rules_jvm_external release, the problem is fixed after adding 2 new attributes to the maven.install() call that basically tell rules_jvm_external to use the rules_android starlark version of aar_import.

To completely fix, we'll have to update the rules_jvm_external version of every project in bazelbuild/examples, and then add those extra attributes. Or, we'll have to update rules_jvm_external to use the Starlark aar_import by default.

@ahumesky
Copy link
Collaborator

We're working on updating the downstream repos

@alexeagle
Copy link
Collaborator

Is there a good reason this repo should use Bazel @Head? As an example for end-users it would be more appropriate to use the latest LTS as the primary CI.
I could imagine you'd still like to see test coverage against the latest Bazel changes, but couldn't you do that with the Bazel+Downstream pipeline rather than introduce instability for maintenance here?

@ahumesky
Copy link
Collaborator

The repo here uses last_green for the firebase example (

bazel: last_green
), we could move that to stable(r) version of bazel, but we also the "bazel@HEAD" suite of tests (https://buildkite.com/bazel/bazel-at-head-plus-downstream/)

is also failing so we'll need to fix this for bazel@HEAD anyway

@ahumesky
Copy link
Collaborator

This was switched to last_green to update platforms: 5e37d3c

I wonder if whatever changes were needed in bazel near head are in 7.x by now

@@ -41,3 +41,15 @@ load("@io_bazel_rules_kotlin//kotlin:core.bzl", "kt_register_toolchains")

kt_register_toolchains()

## NDK
http_archive(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, why do we need the NDK dependency?

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.

5 participants