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

incompatible_enable_android_toolchain_resolution #16285

Closed
katre opened this issue Sep 15, 2022 · 10 comments
Closed

incompatible_enable_android_toolchain_resolution #16285

katre opened this issue Sep 15, 2022 · 10 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: process

Comments

@katre
Copy link
Member

katre commented Sep 15, 2022

We are migrating Android builds to use platforms and toolchain resolution to find the SDK and NDK. The work is progressing well but we need to execute more testing before we provide migration directions and flip this flag.

@katre katre added P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions incompatible-change Incompatible/breaking change labels Sep 15, 2022
@katre katre self-assigned this Sep 15, 2022
ahumesky pushed a commit to bazelbuild/rules_android_ndk that referenced this issue Jan 5, 2023
)

This is work towards enabling platforms and toolchains for Android
builds (bazelbuild/bazel#16285).
copybara-service bot pushed a commit to bazelbuild/rules_android that referenced this issue Jan 6, 2023
Users can use these by adding `--android_platforms=@rules_android//:armeabi-v7a` to their builds (replacing `@rules_android` with the name they use to import https://github.com/bazelbuild/rules_android).

This is work towards enabling platforms and toolchains for Android builds (bazelbuild/bazel#16285).

PiperOrigin-RevId: 499992246
Change-Id: If03f275cb8372cd18a7cd21a94f693ebcc3dd569
mauriciogg pushed a commit to mauriciogg/rules_android that referenced this issue Jan 17, 2023
Users can use these by adding `--android_platforms=@rules_android//:armeabi-v7a` to their builds (replacing `@rules_android` with the name they use to import https://github.com/bazelbuild/rules_android).

This is work towards enabling platforms and toolchains for Android builds (bazelbuild/bazel#16285).

PiperOrigin-RevId: 499992246
Change-Id: If03f275cb8372cd18a7cd21a94f693ebcc3dd569
mauriciogg pushed a commit to mauriciogg/rules_android that referenced this issue Jan 17, 2023
Users can use these by adding `--android_platforms=@rules_android//:armeabi-v7a` to their builds (replacing `@rules_android` with the name they use to import https://github.com/bazelbuild/rules_android).

This is work towards enabling platforms and toolchains for Android builds (bazelbuild/bazel#16285).

PiperOrigin-RevId: 499992246
Change-Id: If03f275cb8372cd18a7cd21a94f693ebcc3dd569
katre added a commit to katre/bazel that referenced this issue Oct 4, 2023
Enable Android Platforms tests for android_local_test.

Part of bazelbuild#16285.
katre added a commit to katre/bazel that referenced this issue Oct 4, 2023
Enable Android Platforms tests for android_local_test.

Part of bazelbuild#16285.
katre added a commit to katre/bazel that referenced this issue Oct 6, 2023
In some cases, this means adding missing dependencies.

In many others, it means test cleanups.

Part of bazelbuild#16285.

PiperOrigin-RevId: 570765426
Change-Id: I36763a45699a37e54b3823be8f70f59d54269ca6
katre added a commit to katre/bazel that referenced this issue Oct 6, 2023
katre added a commit to katre/bazel that referenced this issue Oct 10, 2023
Part of bazelbuild#16285.

PiperOrigin-RevId: 571427717
Change-Id: I47075f87e6e13615391bb056ee769d024e6b089e
copybara-service bot pushed a commit that referenced this issue Oct 10, 2023
Part of #16285.

PiperOrigin-RevId: 572256569
Change-Id: I47075f87e6e13615391bb056ee769d024e6b089e
katre added a commit to katre/bazel that referenced this issue Oct 10, 2023
In some cases, this means adding missing dependencies.

In many others, it means test cleanups.

Part of bazelbuild#16285.

PiperOrigin-RevId: 570765426
Change-Id: I36763a45699a37e54b3823be8f70f59d54269ca6
@katre katre added this to the 7.0.0 branch cut milestone Oct 10, 2023
@katre
Copy link
Member Author

katre commented Oct 10, 2023

Ran a downstream test of #19726:

  • Hard bazel crashes
    • Looks like an NPE in AndroidSdkProvider.getAndroidJar
    • Fix ASAP
    • Bazel
    • rules_jvm_external - examples (Android Robolectric test example)
    • rules_kotlin (Example - Anvil)
    • rules_kotlin (Example - Jetpack Compose)
    • rules_kotlin (Example Bzlmod - Android)
  • Project doesn't set --android_platforms but probably should
    • Bazel Examples (android (Android Firebase Cloud Messaging)
    • Bazel Examples (android (Android NDK)
    • Bazel Examples (tutorials (Bazel end-to-end example)
    • rules_android
    • rules_android_ndk
    • rules_foreign_cc (Examples (spawn_strategy=standalone))
    • rules_jvm_external - examples (Android instrumentation test example)
    • rules_jvm_external - examples (Simple example)
    • rules_kotlin (Example - Android)
    • rules_rust (Android Examples)

@katre
Copy link
Member Author

katre commented Oct 10, 2023

To reproduce the crash (in a clean Bazel checkout with with Bazel built from HEAD):

$ bazel-dev build --nobuild --incompatible_enable_android_toolchain_resolution -- //src/test/java/com/google/devtools/build/android/r8:desugar_testdata_lambda
Building dev version of bazel...
WARNING: Download from https://mirror.bazel.build/github.com/madler/zlib/releases/download/v1.3/zlib-1.3.tar.gz failed: class java.io.FileNotFoundException GET returned 404 Not Found
INFO: Analyzed target //src:bazel-dev (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //src:bazel-dev up-to-date:
  bazel-bin/src/bazel-dev
INFO: Elapsed time: 0.221s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
Executing bazel-dev...
WARNING: Build option --incompatible_enable_android_toolchain_resolution has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
Analyzing: target //src/test/java/com/google/devtools/build/android/r8:desugar_testdata_lambda (0 pack\
ages loaded, 0 targets configured)
FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=@bazel_tools//tools/android:android_jar, config=BuildConfigurationKey[0bf75dd232dadeb1aae161cb11ea8439ac21f80b21714964ff16886adcd0633d]}' (requested by nodes 'ConfiguredTargetKey{label=//src/test/java/com/google/devtools/build/android/r8:desugar_testdata_lambda, config=BuildConfigurationKey[0bf75dd232dadeb1aae161cb11ea8439ac21f80b21714964ff16886adcd0633d]}')
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:550)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:414)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1395)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: java.lang.NullPointerException: Cannot invoke "com.google.devtools.build.lib.rules.android.AndroidSdkProvider.getAndroidJar()" because the return value of "com.google.devtools.build.lib.rules.android.AndroidSdkProvider.fromRuleContext(com.google.devtools.build.lib.analysis.RuleContext)" is null
	at com.google.devtools.build.lib.rules.android.AndroidToolsDefaultsJar.create(AndroidToolsDefaultsJar.java:48)
	at com.google.devtools.build.lib.bazel.rules.android.BazelAndroidToolsDefaultsJar.create(BazelAndroidToolsDefaultsJar.java:20)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:399)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:194)
	at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:1305)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:385)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:312)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:461)
	... 7 more

copybara-service bot pushed a commit that referenced this issue Oct 11, 2023
… android_library.

Part of #16285.

PiperOrigin-RevId: 572479677
Change-Id: If2a19723d4cdfa469e3e8f547baa11ab989835b4
copybara-service bot pushed a commit that referenced this issue Oct 11, 2023
With changes to Android builds, we either need to set up the correct
flags, or just use a custom rule that we control.

Part of #16285.

PiperOrigin-RevId: 572564475
Change-Id: I81887c0babc6513ecd5b228429f6de0252a9e8ef
copybara-service bot pushed a commit that referenced this issue Oct 11, 2023
In some cases, this means adding missing dependencies.

In many others, it means test cleanups.

Part of #16285.

PiperOrigin-RevId: 572585782
Change-Id: I36763a45699a37e54b3823be8f70f59d54269ca6
copybara-service bot pushed a commit that referenced this issue Oct 11, 2023
Part of #16285.

Fixes #19600.

PiperOrigin-RevId: 572680900
Change-Id: I0ef3b00ac70043dea81bcd39764f0ab2c6be2d2a
katre added a commit to katre/bazel that referenced this issue Oct 11, 2023
…is used.

Previously the warning could be shadowed by later errors.

Part of bazelbuild#16285.

PiperOrigin-RevId: 572645472
Change-Id: Ief6ae24b5c4d77e41b167f5d92878eec7d7a6181
copybara-service bot pushed a commit that referenced this issue Oct 12, 2023
…is used.

Previously the warning could be shadowed by later errors.

Part of #16285.

PiperOrigin-RevId: 572899622
Change-Id: Ief6ae24b5c4d77e41b167f5d92878eec7d7a6181
katre added a commit to katre/rules_android that referenced this issue Oct 13, 2023
katre added a commit to katre/rules_android_ndk that referenced this issue Oct 13, 2023
illicitonion pushed a commit to bazelbuild/rules_rust that referenced this issue Oct 16, 2023
katre added a commit to bazelbuild/examples that referenced this issue Oct 16, 2023
Part of bazelbuild/bazel#16285.

Several of these just forcible turn off Android platforms due to other
issues (including bazelbuild/rules_android#77)
jin pushed a commit to bazelbuild/rules_android that referenced this issue Oct 16, 2023
* Add Android-specific build flags.

Part of bazelbuild/bazel#16285.

* Switch java runtime flag back
ahumesky pushed a commit to bazelbuild/rules_android_ndk that referenced this issue Oct 16, 2023
katre added a commit to katre/rules_android that referenced this issue Oct 16, 2023
Add Android Platforms flags.

Closes bazelbuild#162.

Part of bazelbuild/bazel#16285.

PiperOrigin-RevId: 573790740
Change-Id: I4c3389448a2e7158ffecfa9f2ca72799a57ff047
@katre
Copy link
Member Author

katre commented Oct 16, 2023

I started the internal merge for #19726 and ran into test failures that I thought were already resolved. I'm moving this from the "branch cut" milestone to the "release blocker" milestone so that it can be cherry-picked when ready.

@katre
Copy link
Member Author

katre commented Oct 16, 2023

(Copied from #19726)

Downstream projects are mostly clean at this point. A typical error looks like

'android_library' rule '@bazel_tools//src/tools/android/java/com/google/devtools/build/android/incrementaldeployment:incremental_stub_application' requested an android sdk via toolchain resolution but hasn't set an appropriate --android_platforms value: Either set --noincompatible_enable_android_toolchain_resolution or --android_platforms.

If any more downstream projects fail, assign an issue to me to triage.

@iancha1992 iancha1992 removed this from the 7.0.0 release blockers milestone Oct 18, 2023
@iancha1992
Copy link
Member

@bazel-io fork 7.0.0

@cpsauer
Copy link
Contributor

cpsauer commented Nov 15, 2023

Would it make sense to set a usable default value for --android_platforms to avoid having everyone configure manually?
(e.g. all 4 architectures Google would like people to build for)

(Alternatively, --fat_apk_cpu was fairly ergonomic because people didn't have to define out all the platforms manually. (I think there still aren't common platform definitions, right?) I wonder if that convenience might be worth preserving in some form?)

I think we'll probably also need to update the rules_android_ndk readme (depending on the default)

@katre
Copy link
Member Author

katre commented Nov 15, 2023

Unfortunately, there's no useful default to set it to: we very deliberately don't want to define any actual platforms in the @bazel_tools repo, and there's nowhere else to define them that's guaranteed to be present for all users.

@cpsauer
Copy link
Contributor

cpsauer commented Nov 15, 2023

What about in @platforms, parallel to your bazelbuild/platforms#36?

@cpsauer
Copy link
Contributor

cpsauer commented Nov 15, 2023

(Just, ofc, seems like a bummer from a product standpoint to require users to discover boilerplate config on error rather than have a usable (but configurable) default.)

@cpsauer
Copy link
Contributor

cpsauer commented Nov 15, 2023

More importantly, thanks so much for all your work getting these flipped. It's awesome to have platform selection working smoothly without platform_mappings!

jylinv0 pushed a commit to jylinv0/rules_android that referenced this issue Jan 23, 2024
Users can use these by adding `--android_platforms=@rules_android//:armeabi-v7a` to their builds (replacing `@rules_android` with the name they use to import https://github.com/bazelbuild/rules_android).

This is work towards enabling platforms and toolchains for Android builds (bazelbuild/bazel#16285).

PiperOrigin-RevId: 499992246
Change-Id: If03f275cb8372cd18a7cd21a94f693ebcc3dd569
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: process
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants