Commit Graph

395 Commits (b8164180d2c339940c99db74344c865e67ca064d)

Author SHA1 Message Date
Christoffer Jansson 013e47be09 Revert "Fix not-run-test detection."
This reverts commit 41691abe86.

Reason for revert: I thought https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683378 would solve the problem but turns out this is the issue where it now reports it as a critical error.

Original change's description:
> Fix not-run-test detection.
>
> In crrev.com/c/2986400 the skip_shebang_check flag was added to address
> the problem of some tests being run neither on Python 2 or Python 3. At
> the same time reporting was added to warn if tests were run on neither
> Python version.
>
> Unfortunately the reporting code was incorrect. A PresubmitPromptWarning
> object was created but was not added to the list of results, so it had
> no effect. Also the skipped tests name was added where a list was
> expected which meant that each character of the test name showed up on
> its own line.
>
> This change fixes the reporting. It also changes the format of the
> report and makes it an error - a failure to run tests at all deserves
> that severity level.
>
> A test run with the fixed check showed that no errors have crept in -
> all tests were being run with one or the other. However the fix did
> find a bug in an in-progress change I was working on.
>
> Bug: 1223478
> Change-Id: Ibb44b5e60e7a7a5de08302f19ee4035cdfac5212
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3674199
> Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
> Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
> Reviewed-by: Fabrice de Gans <fdegans@chromium.org>

Bug: 1223478
Change-Id: If4395e90784c3f91692f398b4e5230a51139f78a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683105
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Christoffer Jansson c03ebf0e1a Revert "Add Python 3 support to GetPythonUnitTests"
This reverts commit dfc71bbe01.

Reason for revert: Breaks all presubmits that do not have skip_shebang_check=True set.

Original change's description:
> Add Python 3 support to GetPythonUnitTests
>
> GetPythonUnitTests (and by extension RunPythonUnitTests) always ran
> scripts using Python 2. This change adds a python3=False parameter to
> GetPythonUnitTests to give the option of switching to Python 3, while
> leaving the default as Python 2. The child scripts are run under one or
> the other, based on this parameter.
>
> This change is needed in support of crrev.com/c/3679801.
>
> Bug: 1313804
> Change-Id: Ic59287352d4941707adaf7981ed7af4201b7d526
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3680099
> Reviewed-by: Jesse McKenna <jessemckenna@google.com>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>

Bug: 1313804
Change-Id: I1bd7096b6cfdfed6205e6dc0b5d4498b5b821b97
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683378
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Christoffer Jansson <jansson@chromium.org>
Owners-Override: Ben Pastene <bpastene@chromium.org>
Reviewed-by: Tomas Gunnarsson <tommi@chromium.org>
3 years ago
Bruce Dawson dfc71bbe01 Add Python 3 support to GetPythonUnitTests
GetPythonUnitTests (and by extension RunPythonUnitTests) always ran
scripts using Python 2. This change adds a python3=False parameter to
GetPythonUnitTests to give the option of switching to Python 3, while
leaving the default as Python 2. The child scripts are run under one or
the other, based on this parameter.

This change is needed in support of crrev.com/c/3679801.

Bug: 1313804
Change-Id: Ic59287352d4941707adaf7981ed7af4201b7d526
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3680099
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson 41691abe86 Fix not-run-test detection.
In crrev.com/c/2986400 the skip_shebang_check flag was added to address
the problem of some tests being run neither on Python 2 or Python 3. At
the same time reporting was added to warn if tests were run on neither
Python version.

Unfortunately the reporting code was incorrect. A PresubmitPromptWarning
object was created but was not added to the list of results, so it had
no effect. Also the skipped tests name was added where a list was
expected which meant that each character of the test name showed up on
its own line.

This change fixes the reporting. It also changes the format of the
report and makes it an error - a failure to run tests at all deserves
that severity level.

A test run with the fixed check showed that no errors have crept in -
all tests were being run with one or the other. However the fix did
find a bug in an in-progress change I was working on.

Bug: 1223478
Change-Id: Ibb44b5e60e7a7a5de08302f19ee4035cdfac5212
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3674199
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
Reviewed-by: Fabrice de Gans <fdegans@chromium.org>
3 years ago
Bruce Dawson ac96470e3e Tag cpplint errors
A typical cpplint error report during a presubmit looks like this:

    .../linux_ui.cc:64:  Almost always, snprintf is better than strcpy  [runtime/printf] [4]
    ...
    Changelist failed cpplint.py check.

The problem is that the details of the error go to stderr when
CheckChangeLintsClean is run, whereas the "Changelist failed cpplint.py
check." message is printed at the end of the run. If there a lot of
errors, warnings, or messages then the association between them can be
completely lost.

This change adds the tag (cpplint) to the error messages, and adds a
comment to the "failed cpplint.py check" message to suggest looking for
the tag.

Bug: 1309977
Change-Id: I0d073b57f215e28495cbc3e009cab6ac2f23b65e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3632947
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
3 years ago
Bruce Dawson 09c0c073ea Optimize presubmit --all with --no_diffs option
Some of the expensive checks when running presubmit --all, such as
CheckStableMojomChanges (~300 s) and CheckAddedDepsHaveTargetApprovals
(~200 s) only look at diffs and are therefore guaranteed to be NOPs when
running presubmit --all or --files=. Passing along the no_diffs state
lets these expensive checks be skipped, thus allowing for faster
iteration times.

Initial testing suggests that (with some supporting changes in the
Chromium repo) this reduces "presubmit --all" times by about ten
minutes, or a bit more than 10%, and additional improvements may be
possible.

Special handling for the no-diffs case also offers a simple way to avoid
presubmit failures that happen whenever all files are flagged as being
changed.

Finally, and perhaps most importantly for having a presubmit --all bot,
when --no_diffs is passed we can treat errors like "Issue wasn't
uploaded" and "Add a description to the CL" as messages, thus making it
possible to have zero presubmit errors when run on origin/main.

Bug: 1320937, 1322936
Change-Id: I0d09dd4aae8fdaa48c8b2f89337441cf96dcff72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3628368
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
3 years ago
Bruce Dawson 00de780a6e Improve cpplint defaults
While enabling cpplint on base I found an extraneous error and an overly
complicated invocation of CheckChangeLintsClean. This change disables
the extraneous error and improves the file filtering defaults to make it
easier to invoke cpplint.

The warning was:

    base\win\win_util.cc(193):  Missing username in TODO; it should look
    like "// TODO(my_username): Stuff."  [readability/todo] [2]

Chromium has over 10,000 TODO comments that are links to bugs instead of
users so we clearly want to suppress this warning.

Most calls to CheckChangeLintsClean in Chromium pass in custom file
filtering in order to only analyze C++ files. Failing to pass a file
filter leads to errors like this:

    Ignoring base\win\embedded_i18n\create_string_rc.py; not a valid
    file name (cc, cuh, cu, cpp, h)
    Ignoring c:\src\chromium\src\components\arc\PRESUBMIT.py; not a
    valid file name (cu, cc, cuh, cpp, h)

It doesn't make sense for the canned checks version of
CheckChangeLintsClean to default to passing in files that cpplint will
complain about so if source_file_filter is None then I configure an
appropriate filter.

This change also switches to vs7 output format by default on Windows,
and deletes a trailing space.

Bug: 1309977
Change-Id: I342ca9e56a41502755dfc09307b441dfa6a48ec0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3655592
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
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
Edward Lesmes c1aa4ecfcc Reland "presubmit: Don't print comments for missing reviewers."
This is a reland of 276bab4c6e

Fixed to include author on the list of reviewers.

Original change's description:
> presubmit: Don't print comments for missing reviewers.
>
> Don't print owners comments when suggesting missing reviewers,
> as this won't be supported when using Gerrit API.
>
> Change-Id: I644b2f87b58a3c9ece3aa7d107100f3501bd3071
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2607675
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>

Change-Id: I6dfb2087022ad87595e6cabc50d42a8bdf6311b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2615918
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
4 years ago
Edward Lesmes 3c57b27ae1 Revert "presubmit: Don't print comments for missing reviewers."
This reverts commit 276bab4c6e.

Reason for revert:
Possible cause of crbug.com/1163731

Original change's description:
> presubmit: Don't print comments for missing reviewers.
>
> Don't print owners comments when suggesting missing reviewers,
> as this won't be supported when using Gerrit API.
>
> Change-Id: I644b2f87b58a3c9ece3aa7d107100f3501bd3071
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2607675
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Reviewed-by: Josip Sokcevic <sokcevic@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: 1163731
Change-Id: I0b3d0f352e5483e754f6b790f70a9278df4eb3e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2613924
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Edward Lesmes 276bab4c6e presubmit: Don't print comments for missing reviewers.
Don't print owners comments when suggesting missing reviewers,
as this won't be supported when using Gerrit API.

Change-Id: I644b2f87b58a3c9ece3aa7d107100f3501bd3071
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2607675
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
4 years ago
Edward Lesmes d79114a2ed [presubmit] Add check to prevent new metadata in OWNERS files.
Bug: 1113033
Change-Id: I000cd307fea8862b9ef9cab907aafd357ee761bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2582566
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
4 years ago
Edward Lesmes 1ec57124a6 [presubmit] Add option to ignore TBR on PRESUBMIT.py checks.
Change-Id: Ibd1a5fa67aca0c273e8289d0726afeaeb88754d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2568908
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
4 years ago
Edward Lesmes cf49cb8206 [presubmit][owners] Skip owners check when Owners-Override is +1
Add support for Owners-Override label to allow trusted bots, sheriffs,
and global owners to bypass owners checks.

Bug: 1093627
Change-Id: Ie217369a67e4060807c0a12f84430e30a7c57daa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2530488
Reviewed-by: Yulan Lin <yulanlin@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Edward Lesmes 02d4b82580 [presubmit][owners] Skip owners check when Bot-Review is +1
Add support for Bot-Review label to allow trusted bots to
bypass owners checks.

Bug: 1093627
Change-Id: Ie7398041e9e32fd87bb415b239b5ec0a4740e1ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2530516
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Yulan Lin <yulanlin@google.com>
5 years ago
Andrew Grieve 02ffb6f47a CheckDirMetadataFormat: LocalPath() -> AbsoluteLocalPath()
LocalPath() should be used in error messages, but never when actually
trying to access a path.

Bug: 1127507
Change-Id: I03ec5421891ac67a8620cd3c7e88c2de1811e914
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2418914
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
5 years ago
Josip Sokcevic c6aa151180 Warn only if non-inclusive term is used
This fixes the issue where presubmit_canned_checks issues a warning if
optional arguments in CheckAuthorizedAuthor are not used at all.

This relands commit b09f2bb2f2 with some
additional changes.

R=apolito@google.com

Bug: 1098560
Change-Id: If323d90ab7d6bcca68ed89142ea67edc4be057d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2373216
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago
Josip Sokcevic b09f2bb2f2 Revert "Warn when non-inclusive parameters are used"
This reverts commit 149a3b7581.

Reason for revert: warnings are issued if no optional parameters
are passed.

Original change's description:
> Warn when non-inclusive parameters are used
> 
> R=​apolito@google.com
> 
> Bug: 1098560
> Change-Id: Iabc2a5d89a6af1418c1228f3c09eacbbd799b5c7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2355112
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>

TBR=apolito@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1098560
Change-Id: Ib0f188de90121c692f5c0d7271938ee312917fd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2376042
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago
Josip Sokcevic 149a3b7581 Warn when non-inclusive parameters are used
R=apolito@google.com

Bug: 1098560
Change-Id: Iabc2a5d89a6af1418c1228f3c09eacbbd799b5c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2355112
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago
Edward Lesmes 7291750ae2 presubmit: Add dirmd_bin to CheckDirMetadataFormat.
dirmd is not available on bots, so we'll need to receive the path
where it's available as an argument.

Bug: 1102997
Change-Id: I8966225d8ded3eb06c504797280baaa6c6c64a9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2321510
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Edward Lesmes a58287b8ee presubmit: Also validate metadata in OWNERS files.
Bug: 1102997
Change-Id: Id847129d320004fcd121941f2e14cf9ad56ab4a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2318335
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Josip Sokcevic d855b92743 Make presubmit_canned_checks inclusive
This modifies CheckAuthorizedAuthor parameters to be inclusive while
being backwards compatbile. Also, it fixes issue with empty lists.

R=ehmaldonado@chromium.org

Bug: 1098560
Change-Id: I992b25d6408d6cb906fa1696209de253e50c5156
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2316706
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago
Lei Zhang b8c62cf5f4 Add default filters to git cl lint.
Use the same set of default filters as presubmit checks. Presubmit
checks use filters to give better lint results as expected by Google C++
style. Give git-cl's lint subcommand the same benefit.

Bug: 1106028
Change-Id: I51ff0737b310990ab424376d2721e8a6a50dfc6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2298122
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Edward Lesmes 6d938a0d85 presubmit_canned_checks: Add DIR_METADATA presubmit checks.
- Add a check to validate DIR_METADATA files.
- Add a check to enforce that OWNERS files contain no metadata if a
  DIR_METADATA file is present in the same directory.

Bug: 1102997
Change-Id: Iedb09941001577dc81b1dda2241481ff667b2568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2298330
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
5 years ago
local_bot 6402141d1f Rename blocklist/allowlist to skip_list/check_list
This change is backwards compatible.

R=dpranke@google.com

Bug: 1098560
Change-Id: Ic9447041a68b8c379f65900af11c89b006e3f806
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2285156
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
5 years ago
local_bot 30f774ef87 Rename whitelist/blacklist to allowlist/blocklist
R=apolito@google.com, ehmaldonado@chromium.org

Bug: 1098562
Bug: 1098560
Change-Id: I45dc739f9e6d1a2e9bfcd9fa0587f4bb4a115c5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2265255
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
5 years ago
Peter Kasting 5ff7ad23ae Remove lint checks for non-const references.
These are now allowed by the Google style guide.

Bug: none
Change-Id: Ib182011ce489ee218cf673842be633626507cd60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2248714
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
5 years ago
Leandro Lovisolo 44de5e3f65 presubmit_canned_checks.py: Ignore long TypeScript imports.
Change-Id: If9e133002e2cdcff42917a487a677f3ed73d4d3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2241184
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
5 years ago
Aaron Gable 6e7ddb698b Remove agable from depot_tools OWNERS and TODOs
Change-Id: I0667af4562208a111ab3673f3fca6aab655f5f26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2218962
Auto-Submit: Aaron Gable <agable@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Anthony Polito <apolito@google.com>
5 years ago
Robert Brandt beb877fe49 depot_tools: bugfix for py3 syntax and bytes/str changes
Also update copywrite header to conform to expectations.

BUG=chromium:1082354
TEST=locally, can upload commit with this fix

Change-Id: I5527cf9cc58fbab94f777b82008a422a3190f539
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2199884
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Derek Morris 053213d049 Add a pan-project presubmit check for commits in tree
It's possible to add commit objects to the tree, and this can cause
some confusion for git's tools - if this happens to overlap with an
actual folder or a gclient-imported repo, the tree will be dirty.

This change finds this issue by scanning the tree and looking for a
commit object there.

Bug: 869594
Change-Id: Ia1f1fb22ab8f7ac1eed7b32dedad5c1296b1d67b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1842637
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Derek Morris <demorri@microsoft.com>
5 years ago
Samuel Huang c1e1d099bd [Presubmit] Allow GetUnitTestsRecursively() to restrict Python versions.
GetUnitTests() has named params {run_on_python2, run_on_python3}.
This CL adds these params to GetUnitTestsRecursively() so they can be
forwarded to GetUnitTests().

Bug: https://crbug.com/1054018
Change-Id: I06ac25a37f94cdea1cdc91954b70113e54cfdd15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2107610
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
5 years ago
Edward Lemur bce1ec1d57 Reland "presubmit_canned_checks: Run pylint on parent directory for depot_tools."
This is a reland of d3bfd23c6d

Don't modify cwd for paths other than depot_tools.

Original change's description:
> presubmit_canned_checks: Run pylint on parent directory for depot_tools.
>
> On Windows, scripts on the current directory take precedence over PATH.
> When pylint.bat calls vpython, it executes the vpython of the depot_tools
> under test instead of the one in the bot.
> As a workaround, run the tests from the parent directory instead.
>
> Change-Id: I7fa3f5b268df516194063ff39907ada18f7ef544
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1994216
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

No-Presubmit: True
Change-Id: I4236ec17b2642ed7f53844af532ec1f5234b80d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1995783
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
5 years ago
Dean Berris a1266b63b5 Revert "presubmit_canned_checks: Run pylint on parent directory for depot_tools."
This reverts commit d3bfd23c6d.

Reason for revert: Breaks non-depot_tools based presubmits that use canned checks that expect the presubmit to be run in the sub-repository (for example, in Catapult where it's hosted in third_party/catapult instead of being directly in chromium).

Original change's description:
> presubmit_canned_checks: Run pylint on parent directory for depot_tools.
> 
> On Windows, scripts on the current directory take precedence over PATH.
> When pylint.bat calls vpython, it executes the vpython of the depot_tools
> under test instead of the one in the bot.
> As a workaround, run the tests from the parent directory instead.
> 
> Change-Id: I7fa3f5b268df516194063ff39907ada18f7ef544
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1994216
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,apolito@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: If989527a53be90e1ae32cb3fa30f5e4777824480
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1994753
Reviewed-by: Dean Berris <dberris@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Dean Berris <dberris@chromium.org>
5 years ago
Edward Lemur d3bfd23c6d presubmit_canned_checks: Run pylint on parent directory for depot_tools.
On Windows, scripts on the current directory take precedence over PATH.
When pylint.bat calls vpython, it executes the vpython of the depot_tools
under test instead of the one in the bot.
As a workaround, run the tests from the parent directory instead.

Change-Id: I7fa3f5b268df516194063ff39907ada18f7ef544
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1994216
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Garrett Beaty ed0cc5f6fd Add flag to "git cl format" to avoid invoking clang-format.
In repos that are checked out separately from chromium, invoking
"git cl format" on a change that has files that would be formatted using
clang-format causes an error because the depot_tools version of
clang-format looks for a clang-format inside the repo.

This prevents using proto-based properties (the preferred mechanism
going forward) in recipe repos that enforce formatting. The
--no-clang-format flag provides a workaround that can be used to
prevent trying to format any files where clang-format doesn't work.

Bug: 979330
Change-Id: Ice8561d88b29623deb953465253f92c88aa7fc2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1986111
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Dan Beam f392703d63 Exclude ES imports from long lines check
Bug: 1028829
Change-Id: Iafc40952e402884639543436281e93942c0257a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1959813
Auto-Submit: Dan Beam <dbeam@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
5 years ago
Edward Lemur b983024505 presubmit_support: Fix tests on Python 3.
The problems on crbug.com/1017367 should have been fixed by crrev.com/c/1880013

Bug: 1009814
Change-Id: Ia98304ca983b4e7e293fedb9df1fee589de58ba9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1887714
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
6 years ago
Haiyang Pan 5eb757a742 Skip non-text files for CheckDoNotSubmitInFiles
Bug: 1017286
Change-Id: Ibb9f2c94eb87024a3d9b9f334ffa5f8e2429b368
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1881377
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Haiyang Pan <hypan@google.com>
6 years ago
Edward Lemur d2a5a4c043 Revert "presubmit_support: Fix tests on Python 3."
This reverts commit a834f39e21.

Reason for revert:
https://bugs.chromium.org/p/chromium/issues/detail?id=1017367

Original change's description:
> presubmit_support: Fix tests on Python 3.
>
> Bug: 1009814
> Change-Id: I2c2a835fc4fa0a7fc9e68dfff680fb0beacb491e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1869535
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,apolito@google.com

Bug: 1009814
Change-Id: I48907caeca9b9b32e4e3a7cb5a636280d017a4c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1877350
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
6 years ago
Edward Lesmes 401b25c878 presubmit: Convert literals to strings before setting env variables.
On windows on Python 2, only strings can be environment variables, but
literals are unicode, so we need to convert to strings before setting
env vars.

Change-Id: I40e8e6ae4f5497d56efa568153eb65392dbd85e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1875108
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
6 years ago
Edward Lemur a834f39e21 presubmit_support: Fix tests on Python 3.
Bug: 1009814
Change-Id: I2c2a835fc4fa0a7fc9e68dfff680fb0beacb491e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1869535
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
6 years ago
Edward Lemur 5b929a4926 depot_tools: Clean up auth.py
Bug: 1001756
Change-Id: I495991c30f7f00de17e7c098e2d88ef7783aff3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1865265
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
6 years ago
Marc-Antoine Ruel 8e57b4bc55 python3 improvements
Ran:
  vi $(git grep --name-only iteritems | grep -v third_party)
  vi $(git grep --name-only itervalues | grep -v third_party)
  vi $(git grep --name-only 'print ' | grep -v third_party)

and edited the files quickly with adhoc macros. Then ran in recipes/:
  ./recipes.py test train

There was only a small subset of files that had been updated to use
six.iteritems() and six.itervalues(). Since the dataset size that is
being used in gclient is small (pretty much always below 200 items),
it's better to just switch to .items() right away and take the temporary
performance hit, so that we don't need to come back to rewrite the code.

Recipe-Nontrivial-Roll: build
Bug: 984182
Change-Id: I5faf11486b66b0d73c9098ab0f2ce1b15a45c53e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1854900
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org>
6 years ago
Edward Lemur b4a587df5e depot_tools: Remove depot-tools-auth
Users must use luci-auth now.

Bug: 1001756
Change-Id: I04cab6bdbfbd958f386a4cab761dfe4d34073afc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1849810
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
6 years ago
Dan Beam b295105c22 Make presubmit bark at FIXED= in CL description
"Fixed:" is equivalent and the preferred syntax.

Also sets up a more generic mechanism for "unwanted tags",
which BUG= and TBR= may eventually be when we're ready to
move over to git-footers more fully.

Bug: monorail:4470
Change-Id: I2e999d364292447124870b8d3539fde1ac1eb4ec
Recipe-Nontrivial-Roll: chromiumos
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1836607
Auto-Submit: Dan Beam <dbeam@chromium.org>
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
6 years ago
Alan Cutter 3fcadbb943 Allow TBR for CLs with approved changes to OWNERS files
https://chromium-review.googlesource.com/982601 disabled TBR for CLs that
modified OWNERS files. This CL relaxes the restriction to permit TBR for
CLs that have LGTMs for their modifications to OWNERS files.

Bug: 688115
Change-Id: I47fef6b1eb021ca7cdfc003dc57722643b174a6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1797605
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
6 years ago
Edward Lemur 940c282116 depot_tools: Make it possible to run python3 tests on PRESUBMIT.
Add run_on_python2 and run_on_python3 to allow to be tests on
either only python2, only python3 or both.

Bug: 984182
Change-Id: I87d3b67412129cf3a8e627cd6bc97daa81147657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1758849
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: smut <smut@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
6 years ago
Mike Frysinger 53297790de pylint: rework wrapper to use vpython for multiple versions
This allows people to pin a specific version of pylint when running
it.  The default behavior is unchanged: we run pylint-1.5.6 still
as we've created a vpython wheel in cipd for it.  But for projects
that want to move to a newer version, they can pick any of the new
1.x wrappers.  Having the full set of versions here allows projects
to incrementally move to newer versions as the lint delta between
1.5 & 1.6 tends to be much smaller than 1.5 & 1.9.

We don't include support for the newer 2.x series as those only
support Python 3 which the codebase isn't ready for.

Bug: 866772
Test: `./pylint ...` still works & shows pylint-1.5
Test: `./pylint-1.5 ...` runs the v1.5 version
Test: `./pylint-1.8 ...` runs the v1.8 version
Change-Id: I7c47187493564db81e3eb28c6dbd09e6309ead46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1752507
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
6 years ago
John Budorick a151265d61 Make presubmit emit a warning when it can't find the desired luci config set.
Useful in cases like the green presubmit runs on crrev.com/i/1434617/2

Bug: 925774
Change-Id: I37ce2be843d76ace0615cd3313e5c8421504a1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1687122
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
6 years ago
Aaron Gable aca5b6aca8 Add JSON-parsing presubmit check
This is followup from the malformed-json bug which took down
Sheriff-o-Matic on 2019-05-16. This CL just adds the canned check;
future CLs can either add it to PanProjectChecks or add it to
other PRESUBMIT files directly.

Change-Id: I4a445193c1744966a448b12c7eb2915873e484d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1617941
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Sean McCullough <seanmccullough@chromium.org>
6 years ago