Also add oneline status on gclient gitmodules.
R=jojwang@google.com
Change-Id: I05c9f856ce6fd1c3ebf1dc7da672d25196a4cb67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4771975
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
The license header check error message can't distinguish between new and
moved files. We don't want to update the year for moved files, so
clarify instructions for that case.
Bug: 1457007
Change-Id: I47471e56a5d9889b7315d9ad6dd8a5dd5cfe956f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4641350
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
The new flow is that if any affected file is in the config directory
of a config set, the script will call `lucicfg validate` to validate
the whole config directory and then filter out the validation result
that does not belong to affected file(s). This will introduce a bit
more latency in valdiation because before, only affected files are
sent to LUCI Config for validation. However, this is not latency
senstively operation and always validating the full snapshot will
faciliate us breaking down the config as it's possible in order to
validate the affected file, LUCI Config will need an unaffected file.
R=iannucci, yuanjunh
Bug: 1449933
Change-Id: Id330973f59b882569a8ece36edb4b6a8ff0ed2d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4591233
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Yuanjun Huang <yuanjunh@google.com>
CheckOwnersFormat requires network and code owners plugin to be enabled.
This moves that check under condition.
R=jojwang@google.com
Change-Id: Ic9cfd5e33987cac9aae93d744ebfb2482eb309be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4571106
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Calling CheckChangeHasNoUnwantedTags and
CheckChangeHasBugFieldFromChange from a PostUploadHook is useful because
it ensures that users will see a message if they add a bad tag when
creating the commit message during upload. Allowing this requires
creating entry points for these functions that take a change parameter
instead of an input_api parameter. It also requires fixing our use of
the deprecated inspect.getargspec function.
In order to not tell users twice (once before and once after upload)
that they haven't used a Bug: tag (it is optional, after all) a flag
to make these suggestions optional is also added.
This change is required in support of crrev.com/c/4490087.
Bug: 1434702
Change-Id: I3fd909a71c6c7d7671d6f986ad60d5375143a2f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4519532
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
PRESUBMIT.py files can invoke other scripts, and those scripts may be
run under Python 2 or Python 3. There are multiple mechanisms that
govern which version will be used, sometimes requiring several flags
to be passed.
This change removes support for Python 2 from this system, thus making
it simpler to invoke Python 3 scripts.
The function parameters that are used to select Python versions are
passed in from multiple places so they still need to be supported, but
they are now ignored. The parameters are deleted to prevent accidental
use.
This change was tested by running this command in a Chromium repo:
git cl presubmit --force --all
Bug: 1207012
Change-Id: I4adc7164417e155ff80d3a039cf36ed030756456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4328470
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Exclude file paths always use forward slash but tests currently change
based on which OS is running them, so this bug never got caught.
Bug: 1440473
Change-Id: Iafd7e57c2dedd0c9990a1a620abd3ae38631a4f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4481518
Reviewed-by: Sean McCullough <seanmccullough@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
The Bug: syntax is well known as a way of tagging bugs in a commit
message, at least partially because when it is missing there is a
presubmit warning suggesting that it be used.
The Fixed: syntax is less well known because there is no such reminder.
And, if somebody types "Fixes: " instead of "Fixed: " there is no error.
Therefore it is not surprising that "Fixes: " was used seven times in
the last 10,000 commits. See crrev.com/c/4416218 for one example.
This change adds both a reminder that Fixed: exists as well as an error
if Fixes: is used.
Fixed: 1434702
Change-Id: Id940d0af5cfea206ee923fbb779bf686b34c5f33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4436316
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Sometimes, users may send very large config files to luci-config service
to validate (e.g chromeos). The GFE has 32 MiB limit. Therefore,
compress the json body and set a custom `application/json-zlib` content
type header that luci-config service is able to recognize.
Bug: 1417047
Change-Id: I61ee50125bab5746f8094d81cab484d70002ac53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4296563
Commit-Queue: Yuanjun Huang <yuanjunh@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
This change allows repos that use depot_tools to have submodules. It
relies on a "magic" text `SUBMODULE_MIGRATION` and
`use_git_submodules`in DEPS file.
SUBMODULE_MIGRATION is placed in DEPS file when git submodules should be
accepted but parity with DEPS file is expected. We don't rely on gclient
getdep as it's too slow for a presubmit check. Instead, we just naively
check presence of git submodule commit hash in the DEPS file.
use_git_submodules (to be a proper variable in DEPS file) indicates the
project migrated to git submodules and no git dependencies are specified
in DEPS file.
R=aravindvasudev@google.com, jojwang@google.com
Bug: 1417051
Change-Id: I03fcd98a001ee8513740108e58bc1dfe3c9a460e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4262472
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Only tests under tests/ are run on presubmit, so
presubmit_canned_checks_test.py is not run. Move these there and fix
them.
Bug: 1385693
Change-Id: Ib01a816b0a81919722ea49e5551010251c0e3287
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4038514
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
file:// references in OWNERS files can become invalid when the file
is moved or deleted.
Bug: 1385205
Change-Id: Icf1a65b3d96d6ad298eac6645d8f692cb09dc75a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4029143
Auto-Submit: Victor Vianna <victorvianna@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
The current message prints a regular expression.
This new message prints the correct copyright header which can be
copy-pasted into new files.
Bug: 1376752
Change-Id: Ifb03924641ad75d2b58158a752e0a62648436edc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4032368
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Some recent "improvements" to CheckLicense were buggy, at least
partially because I did not increase test coverage when adding new code
paths. This change fixes two known mistakes and greatly increases test
coverage.
One bug was that the code assumed that match.groups()[0] would always be
the current year, but that assumption is invalid when a license regex is
passed in.
Another bug is that the new_license_re took away the possibility of the
license ending with */ but that is used in .css files.
The increased test coverage catches these and validates other
assumptions around new files.
The failures were reproduced and then the fixes validated using this
command:
vpython3 tests/presubmit_unittest.py
Bug: 1098010
Change-Id: Ic91ca9f7a182660aef7b1eead79e841f58e15a3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4032366
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Change crrev.com/c/3967210 compared a regex result (str) to current_year
(int), which will never match. This fixes the comparison by converting
current_year to str. Unit tests to validate the behavior will be added
in a follow-up.
Bug: 1098010
Change-Id: I79d0adc6d6566df71425bfb448091445f57ad2c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3968797
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
In crrev.com/c/3955701 the license checks were made stricter on new
files. Unfortunately moved and new files are indistinguishable in the
presubmit system, and moved files are not supposed to update their
copyright year.
So, CheckLicense() now emits three different types of presubmit results:
1) If a file is new/moved and the license is malformed then an error
is reported.
2) If a file is new/moved and the year is not current then a warning
is issued, with advice to ignore the warning if the file was moved.
3) If a file is not new/moved and it has a bad license then a warning
is issued.
This will ensure that new files do not have bad licenses, and will
usually get the year correct.
The new output looks like this (for one moved file with an old date,
one file with a bad license, and one new file with a bad license):
** Presubmit Warnings: 2 **
License doesn't list the current year. If this is a new file, use the current year. If this is a moved file then ignore this warning.
base\win\moved.cc
License must match:
.*? Copyright (\(c\) )?(2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors(\. All rights reserved\.)?\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
base\win\bad_license.cc
** Presubmit ERRORS: 1 **
License on new files must match:
.*? Copyright (2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.\n
Found a bad license header in these new files:
base\win\new_file.cc
Bug: 1098010
Change-Id: Ia62b0591ee416c55566427bba9fdd91d74a26349
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3967210
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
The Chromium license check is quite forgiving because it has to handle
variations that have accumulated over the years. But, there is no need
for it to be forgiving on new files. This change requires that new files
have a license that _exactly_ matches what we want, including having the
current year. This will catch lots of errors that would otherwise
require an observant reviewer.
This does mean that if an old file is copied that it might require some
updates.
This was tested with presubmit --all where it gave identical results to
the old check. It was also tested with new files with bad licenses to
make sure that the new check behaved as designed.
This change also switches from f.OldContents() to f.Action() == 'A'.
f.OldContents() is very expensive (about _40_ minutes if invoked on all
files during presubmit --all) and f.Action() == 'A'dded better indicates
what we are actually checking.
This change also switches from checking for the presence of key_line
before doing the regex search to just doing the regex search on the
start of the file. Both techniques avoid the extreme cost of a regex
search on large files with no license, but the new technique is simpler.
Bug: 1098010
Change-Id: I028d72cd31f5d0f787a522c54683de32f2a98867
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3955701
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Long lines in source files are bad, but sometimes unavoidable. Since
ignoring warnings is even worse than long lines it is wise to have a way
to disable the warning. This change adds support for a multi-language
tag that can be put in unavoidably long lines.
Bug: 1309977
Change-Id: I205086050b5aa5b4a02a651c06615c82ec0e1a38
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3896084
Reviewed-by: Dmitrii Kuragin <kuragin@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This change also makes CheckVPythonSpec() look for .vpython3 files.
Bug: 1336295
Change-Id: I4bbec48debe42748811a8cfcef6da9602017d4bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3891974
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
We have long had a presubmit that warns when a file lacks a suitable
license, but because this is a warning it can be ignored by the author
and is not visible to reviewers. Every failure that is not prevented
will occur, and indeed newly added files from this year (seen in
crrev.com/c/3706888 and crrev.com/c/3437860, and possibly others) are
missing valid licenses.
This change turns the warning into an error on newly added files.
Bug: 1361031
Change-Id: Ie17bd6e591918affd9d865a3bf2be6c2bf62d1ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3887721
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This is a reland of commit 10dbd7babd
Original change's description:
> Remove old DepotToolsOwners implementation
>
> code-owners should have been enabled for most hosts that depot_tools
> supports by now. Remove our own implementation and rely on code-owners.
>
> Change-Id: Iaf0d3db65b2e5063b67d42b92188c4ec51d2cd9a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3783475
> Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>
> Commit-Queue: Gavin Mak <gavinmak@google.com>
Change-Id: Ic87f34411bb38b91fcf49adb07601ae10244e828
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3881043
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
As per the lawyers, the preferred style has no period after "Authors"
when the "all rights" sentence is dropped. Adjust the regex to not
match a period when there is no "all rights" sentence.
Bug: 1098010
Change-Id: Ib4103c09f9d02d58b9294643bbedd72fc895bbe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3876526
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Avi Drissman <avi@chromium.org>
This is a reland of commit eb16430cf9
Original change's description:
> Use io.open for opening files
>
> Use io.open which is consistent in py2 and py3.
>
> Bug: 1357152
> Change-Id: I49a3df503026bc6918362a9b5600f57111111111
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872429
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Bug: 1357152
Change-Id: I58881c040ca7333f0247875a0c4abe31cf793b08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3877645
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Pylint uses parallelism to improve performance, but high startup costs
means that this only makes sense if there are lots of files to be
processed. So, a while ago a change was made such that if there are
fewer than ten files to be analyzed then no parallelism is used.
Our Pylint wrapper also has a hack where it does one type of check in
serial mode, because that is the only time it is reliable. This
requires running Pylint twice, which is expensive.
If there are a small enough number of files to analyze then we will be
doing serial analysis anyway, so there is no need to do two separate
runs. In this test case:
git cl presubmit -v --files tools\code_coverage\create_js_source_maps\test\create_js_source_maps_test.py
the cost of Pylint is dropped roughly in half, from six seconds to
three seconds, by eliminating one of the three-second runs.
Bug: 1309977
Change-Id: I2e5e96a86d1d76b127f481af7478d807c042b609
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3812436
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This update is per http://crrev.com/1031130 where OSS licensing
requested removal of "All rights reserved" from new files.
Change-Id: I52a9deab99539ea90403798606c7bbeb261df994
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3811744
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Robert Liao <robliao@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Recent gerrit issues made it obvious that "git cl presubmit" relies on
gerrit much more than most people would expect (the expectation is zero
for many people). This makes presubmits flaky or much slower under poor
network conditions, and it means that the presubmit step may drastically
underestimate how long it takes to run because of a
cl.FetchDescription() that may occur outside of the timed portion of the
presubmits.
This change wraps more network-touching steps in try/except blocks, to
make them robust. It also gets them to check for the existence of a
PRESUBMIT_SKIP_NETWORK environment variable. And, it prints the elapsed
time to get the CL description if this is inordinately long.
Bug: 1350227
Change-Id: I7954fd50e928fd24975a4f61a316cb280542ebbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3813095
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Change crrev.com/c/3788227 fixed one instance of errors being treated as
warnings when running "git cl presubmit" with the --upload option. This
is undesirable when testing "git cl presubmit" with --all or --files,
because it makes the errors harder to find. This change fixes four
more newly discovered instances of this behavior.
That is, this change makes it so that pylint issues and other serious
problems will be reported as errors when running:
git cl presubmit --force --all --upload
This will make the pylint errors that this command triggers easier to
find and fix:
git cl presubmit --force --upload --files mojo\public\tools\bindings\*.py
This change does _not_ turn cpplint warnings into errors, even though
they are errors when running non-upload presubmits. That is because
there are several directories that only run cpplint on upload and these
directories have many errors and there is no short-term path to changing
this.
Bug: 1309977
Change-Id: If49f820fc6894dcd1d9aaaf4d932b04f79922bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3791744
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
When upload presubmits are being run then GetUnitTests would treat
errors as warnings, so as to not stop developers from uploading their
changes. This makes sense when patches are being manually uploaded, but
doesn't make sense when running a "git cl presubmit --all --upload" bot.
This behavior caused nine failures (but not all failures) to be missed
when doing these tests.
This change makes these failures errors when testing with --all
--upload, but leaves them as warnings for other --upload purposes.
Bug: 1309977
Change-Id: Ibf149475e4cdee10bbbbc86fd0ab668b7a679089
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3788227
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
If a presubmit test is misconfigured then it may end up running on
neither Python 2 nor Python 3. This was supposed to trigger a warning
but the warning code was broken for almost a year. It was then fixed so
that it issued an error, but that caused some breakage, so it was made a
warning.
The known problems with tests not running at all have been fixed and
the warning has been on for over two weeks so this change turns the
warning back into an error. This is appropriate because a test that
is not run at all is a serious bug. This change will allow us to move
more confidently when switching tests to Python 3, and it shouldn't
cause any additional breakage.
Bug: 1330859
Change-Id: I51028bb9c896c60c5cf3ccb6f472ade0bb1e0c16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3717242
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
The multiprocessing module on Windows has a bug where if you ask for
more than 60 child processes then it will hang. This is related to the
MAXIMUM_WAIT_OBJECTS (64) limit of WaitForMultipleObjects. Other sources
have listed the multiprocessing limit as being 61, or have said that the
the maximum number of objects that can be waited on is actually 63, but
those details don't really matter.
The original fix for this class of issues was crrev.com/c/2785844. This
change extends those fixes to depot_tools, which was missed last year.
This change also updates how PyLint is called by further limiting the
number of jobs to the number of files being processed divided by 10.
This is because there is a significant cost to creating PyLint
subprocesses - each takes about 0.5 s on my test machine. So there needs
to be enough parallelism to justify this.
Patches for PyLint and a bug for cpython are planned.
This will stop PyLint from hanging during presubmits on many-core
machines. The command used to reproduce the hangs and validate the fix
was:
git cl presubmit -v --force --files "chrome/test/mini_installer/*.py"
Prior to this change this command would use (on my many-core test
machine) 96 processes and would hang. How it uses just two processes
because there are only 16 files to analyze.
Output before:
Pylint (16 files using ['--disable=cyclic-import'] on 96 cores)
Output after:
Pylint (16 files using ['--disable=cyclic-import'] on 2 processes)
This is actually not quite true because the hang would prevent the
old message from being displayed.
Bug: 1190269, 1336854
Change-Id: Ie82baf91df4364a92eb664a00cf9daf167e0a548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3711282
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
presubmit --all should succeed regardless of the current state of the
tree. Otherwise a presubmit --all bot will be flaky due to circumstances
beyond its control.
This is a logical follow-on to crrev.com/c/3628368 which turned some
other errors into warnings for presubmit --all.
Bug: 1309977
Change-Id: I6d26708f8c47916cf015dec75e27833db7ca465b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3696070
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
A run of "git cl presubmit --all" shows that PyLint 1.5 is being run
from nine locations, however the deprecation warnings don't say where.
This makes it difficult to file bugs or fix the remaining instances.
This changes the message to list the path to the presubmit that is
running PyLint 1.5.
This also adds instructions on how to change to version 2.7.
Before:
pylint-1.5 is deprecated, please switch to 2.7 before 2022-07-11
After (with word wrapping):
pylint-1.5 is being run on ...src\tools\find_runtime_symbols
and is deprecated, please switch to 2.7 before 2022-07-11 (add
version='2.7' to RunPylint call)
Change-Id: Iece2cb904f5d26ad66e3ab78f7ce7aef1878bfd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3688839
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
When PRESUBMIT is run with a custom --upstream, dry run cl format should
also use this value, otherwise format warnings would be inconsistent.
Change-Id: I1370f614618ecc03706e54cd4b151a207d9b3bae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3688631
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This reverts commit 013e47be09.
Reason for revert: This will be relanded as a warning, although the
bugs which the original change exposed should be fixed soon.
Original change's description:
> 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>
Bug: 1223478, 1330859
Change-Id: I6a0a78e70db77c9c5fda5bab27960cd4e71279e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3685349
Reviewed-by: Fabrice de Gans <fdegans@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This reverts commit c03ebf0e1a.
Reason for revert: the original change was not the cause of the test
failures. The actual cause was crrev.com/c/3683105 which was reverted.
Original change's description:
> 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>
Bug: 1313804
Change-Id: I15ed90cc35d88ece71eb681acc0acc06cb5c3bcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3685229
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Jesse McKenna <jessemckenna@google.com>
Anne Redulla
Josip Sokcevic
mark a. foltz
Yiwei Zhang
Gavin Mak
Joanna Wang
Bruce Dawson
Yuanjun Huang
Collin Baker
Brian Ryner
Victor Hugo Vianna Silva
Vadim Shtayura
Fabrice de Gans
Avi Drissman
Josip Sokcevic
Robert Liao
Aleksey Khoroshilov