PRESUBMIT.py files should not have CRLF line endings, but if a git repo
is misconfigured this may happen and it is important for the presubmits
to run properly so that they can report on this issue.
In particular, the regex which detects USE_PYTHON3 = True did not work
with CRLF line endings.
It was supposed to handle these, and there is even a comment saying:
# Accept CRLF presubmit script.
However the line after that uses 'rU' as the file open mode in order to
try to get universal newlines, but FileRead ignores the file mode. So,
the replace method is now used instead.
Bug: angleproject:7475, angleproject:4905
Change-Id: I38321176feca089a5796b24756b371709d852243
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3739269
Reviewed-by: Eddie Hatfield <eddiehatfield@google.com>
Reviewed-by: Cody Northrop <cnorthrop@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This CL relaxes the check for the .git folder on Windows so that it
may be either a file or a folder, as may be the case if depot_tools
is a git submodule or a worktree.
Bug: 1333997
Change-Id: Ib805912903696fc247f85d15c35b6e3093cd57f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3736684
Commit-Queue: Will Cassella <cassew@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
The depot_tools directory may get into a state in which its no longer a
git repo, in which case we can't update it. On Windows this would cause
the update_depot_tools.bat script to fail with a somewhat confusing
error message, this CL fixes that to inform the user what happened and
what they should do about it.
Bug: 1333997
Change-Id: I2c60fc46f1fae7811ff6610e25380d08010d0597
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3718926
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Will Cassella <cassew@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Regression was introduced as part of gclient_scm change to support cog.
The directory should be cleaned up regardless if is empty or not, and if
statement is used only to warn user about non-empty directory, which si
not expected.
R=jojwang@google.com
Fixed: 1339077
Change-Id: Ie3bc6bcac168bad9cf2cb6adc6ae4024fe9ef167
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3724051
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
AFAICT
- the code never reaches this point because we already check should process before adding it to the queue. http://shortn/_PWFae17qw4
- this might be necessary if the value of should_process changes between adding it to the queue and running it. But I don't see that happening anywhere. should_process is only ever set and modified here: http://shortn/_eirozAoIm4 which happens during in the parent's run() before the deps get added to the queue.
The should_process check that happens at the beginning of run() was added here: https://codereview.chromium.org/3124017/diff/2001/gclient.py
The should_process check before a deps gets enqueued was added later here: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/559150/
where several should_process checks were added and I think the author just didn't realize that, while the other methods might need them, run() already had a should_process check.
Alternatively, we could remove the should_process check before enqueue-ing. But that would result in enqueuing things that we currently don't enqueue, even if we ultimately don't do anything with it.
Change-Id: Ib3be61310c09a681988baa75fd562c7596e9e044
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3721600
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Joanna Wang <jojwang@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/8810528512950350097
recipe_engine:
5a7f940c8e
5a7f940 (yiwzhang@google.com)
roll the pin of luci-go proto (excluding buildbucket proto for...
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: I3cc6b248d7b6e38cf05a61f413ceb1b5458d82ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3722721
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>
If a presubmit test is misconfigured then it may end up running on
neither Python 2 nor Python 3. This was supposed to trigger a warning
but the warning code was broken for almost a year. It was then fixed so
that it issued an error, but that caused some breakage, so it was made a
warning.
The known problems with tests not running at all have been fixed and
the warning has been on for over two weeks so this change turns the
warning back into an error. This is appropriate because a test that
is not run at all is a serious bug. This change will allow us to move
more confidently when switching tests to Python 3, and it shouldn't
cause any additional breakage.
Bug: 1330859
Change-Id: I51028bb9c896c60c5cf3ccb6f472ade0bb1e0c16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3717242
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
This reverts https://crrev.com/c/3697958 and https://crrev.com/c/3704921.
One reason is that recipe bundle is not designed to be mutated.
Another reason is the presence of gomacc binary under .cipd_bin causes an unexpected side effect. https://crbug.com/1337048
Bug: 931218, 1337048
Recipe-Nontrivial-Roll: build_limited
Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: chrome_release
Recipe-Nontrivial-Roll: chromiumos
Recipe-Nontrivial-Roll: infra
Change-Id: Iaea12a0ad9246c0781035aa499cc6c0bb42d71c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3715873
Auto-Submit: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
This CL updates cpplint.py to add some of the c++17 header files to the
list of known c++ headers. We're specifically using optional and variant
in the Tint project and would like to remove the NOLINT lines necessary
to keep the linter happy.
Bug: dawn:1379
Change-Id: I41cacd21e0742f992cdd6913b7f4def556b543c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3709241
Auto-Submit: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
The multiprocessing module on Windows has a bug where if you ask for
more than 60 child processes then it will hang. This is related to the
MAXIMUM_WAIT_OBJECTS (64) limit of WaitForMultipleObjects. Other sources
have listed the multiprocessing limit as being 61, or have said that the
the maximum number of objects that can be waited on is actually 63, but
those details don't really matter.
The original fix for this class of issues was crrev.com/c/2785844. This
change extends those fixes to depot_tools, which was missed last year.
This change also updates how PyLint is called by further limiting the
number of jobs to the number of files being processed divided by 10.
This is because there is a significant cost to creating PyLint
subprocesses - each takes about 0.5 s on my test machine. So there needs
to be enough parallelism to justify this.
Patches for PyLint and a bug for cpython are planned.
This will stop PyLint from hanging during presubmits on many-core
machines. The command used to reproduce the hangs and validate the fix
was:
git cl presubmit -v --force --files "chrome/test/mini_installer/*.py"
Prior to this change this command would use (on my many-core test
machine) 96 processes and would hang. How it uses just two processes
because there are only 16 files to analyze.
Output before:
Pylint (16 files using ['--disable=cyclic-import'] on 96 cores)
Output after:
Pylint (16 files using ['--disable=cyclic-import'] on 2 processes)
This is actually not quite true because the hang would prevent the
old message from being displayed.
Bug: 1190269, 1336854
Change-Id: Ie82baf91df4364a92eb664a00cf9daf167e0a548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3711282
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This is always recommended, since wheel availability varies between
minor versions. We may not support major-version-only specifiers
in the future.
Change-Id: Ib41426fa39212fe3e49d87c1d1c8b5bbc49aef7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3708295
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Brian Ryner <bryner@google.com>
There are still some places that directly calls depot_tools/ninja{,.exe} without ensuring .cipd_bin dir.
This is a binary build on 3pp CIPD pipeline specified in cipd_manifest.txt.
Bug: 931218
Change-Id: I9f455bf47d648ce8e5bf74bd6c6d356867766314
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3708571
Auto-Submit: Junji Watanabe <jwata@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This reverts commit 0a5bae7ebc + some modifications on recipe module.
The previous CL https://crrev.com/c/3697958 ensures depot_tools/.cipd_bin in recipe environments. This CL copies ninja.exe from .cipd_bin to depot_tools root as cipd_bin_setup.bat.
For developers, it's expected to run update_depot_tools which triggers cipd_bin_setup.
For CI/CQ, depot_tools._cipd_bin_setup() should be called before using ninja/autoninja recipe module APIs.
Builds with depot_tools recipe bundle are tested using led.
# Windows
> led get-build 8811491780369249585 | led edit-recipe-bundle -O depot_tools="$HOME/depot_tools" | led launch
https://ci.chromium.org/swarming/task/5b7512ee9d91fc10?
# Linux
> led get-build 8811504372914233697 | led edit-recipe-bundle -O depot_tools="$HOME/depot_tools" | led launch
https://ci.chromium.org/swarming/task/5b7515832d0f7c10
Original change's description:
> Revert "Switch to use CIPD ninja v1.8.2"
>
> This reverts commit c90a982106.
>
> Reason for revert:
> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8811949917908303825/+/u/compile/stdout
>
> Original change's description:
> > Switch to use CIPD ninja v1.8.2
> >
> > https://crrev.com/c/3674981 installed CIPD ninja packages.
> > This CL switches from the old ninja binaries to them.
> >
> > Note that this CL doesn't change ninja version. the old/CIPD ninja binaries are v1.8.2.
> >
> > The differences from the old binaries are
> > - the CIPD packages don't use chromium's clang.
> > - the CIPD package for Linux doesn't use chromium's sysroot.
> > - the min macOS version is 10.13, which was 10.6.
> > Please also see https://crsrc.org/i/3pp/ninja/install_bootstrap.sh
> >
> >
> > On Windows, ninja.exe will be installed under .cipd_bin.
> > But there are many places that assume that ninja.exe exists on depot_tools root. So this CL also copies the ninja.exe from .cipd_bin to root in cipd_bin_setup.bat.
> >
> > Bug: 931218
> > Change-Id: Ib67eee5e9b6ad9b2937b789626970d9c85867dbd
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3674982
> > Commit-Queue: Junji Watanabe <jwata@google.com>
> > Reviewed-by: Philipp Wollermann <philwo@google.com>
> > Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
> > Reviewed-by: Nico Weber <thakis@chromium.org>
> > Reviewed-by: Josip Sokcevic <sokcevic@google.com>
>
> Bug: 931218
> Change-Id: I08d61dad119a0d23ae9ec8b1ed787dd3915de697
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3686854
> Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> Owners-Override: Takuto Ikuta <tikuta@chromium.org>
Bug: 931218
Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: build_limited
Recipe-Nontrivial-Roll: chrome_release
Recipe-Nontrivial-Roll: infra
Change-Id: Ic7a9395c0e858823375f1a6ec79034dbc54230bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3696963
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
The meta quest browser has an issue with the initial design where the all the dependencies within the same tree are overridden with the solution URL's scheme. This fix updates fetch to add a .gclient entry instead of implicitly overriding the URL's scheme.
Bug: 1336027
Change-Id: Iabe5b9017f6eb9e682a0c721f43dd5f347ffcbfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3703942
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
This reverts commit 6754c49e02.
Reason for revert: will be relanded with fixed batch comments that triggered parser error.
Original change's description:
> Revert "Make depot_tools scripts return exit codes properly on all platforms."
>
> This reverts commit 7c4f7ec408.
>
> Reason for revert: bugs reported by users (see comments in Gerrit).
> error: block. was unexpected at this time.
>
> Original change's description:
> > Make depot_tools scripts return exit codes properly on all platforms.
> >
> > Changes:
> > 1. Windows: exit /b %errorlevel% should be used instead of goto :EOF to get valid exit codes during cmd /c <script>.bat invocation.
> > 2. Windows: delayed var expansion is required in update_depot_tools.bat exit code generation.
> > 3. Posix: update_depot_tools returns exit code from update_git_repo function in case of a failure.
> >
> > A rule of thumb on Windows: goto :EOF should not be used if %errorlevel% must be returned for all possible invocations.
> >
> > Test case for update_depot_tools changes:
> > 1. Make a change to depot_tools sources that will conflict with next depot_tools update
> > 2. Run update_depot_tools either directly or via gclient
> > 3. Expect a git error is triggered
> > 4. Inspect %errorlevel% or $? depending on platform
> > 5. Expected 1, but the actual result is 0.
> >
> > Test case for changes in other .bat files:
> > 1. Make a change to depot_tools sources that will conflict with next depot_tools update
> > 2. Run cmd /c gclient
> > 3. Expect a git error is triggered
> > 4. Inspect %errorlevel%
> > 5. Expected 1, but the actual result is 0.
> >
> > Change-Id: I64459982bcd9cc3db1319a9b39224b7a7af8c5aa
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3688632
> > Reviewed-by: Josip Sokcevic <sokcevic@google.com>
> > Commit-Queue: Josip Sokcevic <sokcevic@google.com>
> > Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
> > Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
>
> Change-Id: I85d598af01d75588cdee77165d6af22270ee031d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3694139
> Auto-Submit: Josip Sokcevic <sokcevic@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: I377f966ea1b1a567de815caca703b5e124a76b64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3696396
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.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/8811716339900330657
recipe_engine:
dfb53acac9
dfb53ac (jkusuma@google.com)
Update resultdb recipe module to support gcs uri.
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: I873e31a9bd0d955f7f04b11305874c6f083840e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3699409
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>
While all Chromium PRESUBMIT.py scripts have been running on Python 2
for a long time they continue to invoke child scripts under Python 2.
Part of the reason for slow progress on this transition is that it is
not easy to tell that this is happening, and most developers probably
assume that Python 3 presubmits implies a lack of Python 2.
This change adds a warning when it detects Python 2 scripts being run.
Typical output (edited for clarity) looks like this:
git cl presubmit --files "chrome/updater/tools/*;ppapi/generators/*"
...
Python 2 scripts were run during Python 3 presubmits. Please ask ??? if help is needed in preventing this.
"depot_tools\pylint-1.5" --args-on-stdin from chrome\updater\tools \
"depot_tools\pylint-1.5" --args-on-stdin from chrome\updater\tools \
idl_tests.py from ppapi\generators
If Python 2 scripts launch child scripts, especially if they use
sys.executable, then they will not be reported. However this is a good
thing because it means that the report focuses on the top-level scripts
that drive Python 2 usage.
This change works by modifying vpython.bat to write invocation
information to a text file. The data in this text file is picked up by
presubmit_support.py when it finishes running a set of presubmits.
Bug: 1313804
Change-Id: Ic632b38eae07eca2e02e94358305cc9c998818e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3699002
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
When initializing git_cache repository, we can't assume the default
branch is main. This results in HEAD pointing to invalid ref for repos
that don't have main branch.
Instead, fetch information from remote git repository.
R=gavinmak@google.com
Fixed: 1334889
Change-Id: I84e6d8e09c81bdae7e5e31af88973534431824e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3696906
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Add helpers to gerrit_util for:
* Setting the commit message on a ChangeEdit,
* Creating a cherry-pick,
* Getting the contents of a file in a change.
Bug: v8:12849
Change-Id: I1a5de3054a366d480def32942fd42be179ffb749
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683383
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Richard Wang
recipe-roller
Bruce Dawson
Joanna Wang
manukh
Vadim Shtayura
Yulan Lin
Will Cassella
Yuanjun Huang
Josip Sokcevic
Fumitoshi Ukai
Yiwei Zhang
Junji Watanabe
Ergün Erdoğmuş
dan sinclair
Brian Ryner
Aravind Vasudevan
Aleksey Khoroshilov
Leszek Swirski