While we're at it, ensure that the working directory is reverted even if
exceptions are raised by moving that part into a finally block.
Bug: 1404222
Change-Id: I8606bc5810202ff1ecdc7f202e3f574bf643f91d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4129515
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
.rs files were not part of DEFAULT_FILES_TO_CHECK which means that they
were not being checked for proper licenses, among other things.
Testing with this command reveals that six missing or bad licenses have
crept in:
git cl presubmit --force --files "*.rs"
Output was:
Found a bad license header in these files:
build\rust\tests\test_aliased_deps\main.rs
build\rust\tests\test_aliased_deps\real_name.rs
mojo\public\rust\tests\util\mojom_validation.rs
testing\rust_gtest_interop\gtest_attribute.rs
tools\crates\gnrt\sample_package\foo\src\lib.rs
tools\crates\gnrt\sample_package\src\main.rs
Change-Id: Ia7788d92d428a4060f4a0b854fdcee0450b0a3f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4128762
Commit-Queue: Gavin Mak <gavinmak@google.com>
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
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>
A last minute change to the CheckLicense tests in crrev.com/c/4032366
used the _GetLicenseText function in the wrong place, thus neutering one
test and duplicating code in another. This fixes the tests so that they
minimize duplication and actually test the CSS/C style comments.
Change-Id: Ibbe880dec8d1e85416ed1b7ce94922c6f2c2a543
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4037103
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
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>
upload currently overwrites any external changes, e.g. edits/rebases
in Gerrit. Check for these on upload and provide the ability to
incorporate those into the CL.
Bug: 1382528
Change-Id: Id32bf8804c4cdeb12b6a5a7cf206e033bbccd453
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3975855
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Adds --no-python2-post-upload-hooks to git_cl.py. This causes any
post upload hooks to only run under Python 3. Without this, uploading
CLs from builders that omit Python 2 causes the CL the step to fail
after the CL is uploaded.
Bug: 1340617
Change-Id: I5fb45922fb05b28ad570091f9e285dd533d92336
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3996467
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Currently, the --revision flag to git cl try will cause a gitiles commit
to be set that uses the provided revision, the gerrit host and project
of the CL to test and the target ref of the CL to test. Since the flag
is intended to control the version of source to check out and have the
patch applied to, using the gerrit host instead of the gitiles host will
result in gitiles commits that won't match the repo_path_map in gclient
configs.
Change-Id: Ie391cc9c636f3a9c87116dbb781267031569e67b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3907186
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Garrett Beaty <gbeaty@google.com>
Before this commit, the following switches exist:
- --nohooks
- --no-history
They are not consistent. This commit makes the following switches
available:
- --nohooks, --no-hooks
- --nohistory, --no-history
Bug: None
Change-Id: Iaea2357c05e3dc69e3fac0fb9d823db903d811bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3904446
Auto-Submit: Thiago Perrotta <tperrotta@chromium.org>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
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>
The warning that a PRESUBMIT.py file was still being run under Python 2
was apparently too polite as one was just added. This change adds
instructions on how to prevent this, which should make it clear that
the behavior is not desirable.
The warning was added in crrev.com/c/3826917.
Bug: 1313804
Change-Id: I81a6ea45907b5ea00e7d945c3131a329708cc377
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3878875
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
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>
depot_tools codebase uses py3 and there's limited py2 usage, namely for
presubmits. This CL drops running most of py2 unit tests since there are
no longer useful.
Bug: 1357152
Change-Id: I49a3df503026bc6918362a9b5600f5714eb5c87a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872425
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
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>
Currently multiple reviewers can only be provided with a comma-separated
list, for example: -r foo,bar,baz
This CL adds support for multiple -r arguments such as:
-r foo -r bar -r baz
Or even:
-r foo,bar -r baz
This makes it consistent (and thus less confusing) with `git cl upload` which does accept multiple -r arguments.
Bug: none
Change-Id: I27d03601b488c0c5b27568d86dcb4ed66df6b76f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3863675
Commit-Queue: Thiago Perrotta <tperrotta@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Auto-Submit: Thiago Perrotta <tperrotta@chromium.org>
There are other retries (e.g. in git wrapper, gclient_scm) so we
shouldn't be retrying that much in gclient_utils.
In ideal case, we have a dedicated place where retries are determined,
and we make a retry decision based on stderr / exit code.
R=jojwang@google.com
Bug: 1359109
Change-Id: I97daa0d991a7294635e54b7a3d85a349c03c04c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3871979
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Our Windows CI machine may take longer than 10s to process hooks
resulting in an extra block of information being printed. This change
updates the affected test to handle both fast and slow cases.
R=gavinmak@google.com
Fixed: 1352364
Change-Id: I62edbe49c0efc16cfed2bc218220840120c81c0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3867544
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Vanilla git has a git-send-email(1) subcommand. `git cl upload` should
follow suit by using the same flag name to be consistent and avoid
confusion.
[1]: https://git-scm.com/docs/git-send-email
Bug: none
Change-Id: I47ff8e03bd11915e99804fe1695046d874efc4e0
Tested: `git cl upload --send-email` to upload this very own CL
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3854330
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Auto-Submit: Thiago Perrotta <tperrotta@chromium.org>
If a wrong command name is used (e.g. typo), cmdhelp is triggered.
However, if user specifies any additional arguments (other than -h),
then user is presented with not so useful message.
This CL calls cmdhelp with no arguments.
Fixed: 1352093
Change-Id: I30c0b5f580f18b4fa2cb59d3195b67bb083f3442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3828794
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
When running presubmits you can tell if Python 2 presubmits are running,
but it's subtle, and it isn't obvious which presubmits are running under
Python 2.
This makes it obvious by always printing the path to PRESUBMIT.py
scripts that are run under Python 2. This change also fixes a bug in the
--verbose logic - it would print that a script was running before
evaluating _ShouldRunPresubmit.
Bug: 1352347
Change-Id: I45213acdb3b37f7be0713e5caba775841704817c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3826917
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
ListOwners returns all OWNERS for a file, which is undesirable in cases
when only direct OWNERS are needed.
Bug: 1351212, 1351519
Change-Id: I693b6645c780aa589e8ab24d0b58691f4aeb30f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3823299
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: 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>
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>
- Instead of expecting the caller to run `rev-parse HEAD` before
calling gclient, we can just store the latest commit on disk
- this gets around the problem where gclient might git reset to the
base checkout after applying patch-refs, so calling rev-parse
before calling gclient sync might not actually return the latest
commit of the last sync.
- also moving os.environ[PREVIOUS_CUSTOM_VARS] setting to earlier when
we only have the solutions in self.dependencies because all other
dependencies inherit from the solutions so there's no point storing
custom_vars for the rest.
Bug: 1339472
Change-Id: I6a3570f09153bd8087bbe6bdab7ece4949856aae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3750491
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
This check has been failing for a while, and blocks submission of other
depot_tools CLs.
Change-Id: I0f61ff1657c8cc01d0004ed5df9e37465b73c237
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3759629
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Headers currently have the format: `** Presubmit ${name} **`. Make these
more helpful by including the number of total errors there are.
Bug: 1341987
Change-Id: Ib41a133c31568d4264d73c14350f0bbd9590356a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3756169
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: 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>
When a repository doesn't work with Gerrit, it doesn't make sense to
create GerritAccessor and pass it to presubmit checks.
gerrit.host is stored into git config when codereview.settings file contains GERRIT_HOST item.
Change-Id: I9740950caf85a1da9a6e4ae12e3612625cf0cec5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3687235
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This is a reland of commit 6817010e83
Additional Changes:
This reland fixes https://crbug.com/1330995 by stopping gclient from accidentally updating the scheme when one is not present.
Original change's description:
> Update fetch protocol using --protocol-override flag
>
> This CL updates gclient sync to use the protocol of the URL specified in the solutions for cloning all the child dependencies of it.
>
> Bug: chrome-operations:170
> Change-Id: I33588059788b677fbae8c3b434100af5c7979a67
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3631600
> Reviewed-by: Joanna Wang <jojwang@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bug: 1330995
Change-Id: I1447a5e884e41d671d8556c35193f1635f2f6936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3684112
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Currently cipd packages listed in src/DEPS are being created and
installed in symlink mode (see bug for more info). This CL adds `$OverrideInstallMode copy` to the ensure files for cipd packages installed during gclient runhooks. `$OverrideInstallMode` was
implemented here, https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/3124202,
so that every cipd package can be installed directly to the installation
target, instead of being symlinked to an executable inside the cache/builder/.cipd dir.
The added `$OverrideInstallMode copy` can be removed once all cipd
packages themselves or the default mode is updated to be created in
copy mode.
Bug: 1329641
Change-Id: I68dc718e20f193c384d005c9b6b87603753943d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3673698
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Stephanie Kim <kimstephanie@google.com>
This reverts commit 6817010e83.
Reason for revert: Breaks gclient sync for flutter
Original change's description:
> Update fetch protocol using --protocol-override flag
>
> This CL updates gclient sync to use the protocol of the URL specified in the solutions for cloning all the child dependencies of it.
>
> Bug: chrome-operations:170
> Change-Id: I33588059788b677fbae8c3b434100af5c7979a67
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3631600
> Reviewed-by: Joanna Wang <jojwang@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bug: chrome-operations:170
Change-Id: I76a49030f48fa266eeea336307e7153b22ded4e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3684110
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This CL updates gclient sync to use the protocol of the URL specified in the solutions for cloning all the child dependencies of it.
Bug: chrome-operations:170
Change-Id: I33588059788b677fbae8c3b434100af5c7979a67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3631600
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Joanna Wang
Brian Ryner
Pavol Marko
Bruce Dawson
Gavin Mak
Brian Sheedy
Erik Staab
Garrett Beaty
Thiago Perrotta
Fabrice de Gans
Josip Sokcevic
manukh
Aleksey Khoroshilov
Aravind Vasudevan
Stephanie Kim