The files_to_skip lists in chrome/android/java/src/PRESUBMIT.py were
not working on Windows because they didn't use the verbose [\\\/]
construct for path separators. Fixing this in FilterSourceFile means
that multiple failures may be fixed simultaneously. The only failures
that this is known to fix are a set of AlertDialog.Builder errors that
were showing up only on Windows.
This addresses the issues that were going to be incorrectly fixed in
crrev.com/c/3606128 (now abandoned).
Bug: 1309977
Change-Id: I7a10483448e00df55e2cbf8bff8bad7a5d8db124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3609117
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
presubmit --all tells the presubmit system that all files are 'modified'
but ChangedContents still goes off to see what changes are present. If
most files were unchanged this was all handled perfectly, but if _all_
files were unchanged then _GitDiffCache would interpret the empty
dictionary of changes as a reason to ask git for diffs, millions of
times. This made some checks take more than 100x as long. The overall
effect on presubmit --all time is not known because I was never willing
to wait the multiple days for them to terminate.
That is, this would take many days to run:
git checkout -b empty -t origin/main
git cl presubmit --all
whereas a single-character change to any file would let this run in
about two hours.
After three weeks of working on presubmits I only hit this twice which
is why it took me so long to realize what the problem was.
Bug: 1309977
Change-Id: Ib280ea386107843b9174d835b0895316a5ed240c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3589900
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
It is easy to get type confusion and end up passing a list as the
message parameter to _PresubmitResult. This error will not be detected
until the end of the run - perhaps hours later - when all evidence of
where the list came from is lost.
This change ensures that the message parameter is a string. If it is not
then the exception that is thrown should allow quick identification of
the problematic code.
This also fixes a presubmit unit test that passed None as the message.
We could support that but I don't think that we should.
Bug: 1309977
Change-Id: Ifb1d5100d47922b0ebd8bb834caa6fbba690b43c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3566436
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Chromium's presubmits contain a lot of latent errors - presubmit errors
that will trigger the next time a file is modified, but have been
waiting for years. Flushing those out with "git cl presubmit --all"
works poorly because it exercises all presubmits simultaneously, which
is too slow and triggers too many failures. Adding a --files option lets
small areas of the tree or specific file types to be exercised in a
controlled manner.
Bug: 1311697
Change-Id: I36ec6a759a80000d6ed4a8cc218ece327d45f8d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3559174
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
When running git cl presubmit --all which is handy for finding
latent presubmit bugs, the 'No diff found for %s' gets printed many
thousands of times, making actual issues difficult to find. This change
suppresses that message, to make actual issues easier to find.
The message for slow presubmits prints the name of the slow presubmit,
but there are 243 different CheckChangeOnCommit functions, so the name
is actually not sufficient. Therefore this change plumbs through the
path to the script containing the presubmit and prints it.
Similarly, the cpplint message "Done processing %s" doesn't add enough
value.
These changes will make it easier to find the signal in the presubmit
noise.
Bug: 1309977
Change-Id: Iba40b5748266e3296eeb530bb00182db4814aa5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3556594
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
depot_tools has no versioning. It's hard to know if reported issues were
caused due to outdated depot_tools or actual unresolved bug.
This CL adds basic information about depot_tools version and it's
included in presubmit failure.
R=aravindvasudev@google.com, gavinmak@google.com
Change-Id: If8577c0826063693a7278a57a0cce629d4b1325f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3541061
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
git cl presubmit has --all option which marks all files as modified.
However, that doesn't work well with SCM diff. That is, git diff may not
contain affected file. Instead of throwing an exception in such case,
just return no diff.
R=gavinmak@google.com
Fixed: 808346
Change-Id: Ie1d534b8eebc84bc5206eba7db8057829390fbec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3485501
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Currently, post upload presubmit hooks are exlusively executed with py2,
regardless of USE_PYTHON3 magic variable. This change adds py3 support
in the same fasion as regular presubmit hooks.
R=aravindvasudev@google.com
Fixed: 1297712
Change-Id: Ib464f8563e4135a63fc48692d27c8692fe1f630b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3469285
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: I4b8f8fc521386419a3121bbb07edc8ac83170a94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413679
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
This reverts commit 22bf605bb6.
Reason for revert: breaks gclient sync
Original change's description:
> Use pylint 2.7 for depot_tools
>
> This includes a few fixes for specific errors, and disables several new
> warnings introduced in this version, in order to allow for an incremental migration.
>
> Bug:1262286
> Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bug: 1262286
Change-Id: Ieb946073c7886c7bf056ce843a5a48e82becf7a5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413672
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Gitwatch supports closing bugs with a FIX(ES|ED|ING)? tag in the
commit message, but we only check BUG to ensure a bug is
specified, add the fix fields to BugsFromDescription so that presubmits
will still pass if only eg: FIXES=b:1234 is given.
Gitwatch reference:
http://shortn/_LGPgBjgi3A
FIXED=b:203812728
TEST=presubmit_unittest.py doesn't fail more than it already does
Change-Id: I6afc38c786e281dcefb4d359bb9212ebe5980ed6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3237598
Auto-Submit: Sean McAllister <smcallis@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
When presubmit checks fail, we want to print the error message from the
underlying check/tool. We use _PresubmitResult objects to collect these
diagnostics and output them to the console and json files.
This change ensures that the string values passed to the
_PresubmitResult constructor will be converted, as needed, to 'str'
instances. This way, we will avoid encoding errors and make sure to
print the actual messages instead of repr(message).
SHERIFFS: this patch breaks backwards compatibility of presubmit error
handling but we don't expect issues in downstream projects. Please
revert if downstream presubmit checks start failing spuriously.
Bug: 1225052
Change-Id: If7f5c99cb5d730c1bfbd6d108b912f77b5f2455c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3037262
Commit-Queue: Tom McKee <tommckee@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
This CL adds a new field to the codereview.settings file used by
git_cl for project-wide defaults. If `USE_PYTHON3` is set to True,
then we will run the PRESUBMIT checks under Python3 by default
instead of Python2, unless the PRESUBMIT.py file contains
`USE_PYTHON3 = False` on a line by itself
(as opposed to now, where we'll use Python2 by default unless the
file contains `USE_PYTHON3 = True`).
This will allow us have Python3 be the default for new files
and to eventually eliminate any uses of `USE_PYTHON3` from the
individual presubmit files. Of course, you will have to go in and
explicitly add `USE_PYTHON3 = False` to any Py2-only files prior
to enabling this.
Bug: 1207012
Change-Id: Id8ec45b43940e5bcffeee196398c711c541e733e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2917747
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Currently, GitChange.AllFiles returns a list of byte[]. This is
inconsistent with py2 and may cause issues when byte[] is used when
string is expected.
Change-Id: I3e73bc5f71aa787f55357b8139cdeeabee056047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2946934
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
When presubmit checks create warnings that should raise a prompt
whether or not to continue, the output wasn't being flushed
properly under python3.
Bug: 1213316
Change-Id: Ibb4b4d965d49f0cd65ccc0737282e5f5b5f58004
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2921323
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Commit 6fc394f added some diagnostics, but it didn't quite work out the
way it was planned.
Bug: 1210746
Change-Id: Ia4f30606e87d3f8b1362441bd74bb7ed0f5ca886
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2928771
Auto-Submit: Greg Thompson <grt@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
If a presubmit happened to generate a crash, the logic we have
for reporting the presubmit check times would call `six.reraise()`
incorrectly, leading to a second crash.
This CL fixes the invocation of the second crash, and also adds
an error message to help users tripping over one possible source
of the first crash (reading a binary file as text).
Bug: 1210746
Change-Id: Ic46f38901b6acf2055b3feb7272dc751dc69037c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2921322
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Presubmits when uploading occasionally take a very long time - over a
minute - and after the fact there is no easy way to know why. This makes
fixes to slow presubmits take longer.
This change builds on crrev.com/c/2532895 to print a message when any
individual presubmit function takes longer than ten seconds. During
normal usage this is a NOP but it will presumably find the long poles
when presubmits are running particularly slowly.
This is similar to gclient runhooks where any hooks that take longer
than ten seconds have their run time printed.
Change-Id: If57ed35d7a7d221f6380e9b97cf72af56f75e441
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2911594
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
CheckForWindowsLineEndings is a Chromium presubmit to make sure that
\r\n line endings (Windows line endings) don't get committed. Among
other things these line endings mess up the license checks, leading to
cryptic errors.
The problem is that ChangedContents() was stripping line endings, making
any checking for them futile. This change adds an option to preserve
line endings, to be used by Chromium's CheckForWindowsLineEndings.
It's not clear how long this has been broken.
See also crrev.com/c/2911914 which must land after this.
Bug: 801033
Change-Id: I7cdb9b3581788624f9eca081da43bb070ee412a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2910488
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This fixes CL:2880843 to use a module already present in vpython.
Bug: 1206782
Change-Id: If4158a62011e043e89f40f368c9c644434df6614
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2901758
Commit-Queue: Allen Webb <allenwebb@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
This is a followup to CL:2797935 to make it easier to debug exceptions.
Bug: 1206782
Change-Id: Ibb649e94fd562919737de152e3203ca6b02b38ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2880843
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Allen Webb <allenwebb@google.com>
This CL adds support for running PRESUBMIT.py under either Python2
or Python3 as specified in each PRESUBMIT.py file.
To run the checks under Python3, the PRESUBMIT.py file must contain
a line exactly matching "^USE_PYTHON3 = True$". If the file
does not contain this string, the checks will run under Python2.
Different PRESUBMIT.py files in a single CL may thus contain
a mix of 2- and 3-compatible checks, but each individual file will
only be run in one or the other (it doesn't likely make sense to run
them in both by default).
Bug: 1157663
Change-Id: Ic74977941a6519388089328b6e1dfba2e885924b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2832654
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
This is a reland of 2cf835a9ba
URL-encoded repo path, so chromium/src becomes chromium%2Fsrc
as Gerrit expects
Original change's description:
> presubmit: Skip owners checks if code-owners plugin is enabled.
>
> If code-owners plugin is enable for the repo, skip owners check on
> commit, and skip checking owners format, as that will be done by
> the plugin.
>
> Change-Id: I1663baef4f0f27b00423071343fe740f6da50ce7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2727131
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Change-Id: I3038590f3a92cbf7b6dc0ba6eb47f72593a2ccf7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775840
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
This is a reland of 36de4be91e
Should be fixed after cl/363445481
Original change's description:
> presubmit: Use code-owners plugin if available.
>
> Change-Id: I56734de985731d007360a4a4e65f2b16de28b69a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2753894
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Change-Id: Id7d5c6c3bd0a246e3400ca99584bb41a92853d16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2774218
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This reverts commit e7c0581740.
Reason for revert: Whoops. chromium/src was updated. I was confused by a conversation on the #ops slack channel and my checkout being slightly out of date. I think the original change was actually okay.
Original change's description:
> Revert "presubmit: Don't skip OWNERS check when Bot-Commit+1 is present."
>
> This reverts commit 9757ad5883.
>
> Reason for revert: We need to update callers first (e.g., //PRESUBMIT.py in chromium/src).
>
> Original change's description:
> > presubmit: Don't skip OWNERS check when Bot-Commit+1 is present.
> >
> > Change-Id: I17b07796a86c5214e13a0058428889c1bb2b850d
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2774080
> > Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> > Reviewed-by: Jason Clinton <jclinton@chromium.org>
> > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
>
> Change-Id: I02c3d5ea2e65ef852d34a6816d04fe1cad82823a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775101
> Auto-Submit: Dirk Pranke <dpranke@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: I2c1ae8c60938cbd9316e9075488cc7451068a2f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775103
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Auto-Submit: Dirk Pranke <dpranke@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
This reverts commit 2cf835a9ba.
Reason for revert: doesn't work with depot_tools
error 404 Not found: tools (with applied patch from 2729404)
Bug: 1183975
Original change's description:
> presubmit: Skip owners checks if code-owners plugin is enabled.
>
> If code-owners plugin is enable for the repo, skip owners check on
> commit, and skip checking owners format, as that will be done by
> the plugin.
>
> Change-Id: I1663baef4f0f27b00423071343fe740f6da50ce7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2727131
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Change-Id: Id4d560701e4b0144e0aafc5263e09ed6927f6222
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2729409
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: I22e6a9d4405d92d26dfbfba64735b6e9ce92674c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2729404
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
If code-owners plugin is enable for the repo, skip owners check on
commit, and skip checking owners format, as that will be done by
the plugin.
Change-Id: I1663baef4f0f27b00423071343fe740f6da50ce7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2727131
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Will be used in a follow-up CL to initialize code-owners client.
Recipe-Nontrivial-Roll: build
Change-Id: Iefe9176320b4d1ae7715e88a8db132e815be76ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2717979
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Bruce Dawson
Josip Sokcevic
Louis Romero
Andrew Grieve
Aravind Vasudevan
Sean McAllister
Tom McKee
kyle Ju
Dirk Pranke
Greg Thompson
Allen Webb
Erik Staab
Gavin Mak
Edward Lesmes
Edward Lesmes
Stephen Martinis