New issue
Advanced search Search tips

Issue 607761 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug


Sign in to add a comment

GN Should Fail Builds when Android Lint Warnings Exist

Project Member Reported by agrieve@chromium.org, Apr 29 2016

Issue description

GYP failed builds, but GN never did.

Now we've enabled more checks in GN (that were mistakenly not enabled in GYP), and lint warnings exists. We should fix or silence existing lint warnings and then enable warnings-as-errors.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b9957478bb09c311650460b568f237f45c7db8c

commit 6b9957478bb09c311650460b568f237f45c7db8c
Author: agrieve <agrieve@chromium.org>
Date: Fri Apr 29 01:43:53 2016

Fix android lint warning in TokenSourceImpl.java

Consider using `apply()` instead; `commit` writes its data to persistent
storage immediately, whereas `apply` will handle it in the background:
CommitPrefEdits [warning]

BUG= 607761 

Review-Url: https://codereview.chromium.org/1934473002
Cr-Commit-Position: refs/heads/master@{#390563}

[modify] https://crrev.com/6b9957478bb09c311650460b568f237f45c7db8c/blimp/client/app/android/java/src/org/chromium/blimp/auth/TokenSourceImpl.java

Blocking: 617657
Blockedon: 621769
Blockedon: 621770
Blockedon: 621771
Blockedon: 621772
Blockedon: 621773
Blockedon: 621774
Blockedon: 623259
Blockedon: 623262
Blockedon: 623264
Blockedon: 624234
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35269b7ddd6cf1134ea4e93c9928365bca90db9c

commit 35269b7ddd6cf1134ea4e93c9928365bca90db9c
Author: wnwen <wnwen@chromium.org>
Date: Mon Jul 11 14:28:24 2016

Replace Shared Prefs commit with apply.

BUG= 607761 

Review-Url: https://codereview.chromium.org/2124583003
Cr-Commit-Position: refs/heads/master@{#404653}

[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ManageSpaceActivity.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/javatests/src/org/chromium/chrome/browser/ChromeBackupIntegrationTest.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/javatests/src/org/chromium/chrome/browser/accessibility/FontSizePrefsTest.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java
[modify] https://crrev.com/35269b7ddd6cf1134ea4e93c9928365bca90db9c/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java

Blockedon: 632362
Blockedon: 634948
Blockedon: 635179
Blockedon: 635567
Blockedon: 635967
Cc: agrieve@chromium.org
Owner: estevenson@chromium.org
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49f5130f956057407d2219e37a978437bcdcf7fe

commit 49f5130f956057407d2219e37a978437bcdcf7fe
Author: estevenson <estevenson@chromium.org>
Date: Fri Aug 12 17:36:34 2016

Fail builds when lint warnings are generated.

Suppress all existing lint warnings so that builds will fail when new
warnings are generated. New lint warnings that are generated in files
that are ignored in suppressions.xml will not be caught. However, now
we can remove lint warnings on a file by file basis instead of having to
fix all lint warnings before making lint break builds.

BUG= 607761 

Review-Url: https://codereview.chromium.org/2240573004
Cr-Commit-Position: refs/heads/master@{#411692}

[modify] https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe/build/android/gyp/lint.py
[modify] https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe/build/android/lint/suppressions.xml
[modify] https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe/build/config/android/internal_rules.gni

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71ab99083db4d384fc9ce687c9d1cd5300f3ef89

commit 71ab99083db4d384fc9ce687c9d1cd5300f3ef89
Author: xidachen <xidachen@chromium.org>
Date: Fri Aug 12 18:04:01 2016

Revert of Fail builds when lint warnings are generated. (patchset #2 id:20001 of https://codereview.chromium.org/2240573004/ )

Reason for revert:
Causing compilation failure:
https://build.chromium.org/p/chromium.linux/builders/Android%20Builder/builds/71177

Original issue's description:
> Fail builds when lint warnings are generated.
>
> Suppress all existing lint warnings so that builds will fail when new
> warnings are generated. New lint warnings that are generated in files
> that are ignored in suppressions.xml will not be caught. However, now
> we can remove lint warnings on a file by file basis instead of having to
> fix all lint warnings before making lint break builds.
>
> BUG= 607761 
>
> Committed: https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe
> Cr-Commit-Position: refs/heads/master@{#411692}

TBR=agrieve@chromium.org,estevenson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 607761 

Review-Url: https://codereview.chromium.org/2245663003
Cr-Commit-Position: refs/heads/master@{#411700}

[modify] https://crrev.com/71ab99083db4d384fc9ce687c9d1cd5300f3ef89/build/android/gyp/lint.py
[modify] https://crrev.com/71ab99083db4d384fc9ce687c9d1cd5300f3ef89/build/android/lint/suppressions.xml
[modify] https://crrev.com/71ab99083db4d384fc9ce687c9d1cd5300f3ef89/build/config/android/internal_rules.gni

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce

commit 9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce
Author: estevenson <estevenson@chromium.org>
Date: Fri Aug 12 20:03:13 2016

Fail builds when lint warnings are generated.

Suppress all existing lint warnings so that builds will fail when new
warnings are generated. New lint warnings that are generated in files
that are ignored in suppressions.xml will not be caught. However, now
we can remove lint warnings on a file by file basis instead of having to
fix all lint warnings before making lint break builds.

BUG= 607761 

Committed: https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe
Review-Url: https://codereview.chromium.org/2240573004
Cr-Original-Commit-Position: refs/heads/master@{#411692}
Cr-Commit-Position: refs/heads/master@{#411748}

[modify] https://crrev.com/9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce/build/android/gyp/lint.py
[modify] https://crrev.com/9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce/build/android/lint/suppressions.xml
[modify] https://crrev.com/9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce/build/config/android/internal_rules.gni

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b8904601897107f297027eecbb7572bb7bf902d

commit 0b8904601897107f297027eecbb7572bb7bf902d
Author: caseq <caseq@chromium.org>
Date: Fri Aug 12 21:04:44 2016

Revert of Fail builds when lint warnings are generated. (patchset #3 id:40001 of https://codereview.chromium.org/2240573004/ )

Reason for revert:
Broke Android Arm64 builder.

Original issue's description:
> Fail builds when lint warnings are generated.
>
> Suppress all existing lint warnings so that builds will fail when new
> warnings are generated. New lint warnings that are generated in files
> that are ignored in suppressions.xml will not be caught. However, now
> we can remove lint warnings on a file by file basis instead of having to
> fix all lint warnings before making lint break builds.
>
> BUG= 607761 
>
> Committed: https://crrev.com/49f5130f956057407d2219e37a978437bcdcf7fe
> Committed: https://crrev.com/9a52a7e12ad50f6f85e88c6a5636ce6e0a405dce
> Cr-Original-Commit-Position: refs/heads/master@{#411692}
> Cr-Commit-Position: refs/heads/master@{#411748}

TBR=agrieve@chromium.org,estevenson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 607761 

Review-Url: https://codereview.chromium.org/2236313004
Cr-Commit-Position: refs/heads/master@{#411764}

[modify] https://crrev.com/0b8904601897107f297027eecbb7572bb7bf902d/build/android/gyp/lint.py
[modify] https://crrev.com/0b8904601897107f297027eecbb7572bb7bf902d/build/android/lint/suppressions.xml
[modify] https://crrev.com/0b8904601897107f297027eecbb7572bb7bf902d/build/config/android/internal_rules.gni

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8bde5d301337c0994a9d71fc31ed71b36a26e47

commit f8bde5d301337c0994a9d71fc31ed71b36a26e47
Author: agrieve <agrieve@chromium.org>
Date: Sat Aug 13 02:42:02 2016

Reland #2 of Fail builds when lint warnings are generated

Previous: https://codereview.chromium.org/2236313004/

Reason for revert:
If this fails again, please revert changes that introduce
new warnings rather than this patch.

TBR=estevenson@chromium.org,caseq@chromium.org
BUG= 607761 

Review-Url: https://codereview.chromium.org/2241973002
Cr-Commit-Position: refs/heads/master@{#411864}

[modify] https://crrev.com/f8bde5d301337c0994a9d71fc31ed71b36a26e47/build/android/gyp/lint.py
[modify] https://crrev.com/f8bde5d301337c0994a9d71fc31ed71b36a26e47/build/android/lint/suppressions.xml
[modify] https://crrev.com/f8bde5d301337c0994a9d71fc31ed71b36a26e47/build/config/android/internal_rules.gni

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1155b192671fe54bd88883f511dc98d95a9b5489

commit 1155b192671fe54bd88883f511dc98d95a9b5489
Author: agrieve <agrieve@chromium.org>
Date: Sat Aug 13 03:46:47 2016

Revert of Fail builds when lint warnings are generated. (patchset #1 id:1 of https://codereview.chromium.org/2241973002/ )

Reason for revert:
Broke clank tot roller:
https://uberchromegw.corp.google.com/i/internal.client.clank_tot/builders/clang-clankium-tot-builder/builds/32680/steps/compile/logs/stdio

Original issue's description:
> Reland #2 of Fail builds when lint warnings are generated
>
> Previous: https://codereview.chromium.org/2236313004/
>
> Reason for revert:
> If this fails again, please revert changes that introduce
> new warnings rather than this patch.
>
> TBR=estevenson@chromium.org,caseq@chromium.org
> BUG= 607761 
>
> Committed: https://crrev.com/f8bde5d301337c0994a9d71fc31ed71b36a26e47
> Cr-Commit-Position: refs/heads/master@{#411864}

TBR=estevenson@chromium.org,caseq@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 607761 

Review-Url: https://codereview.chromium.org/2241833003
Cr-Commit-Position: refs/heads/master@{#411871}

[modify] https://crrev.com/1155b192671fe54bd88883f511dc98d95a9b5489/build/android/gyp/lint.py
[modify] https://crrev.com/1155b192671fe54bd88883f511dc98d95a9b5489/build/android/lint/suppressions.xml
[modify] https://crrev.com/1155b192671fe54bd88883f511dc98d95a9b5489/build/config/android/internal_rules.gni

Blockedon: 640267
Project Member

Comment 27 by bugdroid1@chromium.org, Aug 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/166a7cf0a0545439141f2e2fec69010f05c3f499

commit 166a7cf0a0545439141f2e2fec69010f05c3f499
Author: estevenson <estevenson@chromium.org>
Date: Fri Aug 26 14:39:21 2016

Reland of Fail builds when lint warnings are generated.

Previous: https://codereview.chromium.org/2241973002/

Original issue: https://codereview.chromium.org/2240573004/

Suppress all existing lint warnings so that builds will fail when new warnings
are generated. New lint warnings that are generated in files that are ignored
in suppressions.xml will not be caught. However, now we can remove lint
warnings on a file by file basis instead of having to fix all lint warnings
before making lint break builds.

BUG= 607761 

Review-Url: https://codereview.chromium.org/2241233002
Cr-Commit-Position: refs/heads/master@{#414708}

[modify] https://crrev.com/166a7cf0a0545439141f2e2fec69010f05c3f499/build/android/gyp/lint.py
[modify] https://crrev.com/166a7cf0a0545439141f2e2fec69010f05c3f499/build/android/lint/suppressions.xml
[modify] https://crrev.com/166a7cf0a0545439141f2e2fec69010f05c3f499/build/config/android/internal_rules.gni

In https://codereview.chromium.org/2241973002/, we suppressed all existing lint warnings and we now fail builds when lint warnings exist when building with GN. 

We can leave this bug open for tracking until we've addressed the lint warnings we suppressed.
Status: Fixed (was: Assigned)
We're now erroring on lint warnings (hurray!), so closing this.

Removing suppressed warnings can be tracked here:
https://bugs.chromium.org/p/chromium/issues/detail?id=635567

Sign in to add a comment