Commit Graph

338 Commits (6a1494e5d76a4d03d4a0ba1847110df1a8d1c868)

Author SHA1 Message Date
Bruce Dawson e6bbb3a237 Avoid redundant global presubmit checks
PanProjectChecks is called twice, once from the root PRESUBMIT.py and
once from third_party/blink/PRESUBMIT.py. For the file-based checks a
series of filters avoids redundant reporting, but certain global checks
were being run twice, potentially causing duplicate reporting.

This adds an optional global_checks flag which can be set to false in
the blink PRESUBMIT.

Bug: 1309977
Change-Id: Ia43a7a0f29cf7b58ac9bab0e9aa374e9bf0155a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3611409
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
3 years ago
Bruce Dawson 9b9f451add Let git cl presubmit work with no network
Bug: 1309977
Change-Id: Ifca6e60c1fb2ff76f6a8dee9129d4d661ba196a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3606733
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson 89ccf4a805 Don't check header guards in cpplint.py
Header guards are checked by CheckForIncludeGuards which has broader
coverage than cpplint.py and should therefore be preferred.

Having two include-guard checkers that cover different sets of files and
use different tags to silence warnings is just confusing.

Bug: 1309977
Change-Id: Ic0d1ad610cf9c34d6777d852da2e2e22b8686552
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3587725
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson ab2e7f8f2e Improve canned checks output
When CheckLongLines finds problems it prints the first five, but this
gives no indication as to whether there are just five, or hundreds.
This change prints the number of long lines found.

The snapshot function in PanProjectChecks prints the elapsed time for
long checks, however it has two issues that make it inconvenient. One is
that it prints the time in ms which sounds great until you get a warning
that one of the checks is taking 2041373 ms (not kidding, although now
fixed) which is tricky to read. The other problem is that it was
actually measuring CPU time, not wall-clock time. This changes it to
print the time in seconds (to 0.1 seconds) and to measure elapsed
time.

Bug: 1309977
Change-Id: I7564a8cdf7bb3349b10ebbddbfe179188d4bf309
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3587726
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson 07bfa0dd65 Make CheckLicense ~60x faster
After optimizing CheckForIncludeGuards the CheckLicense presubmit was
the second slowest presubmit, taking about 35 minutes when run as part
of "git cl presubmit --all". The cause of its slowness was initially
non-obvious because it runs quite quickly on most files, however
analysis showed that it could take 50+ seconds on some files. The files
that it is slow on are those that lack a good license header, meaning
that the regex match has to painstakingly scan the entire file.

The optimization in this change is to recognize that there is a simple
non-regex line that appears in all valid license headers, regardless of
variants. If that line is absent then there is, necessarily, no valid
license header, and searching for a line of text is something that
Python can do extremely quickly.

This change drops the CheckLicense time from about 35 minutes to about
32 seconds.

Trivia: _CommonChecks in third_party/blink/PRESUBMIT.py passes .* as the
license, so I added an early-out for that to avoid pointlessly scanning
those files.

Bug: 1309977
Change-Id: Ic2e56079675c2c5a2643d20dd492b1cc52e4ead2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3584882
Reviewed-by: Erik Staab <estaab@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson 028f4615ad Fix CheckDirMetadataFormat for Windows command limits
CheckDirMetadataFormat executes dirmd on all applicable files. When
running "git cl presubmit --all" that ends up being ~7,500 files and
the command line that is generated is ~500,000 characters. Windows does
not like that.

This change breaks up the check into multiple invocations of dirmd in
order to avoid these limits. This is important because running all
presubmits is the only way to systematically find bugs, and avoid being
surprised by them when submitting a change.

Bug: 1309977
Change-Id: I24fbc340cdb975dbe7f6a2132e516d6f7e2f9165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3554633
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Aravind Vasudevan c5f0cbb865 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: 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>
3 years ago
Josip Sokcevic 42c5bbbc96 Revert "Use pylint 2.7 for depot_tools"
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>
3 years ago
Aravind Vasudevan 22bf605bb6 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>
3 years ago
Takuto Ikuta 26bfecdd07 include filename in LUCI config validation
Fixed: 1254615
Change-Id: Ic6d5350c7a89a597c1126ec05235a3e63eb2ad09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3312231
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Anthony Polito <apolito@google.com>
Reviewed-by: Anthony Polito <apolito@google.com>
3 years ago
Collin Baker dfff8abbdf Disable whitespace lints handled by clang-format
These lints are redundant and in rare cases may conflict with
clang-format output. We treat clang-format's output as the correct
formatting, so disable the lints.

The following whitespace lints are retained, all others are disabled:
whitespace/blank_line
whitespace/comments
whitespace/empty_conditional_body
whitespace/empty_if_body
whitespace/empty_loop_body
whitespace/ending_newline
whitespace/todo

Bug: chromium:1240500
Change-Id: I22e2d8ea4de021ea389e155d8c9e096c3ae6f8b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3231485
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
4 years ago
Josip Sokcevic 59e3296a7e Add pylint-2.7
R=apolito@google.com, dpranke@google.com

Bug: 1257792
Change-Id: I61a37d4b48dde0b7e1975d95d8fd627b9cbfb65b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3214207
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
4 years ago
Dominic Battre e5d0a56e04 Warn user in case they use commit messages with "Bug=" instead of "Bug:"
Chrome uses the syntax "Bug: xyz" in commit messages to attribute CLs
to bugs while Critique uses "BUG=xyz". As per
```
git log --grep '\(Bug\|Fixed\)=' --since=2021-01-01 --format=oneline \
| wc -l
```
we've had 27 CLs this year using the wrong syntax which led to crbug.com
not learning about the CLs <-> Bug attribution. Yours truly caused one
of them and wants to prevent the problem in the future.

We might relax the requirments in crbug.com or
chromium-review.googlesource.com but then anyone grepping through the
commit logs would need to deal with the ambiguity. Therefore, we can
just enforce the right syntax here.

https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/contributing.md
contains the correct syntax.

Change-Id: I60ee579deac50a74e1a014ceb1d9744cbc10ab41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3141567
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Dominic Battré <battre@chromium.org>
4 years ago
Josip Sokcevic 8daaf5c5b0 Explicitly set python version for pylint 2.6
vpython doesn't parse shebang. If python version is not explicitly set
in vpython file, version 2 will be used. pylint 2.6 requires py3.

R=dpranke@google.com

Bug: 1242737
Change-Id: I21a3c4b26c24860b4c8fd07c6d5cc6d5149af2fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3120321
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
4 years ago
Dirk Pranke 7a262eba20 Fix pylint canned check for pylint-2.6.
We were attempting to run pylint-2.6 with python2, when
it needs to be run with python3. I'm not sure how we
haven't noticed this before now.

Bug: 1157676
Change-Id: I60fc22a805c20649d04f7d0f53c828a0db41e8e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3068833
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
4 years ago
Johann f6f3c7a46e [docs] update links to developer guide
Change-Id: I906654a06f2833c8ca4089aa7eae3bab104f013a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3020735
Auto-Submit: Johann Koenig <johannkoenig@google.com>
Commit-Queue: Anthony Polito <apolito@google.com>
Reviewed-by: Anthony Polito <apolito@google.com>
4 years ago
Josip Sokcevic 48c947534d Add presubmit notice if buganizer bug is malformed
If user enters buganizer bug prefixed with b/, warn user that b: is
expected.

R=apolito@google.com

Bug: 1226474
Change-Id: Ifbced843f9eecc00560cc273ae06bff9d435a815
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3016587
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
4 years ago
Josip Sokcevic f0d7ed89cf Allow custom filters for json validation
JSON check may be useful for files that don't necessaraly end with
.json. For example, infra/config/recipes.cfg should be a json file. This
change allows users to add custom file_filter that can include such
files.

R=apolito@google.com

Bug: 1223923
Change-Id: Ia6fc7f86fa368510baaee978d9a0a27eccb6b31f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2992735
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
4 years ago
Peter Wen 866be0f290 Add an option to specify pylint version
This allows us to migrate PRESUBMIT.py files over to pylint-2.6 bit by
bit. It also allows us to support and upgrade pylint to newer versions
in the future.

Bug: 1221143
Fixed: 1221143
Change-Id: I9af96f5f06caf48e9923ad5fae75b98a0a7aeb49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2994723
Commit-Queue: Peter Wen <wnwen@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
4 years ago
Fabrice de Gans a73eec2873 Add a new argument to skip shebang checks on python3
A bug in the logic enabling tests to be run on python3 resulted in
tests never being run in python3 unless they also have a shebang line
pointing to python3. Since this bug was introduced, effectively, most
tests that are supposed to be run on python3 do not run at all. Since
then a lot of python3 tests have regressed in Chromium.

In order to fix the issue moving forward, this introduces a new
argument that skips the shebang check to run a test in python3. Tests
will be re-enabled one by one until every single instance in Chromium
has been updated.

Bug: 1223478
Change-Id: I91a0688c6f4d9b4fbf18e3d446366cded8c7f2f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2986400
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
4 years ago
Bruce Dawson 3b56309183 Ignore \r characters in CheckLicense
If you copy\paste a license header in such a way that \r\n line endings
are used (I have done this) then CheckLicense would fail. This is quite
confusing because the license looks perfect in most text editors. This
change tells CheckLicense to treat \r\n line endings as equivalent to \n
and let CheckForWindowsLineEndings do its job of warning about the \r\n
line endings. This avoids confusing presubmit messages.

Bug: 801033
Change-Id: I37fa4a6d9cd0f1a4dcce1267175f4b8fd298b906
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2951788
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
4 years ago
Josip Sokcevic 104ca4d094 Use input_api.sys instead of import sys
Follow up on https://crrev.com/c/2929386

R=dpranke@google.com

Change-Id: Id977cfae561ecfb231372348387b084e098e6747
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2947981
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
4 years ago
Dan Harrington a0b0f12874 Fix GetPylint with USE_PYTHON3=True
kwargs['stdin'] is passed in to subprocess communicate(), which
expects bytes, not str.

Bug: 1215602
Change-Id: I95326eed08cffecd31abc6b0380de2a3c2ff3dd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2929386
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Dan H <harringtond@chromium.org>
4 years ago
Bruce Dawson b0bfaf1b3d Strip Windows line endings when checking licenses
The CheckLicense presubmit is supposed to check for correct licenses,
not for correct line endings. By reading the input file in binary mode
it implictly does both, leading to confusing error messages.

This is exacerbated by bugs in the line ending checks which are being
addressed separately. This change will avoid false positives on files
that have Windows line endings.

Change-Id: I2ff8632f273ec4896cb4918c386e0d1c12e72935
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2910486
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
4 years ago
Dirk Pranke 51e3708841 Reland "Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor."
This reverts commit 3e9eda1734.

Reason for revert: Trying again with a fix.

Original change's description:
> Revert "Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor."
>
> This reverts commit dd0076703b.
>
> Reason for revert: Change wasn't correct.
>
> Original change's description:
> > Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor.
> >
> > This will attempt to do so in a Python2-compatible way.
> >
> > Bug: 1210746
> > Change-Id: I09edc21a5c47106803c0ac5ca449e0f8732efb24
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2906501
> > Auto-Submit: Dirk Pranke <dpranke@google.com>
> > Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
>
> Bug: 1210746
> Change-Id: Ia101d73857abe3d65ba48e05e7a0bc98efb2fd37
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2908152
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Dirk Pranke <dpranke@google.com>

Bug: 1210746
Change-Id: I6b7b2a24dd5565740a0561c8eeb860d62375d4bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2908153
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
4 years ago
Dirk Pranke 3e9eda1734 Revert "Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor."
This reverts commit dd0076703b.

Reason for revert: Change wasn't correct.

Original change's description:
> Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor.
>
> This will attempt to do so in a Python2-compatible way.
>
> Bug: 1210746
> Change-Id: I09edc21a5c47106803c0ac5ca449e0f8732efb24
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2906501
> Auto-Submit: Dirk Pranke <dpranke@google.com>
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

Bug: 1210746
Change-Id: Ia101d73857abe3d65ba48e05e7a0bc98efb2fd37
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2908152
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
4 years ago
Dirk Pranke dd0076703b Try again to fix UnicodeDecodeError in CheckAuthorizedAuthor.
This will attempt to do so in a Python2-compatible way.

Bug: 1210746
Change-Id: I09edc21a5c47106803c0ac5ca449e0f8732efb24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2906501
Auto-Submit: Dirk Pranke <dpranke@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Dirk Pranke 4914c83778 Revert "Specify encoding for AUTHORS file"
This reverts commit 68324eff2f.

Reason for revert: I think we need to revert this, because if someone tries to upload a CL from a revision older than when we switched to Python3, this'll run under Python2, where that parameter isn't accepted.

Sorry, I didn't think that part through.

Original change's description:
> Specify encoding for AUTHORS file
>
> Needed to resolve "UnicodeDecodeError" for Python 3 presubmit checks.
>
> Bug: 1210746
> Change-Id: If55190ffc9f99c671a84fe9b3539317e464639cc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2904927
> Commit-Queue: Dirk Pranke <dpranke@google.com>
> Auto-Submit: Alan Screen <awscreen@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@google.com>

Bug: 1210746
Change-Id: I4e5a58869baa897066f4e84c2e18b673c25bcc34
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2906499
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>
4 years ago
Alan Screen 68324eff2f Specify encoding for AUTHORS file
Needed to resolve "UnicodeDecodeError" for Python 3 presubmit checks.

Bug: 1210746
Change-Id: If55190ffc9f99c671a84fe9b3539317e464639cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2904927
Commit-Queue: Dirk Pranke <dpranke@google.com>
Auto-Submit: Alan Screen <awscreen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
4 years ago
Evan Stade 288e51c0ee Refactor canned presubmit check for license header.
This allows it to be more easily reused in a project, e.g.
https://chromium-review.googlesource.com/c/chromium/src/+/2895086

Bug: 1209079
Change-Id: Ib50f2fcca3489dda60328589a7489d76f1d83ffe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2896584
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Evan Stade <estade@chromium.org>
4 years ago
Dirk Pranke 04cf3de933 Fix a Py3 bug in presubmit_canned_checks.py.
There were a couple of references to using filter() that
needed to be replaced with list comprehensions as the return
values may be used repeatedly.

Bug: 1207012
Change-Id: Iae0abd38aeb186d6f7945f8c848c1f7312cb45bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2890329
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
4 years ago
Dirk Pranke 61bf6e8d69 Add support for running PRESUBMIT.py checks under Python2 or 3.
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>
4 years ago
seanmccullough faaed2f486 [depot_tools, chromium/src] mv CheckInclusiveLanguage into canned checks
Also adds a new canned_presubmit_checks_test.py and supporting mocks,
based on the PRESUBMIT_*.py under chromium/src.

If this is OK, there are subsequent CLs for removing the original code
from chromuim/src and infra/infra, and having their PRESUBMIT scripts
just reference this canned check instead.

Change-Id: I67dfb7ac0b4cdc36bd62ec2dc062ca5c78c2244e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2805268
Commit-Queue: Sean McCullough <seanmccullough@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
4 years ago
Stephen Martinis 7837307d38 Delete CheckBuildbotPendingBuilds
This check has been disabled basically everywhere since about 2013. It
should be safe to remove this; there will be warnings when trying to
skip it, but it shouldn't fail any builds.

Recipe-Nontrivial-Roll: build
Bug: 1194216
Change-Id: I84878f9e46543ae71f7e4e057b99d15a034e0d0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2794799
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
4 years ago
Edward Lesmes c900fe5760 presubmit: Also skip owners check on upload if code-owners is enabled.
Change-Id: I6f136321750b4e9f2514f04a6f357272d35e9b4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775715
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Edward Lesmes 8170c29fd1 Reland "presubmit: Skip owners checks if code-owners plugin is enabled."
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>
4 years ago
Edward Lesmes c456617ddd presubmit: Allow Bot-Commit+1 to self-approve change.
Change-Id: I29a7aeb6f797be015f29e1e514bbe44988a1221e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775839
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Dirk Pranke e4bab45474 Reland "presubmit: Don't skip OWNERS check when Bot-Commit+1 is present."
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>
4 years ago
Dirk Pranke e7c0581740 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>
4 years ago
Edward Lesmes 9757ad5883 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>
4 years ago
Josip Sokcevic ec3c39536a Revert "presubmit: Skip owners checks if code-owners plugin is enabled."
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>
4 years ago
Edward Lesmes 2cf835a9ba 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>
4 years ago
Edward Lesmes ac34821d37 presubmit: Make email_regexp argument optional.
Make email_regexp argument to GetCodeReviewOwnerAndReviewers
optional. This will make it possible to remove it from
downstream PRESUBMIT.py scripts that pass it.

Change-Id: I45168c1f4137e7e797b30d050e4ee82f6e26daf3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2679763
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Edward Lesmes e342fb16f9 Don't suggest change author as reviewer.
Change-Id: Ie34fc34883b2c86fc134f10ded24b26a631886ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2676835
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Josip Sokcevic 8c95595001 Remove non-inclusive parameters from presubmit
R=ehmaldonado@google.com

Bug: 1118435, 1118436
Change-Id: Ie66b2cb39a3d8abc83679bee44d2e594eba77442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2657981
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
4 years ago
Edward Lesmes 1076f38947 Reland "presubmit: Use new API to check for owners approval"
This is a reland of 0489cc12af

Don't remove email_regexp argument, as downstream still passes it.

Original change's description:
> Reland "presubmit: Use new API to check for owners approval"
>
> New API was updated to properly support '*' as owner.
>
> Change-Id: If14144f83484731fd5534c03cb9fde4b18f49fe9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2628703
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

Change-Id: Id137aca0036c2ebf11ec56a12f4e053cd2cc6637
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2639411
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
4 years ago
Edward Lesmes 0eb104764a Revert "Reland "presubmit: Use new API to check for owners approval""
This reverts commit 0489cc12af.

Reason for revert:
Chromium presubmit needs to be updated too

Original change's description:
> Reland "presubmit: Use new API to check for owners approval"
>
> New API was updated to properly support '*' as owner.
>
> Change-Id: If14144f83484731fd5534c03cb9fde4b18f49fe9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2628703
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,gavinmak@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com

Change-Id: Id6f55a8fbc692ad1a82154a6646d487bb37cbe63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2637539
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Edward Lesmes 0489cc12af Reland "presubmit: Use new API to check for owners approval"
New API was updated to properly support '*' as owner.

Change-Id: If14144f83484731fd5534c03cb9fde4b18f49fe9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2628703
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Ben Pastene 703f21e217 Revert "presubmit: Use new API to check for owners approval"
This reverts commit 64d94deaa6.

Reason for revert: tentative revert for https://crbug.com/1166467

Original change's description:
> presubmit: Use new API to check for owners approval
>
> It also allows us to improve unit tests.
>
> Change-Id: I356a2fddcbcc5af0e628f79ede1ba277008f5cde
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2612222
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,gavinmak@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com

Bug: 1166467
Change-Id: I1df97f8fdbc56942fdcc7bafffed517e24a9481d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2628574
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
4 years ago
Edward Lesmes 64d94deaa6 presubmit: Use new API to check for owners approval
It also allows us to improve unit tests.

Change-Id: I356a2fddcbcc5af0e628f79ede1ba277008f5cde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2612222
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago