1289585 - localOldDebug builds semi-broken (ZipException when building via ./gradlew clean app:assembleDebug)

Status: NEW

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Jan Henning [:JanH]]

Bug 1285511 (http://hg.mozilla.org/mozilla-central/rev/6972bef6693c) is breaking localOldDebug builds for me. When trying to build Firefox via Gradle/from Android Studio, the build errors out with this message:

* What went wrong:
Execution failed for task ':app:transformClassesWithDexForLocalOldDebug'.
> com.android.build.api.transform.TransformException: java.util.zip.ZipException: error in opening zip file

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

I can still build successfully (mozilla-central, assembleLocalOldDebug). Does 'clean' help? Did you try running 'mach bootstrap' again?

Comment 2

[Jan Henning [:JanH]]

(In reply to Sebastian Kaspari (:sebastian) from comment #1)
> I can still build successfully (mozilla-central, assembleLocalOldDebug).
> Does 'clean' help?

As in './gradlew clean app:assembleDebug'? No, unfortunately that doesn't help. I'll give rerunning 'mach bootstrap' a try, even though that'll probably want to redownload the whole SDK and NDK as well...

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Jan Henning [:JanH] from comment #2)
> (In reply to Sebastian Kaspari (:sebastian) from comment #1)
> > I can still build successfully (mozilla-central, assembleLocalOldDebug).
> > Does 'clean' help?
> 
> As in './gradlew clean app:assembleDebug'? No, unfortunately that doesn't
> help. I'll give rerunning 'mach bootstrap' a try, even though that'll
> probably want to redownload the whole SDK and NDK as well...

Oh, I see the same error when running assembleDebug. But the actual task (that AS should run too) is assembleLocalDebug or assembleLocalOldDebug.

Can you run this locally?
> ./mach gradle clean assembleLocalOldDebug

Comment 4

[Jan Henning [:JanH]]

I'll give it a try when I get back home. However apart from manually triggering the build on the command line, whatever command Android Studio is using internally when attempting to build (as in pressing the green 'Run' arrow) is giving me the same error.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I can't reproduce this locally, on the command line w/ `mach gradle assembleLocalOldDebug` or with AS.

Are you using `--with-gradle` to set up gradle builds? I'm not and that could be the discrepancy here. `--with-gradle` will change the code that is being built after the changeset you referenced in comment 0.

Comment 6

[Jan Henning [:JanH]]

Hmm, running the assembleLocalOldDebug task directly is indeed working. The weird thing is that even though the Android Studio Gradle console claims that it's always executing the :app:assembleLocalOldDebug task, its success seems to be determined by the last build I ran from the command line - if I last used assembleDebug (as originally suggested on the Simple Firefox Build MDN page), then Android Studio will fail to build, too. If I run a assembleLocalOldDebug build from the command line instead, Android Studio starts building successfully as well, and so on.

@mcomella: Can you reproduce it if you try running ./gradlew clean app:assembleDebug and then doing an AS build afterwards?

Comment 7

[Jan Henning [:JanH]]

Oh, and re `--with-gradle`, you mean in my mozconfig? No, I'm not using that.

Comment 8

[Jan Henning [:JanH]]

Whoops, accidentally managed to reset the assignee when restoring the comment field after our comments had mid-aired.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Jan Henning [:JanH] from comment #6)
> @mcomella: Can you reproduce it if you try running ./gradlew clean
> app:assembleDebug and then doing an AS build afterwards?

Yes, running this command causes me to get the exception. After backing out revision 6972bef6693c, which adds the multidex code, I can no longer reproduce.

app:assembleDebug appears to run both "*AutomationDebug*" tasks as well as "*LocalOldDebug*" – I wonder if we're hitting conflicts because the jobs are trying to share compiled resources that change between the two configurations, now that we enable multidex on "automation" but not on "local" builds.

Nick, does that sound plausible? Do you know what the proper fix for this issue might be?

Running both "*AutomationDebug*" and "*LocalOldDebug*" in a single task seems useful to ensure we didn't break the build in all configurations but it doesn't seem very useful to the average developer when making changes – I wonder if we should just not support it.

Comment 10

[Nick Alexander :nalexander [he/him]]

(In reply to Michael Comella (:mcomella) from comment #9)
> (In reply to Jan Henning [:JanH] from comment #6)
> > @mcomella: Can you reproduce it if you try running ./gradlew clean
> > app:assembleDebug and then doing an AS build afterwards?
> 
> Yes, running this command causes me to get the exception. After backing out
> revision 6972bef6693c, which adds the multidex code, I can no longer
> reproduce.
> 
> app:assembleDebug appears to run both "*AutomationDebug*" tasks as well as
> "*LocalOldDebug*" – I wonder if we're hitting conflicts because the jobs are
> trying to share compiled resources that change between the two
> configurations, now that we enable multidex on "automation" but not on
> "local" builds.
> 
> Nick, does that sound plausible?

Yes.  I have seen issues around multiDex and preDexLibraries in Gradle, although not really /across/ configurations.

Do you know what the proper fix for this
> issue might be?

Sadly, I really don't.  I think the Gradle Android plugin doesn't partition across configurations sufficiently; and I /know/ it doesn't invalidate it's intermediate dex caches sufficiently.  I think part of this is that all configurations share the same release configuration for each required library (and sub-project), and those seem to get confused.  That is, the various debug builds depend on the release builds of all the GPS and Android support libraries, and that seems to cause the confusion with DEX.

Sorry that I can't help more -- I've seen these ZipExceptions and always just needed to clean :(

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Given my level of expertise, I don't have the time to come up with a proper solution. NI sebastian if he wants to tackle it (but I'm not sure it's actually worth investing in because developers generally will only use the "local" configurations & automation will use the "automation" configurations).

If one runs into this issue anyway, it sounds like the proper workaround is to clean between configurations (as per Nick's comment 10).

I've updated the build docs to say `./gradlew assembleLocalDebug` instead but here are some other ways we could potentially help users not run `assembleDebug`:

1) Disable the task. These tasks are auto-generated so I'm not sure how one might do this (if it's possible)
2) Add a message to the task that this causes issues because of predex/multi-dex and we're aware of it. Mention the workaround.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

If you are unblocked now then I'll drop the ball too. I often have problems with a broken AS build because of a missing AndroidManifest.xml. This requires a clean or a command line build to be fixed too.

I tend to WONTFIX here: Our build is still very special and I think the time is better spent improving our build and migrating to gradle than fixing some broken aspect of this mixed build system. However does it make sense to collect some of those troubleshooting tips on the wiki?

Comment 13

[Jan Henning [:JanH]]

(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> If you are unblocked now then I'll drop the ball too. I often have problems
> with a broken AS build because of a missing AndroidManifest.xml. This
> requires a clean or a command line build to be fixed too.

Yup, I think I've seen AS refusing to build successfully after a clobber until I ran a command line build.

> I tend to WONTFIX here:

Since calling assembleLocalOldDebug does the trick as well and the MDN docu has been updated, no objection.

Comment 14

[Tom Klein]

As I think Jan hints at in comment 13 (and comment 6 I guess), I needed to do './gradlew clean app:assembleLocalOldDebug' for my setup, not './gradlew clean app:assembleLocalDebug' as the current wiki doc says.  I guess someone more experienced could guess that you need to insert 'Old' in the clean command if you're using Build Variant localOldDebug, but it's not that obvious to me, especially since the stuff about setting Build Variant comes after the instruction to run clean without 'Old' - if that's right, could the doc be updated again?  It also seems like the first bullet under Troubleshooting should be updated as well.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Tom Klein from comment #14)
> if that's right, could the doc be updated again?  It also seems like the first bullet under
> Troubleshooting should be updated as well.

That's good feedback – thanks! I updated the docs.