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

Updating android remote tools broke downstream projects #13409

Closed
c-parsons opened this issue Apr 27, 2021 · 25 comments
Closed

Updating android remote tools broke downstream projects #13409

c-parsons opened this issue Apr 27, 2021 · 25 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: support / not a bug (process)

Comments

@c-parsons
Copy link
Contributor

b6f87f1 broke a number of downstream projects (Bazel Examples, rules_jvm_external, Android Testing).

Example failure:
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2010#2595dff6-44ac-4996-9b38-cdacf2842215

We're issuing a rollback now, but perhaps the correct solution is to update AAPT2 on the buildbots before rolling forward?

@c-parsons c-parsons changed the title Updating android remote tools broke downstream proejcts Updating android remote tools broke downstream projects Apr 27, 2021
@ahumesky
Copy link
Contributor

The new tools use a newer aapt2 flag, so we just need to update the android sdk to build tools, with the latest 30.0.3:

~/android-sdk$ build-tools/30.0.3/aapt2 link 2>&1 | grep -- "--no-proguard-location-reference"
 --no-proguard-location-reference                  Keep proguard rules files from having a reference to the source file

@philwo can you update the build image?

@philwo
Copy link
Member

philwo commented Apr 28, 2021

@ahumesky Will do!

@philwo philwo added P0 This is an emergency and more important than other current work. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: support / not a bug (process) labels Apr 28, 2021
@ahumesky
Copy link
Contributor

thanks!

@sventiffe
Copy link
Contributor

Is this still a P0 and has there been any progress?

@philwo philwo added P1 I'll work on this now. (Assignee required) and removed P0 This is an emergency and more important than other current work. (Assignee required) labels May 7, 2021
@philwo
Copy link
Member

philwo commented May 7, 2021

I reduced it to a P1 as I understand that the problematic changes were rolled back. Nevertheless I plan to build new images and containers this evening so it should be fixed then. 😊

@ahumesky
Copy link
Contributor

hey philwo, have the images and containers been updated?

@philwo
Copy link
Member

philwo commented May 17, 2021

Not yet, I'll get to it tomorrow, promise.

@philwo
Copy link
Member

philwo commented May 20, 2021

Discussed with @ahumesky that the observed issue is not caused by an outdated version of build-tools, but because rules_jvm_external and android-testing pin their build version to 28.0.2:

I'll still upgrade the build-tools and remove the unused 29.x versions from our images, but for now at least we're not responsible for the issues ;)

ahumesky added a commit to ahumesky/testing-samples that referenced this issue May 20, 2021
Remove `build_tools_version` and `api_level` to let `android_sdk_repository` automatically select the latest installed version.

One of the fixes for bazelbuild/bazel#13409 (comment)
@ahumesky
Copy link
Contributor

Thanks Phil! I've created pull requests to remove the hard-coded build_tools_version. Once those are merged I'll try submitting my original change again.

jin pushed a commit to bazel-contrib/rules_jvm_external that referenced this issue May 20, 2021
Remove `build_tools_version` and `api_level` to let `android_sdk_repository` automatically select the latest installed version.

One of the fixes for bazelbuild/bazel#13409 (comment)
bazel-io pushed a commit that referenced this issue Jun 7, 2021
*** Reason for rollback ***

Downstream projects on bazelci have been updated to not hardcode the buildtools versions: #13409 (comment)

*** Original change description ***

Automated rollback of commit b6f87f1.

*** Reason for rollback ***

Breaks downstream bazel projects because AAPT2 needs to be updated in the build images:
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2010#2595dff6-44ac-4996-9b38-cdacf2842215

*** Original change description ***

Updates Android remote tools to 0.23.0 to include Update remote android tools to include c3edf13.

RELNOTES: None.
PiperOrigin-RevId: 377971964
bazel-io pushed a commit that referenced this issue Jun 7, 2021
Bazel's Android tools requires Android Build Tools 30.0.0 or newer.

See #13409

RELNOTES: The minimum Android build tools version for the Android rules is now 30.0.0
PiperOrigin-RevId: 378014192
@meteorcloudy
Copy link
Member

@ahumesky Looks like the roll forward of 80feea0 did some breakage again:

https://buildkite.com/bazel/bazel-auto-sheriff-face-with-cowboy-hat/builds/546

  • Android Testing
  • Bazel Examples
  • rules_jvm_external

@meteorcloudy
Copy link
Member

meteorcloudy commented Jun 8, 2021

The VMs currently provide these versions:

$ ls -l /opt/android-sdk-linux/build-tools/
total 16
drwxr-xr-x 5 root root 4096 Sep 16  2020 28.0.2
drwxr-xr-x 5 root root 4096 Sep 16  2020 29.0.2
drwxr-xr-x 5 root root 4096 Sep 16  2020 29.0.3
drwxr-xr-x 6 root root 4096 Sep 16  2020 30.0.1

rules_jvm_external should already been fixed to not hard code android build tools version, but from the error message:

(02:06:17) ERROR: /var/lib/buildkite-agent/builds/bk-docker-sz3g/bazel-downstream-projects/rules_jvm_external/examples/simple/WORKSPACE:1:23: fetching android_sdk_repository rule //external:androidsdk: Bazel requires Android build tools version 30.0.0 or newer, 28.0.2 was provided
(02:06:17) ERROR: Analysis of target '//:my_app' failed; build aborted: Bazel requires Android build tools version 30.0.0 or newer, 28.0.2 was provided

It seems Bazel picked the oldest one by default?

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2060#afcdcf62-b737-4792-8011-dea27885cc1b

@ahumesky
Copy link
Contributor

ahumesky commented Jun 11, 2021

So the error itself is coming from 0e65273 where I updated the minimum required sdk tools. Bazel should be picking the latest version, as far as I know nothing has changed in that code in quite a while. Let me see if I can repro this problem locally.

@ahumesky
Copy link
Contributor

Ah, well for rules_jvm_external, there are several other workspace files in the examples directory that also hardcode the build tools version, e.g.
https://github.com/bazelbuild/rules_jvm_external/blob/master/examples/android_instrumentation_test/WORKSPACE#L6
https://github.com/bazelbuild/rules_jvm_external/blob/master/examples/simple/WORKSPACE#L4

etc

@ahumesky
Copy link
Contributor

And it seems the reason that android testing failed is that CI ran on the commit just 1 before the fix:
ran at: ac535c1a37119470014bd93b09b47d6188b3caa3
fixed at: 482321c0adc00b2c0238f7a12ec39418f4d462af
https://github.com/android/testing-samples/commits/main

So that will presumably get fixed on its own when CI runs on a newer version of the android testing repo

@meteorcloudy
Copy link
Member

@ahumesky I see, do you know why Android Testing is failing with Bazel 4.1.0?
It happens since bazel-contrib/rules_jvm_external#562 is submitted, filed android/testing-samples#387

@fweikert
Copy link
Member

Friendly ping - I still see multiple "Bazel requires Android build tools version 30.0.0 or newer, 28.0.2 was provided" failures, e.g. https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2070#81fa8b3c-a2e5-4532-ad61-d28fa5729446

@ahumesky
Copy link
Contributor

Looks like I missed one instance of the build tools version being hardcoded in the bazel examples repo:
bazelbuild/examples#211

For the Android Testing repo, for some reason it's still getting a commit 1 before the fix, same as in the comment above (#13409 (comment)). How does the test infrastructure select the commit to clone at?

And for rules_jvm_external, there are several examples that hardcode the build tools version that I didn't see. I'll remove those.

By the way, I noticed a failure in Tensorflow resulting from aefd107

I created
tensorflow/tensorflow#50310

@meteorcloudy
Copy link
Member

How does the test infrastructure select the commit to clone at?

It select the last green commit in the pipeline with Bazel@latest release, because if it's already red, it doesn't make sense to test against that commit in downstream.

Thanks for all the fixes!

comius pushed a commit to bazelbuild/examples that referenced this issue Jun 24, 2021
…to let android_sdk_repository automatically select the latest installed version. See bazelbuild/bazel#13409 (#210)
@comius
Copy link
Contributor

comius commented Jun 24, 2021

There are two open PRs failing on RBE, zipalign issue, https://buildkite.com/bazel/rules-jvm-external-examples/builds/1884#321c38ff-91c5-43fc-b54f-1a02316d3c91

Is this a infrastructure or caching problem? cc @philwo, @meteorcloudy

@meteorcloudy
Copy link
Member

meteorcloudy commented Jun 24, 2021

This looks like the same issue in android/testing-samples#387 which happens without RBE (it does), I'll do some debugging.

Related discussion: android/testing-samples#384

@comius
Copy link
Contributor

comius commented Aug 9, 2021

This is reappearing again. "Bazel requires Android build tools version 30.0.0 or newer, 28.0.2 was provided"

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2122#cb584520-989b-400b-ac07-5407601331ff

@meteorcloudy Can you check what's going on? If the SDK is really installed on the CI, it should error out.

@coeuvre
Copy link
Member

coeuvre commented Sep 23, 2021

The VMs now provide these versions:

$ ls -l /opt/android-sdk-linux/build-tools/
total 8
drwxr-xr-x 5 root root 4096 Aug  8 11:09 28.0.2
drwxr-xr-x 6 root root 4096 Aug  8 11:09 30.0.3

Any ideas why the error "Bazel requires Android build tools version 30.0.0 or newer, 28.0.2 was provided" reappears?

@coeuvre
Copy link
Member

coeuvre commented Sep 24, 2021

Fix for rules_jvm_external bazel-contrib/rules_jvm_external@404daf4 is landed and build is green now.

@meteorcloudy
Copy link
Member

Thanks!!

@meteorcloudy
Copy link
Member

It's green already on CI!

mukundsrinivasb pushed a commit to mukundsrinivasb/espresso-testing-tuorial that referenced this issue Mar 31, 2023
Remove `build_tools_version` and `api_level` to let `android_sdk_repository` automatically select the latest installed version.

One of the fixes for bazelbuild/bazel#13409 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: support / not a bug (process)
Projects
None yet
Development

No branches or pull requests

8 participants