1667948 - Creating fat .aar file breaks during SyncLibsAndUpdateGenerationID task creation

Status: RESOLVED FIXED

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

Description

[Georg Koppen]

We rebased out patchset onto 82.0b2 and suddenly creating the fat GeckoView .aar file is broken (while compiling for all the supported architectures still works):

 0:27.88 FAILURE: Build failed with an exception.
 0:27.88 * Where:
 0:27.88 Script '/var/tmp/build/geckoview-1e87e9af192c/mobile/android/gradle/with_gecko_binaries.gradle' line: 71
 0:27.88 * What went wrong:
 0:27.88 A problem occurred configuring project ':geckoview'.
 0:27.88 > Could not create task ':geckoview:syncLibsAndUpdateGenerationIDFromDistDirForWithGeckoBinariesDebug'.
 0:27.88    > Could not create task of type 'SyncLibsAndUpdateGenerationID'.
 0:27.88       > No signature of method: groovy.util.FileNameFinder.getFileNames() is applicable for argument types: (File, String) values: [/var/tmp/dist/android-toolchain/android-ndk/android-ndk-r20, ...]
 0:27.88         Possible solutions: getFileNames(java.lang.String, java.lang.String), getFileNames(java.util.Map), getFileNames(java.lang.String, java.lang.String, java.lang.String)

I started by reverting the change for bug 1661406 which just resulted in a different error message, though:

 1:48.28 FAILURE: Build failed with an exception.
 1:48.28 * What went wrong:
 1:48.28 A problem was found with the configuration of task ':geckoview:syncLibsFromDistDirForWithGeckoBinariesDebug'.
 1:48.28 > Directory '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/dist/geckoview/lib' specified for property 'source' does not exist.

Then I reverted the change for bug 1661158 on top of that which did not change anything. Finally, on top of those two reverts I backed out the change for bug 1627796 and the creation of the fat .aar succeeded as expected.

Comment 1

[Nick Alexander :nalexander [he/him]]

The relevant code block is here: https://searchfox.org/mozilla-central/rev/dfd9c0f72f9765bd4a187444e0c1e19e8834a506/mobile/android/gradle/with_gecko_binaries.gradle#186-199. It reads:

    // For !MOZILLA_OFFICIAL builds, work around an Android-Gradle plugin bug that causes startup
    // crashes with local substitution.  But -- we want to allow artifact builds that don't have the
    // NDK installed.  See class comment above.
    def shouldUpdateGenerationID = {
        if (mozconfig.substs.MOZILLA_OFFICIAL) {
            return false
        } else if (ext.getNDKDirectory() == null) {
            logger.warn("Could not determine Android NDK directory.")
            logger.warn("Set `ndk.dir` in `${project.topsrcdir}/local.properties` to avoid startup crashes when using `substitute-local-geckoview.gradle`.")
            logger.warn("See https://bugzilla.mozilla.org/show_bug.cgi?id=1657190.")
            return false
        }
        return true
    }()

It must be that Tor is !MOZILLA_OFFICIAL (makes sense) but has a full build environment (even for the fat AARs). Let's just check for mozconfig.substs.MOZ_ANDROID_FAT_AAR_ARCHITECTURES as well, like:

...
    def shouldUpdateGenerationID = {
        if (mozconfig.substs.MOZILLA_OFFICIAL) {
            return false
        } else if (mozconfig.substs.MOZ_ANDROID_FAT_AAR_ARCHITECTURES) {
            return false
        } ...

gkoppen: could you verify that's sufficient for your use case? Yet again we need a "I'm not Mozilla but I'm building for a distributed release" flag :(

Comment 2

[Georg Koppen]

Yep. Adding

+        } else if (mozconfig.substs.MOZ_ANDROID_FAT_AAR_ARCHITECTURES) {
+            return false

unbreaks our build. Thanks!

Comment 3

[Georg Koppen]

Another idea :acat came up with today: could we just set MOZILLA_OFFICIAL, too? Would that be okay from a build PoV? I realized we do this already for all of our other mozconfig files to be as close to Mozilla's official build config as possible. This works for us even though I just ran into another build issue later on:

 1:19.39 /var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/_virtualenvs/init_py3/bin/python /var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py -R -m 644 '/var/tmp/build/geckoview-1e87e9af192c/toolkit/library/libxul.so-gdb.py' '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/toolkit/library/build'
 1:19.46 Traceback (most recent call last):
 1:19.46   File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 162, in <module>
 1:19.46     sys.exit(_nsinstall_internal(sys.argv[1:]))
 1:19.46   File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 151, in _nsinstall_internal
 1:19.46     copy_all_entries(args, target)
 1:19.46   File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 112, in copy_all_entries
 1:19.46     os.chmod(dest, options.m)
 1:19.46 FileNotFoundError: [Errno 2] No such file or directory: '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/toolkit/library/build/libxul.so-gdb.py'

Comment 4

[Georg Koppen]

(In reply to Georg Koppen from comment #3)

1:19.39 /var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/_virtualenvs/init_py3/bin/python /var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py -R -m 644 '/var/tmp/build/geckoview-1e87e9af192c/toolkit/library/libxul.so-gdb.py' '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/toolkit/library/build'
1:19.46 Traceback (most recent call last):
1:19.46 File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 162, in <module>
1:19.46 sys.exit(_nsinstall_internal(sys.argv[1:]))
1:19.46 File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 151, in _nsinstall_internal
1:19.46 copy_all_entries(args, target)
1:19.46 File "/var/tmp/build/geckoview-1e87e9af192c/config/nsinstall.py", line 112, in copy_all_entries
1:19.46 os.chmod(dest, options.m)
1:19.46 FileNotFoundError: [Errno 2] No such file or directory: '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/toolkit/library/build/libxul.so-gdb.py'

Interestingly enough, this is gone after I rebuilt and the build succeeds (I did not change anything). So, maybe scratch that issue for now...

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Georg Koppen from comment #3)

Another idea :acat came up with today: could we just set MOZILLA_OFFICIAL, too? Would that be okay from a build PoV? I realized we do this already for all of our other mozconfig files to be as close to Mozilla's official build config as possible.

I am going to redirect to :glandium, because I don't understand the tradeoffs involved. I think that the face value choice can be argued in both directions: Tor isn't part of the official Mozilla build suite, so we should have !MOZILLA_OFFICIAL. But the trend (and the desire) is towards Tor being part of the official Mozilla build, so we should just have MOZILLA_OFFICIAL.

Given that you're setting MOZILLA_OFFICIAL for the builds you might as well set it for the fat AAR tasks too, since the set of things controlled are relevant to both and should agree between the tasks.

The issue with that file probably has to do with swizzling the flag; I wouldn't worry about it.

Comment 6

[Nick Alexander :nalexander [he/him]]

Oh -- could you submit this as a patch, r?nalexander? Thanks!

Comment 7

[Mike Hommey [:glandium]]

1:19.46 FileNotFoundError: [Errno 2] No such file or directory: '/var/tmp/build/geckoview-1e87e9af192c/obj-arm-unknown-linux-androideabi/toolkit/library/build/libxul.so-gdb.py'

That sounds like bug 1646445

As for MOZILLA_OFFICIAL, I don't understand why gradle checks that.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1627796

Comment 9

[Nick Alexander :nalexander [he/him]]

As for MOZILLA_OFFICIAL, I don't understand why gradle checks that.

Because there are things we don't want to do in automation. What would you prefer we check? DEVELOPER_OPTIONS?

Comment 10

[Mike Hommey [:glandium]]

MOZ_AUTOMATION?

Comment 11

[Georg Koppen]

Nick: what is the preferred way forward here? Should we still submit the patch you suggested? And make the MOZ_AUTOMATION change, too, while we are at it? Or do you (or anyone else) want to make those changes? Or?

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Georg Koppen from comment #11)

Nick: what is the preferred way forward here? Should we still submit the patch you suggested? And make the MOZ_AUTOMATION change, too, while we are at it? Or do you (or anyone else) want to make those changes? Or?

Yes, let's submit what worked, and someday we'll move away from MOZILLA_OFFICIAL in these .gradle files. But that can be follow-up.

Thanks!

Comment 13

[Georg Koppen]

Attachment: Bug 1667948 - Don't break fat .aar creation if MOZ_ANDROID_FAT_AAR_ARCHITECTURES is set r=nalexander

Comment 14

[Georg Koppen]

Okay, I submitted my first patch via Phabricator, please bear with me that I got all the flags etc. right. :)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3154e6ace1a3
Don't break fat .aar creation if MOZ_ANDROID_FAT_AAR_ARCHITECTURES is set r=nalexander

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Georg Koppen from comment #14)

Okay, I submitted my first patch via Phabricator, please bear with me that I got all the flags etc. right. :)

Huh, I didn't look at anything too closely since it was straight-forward and small. I queued the landing for you -- should have had you do that, since it's your first run through. Next time. Thanks!

Comment 17

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3154e6ace1a3