Commit Graph

2063 Commits (ae1a70891738fb14f64fbb884e00b87ac663aa15)

Author SHA1 Message Date
Brian Sheedy 7326ca218b Add post upload Python 3 option
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>
3 years ago
Erik Staab 9f38b63b4e Add snippets to presubmit failures in resultdb
Fixed: 1174446
Change-Id: Ie57818846704ed24fe1af49a70e24ad7a118d44c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3965229
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Erik Staab <estaab@chromium.org>
3 years ago
Gavin Mak ff5d25bb10 Add canned presubmit checks for corp links
Only googlesource links should be used publicly.

Bug:b/253074243
Change-Id: Id972821acbfde635717e21bba879880e5312f92c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3955706
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
3 years ago
Garrett Beaty 08bb5c487f Use the gerrit host when setting gitiles commit.
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>
3 years ago
Thiago Perrotta 512dfd676b fetch: make --no-history and --no-hooks switches consistent
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>
3 years ago
Joanna Wang 46ffd1bca1 Allow sso remotes for `git cl status`.
Bug:1364237
Change-Id: Iff3419097f86db125990001fe316c9c52974e3b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3899165
Auto-Submit: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
Fabrice de Gans 9ebcfa6be1 [code-health] Run CheckVPythonSpec under python3
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>
3 years ago
Bruce Dawson 443f9851a4 Improve Python 2 presubmit warning
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>
3 years ago
Gavin Mak 0f1addcbe9 Reland "Remove old DepotToolsOwners implementation"
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>
3 years ago
Josip Sokcevic 95c7da10c5 Only run selected tests on py2
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>
3 years ago
Josip Sokcevic 6efb3ed962 Reland "Use io.open for opening files"
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>
3 years ago
Thiago Perrotta a089281a82 roll_dep: support multiple -r/--reviewer arguments
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>
3 years ago
Gavin Mak a3a95d439b Revert "Use io.open for opening files"
This reverts commit eb16430cf9.

Reason for revert: blocking upload of CLs, https://crbug.com/1357152#c8

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: Iec18fe8fd37f28887507d47d7b35b93ccee72375
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872487
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Gavin Mak <gavinmak@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
3 years ago
Josip Sokcevic eb16430cf9 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>
3 years ago
Josip Sokcevic 38d669fe6f Reduce retries in gclient_utils
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>
3 years ago
Josip Sokcevic 5084800dc3 Fix flaky testPreDepsHooks test
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>
3 years ago
Thiago Perrotta b0fb8d570d Introduce --send-email flag as an alias to --send-mail for `git cl upload`
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>
3 years ago
Josip Sokcevic 940cd8e20f Use empty argument list in cmdhelp
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>
3 years ago
Bruce Dawson 8d78bd1fc1 Print which PRESUBMIT.py scripts run on Python 2
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>
3 years ago
Gavin Mak e0fee9fa4b Add BatchListBestOwners and ListBestOwners to GerritClient
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>
3 years ago
Bruce Dawson 0af709772e Don't do double-serial Pylint checks
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>
3 years ago
Gavin Mak a9677a5e9b Revert "Remove old DepotToolsOwners implementation"
This reverts commit 10dbd7babd.

Reason for revert: removal broke builders

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: I781648f470e8f2c9d408ae7112cfc41060f5385d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3818251
Auto-Submit: Gavin Mak <gavinmak@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
3 years ago
Gavin Mak 10dbd7babd 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>
3 years ago
Joanna Wang 96b57b9b59 [no-sync] bot_update: Remove previous no-sync exp changes and implement new one.
Bug: 1339472
Change-Id: I250dd72dd806d26ba6cbe213280ecd65fbc66123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3789864
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
3 years ago
Joanna Wang 0187079c09 [no-sync] Save previous values to files.
Tested locally with chromium/src.
going from 17s -> <1s

Bug: 1339472
Change-Id: I71b90dcd6a7934ea7c9722dd040d63645792078e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3791746
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
3 years ago
Joanna Wang a84a16b863 [no-sync] Set _should_sync and add flag to control if the experiment should be enabled.
Bug:1339472
Change-Id: I19abca1f4319a89cc461935a83d136c8870944dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3721601
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
Joanna Wang 77da3f2347 Fix flaky testRecursedepsCustomdepsOverride.
Fixed: 1345847
Change-Id: I3d4ea30210f0bf7cb5ace5095d06ca459a7039b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3783155
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Auto-Submit: Joanna Wang <jojwang@chromium.org>
3 years ago
Joanna Wang f3edc50445 [no-sync] Store previous sync commit in memory.
- 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>
3 years ago
Gavin Mak c3ac73ebfa Remove 'Fetching' check in testTrackTag
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>
3 years ago
Gavin Mak d22bf60721 Add number of presubmit errors/warnings in header
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>
3 years ago
Joanna Wang 5a7c824fee [no-sync] Add method to check for diffs between the current checkout and another commit.
Bug: 1339472

Change-Id: If97d2fad7e5c61ec907e15a00db225b653a62bef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3738365
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
Joanna Wang 18af7efdd8 [no-sync] Update ParseDepsFile to process deps and hooks appropriately with should_sync.
Bug: 1339472
Change-Id: Iea077cdb655105d27431ff4f677d93eec121bc31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3738361
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
3 years ago
Joanna Wang 6628661eba [no-sync] Add a skip_sync_revisions and process it before running deps.
Bug: 1339472
Change-Id: I429489f7deea035c47e27e32ada858fb8a827643
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3735487
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
manukh 566e9d0177 [git-cl] If set, use `options.title` for initial uploads.
Change-Id: Ia57b7a7b769d352e05820af0e40a52fcab471ddb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3736424
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
3 years ago
Bruce Dawson 8254f06ba7 Mitigate Python 3 multiprocessing bug on Windows
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>
3 years ago
Joanna Wang a9467d8216 Remove duplicate sparse-checkout call.
Bug:1330629
Change-Id: Ieccf72c75dda7cbec473fcfa07d25fb59f8eb7c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3689728
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
3 years ago
Aleksey Khoroshilov 35ef5ada1d Run presubmit checks without GerritAccessor if GERRIT_HOST is not set.
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>
3 years ago
Joanna Wang 1a977bdc2d Update gclient cloning to work with cog.
Bug:1330629
Change-Id: I522255528e36d4806eada70c22afee1277a6778d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3668429
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
Aravind Vasudevan 5965d3e6fb Reland "Update fetch protocol using --protocol-override flag"
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>
3 years ago
Stephanie Kim 700aee769d Override cipd install mode for runhooks
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>
3 years ago
Aravind Vasudevan fe3a70a926 Revert "Update fetch protocol using --protocol-override flag"
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>
3 years ago
Aravind Vasudevan 6817010e83 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>
3 years ago
Bruce Dawson 10a8286824 Don't halt presubmits on exceptions
Some presubmit failures result in uncaught exceptions which historically
have halted presubmit runs. This behavior makes it more difficult to
find all failures, measure presubmit times, or detect other behavior
over time. This is particularly problematic when running "git cl
presubmit --all".

As an example, presubmit --all was broken and would not complete from
when crrev.com/3642633 landed until crrev.com/c/3657600. The minimal
repro was this presubmit command:

    git cl presubmit --files ui\accessibility\ax_mode.h

This command, or presubmit --all, triggered this error, which halts the
presubmits:

Evaluation of CheckChangeOnCommit failed: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'
Traceback (most recent call last):
  File "presubmit_support.py", line 2038, in <module>
    sys.exit(main())
  File "presubmit_support.py", line 2015, in main
    return DoPresubmitChecks(
  File "presubmit_support.py", line 1738, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "presubmit_support.py", line 1602, in ExecPresubmitScript
    self._run_check_function(function_name, context, sink,
  File "presubmit_support.py", line 1640, in _run_check_function
    six.reraise(e_type, e_value, e_tb)
  File "C:\Users\brucedawson\.vpython-root\e726d2\lib\site-packages\six.py", line 686, in reraise
    raise value
  File "presubmit_support.py", line 1630, in _run_check_function
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "PRESUBMIT.py", line 314, in CheckChangeOnCommit
    errs.extend(CheckModesMatch(input_api, output_api))
  File "PRESUBMIT.py", line 283, in CheckModesMatch
    ax_modes_in_js = GetAccessibilityModesFromFile(
  File "PRESUBMIT.py", line 245, in GetAccessibilityModesFromFile
    for line in open(fullpath).readlines():
FileNotFoundError: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'

With this change the exception is handled and presubmits continue after
printing this message:

Evaluation of CheckChangeOnCommit failed: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js', Traceback (most recent call last):
  File "presubmit_support.py", line 1630, in _run_check_function
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "PRESUBMIT.py", line 314, in CheckChangeOnCommit
    errs.extend(CheckModesMatch(input_api, output_api))
  File "PRESUBMIT.py", line 283, in CheckModesMatch
    ax_modes_in_js = GetAccessibilityModesFromFile(
  File "PRESUBMIT.py", line 245, in GetAccessibilityModesFromFile
    for line in open(fullpath).readlines():
FileNotFoundError: [Errno 2] No such file or directory: 'chrome/browser/resources/accessibility/accessibility.js'

That is, the crucial information regarding the failure is printed, but
subsequent presubmits are not skipped.

Bug: 1309977
Change-Id: I7cfeda85c830e6c8e567c0df3c50f27af1dbe835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3653978
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
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 76fe1a701b Print presubmit messages in a consistent order
Presubmit message types were printed in an inconsistent order, generally
determined by which type was encountered first. This unpredictability
can be confusing when comparing runs. This change enforces a consistent
order, in increasing order of severity. ERRORS always go last so that
they will be the most visible when running presubmits locally.

This also adds a "There were Python %d presubmit errors." message to
the end to make it easier to tell when there were presubmit errors.
This is particularly useful when manually running presubmits.

Bug: 1309977
Change-Id: Ib2b73c7625789bad5b21ae12abf238b746cd11e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3668724
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Josip Sokcevic 632bbc0cb1 Skip Python 2 presubmit step when unneeded
All of the PRESUBMIT.py files in the Chromium repo are running under
Python 3. However "git cl presubmit" also works with other repos where
some PRESUBMIT.py scripts still run under Python 2. This means that
the Python 2 presubmit commit checks step cannot simply be disabled.
That meant that Chromium was paying up to a one-minute cost just to
setup for and look for Python 2 scripts that it doesn't run.

This change runs the Python 3 PRESUBMIT.py scripts first, and keeps
track of whether any were skipped. If none were skipped then the
Python 2 PRESUBMIT.py stage can be skipped.

Note that the child scripts of PRESUBMIT.py scripts may still be run
under Python 2, but that is orthogonal to this change.

Bug: 1313804
Change-Id: Ib65838223f232f1e78058d6a08ea15a89f442310
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3614453
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Joanna Wang 50c631e318 Skip 'ls' when downloading from gs.
Bug: 959167
Change-Id: I160658264dac71a9cff280cf3ba606d583d8f2b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3646593
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
3 years ago
Bruce Dawson 31bfd51995 Actually add a trailing slash to dir_with_slash
AffectedFiles carefully added a slash to dir_with_slash, but then called
normpath - which trims trailing path separators. This meant that
ui/views/PRESUBMIT.py would analyze files in ui\views_content_client,
leading to spooky action at a distance. This was noticed when this
command triggered a presubmit error:

    git cl presubmit "--files=ui/views_content_client/*.cc;ui/views/readme.md"

when running presubmit on either file individually found nothing.

This change was tested by getting CheckChangeLintsClean to print the
files that it was asked to analyze, making the behavior quite obvious.

This bug appears to have existed for about thirteen years, but only
triggers in rare cases, and even then the incorrect behavior was almost
impossible to notice.

One of the tests was inadvertently testing the broken AffectedFiles
behavior and another seemed to be requiring it to handle '.'
correctly which I'm not sure we want to support.

Bug: 1309977
Change-Id: Ibdc39981d69664b03448acb228d4ab05b49436f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3632034
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
3 years ago
dpapad 443d9135cc Include TypeScript .ts files in DEFAULT_FILES_TO_CHECK.
These were previously erroneously skipped resulting in missing
violations.

Fixed: 1322170
Change-Id: I62d27f60bc1761f1f8809c6a7db2ca5f4e05e120
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3626289
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
3 years ago
Aravind Vasudevan fb8cf9cc78 Add --protocol-override to fetch.py
This CL adds a non-functional `--protocol` flag to fetch.py. This will be used in the follow-up CL to update fetching protocol.

Bug: 1322156
Change-Id: I7eeebb9993face20bb671d6942ee23fd04802d8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3609453
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
3 years ago