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.

Sign up for GitHub

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

Jump to bottom

Failing to run d8_dexbuilder with buildtools 31 #13989

Closed
Kernald opened this issue Sep 14, 2021 · 14 comments
Closed

Failing to run d8_dexbuilder with buildtools 31 #13989

Kernald opened this issue Sep 14, 2021 · 14 comments
Assignees
@ted-xie
Labels
P1 I'll work on this now. (Assignee required) team-Android Issues for Android team type: bug

Comments

@Kernald
Copy link
Contributor

Kernald commented Sep 14, 2021

Description of the problem / feature request:

Using buildtools 31, Bazel can't build dex files anymore. With d8 disabled (default):

ERROR: /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/external/androidsdk/BUILD.bazel:13:25: Extracting interface @androidsdk//:dx_jar_import failed: missing input file 'external/androidsdk/build-tools/31.0.0/lib/dx.jar', owner: '@androidsdk//:build-tools/31.0.0/lib/dx.jar'
ERROR: /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/external/androidsdk/BUILD.bazel:13:25 Extracting interface @androidsdk//:dx_jar_import failed: 1 input file(s) do not exist

With d8 enabled with the following flags:

# Enable d8 merger
build --define=android_dexmerger_tool=d8_dexmerger

# Flags for the d8 dexer
build --define=android_incremental_dexing_tool=d8_dexbuilder
build --define=android_standalone_dexing_tool=d8_compat_dx
build --nouse_workers_with_dexbuilder

ERROR: /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/external/maven/BUILD:4748:11: Dexing external/maven/_dx/androidx_transition_transition/classes_and_libs_merged.jar_desugared.jar with applicable dexopts [] failed: (Exit 1): d8_dexbuilder failed: error executing command bazel-out/darwin-py2-opt-exec-2B5CBBC6/bin/external/bazel_tools/tools/android/d8_dexbuilder ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
Error: Could not find or load main class com.android.tools.r8.compatdexbuilder.CompatDexBuilder
Caused by: java.lang.ClassNotFoundException: com.android.tools.r8.compatdexbuilder.CompatDexBuilder

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

In a WORKSPACE:

android_sdk_repository(
    name = "androidsdk",
    api_level = 31,
    build_tools_version = "31.0.0",
)

(I don't think the api_level actually matters here.)

Try to build anything that requires dexing.

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 4.2.1
@philwo philwo added team-Android Issues for Android team untriaged labels Sep 21, 2021
@c16a
Copy link

c16a commented Oct 9, 2021 •
edited
Loading

I'm having the same issue. Any clues what might have gone wrong? I can confirm this works with the latest hotfix of buildtools 30 as well (30.0.3)

@ahumesky
Copy link
Contributor

ahumesky commented Nov 10, 2021 •
edited
Loading

With build tools 31, dx has been removed, and the latest version of d8 moved some classes around. I think his should fix it:
ceb498f
Can you try again with d8 and the latest bazel release? Looks like this is in 5.0 and 6.0

@alyssawilk
Copy link

Is there any advice of what to do without bumping bazel? We've just started having the same problem recently but haven't changed android_sdk_repository or our bazel version so I'm not sure why something broke?

@nkoroste
Copy link
Contributor

I've managed to cherry-pick ceb498f into 4.2.2 pretty easily and it seemed to solve the problem when build_tools_version = "31.0.0", is used with d8

@alyssawilk
Copy link

Ah if anyone else ends up in our situation (where updating bazel isn't trivial) explicitly hard-coding build_tools_version to a lower version worked for us
-android_sdk_repository(name = "androidsdk", api_level = 30)
+android_sdk_repository(name = "androidsdk", api_level = 30, build_tools_version = "30.0.2")

@nkoroste
Copy link
Contributor

@ahumesky @sgjesse

Good news, ceb498f fixes the issue that allows you to use build_tools_version = "31.0.0",

Bad news, the above change introduces some new issues with d8/r8 where Bazel DexMerger is falling back to Platform SDK's D8. This is causing a crash with too many methods being fit into a singular DEX.

Error: Cannot fit requested classes in a single dex file (# methods: 890463 > 65536 ; # fields: 468383 > 65536). Try supplying a main-dex list

The root cause is that D8 is being invoked with the default min-api value of 1, which forces single DEX.

It appears we should pass through —min-api to avoid this problem.

@Kernald Kernald mentioned this issue Jan 15, 2022
Merged
@Kernald
Copy link
Contributor Author

Kernald commented Feb 14, 2022

@ahumesky @sgjesse

Good news, ceb498f fixes the issue that allows you to use build_tools_version = "31.0.0",

Bad news, the above change introduces some new issues with d8/r8 where Bazel DexMerger is falling back to Platform SDK's D8. This is causing a crash with too many methods being fit into a singular DEX.

Error: Cannot fit requested classes in a single dex file (# methods: 890463 > 65536 ; # fields: 468383 > 65536). Try supplying a main-dex list

The root cause is that D8 is being invoked with the default min-api value of 1, which forces single DEX.

It appears we should pass through —min-api to avoid this problem.

Another work-around is to manually passing a number of dex_shards to every android_binary. It's simple enough to compute, just a bit annoying.

@EdbertChan
Copy link

@Kernald How were you able to even get this to run?

I'm getting Error: Could not find or load main class com.android.tools.r8.compatdx.CompatDx. If you peek inside the packaged d8.jar file that is in build_tools 31.0.0, CompatDx doesn't even exist as a class. I can confirm this exists in 29.0.0 in the d8.jar. but not 31.0.0.

In fact, if you look at the R8 source, CompatDx got removed entirely. Maybe I should file this as an entirely separate issue.

@sgjesse
Copy link
Contributor

sgjesse commented Mar 31, 2022

CompatDx was moved from R8 to bazel com.google.devtools.build.android.r8.CompatDx. It was removed from R8 almost two years ago.

@ahumesky
Copy link
Contributor

@EdbertChan what version of bazel are you running? This should be fixed with ceb498f which is in bazel 5.0.0 or newer

@EdbertChan
Copy link

@ahumesky We are running 5.1.0. We have forked it but we only have a few small diffs ontop of it mostly related to manifest merging. I am using the same configs that Kernald is using.

The error I get from --verbose_failures boils down to this line
bazel-out/k8-opt-exec-2B5CBBC6/bin/external/androidsdk/d8_compat_dx --dex '--num-threads=5' '--output=bazel-out/k8-fastbuild/bin/apps/presidio/helix/app-apk/_mobile_install/bin_debug/stub_application/classes.dex' bazel-out/k8-fastbuild/bin/apps/presidio/helix/app-apk/_mobile_install/bin_debug/stub_deploy.jar

When I look into the d8_compat_dx, I only see that a manifest file exists in it where it says it was created by "Target-Label: @androidsdk//:d8_compat_dx" and no other classes. Looking at this line in android_sdk_repository.bzl, it looks like Bazel is still trying to use the d8 packaged in build-tools in the Android SDK rather than trying to create its own.

@EdbertChan EdbertChan mentioned this issue Apr 11, 2022
Closed
@ahumesky
Copy link
Contributor

Gotcha yeah, I see now it's CompatDx vs CompatDexBuilder. We have some changes we'll push out later today that should address this

@ahumesky ahumesky added P1 I'll work on this now. (Assignee required) and removed untriaged labels Apr 11, 2022
bazel-io pushed a commit that referenced this issue Apr 19, 2022
@ted-xie @copybara-github
Automated rollback of commit b93f828.
a603c23 
*** Reason for rollback ***

Breaks a subset of //src/test/java/com/google/devtools/build/android/r8:AllTests.

See b/229725674

*** Original change description ***

Switch to using the d8 jar from maven.google.com instead of the jar from the
Android SDK.

Also fixes #13989

RELNOTES: Bazel uses the D8 jar from Maven instead of the SDK.
PiperOrigin-RevId: 442875035
bazel-io pushed a commit that referenced this issue Apr 22, 2022
@ted-xie @copybara-github
Re-attempt to use the D8 jar from maven.google.com. Update to the lat…
ae24714 
…est version of D8, 3.3.28.

*** Original change description ***

Switch to using the d8 jar from maven.google.com instead of the jar from the
Android SDK.

Also fixes #13989

RELNOTES: Bazel uses the D8 jar from Maven instead of the SDK.
PiperOrigin-RevId: 443517033
fweikert pushed a commit that referenced this issue Apr 26, 2022
@ted-xie @fweikert
Automated rollback of commit b93f828.
93be882 
*** Reason for rollback ***

Breaks a subset of //src/test/java/com/google/devtools/build/android/r8:AllTests.

See b/229725674

*** Original change description ***

Switch to using the d8 jar from maven.google.com instead of the jar from the
Android SDK.

Also fixes #13989

RELNOTES: Bazel uses the D8 jar from Maven instead of the SDK.
PiperOrigin-RevId: 442875035
oliviernotteghem pushed a commit to oliviernotteghem/bazel that referenced this issue Apr 27, 2022
@EdbertChan
Switch to using the d8 jar from maven.google.com instead of the jar f…
75c6a67 
…rom the

Android SDK.

Also fixes bazelbuild#13989

RELNOTES: Bazel uses the D8 jar from Maven instead of the SDK.
PiperOrigin-RevId: 440990736
franksalim added a commit to franksalim/markers that referenced this issue May 4, 2022
@franksalim
Add Bazel build.
2f15471 
I realize this will make upstreaming changes less likely.

At the moment, building requires Bazel 6.0.0-pre.20220421.3
due to bazelbuild/bazel#13989.
@BenHenning BenHenning mentioned this issue Jun 11, 2022
Closed
@dhbaird dhbaird mentioned this issue Jun 29, 2022
Closed
@nkoroste
Copy link
Contributor

What version of Bazel did this fix end up landing in?

@ahumesky
Copy link
Contributor

Right, because of the rollbacks and rollback of rollbacks it's a bit unclear, the final commit is ae24714
and as of right now that's in the recent rolling releases:

6.0.0-pre.20220608.2
6.0.0-pre.20220601.1
6.0.0-pre.20220526.1
6.0.0-pre.20220520.1
6.0.0-pre.20220517.1

@adhiamboperes adhiamboperes mentioned this issue Sep 28, 2022
Closed
@BenHenning BenHenning mentioned this issue May 25, 2024
Merged
6 tasks
BenHenning added a commit to oppia/oppia-android that referenced this issue Jun 13, 2024
@BenHenning @adhiamboperes @seanlip
Fix #5370, part of #59: Migrate to Bazel 6.5.0 (#4886)
add9f74 
## Explanation

Fixes #5370
Fixes part of #59

This PR updates the project to use Bazel 6.5.0 instead of 4.0.0.

Note that most of the changes done so far in addressing #59 are centered
around the concept of simplifying the Bazel maintenance as much as
possible so that it's not too much more difficult than Gradle by the
time we fully remove Gradle support from the project. While Bazel will
always require more effort, there are many things that can be done to
narrow the gap. This is a major step in that process since Bazel 4.x
required using a custom Android toolchain
(https://github.com/oppia/oppia-bazel-tools) which is not at all user
friendly. Plus, there are many compatibility and performance
improvements in later versions of Bazel that we want to be able to
incorporate within the broader Oppia Android project.

Bazel 6.x was specifically chosen because:
- Bazel 4.x was missing support for the new D8 version which made it
impossible to upgrade past 29.0.2 build tools
bazelbuild/bazel#13989.
- Bazel 5.x had some additional compatibility issues with the D8 change,
so we weren't able to use it, either:
bazelbuild/bazel#15957.
- Bazel 7.x (which wasn't released when this work was originally done)
introduces new bzlmod support that causes some additional build
headaches that can be figured out later.
- Bazel 6.5.0 specifically was chosen since it's the latest 6.x version
(as of this edit) and seems to work correctly with existing unit tests.

Some other important details to note:
- rules_kotlin 1.7.x is needed at a minimum for Bazel 5.x+ support.
However, an additional fix was needed
(bazelbuild/rules_kotlin#940) in order to fix a
deviation in functionality that occurred starting in Bazel 5.x's
java_plugin support which led to some file duplication in rules_kotlin
(that was fortunately easy to fix). Unfortunately, this change wasn't
backported to 1.7.x so this PR makes use of a custom patch to
rules_kotlin 1.7.1 (https://github.com/oppia/rules_kotlin) that includes
the needed change. We'll get this change properly once we can upgrade to
1.8.x, though that will also require updating Kotlin itself to 1.8.x due
to bazelbuild/rules_kotlin#1019.
- Bazel 6.x (maybe 5.x) requires at least build tools 30.0.0 since it
completely removed support for the old D8 compat dexer. 32.0.0 was
chosen in this PR as it's simply a newer, more up-to-date build tools
(and removes D8 completely). With this upgrade to Bazel 6.x we'll be
able to update the build tools version more often (so long as it doesn't
introduce AGP incompatibilities since we can't upgrade Gradle).
- As of Bazel 6.x, we're able to reenable Java header compilation and
incremental dexing, both of which should have _significant_ performance
improvements for incremental builds of the app (and in fact we will have
build errors if we disable incremental dexing).
- In CI, we opted to **not** support build tools 29.0.2 or old builds of
the app. Instead, we'll rely on build tools failing for certain PRs as
an indicator that those PRs will require an update (once this PR is
merged) in order to have CI run correctly. This is a lot easier than
trying to figure out how to support before/after changes with some
fairly complex environment differences.
- There are a bunch of version updates that were needed to support the
minimum version of Kotlin for rules 1.7.x (1.6 I think) as well as JDK
11 (which I think was needed for Bazel 5.x), and these have largely been
taken care of in previous PRs to this one (though the JDK 11 update in
CI was done in this PR, along with wiki documentation updates to address
#5370). One such case of a necessary version upgrade:
google/dagger#2511.
- There was a change needed for the databinding java_plugin declaration
to specify that it generates an API (in order for it to be used
correctly in builds).
- rules_java needed to be updated to support the newer version of Bazel.
- The desugaring hack needed for kotlinx-coroutines-core-jvm was removed
since it's no longer needed with the build tools & Bazel upgrade
introduced in this PR.
- This includes one small change in third-party to change all
single-export wrappers that don't have additional plugins being enabled
to aliases instead. This is more semantically correct as the wrappers
may lose information (which caused problems when investigating adding
Jetpack Compose support in #5401). While this isn't directly required
for the Bazel upgrade, this is the last PR needed for Jetpack Compose
support so it's being added here for simplicity.
- ``.bazelrc`` was updated to configure tools, tests, and builds to all
use the remote JDK 11 available via Bazel rather than ever using the
user's local JDK. This should improve build hermeticity and consistency
across different user environments (see
https://bazel.build/docs/bazel-and-java).
- Setup docs were updated to remove setting up JDK 11 (or Java at all
for Linux & Mac) now that the user no longer needs to install Java (see
previous point) except for Windows. The Python instructions were also
removed since Bazel 6.x includes fixes for Android tools that previously
depended on Python 2.x.
- CI was unchanged for Java setup since, as far as I can tell, it's
still needed for sdkmanager.

There was also some small cleanup in unit_tests.yml that I noticed when
updating CI versions.

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
N/A -- This is a build infrastructure change. It shouldn't impact the
end user experience.

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Co-authored-by: Sean Lip <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ted-xie ted-xie

Labels
P1 I'll work on this now. (Assignee required) team-Android Issues for Android team type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@Kernald @philwo @sgjesse @EdbertChan @c16a @nkoroste @ted-xie @gregestren @ahumesky @alyssawilk