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>
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>
In https://crbug.com/1327371, we started encountering hanging gsutil
calls. These were triggered under very specific circumstances, but
were essentially due to a multi-processing bug in python on MacOS:
https://bugs.python.org/issue33725
When running gsutil on mac, it explicitly suggests disabling
multi-processing for this reason:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/catapult/third_party/gsutil/gslib/command.py;drc=3a12d6ccdec28da8bda09d9ff826aae1f9504e59;l=1331
So this CL simply puts that suggestion into practice. There are two
options for doing so: adding a line to an active Boto file, or adding
a "-o" arg to every gsutil invocation. Since chromium has multiple
copies of gsutil and multiple different locations where `gsutil` is
invoked, the latter option is considered intractable.
Instead, we choose the Boto file option by adding a context manager
to the gsutil recipe module that will insert a custom Boto file into
the environment that only disables multi-threading. Due to the fact
that the BOTO_PATH env var takes multiple paths, we can safely
incorporate both our custom Boto along with LUCI's per-build Boto file
in the same gsutil invocation. And when the context manager exits, the
environment should revert back to its original state.
Chromium incorporates this method in crrev.com/c/3662023. See it take
effect during the "gclient runhooks" step in this led build:
https://ci.chromium.org/swarming/task/5b0c5174e70cb010?server=chromium-swarm.appspot.com
Bug: 1327371
Change-Id: I75cfdf16d0ec023bf81ea57991dde996694a46c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3661949
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
It's possible that remote closes connection, which would result in a
stacktrace being shown in the stderr. Silence this exception, as we do
with other exceptions.
R=gavinmak@google.com
Bug: 1328518
Change-Id: Ifae2aca64fe5d572c0b71008f6dacbb63ba77e06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3664178
Reviewed-by: Gavin Mak <gavinmak@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8813329470605599937
recipe_engine:
1caa943e0c
1caa943 (bpastene@chromium.org)
context: Correct misleading comment about inspect the recipe env
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: I509e3c5f308fd7ebf127d78985047a430348f823
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3661516
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Bot-Commit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8813337018642090161
recipe_engine:
ebae866e0d
ebae866 (mohrr@google.com)
[url] Correct default step name in docstrings
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: Ic2f9e599f439ff9b0ca91314df474b01572112b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3661511
Bot-Commit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
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>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8813625169557805905
Please review the expectation changes, and LGTM+CQ.
recipe_engine:
a58dd35fb7
a58dd35 (tikuta@chromium.org)
file: use vpython3 to run scripts in resources dir
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: I116fa1e5faafade1c85a4cca7077b926af788a8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3657049
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Auto-Submit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
This reverts commit b6a0b58b9f.
Reason for revert: we should have a good coverage now within just
chops source. You are welcome to stay owner if you'd like tho.
Original change's description:
> Add iannucci to depot_tools owners
>
> Change-Id: Ic85057a6075ba398e4800edffd183f5e2eccd259
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3440929
> Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
> Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Change-Id: I0a5138d8a119d2d2699e04857c95e6f2875aaac0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3657058
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
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>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8813803843140845281
recipe_engine:
e3cd9ebd63
e3cd9eb (kuanhuang@chromium.org)
Add recipes API swarming.show_request(name, task) -> TaskRequest
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: If2153a7dcef68304f7f83764639d13c478576757
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3653983
Bot-Commit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
This version was built with go1.17. Has no other significant
changes.
R=iannucci@chromium.org, chanli@chromium.org
Change-Id: Ic1b3aa1fd74f5e87b7e0429fa0f52d7989400ee3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3651160
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Chan Li <chanli@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
This snippet of code:
if constexpr (n == 1) {
return 2;
} else if constexpr (n == 2) {
int x = 2;
int y = 2;
return x + y;
}
was triggering the warning:
If/else bodies with multiple statements require braces [readability/braces]
And, in general, cpplint.py was assuming that `if (cond)` was
the only `if` construction to expect, forgetting about
`if constexpr(cond)`.
Change-Id: I4cdc8e7f134c1ebd14d00354a8baadf87c796763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3644147
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8814340506914805185
recipe_engine:
d047727862
d047727 (gbeaty@chromium.org)
Inhibit displaying coverage information on python3 failures wit...
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: Id1b76231a73ae40177efd7f62123488fd80dba7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3645589
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Bot-Commit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
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>
Even when increasing the maximum number of file descriptor
as recommended in the documentation, using a large number
of concurrent jobs cause build error on certain macOS devices
(mostly iMacPro with 18 cores which ends up with a limit of
1440 jobs, and end up reaching the heightened file descriptor
limit).
Put a limit of 800 which has been found to work on all of
the devices available and still allow to have a multiplier
of 80 on highest end M1 devices (currently 10 cores).
Bug: 1317620, 936864
Change-Id: I32560c5ae9462e94f61a773d625ef3758bf05ee5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3634807
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
This is very usable for non Google-sized Goma clusters. We want to use
NINJA_CORE_MULTIPLIER logic, but at the same time we don't want to stall
our cluster with super-high -j values.
Bug: 1323475
Change-Id: I2ed414463b4f397a7bcebe42ea0b996e52006cda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3632492
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Include crrev.com/c/3607770 via 4047a98ee7
Regenerate cipd_manifest.versions using cipd ensure-file-resolve -ensure-file ./cipd_manifest.txt
BUG=1314194
Change-Id: Ic72fecb38fc3adff4c1d7b5f0954cdf9e604d79e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3627691
Reviewed-by: Chan Li <chanli@chromium.org>
Commit-Queue: Chan Li <chanli@chromium.org>
Auto-Submit: Patrick Meiring <meiring@google.com>
LUCI bucket can take a few different formats, and it's not clear which
one `git cl try` wants. So this clarifies it a bit.
Bug: None
Change-Id: I4816cf17e3af376d447395021fbc3544616c5451
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3624550
Auto-Submit: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
This is a reland of commit 5e49eda5c4
Original change's description:
> Break make_encoded_file into two functions
>
> The python3 fallback for `make_encoded_file()` produces different results on py2 and py3 when bytes are passed. This CL makes it clear breaking it into two methods, one for text and another for bytes. This CL also adds a fallback in `download_file` for py3 to return bytes when it cannot decode the downloaded file into utf-8.
>
> Recipe-Manual-Change: chromiumos
> Change-Id: I3d313e430c852e179825bc24bf4a58ce84440b2a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3595026
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Reviewed-by: Greg Edelston <gredelston@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Recipe-Manual-Change: chromiumos
Change-Id: I321f797ae4a00deed7920ee6f80d666954b07c7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3625907
Reviewed-by: Greg Edelston <gredelston@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
When doing presubmit bisects (walking back through git history to see
when a presubmit regression was introduced) it is inconvenient to have
to create a branch at every step, and clean up the branches later. This
change makes having a branch optional, when using --force mode.
Bug: 1309977
Change-Id: I9fb6235620cf6c2e856359d2c25f1ef00c5da554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3611025
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
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>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8815218164711976977
recipe_engine:
82fb654d0e
82fb654 (meiring@google.com)
[recipes-py] Update ResultDB exoneration example to populate re...
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: Id9a79a84f2bd023d0f9ebe3c4d9a227efe89032b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3622625
Bot-Commit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
This is an automated CL created by the recipe roller. This CL rolls
recipe changes from upstream projects (recipe_engine) into this repository.
The build that created this CL was
https://ci.chromium.org/b/8815259687168433617
Please review the expectation changes, and LGTM+CQ.
recipe_engine:
4cb1bf0fb0
4cb1bf0 (gbeaty@chromium.org)
Don't set the -u flag when running python unbuffered.
More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug.
R=iannucci@chromium.org
Recipe-Tryjob-Bypass-Reason: Autoroller
Ignore-Freeze: Autoroller
Bugdroid-Send-Email: False
Change-Id: I0859db0f9db1c7548068073300150e979ac460b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3621157
Commit-Queue: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: Garrett Beaty <gbeaty@google.com>
Auto-Submit: Recipe Roller <recipe-mega-autoroller@chops-service-accounts.iam.gserviceaccount.com>
Reviewed-by: Garrett Beaty <gbeaty@google.com>
This should make it possible to integrate into more tools.
Bug: 1013096
Change-Id: Idd1502996dafc28b1e5b1bf731125bd64c4f5162
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3613833
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Takuto Ikuta
Fumitoshi Ukai
Bruce Dawson
recipe-roller
Ben Pastene
Josip Sokcevic
David Sanders
Chan Li
Louis Romero
Vadim Shtayura
Jochen Eisinger
Joanna Wang
Darius M
Mike Frysinger
Sylvain Defresne
Aleksey Khoroshilov
Patrick Meiring
dpapad
Aravind Vasudevan