Closed Bug 1268724 Opened 9 years ago Closed 9 years ago

Local android lint errors after gradle 2.1 upgrade

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox49 fixed)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed

People

(Reporter: mcomella, Assigned: sebastian)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

There are no errors on sebastian's push: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=830f7765555a3740103c31100118bf318cf785b2 But there are errors locally for me, Grisha, and liuche. This makes me skeptical the builders were not updated correctly. Sebastian, any ideas? Do you see the errors locally?
Flags: needinfo?(s.kaspari)
It looks like the CrashReporter.EditText style is the offending item here. We don't build CrashReporter locally (and I assume the treeherder builds do) so that's part of the problem here. Basically, lint does not complain about crashreporter_layout.xml, but does complain about the style included in that file. A hack is to add the CR.EditText style to the UnusedResourcesUtils [1], but this isn't a temporary disuse. https://hg.mozilla.org/integration/fx-team/rev/830f7765555a3740103c31100118bf318cf785b2
I can reproduce. As liuche pointed out it's the crash reporter (the builders are fine). The default is to build without crash reporter and in addition to that setting MOZ_CRASHREPOTER doesn't seem to affect artifact builds (I need to investigate this a bit more. This should be filed too).
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Flags: needinfo?(s.kaspari)
In our gradle configuration we exclude the source code (CrashReporter.java) and resources (m/a/b/crashreporter/res) based on MOZ_CRASHREPORTER. We should do the same for the styles/colors/layouts and move them to m/a/b/crashreporter/res if they are only used by the CrashReporter.
(In reply to Sebastian Kaspari (:sebastian) from comment #6) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=37dc369bc684 It's getting weirder. After moving the resources to m/a/b/crashreporter/res everything's green locally. But now suddenly the builders think the resources are unused: https://public-artifacts.taskcluster.net/DDNZzaKYRx-gdFkxvwraQQ/0/public/android/lint/lint-results-automationDebug.html#UnusedResources
I can reproduce this locally after manually enabling the crash reporter. It looks like the gradle plugin / lint has problems with multiple resource folders.
It looks like this is fixed locally after switching to the 2.1 plugin. So let's update the builders again! -> bug 1268453.
This is the patch for the lint locally (without crash reporter). Landing this has to wait until we updated the lint plugin (2.0 -> 2.1) on the builders (bug 1268453).
Depends on: 1268453
With the patches from bug 1268453 there's only one new ridiculous lint error left: > ../generated/source/preprocessed_resources/raw/.mkdir.done: The resource R.raw..mkdir.done appears to be unused
I remember somewhere that MOZ_CRASHREPORTER actually only works on linux builds, not osx. I don't remember where exactly this is documented (possibly just in configs/code?), but when I did some crashreporter testing, I remember needing to have a linux VM.
Depends on: 1268948
Comment on attachment 8747043 [details] MozReview Request: Bug 1268724 - Move crash reporter colors, styles and drawables to m/a/b/crashreporter/res. r?mcomella Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49703/diff/1-2/
Comment on attachment 8747043 [details] MozReview Request: Bug 1268724 - Move crash reporter colors, styles and drawables to m/a/b/crashreporter/res. r?mcomella https://reviewboard.mozilla.org/r/49703/#review46573 Were you able to test this locally? If this was working before, I'm afraid that something might break if we change it (but then again, I'm sure someone will see it at some point if it did break :P).
Attachment #8747043 - Flags: review?(michael.l.comella) → review+
https://reviewboard.mozilla.org/r/49703/#review46573 I did a build with and without resources. It worked before because the crashreporter isn't actually a separate gradle module. Instead we just include/exclude files depending on the build flags. With the crashreporter stuff in our base styles/colors/.. folder we'd always include them, even if we build without the crashreporter. Aparently lint was just unable to detect that they are unused before.
https://hg.mozilla.org/integration/fx-team/rev/33b4d01f08e4b89e172be41517a65586b4e59699 Bug 1268724 - Move crash reporter colors, styles and drawables to m/a/b/crashreporter/res. r=mcomella
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: