Siso is going to generate non-empty .ninja_log for the analysis tools.
https://crrev.com/c/4907677
This CL fixes the comment about empty ninja_log.
It's not compatible with Ninja's .ninja_log. So `gn clean` will still
be required.
Bug: b/298594790
Change-Id: Ib4c60f3ed22f516d6f7e2847aaf57e228121eccf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4937692
Auto-Submit: Junji Watanabe <jwata@google.com>
Commit-Queue: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Instead of printing a command-line, we just directly call into the respective main functions from Python. This saves spawning another interpreter and prevents things that can go wrong from having to quote, unquote, split and tunnel arguments through shells.
Part of my bigger auto{ninja,siso} refactoring.
Tested:
- Handling of the ^^ suffix on Windows still works correctly.
- Handling of error codes - i.e.; making sure
"autoninja base_unittests && base_unittests.exe" behaves properly
in the success/failure case.
- Make sure the command prompt title is reliably reset on exit.
I tested autoninja with all combinations of these:
- Host platform: Linux, macOS, Windows
- Remote GN args: <none>, use_goma=true, use_remoteexec=true
- Siso GN args: <none>, use_siso=true
- Targets: base, ../../base/types/expected_macros_unittest.cc^ (on Linux) and ../../base/types/expected_macros_unittest.cc^^ (on Windows)
R=brucedawson@chromium.org, jwata@google.com, tikuta@chromium.org
Bug: b/293657720
Change-Id: I275a775fdc5abb6555f79d4beab76cd0914d4bd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4924185
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Junji Watanabe <jwata@google.com>
Commit-Queue: Philipp Wollermann <philwo@chromium.org>
macOS limits the ninja j value to 1000, because ninja has a limit to the
number of open file descriptors of FD_SETSIZE, which is 1024 on Darwin.
On Linux, the ninja binary distributed on Chromium seems to be compiled
with poll.h support, so that this limitation doesn't exist:
22b778ca19/src/subprocess-posix.cc (L59)
Change-Id: I97848bb99c08fe118dbdaea525da713382373c9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4866223
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Did you know that args.gn files can have import statements and
conditionals? I did not, but apparently some developers make use of both
of these.
Supporting import statements is not too hard, so this change adds this
support. Supporting conditionals is possible, but risks turning
autoninja into a turing complete language which is more than I think we
want to do.
This doesn't use the similar code in tools/mb/mb.py because that
code is complex, and relies on the script location to find the src
directory.
This change also updates two of the existing test conditionals that
were not quite sufficient - ninja/autoninja default to num-cores
plus 2 so > cpu_count() is actually not sufficient to prove
anything.
Bug: 1482404
Change-Id: I0539d8068af59d11927cbdad260278a24ab912e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4864898
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This CL makes autoninja raise the number of allowed open file
descriptors to its hard limit, instead of a random value.
Also, independently of whether this operation was successful or not, the
ninja j value is capped at 80% of the current value of allowed open file
descriptors. This approximately matches the previous value of 200 for
the default value of 256 in macOS, but it's also more future-proof.
Lastly, this changes are also applied on Linux.
Change-Id: Idf2cd08384fe9f2bc699293d7062122590284dba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4852717
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Leave the recipes/ code at 2 space to match the rest of the recipes
project in other repos.
Reformatted using:
files=( $(
git ls-tree -r --name-only HEAD | \
grep -Ev -e '^(third_party|recipes)/' | \
grep '\.py$';
git grep -l '#!/usr/bin/env.*python' | grep -v '\.py$'
) )
parallel ./yapf -i -- "${files[@]}"
~/chromiumos/chromite/contrib/reflow_overlong_comments "${files[@]}"
The files that still had strings that were too long were manually
reformatted because they were easy and only a few issues.
autoninja.py
clang_format.py
download_from_google_storage.py
fix_encoding.py
gclient_utils.py
git_cache.py
git_common.py
git_map_branches.py
git_reparent_branch.py
gn.py
my_activity.py
owners_finder.py
presubmit_canned_checks.py
reclient_helper.py
reclientreport.py
roll_dep.py
rustfmt.py
siso.py
split_cl.py
subcommand.py
subprocess2.py
swift_format.py
upload_to_google_storage.py
These files still had lines (strings) that were too long, so the pylint
warnings were suppressed with a TODO.
auth.py
gclient.py
gclient_eval.py
gclient_paths.py
gclient_scm.py
gerrit_util.py
git_cl.py
presubmit_canned_checks.py
presubmit_support.py
scm.py
Change-Id: Ia6535c4f2c48d46b589ec1e791dde6c6b2ea858f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4836379
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
autoninja tries to detect if a user switches an output directory from
ninja to siso without doing a "gn clean" between. Initiallly this
detection looked for .ninja_deps, but this file doesn't always exist
after a ninja build. This change switches to looking for .ninja_log.
However autosiso creates a .ninja_log file (currently zero length) so
the new check is:
a) If .siso_deps exists then a siso build has been done and only siso
builds are allowed.
b) If .siso_deps doesn't exist but .ninja_log does then a ninja build
has been done and only ninja builds are allowed.
c) If neither file exists then the directory was cleaned and any type of
build is allowed.
Bug: b/293657720
Change-Id: I2cdba74f0894e62aa0e68f91e225f497b2425f45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4784103
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Junji Watanabe <jwata@google.com>
Reviewed-by: Philipp Wollermann <philwo@google.com>
This ensures that for ninja+reclient builds we still start reproxy to allow `rewrapper clang ...` to run locally.
Also this fixes a preexisting bug where RBE_remote_disabled=1 did not apply to the ninja call when running on mac
Bug: b/295192141
Change-Id: I37c4ffdd5f30d423b84f6c469f5edfa88efe1344
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4659225
Auto-Submit: Ben Segall <bentekkie@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
autoninja is intended to mean "build this directory, don't bother me
with the details". As we transition to siso it is therefore appropriate
that it should take on the duty of deciding whether to invoke ninja or
siso.
By looking for a use_siso gn arg autoninja can easily do this.
This change relies on crrev.com/c/4753433 to add the use_siso gn arg.
This change also teaches autoninja to detect if a user switches
between siso and ninja without doing a gn clean inbetween.
Note that this change also teaches autoninja to invoke autosiso or
siso ninja based on whether use_remoteexec is true.
Bug: b/293657720
Change-Id: I3ad36a81857e75ffe6babc4f107b777e733a285b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4749722
Reviewed-by: Philipp Wollermann <philwo@google.com>
Reviewed-by: Junji Watanabe <jwata@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
On macOS the default limit on open file descriptors is really low
and causes ninja to fail with 'Too many open files' if run with a
value of -j greater than 200.
Running only 200 jobs however slow down the build considerably so
check whether the limit has been raised by the user or if it can
be raised programmatically. In the positive case, use a limit of
800 jobs, otherwise settle down on 200 jobs.
This should allow running ninja with a large number of jobs even
on macOS Ventura 13.5 which now requires to set the limit both in
/Library/LaunchDaemons/limit.maxfiles.plist and via ulimit -n.
Bug: 1467777
Change-Id: Ib8b7d0d1ee47d243c1872229c5340e7795c1b42e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4725183
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
The limit set in /Library/LaunchDaemons/limit.maxfiles.plist appears
to no longer be respected in macOS Ventura 13.5 and the OS forces a
limit of 256 file descriptors, causing build with autoninja to fail
on macOS running this most recent version of the OS.
Force the max number of process to 200 until a new working way to
increase the file descriptor limit is found. This will allow devs
working on macOS to be able to build (albeit slower than before).
Fixes the following build failure when goma is enabled and running
on macOS Ventura 13.5:
$ autoninja -C out/Debug-iphonesimulator chrome
ninja: Entering directory `out/Debug-iphonesimulator'
[0/3574] CXX ....oninja: fatal: pipe: Too many open files
Bug: 1467777
Change-Id: Ia7eaab552f7e6d26a2f48d72bb8235a70d6d442f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4720227
Reviewed-by: Dirk Pranke <dpranke@google.com>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
This is required as it is impossible to catch a keyboard interrupt in a
windows batch file and we dont want zombie reproxy instances running on
developer's machines for performance and metric collection reasons.
Bug: b/264405266
Change-Id: I00f036c8f14451cdb1b99a5cad1c2af03dd57d57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4171506
Reviewed-by: Dirk Pranke <dpranke@google.com>
Auto-Submit: Ben Segall <bentekkie@google.com>
Commit-Queue: Ben Segall <bentekkie@google.com>
Chromecast builds use a buildflag called `use_rbe` instead of
`use_remoteexec` or `use_goma`.
Previous attempt at crrev.com/c/4144469 tried recycling the
use_remoteexec variable for the use_rbe case, but that caused
problems with Cast CI because it would hit the error case
where the reclient binary isn't found.
This new attempt introduces a new dedicated use_rbe variable
that skips that check intended only for use_remoteexec.
Bug: b/266099996
Test: run `autoninja` in the chromecast internal repo
Change-Id: Ieaf3af709589fe1b8611904afc2fd80284b333b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4179133
Auto-Submit: Simeon Anfinrud <sanfin@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
This fixes the issue for linux. I will follow up with a windows fix
Bug: 264405266
Change-Id: I518412b06e410d82d02a085180a24987f9ba98fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4134101
Auto-Submit: Ben Segall <bentekkie@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
ninja.exe will be removed from depot_tools soon.
Calling ninja.py finds the ninja installed by DEPS or one in PATH.
Bug: 1340825
Change-Id: I32ed6d8e5b0498347042490eedeca19972d35233
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4061247
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Auto-Submit: Junji Watanabe <jwata@google.com>
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Junji Watanabe <jwata@google.com>
This reverts commit f52f44b287.
Reason for revert: Breaks various internal Nest builds
Original change's description:
> Update autoninja to check for enable_rbe_bootstrap on builds that use
> it.
>
> Autoninja at this point still assumes that build directories are two
> levels up from the chromium/src directory. This will be addressed in a
> follow-on cl.
>
> Bug: b/169983312,b/253452972
> Change-Id: I13eb1e5d8ebfb83d565ea84b6a89c7784eb7fde3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3975409
> Commit-Queue: Michael Savigny <msavigny@google.com>
> Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Bug: b/169983312,b/253452972
Change-Id: Idad4caf46f12860fb4804251f62d0fb498add27d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3982826
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
it.
Autoninja at this point still assumes that build directories are two
levels up from the chromium/src directory. This will be addressed in a
follow-on cl.
Bug: b/169983312,b/253452972
Change-Id: I13eb1e5d8ebfb83d565ea84b6a89c7784eb7fde3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3975409
Commit-Queue: Michael Savigny <msavigny@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
At the moment reclient isn't supported for developer builds. Add
messaging to autoninja to indicate this if a developer attempts a build
with use_remoteexec set.
Bug:b/199192766
Change-Id: I9e016a09c98b756018505f661afdb6eaf3db3243
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3839070
Commit-Queue: Michael Savigny <msavigny@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
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>
M1 mac seems to have capacity to build with higher parallelism.
This also remove limit in macOS.
Bug: 1317620
Change-Id: I4460915c405cbb27ed977dcee631adb8753f2335
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3596361
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Junji Watanabe <jwata@google.com>
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Philipp Wollermann <philwo@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: I4b8f8fc521386419a3121bbb07edc8ac83170a94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413679
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
This reverts commit 22bf605bb6.
Reason for revert: breaks gclient sync
Original change's description:
> Use pylint 2.7 for depot_tools
>
> This includes a few fixes for specific errors, and disables several new
> warnings introduced in this version, in order to allow for an incremental migration.
>
> Bug:1262286
> Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Bug: 1262286
Change-Id: Ieb946073c7886c7bf056ce843a5a48e82becf7a5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413672
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.
Bug:1262286
Change-Id: Ie97d686748c9c952e87718a65f401c5f6f80a5c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400616
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
We don't want to use python2 and this doesn't use third party library
too so no need to be vpython which has a small overhead.
Change-Id: Iff924a87f0562069e29195a5693e2260e4a6684e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400397
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
The change is a part of initiative to replace
use_rbe that infers using Remote Build Execution
with RE (Remote Execution) server agnostic use_remoteexec flag
Bug: 1247781
Change-Id: Ic58d70b2e604a822c03c43049417eb0a21eca728
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3387614
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bozydar Szadkowski <bozydar@google.com>
Chromium + Fuchsia + Flutter Engine tools use GOMA_DIR to determine the
location of goma's scripts and binaries. Goma installs track some state
separately, and so when a different goma distribution's `compiler_proxy`
is launched and autoninja attempts to run `.cipd_bin/gomacc port`, the
check fails even though goma is running.
Additional context:
https://github.com/flutter/flutter/issues/81510#issuecomment-839489061
Change-Id: I93775628c6efba62bb226ac1995301c3ae7f5fa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2891250
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Brandon DeRosier <bdero@google.com>
If autoninja.py detects that goma is not running when it should be then
it prints 'cmd /c exit 1' as the command to be run. This ensures that
errorlevel will be set so that the failure will be detected.
Unfortunately this doesn't work when running autoninja from git bash.
Instead of launching a cmd.exe instance which immediately exits, it
launches a cmd.exe instance which the user must manually exit.
This change adds quotes so that this works from git bash (which invokes
the shell script) as well as from cmd.exe (invoking the batch file).
Bug: 868590
Change-Id: I482f22830f9bd4f7b70c51de9647a70d946ec145
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2726085
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Owners-Override: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
This reverts commit 428143ee24.
Reason for revert: Fixing the issues revealed by the original change by
avoiding python3 and by checking for the existence of gomacc[.exe]
before running it.
This also relands 2241db8a1f - "Avoid
capture_output to support Python 3.6", to simplify relanding and any
possible reverts.
Original change's description:
> Revert "Check whether goma is running when it is enabled"
>
> This reverts commit b7ddc5a009.
>
> Reason for revert:
> This broke the builder where depot_tools is not in PATH.
> https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858077852309878080/+/u/build/stdout
>
> Original change's description:
> > Check whether goma is running when it is enabled
> >
> > One of the mistakes one can make when running ninja is having goma
> > enabled (use_goma=true in args.gn) but not having goma running. This can
> > lead to ~1,000 failed compile steps, which is messy.
> >
> > This change teaches autoninja.py to check whether goma is running. If
> > not then it tells autoninja to just print a warning message. The
> > check costs roughly 30 ms which seems reasonable.
> >
> > In fact, because this change also switches away from vpython (necessary
> > to use python3 to use subprocess.run) it actually runs about 600 ms
> > _faster_ than before this change.
> >
> > If build acceleration is requested through use_rbe then no checking for
> > whether the service is running is done. That could be added in the
> > future.
> >
> > autoninja.py could auto-start goma but that is error prone and has
> > limited additional value.
> >
> > This was tested on Linux, OSX, and Windows.
> >
> > Bug: 868590, b/174673874
> > Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
> > Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> > Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
>
> TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
>
> Change-Id: I57a6c73ea853259f3d1ec7ad0ce51e495acc96db
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 868590
> Bug: b/174673874
> Bug: 1167064
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
# Not skipping CQ checks because this is a reland.
Bug: 868590
Bug: b/174673874
Bug: 1167064
Change-Id: I8aa6830259bc18f8e7926cd0bf5c62e671c74a2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2634201
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Adds reproxy setup and teardown to autoninja. Since reproxy is intended
to run for a single build (unlike the goma proxy), having setup and
teardown happen as part of autoninja makes management of the proxy
execution easier.
To use this as it is currently implemented, set the RBE_BIN_DIR and
RBE_CFG_DIR environment variables to point to the reclient binaries and
reclient configuration files. Note the reproxy.cfg file is NOT included
in this change, and at the time of this CL you need to provide one
yourself.
Bug: 1149386
Change-Id: I23601cc9b13193ac617ffc5963b9d443f6840d33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2597837
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Ye Kuang <yekuang@google.com>
Commit-Queue: Michael Savigny <msavigny@google.com>
Auto-Submit: Michael Savigny <msavigny@google.com>
This reverts commit b7ddc5a009.
Reason for revert:
This broke the builder where depot_tools is not in PATH.
https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858077852309878080/+/u/build/stdout
Original change's description:
> Check whether goma is running when it is enabled
>
> One of the mistakes one can make when running ninja is having goma
> enabled (use_goma=true in args.gn) but not having goma running. This can
> lead to ~1,000 failed compile steps, which is messy.
>
> This change teaches autoninja.py to check whether goma is running. If
> not then it tells autoninja to just print a warning message. The
> check costs roughly 30 ms which seems reasonable.
>
> In fact, because this change also switches away from vpython (necessary
> to use python3 to use subprocess.run) it actually runs about 600 ms
> _faster_ than before this change.
>
> If build acceleration is requested through use_rbe then no checking for
> whether the service is running is done. That could be added in the
> future.
>
> autoninja.py could auto-start goma but that is error prone and has
> limited additional value.
>
> This was tested on Linux, OSX, and Windows.
>
> Bug: 868590, b/174673874
> Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I57a6c73ea853259f3d1ec7ad0ce51e495acc96db
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 868590
Bug: b/174673874
Bug: 1167064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
This reverts commit 2241db8a1f.
Reason for revert:
Cannot merge https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018 due to conflict.
Original change's description:
> Avoid capture_output to support Python 3.6
>
> autoninja.py uses subprocess.run which requires Python 3 and used
> capture_output which requires Python 3.6. One user reported this as an
> issue and it turns out that it is easy to avoid by using subprocess.NULL
> which then means that Python versions back to 3.3 are supported.
>
> Bug: 868590, b/174673874
> Change-Id: Ife5e186d9c54747d35ff989dc2afadba5b9a57f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2630525
> Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: Justin Cohen <justincohen@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
TBR=justincohen@chromium.org,brucedawson@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I5131a43eeb9410a6b45920d409a392a2df9d9af0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 868590
Bug: b/174673874
Bug: 1167064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632022
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
autoninja.py uses subprocess.run which requires Python 3 and used
capture_output which requires Python 3.6. One user reported this as an
issue and it turns out that it is easy to avoid by using subprocess.NULL
which then means that Python versions back to 3.3 are supported.
Bug: 868590, b/174673874
Change-Id: Ife5e186d9c54747d35ff989dc2afadba5b9a57f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2630525
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
One of the mistakes one can make when running ninja is having goma
enabled (use_goma=true in args.gn) but not having goma running. This can
lead to ~1,000 failed compile steps, which is messy.
This change teaches autoninja.py to check whether goma is running. If
not then it tells autoninja to just print a warning message. The
check costs roughly 30 ms which seems reasonable.
In fact, because this change also switches away from vpython (necessary
to use python3 to use subprocess.run) it actually runs about 600 ms
_faster_ than before this change.
If build acceleration is requested through use_rbe then no checking for
whether the service is running is done. That could be added in the
future.
autoninja.py could auto-start goma but that is error prone and has
limited additional value.
This was tested on Linux, OSX, and Windows.
Bug: 868590, b/174673874
Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Chromium does not use CMake, however ninja with Goma is used by LLVM
team.
Change-Id: I38349d0574048b22b32365e45c587e604e045442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2572805
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
In addition to setting GOMA_DISABLED, this CL also sets the
RBE_remote_disabled environment variable to autoninja.
Bug: 1149386
Change-Id: I9aa8be005a9cd473b7371eadcab83d6068e0e070
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2565550
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Michael Savigny <msavigny@google.com>
It is sometimes helpful to do a full build of Chrome using goma while
online and then do incremental builds without goma while offline or with
poor network bandwidth. Turning off goma in gn args triggers a full
rebuild but the GOMA_DISABLED environment variable can be used to
disable goma without triggering a rebuild.
In order to make this feature easier to discover and use this change
adds support for -o/--offline to autoninja. autoninja -h will mention
this flag and autoninja.bat/autoninja.py handle it appropriately. This
means setting the environment variable in autoninja.bat, and using it
to adjust the -j value and stripping it out in autoninja.py.
The bash script that wraps autoninja.py on Linux and OSX did not need
updating because the Python script can emit 'GOMA_DISABLED=1 ninja ...'
and this is executed by bash.
This was produced during pair programming with jessemckenna@
Bug: b/172039612
Change-Id: Ifcfbc598ac20f23e5fe013e02979b5b8c2851b01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2510818
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Here we add slightly improved detection for the goma flag.
The detection of goma in autoninja does not work for the use_goma flag
when it does not simply appear at the beginning of the line.
Below are examples of multi-arg lines we now support:
is_debug=false use_goma=true is_official_build=false
use_goma=false# use_goma=true This comment is ignored
Also adding linux nice process launch feature to improve the user
experience when running a large build. This feature is disabled by
default like other autoninja features.
R=brucedawson@chromium.org
Change-Id: I8a04e7480c5e8d37d57fb71143c6066d46abb004
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2479683
Commit-Queue: Peter McNeeley <petermcneeley@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
The goma-on-borg backend is turning down. While chromium can
continue using the goma client for build acceleration, the
backend is swapped out to instead use Remote Build Execution,
a gcloud service.
Other downstream projects (e.g. Nest) switched out the goma
frontend as well as the backend to RBE. These projects use a
|use_rbe| gn flag to enable build acceleration through RBE.
This change allows autoninja to recognize this flag and treat it
as if it was using goma for the purposes of passing an
appropriate -j flag.
Bug: None
Test: run on an out/ directory with use_rbe enabled
Change-Id: I52d5771e92e9163234811b154fd36816639bf1b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2391009
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
This was made by running `codespell` and `scspell`
and then checking the results.
Change-Id: I169fd5b40294f83015075b4a899fbca263821f25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2144602
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Auto-Submit: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
This updates the comments and code regarding GOMA_DISABLED to more
precisely match its behavior and therefore avoid confusion.
Bug: b/140312943
Change-Id: I6208de282bd5ebf1d88f98c1f905cfc626de792e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1786676
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
The NINJA_SUMMARIZE_BUILD already triggers two types of build
performance data - setting NINJA_STATUS to give more details during the
build and running post_build_ninja_summary.py after the build.
One of the most common causes of slow builds is when CreateProcess
becomes slow, usually due to anti-virus overhead. Neither of these
reports make this problem easily visible, but ninja -d stats does. In
particular the StartEdge data is an excellent summary of CreateProcess
timing. If the StartEdge average (avg) is more than about 6,000 us
(6.0 ms) it might be worth investigating why.
Therefore this change makes it so that autoninja.py adds -d stats to the
command line when NINJA_SUMMARIZE_BUILD is set to 1.
Change-Id: I1b6b990708c784a2cd71d153713b422f090775e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1788399
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
On the wasm team we build several CMake-based projects with ninja
(LLVM, binaryen, etc), and its useful for us (me at least) to be able
to use autoninja with these project too.
Change-Id: I7e213448dbbe95ffe3d249c9c6a3d4baa41f50d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1682742
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Sam Clegg <sbc@chromium.org>
This incorporates a fix to psutil's calculation of
the number of processors on Windows systems with
more than 64 cores.
Windows systems with more than 64 logical
processors divide the processors into groups, with
most applications using only one group, i.e. a
portion of the actual processing power available.
Prior to v5.4.4, psutil only counted the number of
processors in the first processor group. This
resulted in only partial utilization of high-
processing-power systems.
For example, on the 72-core P920, this change has
the following effect:
Before:
> autoninja -C out\nogoma chrome
"c:\src\depot_tools\ninja.exe" -C out\nogoma base -j 38
After:
> autoninja -C out\nogoma chrome
"c:\src\depot_tools\ninja.exe" -C out\nogoma base -j 74
Using this new version of psutil doubles the
number of processors used for building Chrome on
the P920 (when not using goma).
A similar bug exists in ninja.exe, so using
autoninja will be particularly important after
this fix for users of the P920 and other systems
with >64 cores. More fixes will be needed
elsewhere for similar bugs - see crbug.com/980967
for general progress.
Bug: 980270
Change-Id: I8de61a72cf95acf28ef1bcef1b0057b7b1225832
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1686081
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Also make autoninja.py a vpython script to use reliable version of
psutil.
Note: this change also makes autoninja always make a decision about
-j; there's no longer a default where it lets ninja pick. The code is
simpler this way and I think it's better because it lets developers
always see which -j is in effect when using autoninja (and that's its
exact purpose, if you wanted default you shouldn't have used autoninja).
R=dpranke@chromium.org
Bug: 976265
Change-Id: Ic9d12469729e4bf58da1ec1bd70437329519fc46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1663904
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Ran "2to3 -w -n -f print ./" and manually added imports.
Ran "^\s*print " and "\s+print " to find batch/shell scripts, comments and the like with embedded code, and updated them manually.
Also manually added imports to files, which used print as a function, but were missing the import.
The scripts still work with Python 2.
There are no intended behaviour changes.
Bug: 942522
Change-Id: Id777e4d4df4adcdfdab1b18bde89f235ef491b9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595684
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Auto-Submit: Raul Tambre <raul@tambre.ee>
Regex strings need to be raw or else double-escape all the \ characters,
but Python 2.7 doesn't reliably enforce this, so bugs have crept in.
The buggy regex from create_installer_archive.py that this was copied
from was deleted a while ago. However this bad pattern is pervasive in
Chrome and depot_tools, as crudely found by searching for:
re\.[a-z]*\('.*\\[dsw\(\.]
An earlier instance of this was found when running some of our scripts
with Python 3.8.
Bug: 958138
Change-Id: If7ded5ae13f8cc36a5f6277c6ae0a2f54f88c3e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1590191
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Large parallelism causes too many open files error.
Let me limit to 500 for Mac now.
Bug: 936864
Change-Id: I2fccc8cf14483c6f34d84c84d82c44df6e4f3177
Reviewed-on: https://chromium-review.googlesource.com/c/1496675
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
On Windows, without quote, whole return value would be treated as
a command, and the execution would fail.
On Linux and Mac, without quote, if depot_tools is settled under
directories with ' ', command execution would fail because paths are
separated in a wrong way.
To make such a return value work on Linux and Mac, the shell script
started to use eval.
Bug: 902930
Change-Id: I9bb74585294af565988c0b844b6b113a5c685530
Reviewed-on: https://chromium-review.googlesource.com/c/1325249
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Since ninja uses getopt_long to parse the argument, we need to suppose
-j100 and -tclean.
Please see ReadFlags function in:
https://github.com/ninja-build/ninja/blob/master/src/ninja.cc
Change-Id: If61f05e66be546a591549f6f153cce994c972309
Reviewed-on: https://chromium-review.googlesource.com/c/1335428
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
When people execute autoninja, I believe people would expect ninja
in the autoninja directory would be used. However, the original code
find ninja from PATH, and ninja in other directory could be used.
People usually do not notice this because having depot_tools in
PATH is recommended for Chromium developers. However, in some
environments, an old version ninja is pre-installed, and unexpected
version ninja could be used upon PATH environment.
Also, in most of bots, depot_tools directory is not included in PATH.
Autoninja execution would fail there because the system cannot find
ninja.
Bug: b/77176746
Change-Id: Iad8bd952dc1e34a9d303fd5b493c555156369a17
Reviewed-on: https://chromium-review.googlesource.com/c/1319489
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Fumitoshi Ukai <ukai@chromium.org>
Reviewed-by: Shinya Kawanaka <shinyak@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Auto-Submit: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
autoninja automatically sets -l <num_cores>. -l option makes ninja
not invoke a new command if current CPU load average is above -l.
However, as far as I investigated, -l <num_cores> make the build
much slower than without -l especially on Linux & OSX machines with
small numbers of cores. I should say -l decreases the build
performance. When I build Chromium with Goma with the same -j with
autoninja without -l, the load average goes more than the number of
cores while keeping the machine working as usual.
Also, ninja can invoke commands until the spike of command invocation
is reflected to load average, -l might not mitigate for a machine
to get stuck by too high load.
Note that from what I understand from the implementation, Windows
ninja's posix-compatible load average is always less than num_cores.
i.e. -l <num_cores> won't limit the process invocation.
5984986459/src/util.cc (L479)
Let me make autoninja not set -l and bring better build performance
on machines with small number of cores.
Bug: b/117810340
Change-Id: I50f231f1a8976f8ecfc3a0c778f0f1ac98d3827f
Reviewed-on: https://chromium-review.googlesource.com/c/1290611
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Nina supports -C out/Default and -Cout/Default to specify the build
directory so autoninja should also. This change adds that support.
Bug: 890744
Change-Id: I5e824242ed4b333ac99f1ee9a649ffcfa03a812e
Reviewed-on: https://chromium-review.googlesource.com/c/1257586
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
When GOMA_DISABLED is set goma will use the local compiler. Autoninja
needs to understand this in order to avoid requesting too much
parallelism.
Change-Id: Ic124893dd583a401d0d9ad7fbd27ee9b6715fcfe
Reviewed-on: https://chromium-review.googlesource.com/1015402
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
This is the second attempt at fixing autoninja to allow passing '^^' to
it to specify that ninja should build the outputs of the specified file
instead of building that file itself. The problem is that '^' is a
special character and when extra layers of indirection are added the
number of '^' characters needed grows exponentially in some poorly
understood way. The first fix attempt just quoted the arguments that
autoninja.bat passed to autoninja.py, but that meant they came in as
one argument. This fix expands on that by modifying autoninja.py to
understand how to deal with the monolithic argument. With this change
this once again works:
autoninja -C out\debug_component ..\..\base\win\enum_variant.cc^^
It can be convenient to have a ninja.bat file which starts goma and lets
users keep typing the same build commands. However even with this fix
the previously recommended ninja.bat file must be invoked with four
'^' characters. If that is too much then the new recommended ninja.bat
is to copy autoninja.bat and modify as needed, perhaps like this:
@echo off
call python c:\goma\goma-win64\goma_ctl.py ensure_start >nul
FOR /f "usebackq tokens=*" %%a in (`python c:\src\depot_tools\autoninja.py "%*"`) do echo %%a & %%a
BUG: 758725
Change-Id: Ieee9cf343ee5f22e9988a1969cb7a7a90687666b
Reviewed-on: https://chromium-review.googlesource.com/656478
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
The -t tools in ninja fail if -j or -l are specified. So, autoninja.py
needs to watch for -t and omit -j and -l if it is noticed.
R=sebmarchand@chromium.org
BUG=740227
Change-Id: I1418193daeab154178d15be60ab09551bacaf3af
Reviewed-on: https://chromium-review.googlesource.com/563775
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Using goma requires the developer to remember which build directories
use goma and which don't so that they can pass an appropriate -j number.
Getting this wrong makes builds slower, either by under utilizing
resources or by causing a self-inflicted DOS attack. Usage:
autoninja -C out/debug
autoninja looks at the settings for the specified build directory and
then selects either -j num_cores*20 or no -j flag based on the
use_goma setting.
You can set the NINJA_CORE_MULTIPLIER variable to change from the
default 20* multiplier. You can also use NINJA_CORE_ADDITION if you
want non-goma builds to specify -j with an offset to the number of
cores, such as this Linux command:
NINJA_CORE_ADDITION=-2 autoninja -C out/release base
This will tell autoninja to pass -j to ninja with num_cores-2 as the
parameter.
On Windows you can have a ninja.bat file (ahead of ninja on the path)
such that autoninja will automatically be used. It should contain this:
@call autoninja.bat %*
Change-Id: I4003e3fc323d1cbab612999c945b5a8dc5bc6655
Reviewed-on: https://chromium-review.googlesource.com/517662
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Fumitoshi Ukai <ukai@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>