1374182 - Be more general on what files would trigger android-lint task on task cluster.

Status: NEW

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

Description

[Teng-pao Yu [:tyu]]

The Treeherder is currently watching these paths and will be optimized out if none of the following files is changed:

            - - "mobile/android/**/*.java"
              - "mobile/android/**/*.jpeg"
              - "mobile/android/**/*.jpg"
              - "mobile/android/**/*.png"
              - "mobile/android/**/*.svg"
              - "mobile/android/**/*.xml" # Manifest & android resources
              - "mobile/android/**/*.gradle"
              - "mobile/android/**/Makefile.in"
              - "mobile/android/**/moz.build"

However, it should be more general, since we have more java files associated in the build:

            java {
                srcDir "${topsrcdir}/mobile/android/base/java"
                srcDir "${topsrcdir}/mobile/android/search/java"
                srcDir "${topsrcdir}/mobile/android/services/src/main/java"

                if (mozconfig.substs.MOZ_ANDROID_MLS_STUMBLER) {
                    srcDir "${topsrcdir}/mobile/android/stumbler/java"
                }

                if (!mozconfig.substs.MOZ_CRASHREPORTER) {
                    exclude 'org/mozilla/gecko/CrashReporter.java'
                }

                if (!mozconfig.substs.MOZ_NATIVE_DEVICES) {
                    exclude 'org/mozilla/gecko/ChromeCastDisplay.java'
                    exclude 'org/mozilla/gecko/ChromeCastPlayer.java'
                    exclude 'org/mozilla/gecko/GeckoMediaPlayer.java'
                    exclude 'org/mozilla/gecko/GeckoPresentationDisplay.java'
                    exclude 'org/mozilla/gecko/MediaPlayerManager.java'
                }

                if (mozconfig.substs.MOZ_WEBRTC) {
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/base/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/audio_device/android/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/video_capture/android/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/video_render/android/java/src"
                }

                if (mozconfig.substs.MOZ_INSTALL_TRACKING) {
                    exclude 'org/mozilla/gecko/adjust/StubAdjustHelper.java'
                } else {
                    exclude 'org/mozilla/gecko/adjust/AdjustHelper.java'
                }

                if (mozconfig.substs.MOZ_ANDROID_MMA) {
                    exclude 'org/mozilla/gecko/mma/MmaStubImp.java'
                } else {
                    exclude 'org/mozilla/gecko/mma/MmaLeanplumImp.java'
                }

                if (!mozconfig.substs.MOZ_ANDROID_GCM) {
                    exclude 'org/mozilla/gecko/gcm/**/*.java'
                    exclude 'org/mozilla/gecko/push/**/*.java'
                }

                srcDir "${project.buildDir}/generated/source/preprocessed_code" // See syncPreprocessedCode.
            }

I'm not sure if there are more non-java files that should be supervised too.

A recent example would be https://bugzilla.mozilla.org/show_bug.cgi?id=1341285#c176 . Some relevant files ( media/webrtc/trunk/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java ) are changed but the android-lint is optimized out. This should make Sheriffs hard to investigate what breaks the android-lint since problematic code would pass the test (since not scheduled) and later breaks another test.

Comment 1

[Teng-pao Yu [:tyu]]

Some links to the aforementioned code.

https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/android-stuff/kind.yml

https://dxr.mozilla.org/mozilla-central/source/mobile/android/app/build.gradle

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Teng-pao Yu [:tyu] from comment #0)
> The Treeherder is currently watching these paths and will be optimized out
> if none of the following files is changed:
> 
>             - - "mobile/android/**/*.java"
>               - "mobile/android/**/*.jpeg"
>               - "mobile/android/**/*.jpg"
>               - "mobile/android/**/*.png"
>               - "mobile/android/**/*.svg"
>               - "mobile/android/**/*.xml" # Manifest & android resources
>               - "mobile/android/**/*.gradle"
>               - "mobile/android/**/Makefile.in"
>               - "mobile/android/**/moz.build"
> 
> However, it should be more general, since we have more java files associated
> in the build:

The only ones I see here that are not covered by mobile/android/**/*.java are

                if (mozconfig.substs.MOZ_WEBRTC) {
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/base/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/audio_device/android/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/video_capture/android/java/src"
                    srcDir "${topsrcdir}/media/webrtc/trunk/webrtc/modules/video_render/android/java/src"
                }

I guess we can add them, but I'm not sure that they're even linted.  In fact, they probably should _not_ be linted, since they're third-party files imported from upstream.

> I'm not sure if there are more non-java files that should be supervised too.

It's possible, but I'm not aware of any.  If you know of any, add them to this ticket.

Comment 3

[Teng-pao Yu [:tyu]]

They are linted back when I filed this bug. I'm bringing this up since in the aforementioned bug media/webrtc/trunk/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioTrack.java is changed and violates a lint check (newapi) but it is not caught in the m-c build which this change subsides, but in a later build which finally triggers android-lint. This sounds undesirable.

Since we're changing it and it is already under our radar, personally I think it is more appropriate to keep them linted and add them to android-lint triggers. (Only if what I mentioned still applies, I haven't been working on Fennec for quite a while. I've heard webrtc are backed out now due to dex limits, too.)

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Teng-pao Yu [:tyu] from comment #3)
> They are linted back when I filed this bug. I'm bringing this up since in
> the aforementioned bug
> media/webrtc/trunk/webrtc/modules/audio_device/android/java/src/org/webrtc/
> voiceengine/WebRtcAudioTrack.java is changed and violates a lint check
> (newapi) but it is not caught in the m-c build which this change subsides,
> but in a later build which finally triggers android-lint. This sounds
> undesirable.
> 
> Since we're changing it and it is already under our radar, personally I
> think it is more appropriate to keep them linted and add them to
> android-lint triggers. (Only if what I mentioned still applies, I haven't
> been working on Fennec for quite a while. I've heard webrtc are backed out
> now due to dex limits, too.)

I agree: if we're changing it, we should lint it in android-lint.  If we're not, we should exclude it in http://searchfox.org/mozilla-central/source/mobile/android/app/lint.xml.

Comment 5

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.