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)
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)
Comment 1•9 years ago
|
||
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
Assignee | ||
Comment 2•9 years ago
|
||
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)
Assignee | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
Assignee | ||
Comment 6•9 years ago
|
||
Assignee | ||
Comment 7•9 years ago
|
||
(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
Assignee | ||
Comment 8•9 years ago
|
||
I can reproduce this locally after manually enabling the crash reporter. It looks like the gradle plugin / lint has problems with multiple resource folders.
Assignee | ||
Comment 9•9 years ago
|
||
It looks like this is fixed locally after switching to the 2.1 plugin. So let's update the builders again! -> bug 1268453.
Assignee | ||
Comment 10•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49703/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49703/
Attachment #8747043 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 11•9 years ago
|
||
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
Assignee | ||
Comment 12•9 years ago
|
||
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
Comment 13•9 years ago
|
||
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.
Assignee | ||
Comment 14•9 years ago
|
||
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/
Assignee | ||
Comment 15•9 years ago
|
||
Reporter | ||
Comment 16•9 years ago
|
||
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+
Assignee | ||
Comment 17•9 years ago
|
||
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.
Assignee | ||
Comment 18•9 years ago
|
||
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
Comment 19•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•