In repos that are checked out separately from chromium, invoking
"git cl format" on a change that has files that would be formatted using
clang-format causes an error because the depot_tools version of
clang-format looks for a clang-format inside the repo.
This prevents using proto-based properties (the preferred mechanism
going forward) in recipe repos that enforce formatting. The
--no-clang-format flag provides a workaround that can be used to
prevent trying to format any files where clang-format doesn't work.
Bug: 979330
Change-Id: Ice8561d88b29623deb953465253f92c88aa7fc2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1986111
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
The problems on crbug.com/1017367 should have been fixed by crrev.com/c/1880013
Bug: 1009814
Change-Id: Ia98304ca983b4e7e293fedb9df1fee589de58ba9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1887714
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
On windows on Python 2, only strings can be environment variables, but
literals are unicode, so we need to convert to strings before setting
env vars.
Change-Id: I40e8e6ae4f5497d56efa568153eb65392dbd85e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1875108
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Ran:
vi $(git grep --name-only iteritems | grep -v third_party)
vi $(git grep --name-only itervalues | grep -v third_party)
vi $(git grep --name-only 'print ' | grep -v third_party)
and edited the files quickly with adhoc macros. Then ran in recipes/:
./recipes.py test train
There was only a small subset of files that had been updated to use
six.iteritems() and six.itervalues(). Since the dataset size that is
being used in gclient is small (pretty much always below 200 items),
it's better to just switch to .items() right away and take the temporary
performance hit, so that we don't need to come back to rewrite the code.
Recipe-Nontrivial-Roll: build
Bug: 984182
Change-Id: I5faf11486b66b0d73c9098ab0f2ce1b15a45c53e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1854900
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Marc-Antoine Ruel <maruel@chromium.org>
"Fixed:" is equivalent and the preferred syntax.
Also sets up a more generic mechanism for "unwanted tags",
which BUG= and TBR= may eventually be when we're ready to
move over to git-footers more fully.
Bug: monorail:4470
Change-Id: I2e999d364292447124870b8d3539fde1ac1eb4ec
Recipe-Nontrivial-Roll: chromiumos
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1836607
Auto-Submit: Dan Beam <dbeam@chromium.org>
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
https://chromium-review.googlesource.com/982601 disabled TBR for CLs that
modified OWNERS files. This CL relaxes the restriction to permit TBR for
CLs that have LGTMs for their modifications to OWNERS files.
Bug: 688115
Change-Id: I47fef6b1eb021ca7cdfc003dc57722643b174a6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1797605
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Add run_on_python2 and run_on_python3 to allow to be tests on
either only python2, only python3 or both.
Bug: 984182
Change-Id: I87d3b67412129cf3a8e627cd6bc97daa81147657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1758849
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: smut <smut@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
This allows people to pin a specific version of pylint when running
it. The default behavior is unchanged: we run pylint-1.5.6 still
as we've created a vpython wheel in cipd for it. But for projects
that want to move to a newer version, they can pick any of the new
1.x wrappers. Having the full set of versions here allows projects
to incrementally move to newer versions as the lint delta between
1.5 & 1.6 tends to be much smaller than 1.5 & 1.9.
We don't include support for the newer 2.x series as those only
support Python 3 which the codebase isn't ready for.
Bug: 866772
Test: `./pylint ...` still works & shows pylint-1.5
Test: `./pylint-1.5 ...` runs the v1.5 version
Test: `./pylint-1.8 ...` runs the v1.8 version
Change-Id: I7c47187493564db81e3eb28c6dbd09e6309ead46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1752507
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Useful in cases like the green presubmit runs on crrev.com/i/1434617/2
Bug: 925774
Change-Id: I37ce2be843d76ace0615cd3313e5c8421504a1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1687122
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
This is followup from the malformed-json bug which took down
Sheriff-o-Matic on 2019-05-16. This CL just adds the canned check;
future CLs can either add it to PanProjectChecks or add it to
other PRESUBMIT files directly.
Change-Id: I4a445193c1744966a448b12c7eb2915873e484d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1617941
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Sean McCullough <seanmccullough@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>
The previous implementation of CheckLongLines did not
handle global pylint disable/enable directives properly,
i.e. the difference between:
# pylint: disable=line-too-long
.... checks disabled for all lines here.
# pylint: enable=line-too-long
versus:
# Check only disabled for the line below
some python statements # pylint: disable=line-too-long
This CL changes the implementation to support Python files
properly. Note that in order to not disturb the mock-based
unit-tests, a new function is introduced to be able to
filter the list of affected files based on their file
extension.
BUG=890734
R=mattcary@chromium.org,ehmaldonado@chromium.org,dpranke@chromium.org
Change-Id: Id52deff53913b8d47a4157f42b1fffbd3b103201
Reviewed-on: https://chromium-review.googlesource.com/c/1396094
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Fix CheckOwnersFormat so that it checks the new OWNERS files instead of the
original ones.
Change-Id: I4abb1cfc847bd28f45b9948fe7775c10dcc8a420
Reviewed-on: https://chromium-review.googlesource.com/c/1382946
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Right now, if clang-format is broken, a CL will still pass CQ.
This adds a flag so that it will fail CQ.
Bug: 911708
Change-Id: I2c71b7bc434fc611d51f688266be6a265b80f4da
Reviewed-on: https://chromium-review.googlesource.com/c/1361560
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Ryan Tseng <hinoka@chromium.org>
Enables python formatting in git cl format by default
for all files that have a .style.yapf file in a parent
directory up to the repository root. If no .style.yapf
file is found the --python flag still needs to be
set explicitly for python formatting to be enabled.
Also adds a --no-python to explicitly disable python
formatting on any file even if a .style.yapf file is found.
This allows default formatting to be enabled on a per
project/directory basis by adding a .style.yapf file.
Bug:846432
Change-Id: I40d899ec1a3e0dfca445e04b91befab113416175
Reviewed-on: https://chromium-review.googlesource.com/c/1316415
Commit-Queue: Aiden Benner <abenner@google.com>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Not enabling it yet, since PRESUBMIT.py uses presubmit_canned_checks.py from
depot_tools at HEAD, not from a CL (thus enabling this check in this CL will
make it fail the presubmit).
R=nodir@chromium.org
BUG=870166
Change-Id: I97802ec37f1f7513dfca3950f7f38a5c51ab0350
Reviewed-on: https://chromium-review.googlesource.com/1227432
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
The message was previously not clear that if the entire CL has any change
to an OWNERS file, TBR will not apply.
Change-Id: Ib30a93f80196f45b02a29701faab7cf581e37f50
Reviewed-on: https://chromium-review.googlesource.com/1176321
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Add check_python parameter to CheckPatchFormatted to check that python
files are well formatted.
Change-Id: Ibd20d5ec9d1d5142a26f7f4cd5f05c51223da6ca
Reviewed-on: https://chromium-review.googlesource.com/1081342
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Expansion of the tests to check for this are required.
Change-Id: Iae6294501659738df5097ce5a72da79e72818d98
Reviewed-on: https://chromium-review.googlesource.com/1030956
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Since no one can upload or land changes from Rietveld anymore,
PRESUBMIT and its support files no longer need to be able to
support it.
Bug: 770408
Change-Id: I4ca6391291e7b0755c78af453c3d006ad3666a17
Reviewed-on: https://chromium-review.googlesource.com/991053
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This reverts commit f3eed0016e.
Reason for revert: Some PRESUBMIT checks still expect
CheckRietveldTryJobExecution to exist.
Original change's description:
> Remove Rietveld support from presubmit
>
> Since no one can upload or land changes from Rietveld anymore,
> PRESUBMIT and its support files no longer need to be able to
> support it.
>
> R=tandrii@chromium.org
>
> Bug: 770408
> Change-Id: I749092b66fdca16d5cef77e8cefc905aa5375b50
> Reviewed-on: https://chromium-review.googlesource.com/693380
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
TBR=agable@chromium.org,tandrii@chromium.org
Change-Id: I72e29bd8a9739406f29190adbeb7eb7718ed21cd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 770408
Reviewed-on: https://chromium-review.googlesource.com/991012
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
TBRs bypass owners checks, but are also semi-suspicious: a
contributor TBRing many changes in a row to submit code to
a directory they don't own would be seen, frowned upon, and
inquired into. However, a contributor could bypass this by
simply TBRing a single change to add themselves to an OWNERS
file, and then contributing as normal from there. This CL
removes that loophole.
This will not affect sheriffs who TBR reverts for two reasons:
first, it is rare that a chance touches both code and an
OWNERS file, and therefore it is rare that OWNERS changes
get reverted; second, quick reverts (the kind sheriffs do)
bypass PRESUBMIT entirely, and therefore also skip OWNERS
checks.
Bug: 688115
Change-Id: If2b5c9d058c62caf95389287e0bb706aef721baf
Reviewed-on: https://chromium-review.googlesource.com/982601
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Since no one can upload or land changes from Rietveld anymore,
PRESUBMIT and its support files no longer need to be able to
support it.
R=tandrii@chromium.org
Bug: 770408
Change-Id: I749092b66fdca16d5cef77e8cefc905aa5375b50
Reviewed-on: https://chromium-review.googlesource.com/693380
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
On buildbot (only), we don't trust PYTHONPATH, so we have set
VPYTHON_CLEAR_PYTHONPATH to make the first invocation of vpython in a
given chain of python invocations clear PYTHONPATH.
This has the unfortunate side-effect that python scripts that set
PYTHONPATH and then call into vpython (like presubmit) end up
having their PYTHONPATH modifications removed.
As a workaround (until I clean up PYTHONPATH in buildbot for real), fix
presubmit_canned_checks so that it clears VPYTHON_CLEAR_PYTHONPATH when it
adjusts PYTHONPATH.
R=charlie@chromium.org, nodir@chromium.org, vadimsh@chromium.org
Bug: 825290,825174
Change-Id: Ib5f73add1726fdf3c335d26fc0af76cfe3b747b2
Reviewed-on: https://chromium-review.googlesource.com/978632
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Currently in LUCI `git cl` is invoked on chromium_presubmit using vpython. This
means that it operates under the depot_tools .vpython environment (which is
blank).
Some features in presubmit_support allow invocation of python subprocesses, and
previously they would use `sys.executable` (especially on windows, but
occasionally on non-windows as well).
In non-vpython environments, this just picks up the system python and whatever
modules happen to be installed there. Some python unittests work around this on
non-windows systems by having a shebang line which explicitly invokes vpython.
This CL changes presubmit_support so that invocations of 'python', or executions
of '.py' scripts will always end up invoking vpython. In the best case, this will
pick up the invoked scripts' vpython environment. In the worst case, this will
invoke the script under an empty vpython environment (aka "stock python").
R=dpranke@chromium.org, jbudorick@chromium.org, tandrii@chromium.org, tikuta@chromium.org
Bug: 821669
Change-Id: I5d2d5dfd0364022d56833c2c8af4983553a29c7a
Reviewed-on: https://chromium-review.googlesource.com/961865
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
This check will be used in Chromium, V8 and Catapult PRESUBMIT scripts.
R=maruel@chromium.org, tandrii@chromium.org
Bug: 777893
Change-Id: I2ca1e774b89787c4d3b5f336315d145571858864
Reviewed-on: https://chromium-review.googlesource.com/738169
Commit-Queue: Sergiy Byelozyorov <sergiyb@google.com>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Follow-up to d86c803715. The error message
was still saying "src" rather than the repository root.
Bug: 768962
Change-Id: I781191e561db889a28acf51ee49a20d6d352b489
Reviewed-on: https://chromium-review.googlesource.com/719718
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Adding inherit-review-settings-ok to a subrepository causes the
PRESUBMIT.py of the parent repository to be run. In this case, the
CheckPatchFormatted() check needs to run "git cl format" with
CWD=repository root.
Bug: 768962
Change-Id: I44c642185efa78ba3a1bd752f94c3c09feff9cf1
Reviewed-on: https://chromium-review.googlesource.com/685355
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
This allows us to skip 'git cl format' for histograms.xml, which in
turn allows us to issue a presubmit error rather than a warning when
that file is malformatted, without running the formatting code twice.
The formatter is somewhat slow for this file, so it's helpful to avoid
duplicating that work.
BUG=none
TEST=none
R=iannucci@chromium.org
Change-Id: Ia86d45c7c56a9f89db34ca7175ef686589084f85
Reviewed-on: https://chromium-review.googlesource.com/569105
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
I added this when we were moving from gcc to clang on OS X many years
ago. Now that gcc has been deprecated on mac for many years, this
is safe to remove (clang errors on this pattern).
This check took 0.6s during `git cl presubmit` on a recent change of
mine, so it should speed presubmit up a bit.
Bug: none
Change-Id: Ia29b046807582e056115519fb5b34ee8a1b9ff91
Reviewed-on: https://chromium-review.googlesource.com/553238
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
We want PRESUBMIT to be able to equally support
BUG= tags (old style) and Git-Footer: footers
(new style). This change refactors the way that
the presubmit api gives access to those properties
so that it is easier to add support for equivalent
footers.
It also limits the scope of tags/footers that it
exposes, as code search shows no PRESUBMIT files
that take advantage of any of the more esoteric
ones.
Bug: 710327, 710803
Change-Id: I86f1d6cb2e1f0aff9653ef3fb455e0a6f47acf5d
Reviewed-on: https://chromium-review.googlesource.com/506450
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Previously, the <link lines in HTML are allowed to have longer than 80 characters.
This CL adds <g and <path to the HTML lines that may have unlimited characters.
(These g and path tags are used in svg image data).
These HTML exemptions also have a trailing space. The space is intended to reduce
false-positives in matching. There are some occurrences of these tags followed by a
\n which will not match (but the line ends there, so it's not an issue).
These new tags may be preceded by blank space so .lstrip() was added to
the test. I believe this .lstrip() should not be an issue for the other
languages and exemptions present.
Bug: None
Change-Id: Ie9c2bca0e7147033e360ddcfc6eee3b73e167228
Reviewed-on: https://chromium-review.googlesource.com/477371
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Windows paths use backward slash separator, which
escapes regular expressions for white and black lists
in PRESUBMIT.py filters
See https://goo.gl/6c7aku for discussion
Bug:
Change-Id: Ia1d133c5b6cfa6cb364b1500b7b7abe583bce346
Reviewed-on: https://chromium-review.googlesource.com/461141
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
This CL is a step toward checking for long lines in HTML webui files.
Those files may have long <link ...> lines which should be exempted from
the max line length.
BUG=None
Change-Id: I6fa33c004a58ea599651ae03b814fcdefbc887a1
Reviewed-on: https://chromium-review.googlesource.com/448967
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
This CL moves JS formatting behind the --js flag for git cl format
due to issue with LayoutTests. It also adds an option to check
javascript for the presubmit canned formatting check.
BUG=567770
Change-Id: I9c080b1136f6ffef9fb1b08d3bfc97ce5b3185dc
Reviewed-on: https://chromium-review.googlesource.com/430526
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Christopher Lam <calamity@chromium.org>
This reverts commit 66c50ea1f1.
Reason for revert:
Breaks some OWNERS checks. See https://crbug.com/684270 for details.
Original change's description:
> Fix OWNERS canned check to avoid duplicate output.
>
> 1. Update PanProjectChecks() to supply source_filter to CheckOwners().
> This prevents files under e.g. third_party/WebKit being checked both
> by the top-level and sub-directory PRESUBMIT rules.
> 2. Fix CheckOwners() to list missing-OWNERS only for the sub-directory
> it is invoked on, rather than the whole CL. This prevents files under
> sub-directories with their own PRESUBMIT rule, within the same repo,
> from causing OWNERS to be checked for files outside that directory.
>
> BUG=
>
> Reviewed-on: https://chromium-review.googlesource.com/426003
TBR=wez@chromium.org,dcheng@chromium.org
BUG=684270
Change-Id: I094818a6fa3e16fbac0d37d2596a40210611ee05
Reviewed-on: https://chromium-review.googlesource.com/431656
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
On larger CLs, this can be used to see whether all needed LGTMs are
there on a dry run.
R=tandrii@chromium.org
Change-Id: I5fe4022feaca73d08ead9aed14ca270192740675
Reviewed-on: https://chromium-review.googlesource.com/426819
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: smut <smut@chromium.org>
1. Update PanProjectChecks() to supply source_filter to CheckOwners().
This prevents files under e.g. third_party/WebKit being checked both
by the top-level and sub-directory PRESUBMIT rules.
2. Fix CheckOwners() to list missing-OWNERS only for the sub-directory
it is invoked on, rather than the whole CL. This prevents files under
sub-directories with their own PRESUBMIT rule, within the same repo,
from causing OWNERS to be checked for files outside that directory.
BUG=
Change-Id: I9bd5677a69efe9c78b4174a917780d6688991a38
Reviewed-on: https://chromium-review.googlesource.com/426003
Commit-Queue: James Weatherall <wez@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
This affects a bunch of files, but only changes comments,
and shouldn't make any difference to behavior.
The purpose is to slightly improve readability of pylint
disable comments.
Change-Id: Ic6cd0f8de792b31d91c6125f6da2616450b30f11
Reviewed-on: https://chromium-review.googlesource.com/420412
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
"gn format" changed to remove the --in-place argument. This updates the
instructions for formatting and fixes existing usage.
git cl format --diff never worked, it just printed the full formatted output
unconditionally. This change improves things slightly by indicating there's a
diff for a file. A proper diff can be hooked up in a later stage if somebody
so desires.
Review-Url: https://codereview.chromium.org/2220123002
Ensure consistent formatting for Mojo typemap files that define the
build rules to generate StructTraits.
BUG=620799
TEST=Tested locally, both presubmit and git cl format work as expected.
Review-Url: https://codereview.chromium.org/2074793002
Also make _RietveldOwnerAndReviewers have same semantics as before,
to resolve immediate bug until chromium checkouts of developers are
updated.
BUG=609832
R=machenbach@chromium.org
Review-Url: https://codereview.chromium.org/1955223002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300494 0039d316-1c4b-4281-b951-d872f2087c98
Reason for revert:
now it doesn't work for gerrit. Damn it.
Original issue's description:
> Implement owners check in presubmit for Gerrit.
>
> R=machenbach@chromium.org,phajdan.jr@chromium.org
> BUG=605563
>
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300350
TBR=machenbach@chromium.org,phajdan.jr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=605563
Review-Url: https://codereview.chromium.org/1935563002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300352 0039d316-1c4b-4281-b951-d872f2087c98
It's now passed using command line flags.
This depends on https://codereview.chromium.org/1931633002,
which actually uses this command line flag.
R=sergiyb@chromium.org,phajdan.jr@chromium.org
BUG=601128
Review-Url: https://codereview.chromium.org/1929653002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300268 0039d316-1c4b-4281-b951-d872f2087c98
Also, implement skipping of CheckOwners presumbit canned check. To be used by run_presubmit recipe in build repo in
https://codereview.chromium.org/1931633002
R=sergiyb@chromium.org,phajdan.jr@chromium.org,machenbach@chromium.org
BUG=594127
Review-Url: https://codereview.chromium.org/1923623004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@300262 0039d316-1c4b-4281-b951-d872f2087c98
Now that we're using real override/final and not also using
virtual on the same methods, this transitional blacklist
can go away.
R=agable, dcheng
Review URL: https://codereview.chromium.org/1564403003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@298201 0039d316-1c4b-4281-b951-d872f2087c98
It's basically like C++'s #include but in JS.
Why write 1 language when you can write 2?!
R=maruel@chromium.org
BUG=540977
Review URL: https://codereview.chromium.org/1394563003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@297065 0039d316-1c4b-4281-b951-d872f2087c98
* make RunTest's multiprocessing.Pool in the constructor of InputApi
to avoid getting tripped up by chdir manipulation.
* Don't do the split cyclic-import check when the invoker of the
Pylint presubmit checks explicitly sends cyclic import check
parameters via extra_args
* fix pseudobug where ownership of the files variable was unclear,
and pass all arguments on stdin (instead of mix of CLI + stdin).
* fix bug in pylint which caused it to manipulate sys.path before
spawning its subprocesses, which caused multiprocessing to fail
on windows.
* Note: This may carry a slight semantic change. Before, pylint would
add all .py files' directories to sys.path while checking any of
them. Now in parallel mode, pylint will only add the path of the
single file to sys.path. This behavior actually mirrors Python's
own behavior, so the check should be more-correct than before (and
should cut down on pylint import scanning time with very large
sys.path's).
* If someone encounters an issue with this, please note that the
GetPylint check also includes an extra_paths_list which is
expressly for this purpose.
R=dpranke@chromium.org, kbr@chromium.org, maruel@chromium.org
BUG=501012
Review URL: https://codereview.chromium.org/1208743002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@295908 0039d316-1c4b-4281-b951-d872f2087c98
The CQ dry run feature was announced in https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/G5-X0_tfmok
The primary complain is that the presubmit builders fail because of OWNERS checks or because of "Missing LGTM from someone other than...".
This change skips those checks for dry runs.
BUG=chromium:477190
Review URL: https://codereview.chromium.org/1090943003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294853 0039d316-1c4b-4281-b951-d872f2087c98
Blink CL https://codereview.chromium.org/879533003/ defines own
casts to convert between bridged types. The cast helpers in base
cannot be used in Blink. Fine tune the presubmit check to allow
for those by adding a negative look behind for reinterpret_cast.
BUG=
Review URL: https://codereview.chromium.org/1034523003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294560 0039d316-1c4b-4281-b951-d872f2087c98
Thanks to tfarina for the code suggestion.
BUG=443718
Review URL: https://codereview.chromium.org/990403002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294436 0039d316-1c4b-4281-b951-d872f2087c98
If the repository has third_party/dart-sdk/ unpacked, use that to
format dart files modified in the current patch.
BUG=459376
R=maruel@chromium.org, zra@google.com
Review URL: https://codereview.chromium.org/933383002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294179 0039d316-1c4b-4281-b951-d872f2087c98
This check is specific to Chromium codebase, there's no point in having it in depot_tools.
BUG=None
R=maruel@chromium.org
Review URL: https://codereview.chromium.org/924863002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@294124 0039d316-1c4b-4281-b951-d872f2087c98
Way back in r185294 I added the ability to request that, when
checking owners, the patch author *should not* count (normally it does
count). We did this in order to be able to mimic the review processes
of WebKit in the Blink repo.
However, we ended up never using this feature, so I'm finally getting
around to deleting it.
R=maruel@chromium.org
BUG=none
Review URL: https://codereview.chromium.org/851103004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@293685 0039d316-1c4b-4281-b951-d872f2087c98
The idea here is that when one of the filters are cleaned up in Chromium,
they should not be filtered out, otherwise we won't catch them in
the presubmit step.
See for example -> https://codereview.chromium.org/788493002/
BUG=None
R=agable@chromium.org, dpranke@chromium.org
Review URL: https://codereview.chromium.org/779033003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@293357 0039d316-1c4b-4281-b951-d872f2087c98
input_api.basename is a function, so printing it out yields:
"The <function basename at 0x7fccd3735410> directory requires..>"
Instead, use it to resolve the local presubmit path that has
requested clang formatting.
R=dpranke@chromium.org
BUG=422332
Review URL: https://codereview.chromium.org/665493002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292491 0039d316-1c4b-4281-b951-d872f2087c98
If presubmit.py for a directory (such as net/) fails due to not being clang formatted, then 'git cl format' needs to be run for that directory, not the entire patch.
BUG=422332
Review URL: https://codereview.chromium.org/648713003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292407 0039d316-1c4b-4281-b951-d872f2087c98
This check never triggered before, since cpplint.py was looking for
override not OVERRIDE. Since Chromium prefers override now, the
linter is now (correctly) warning. However, the old Chromium convention
(and the one implemented by the clang plugin) is to explicitly annotate
all overrides with both virtual and override. The clang plugin code has
been updated, but new binaries have not yet been built. Until clang
rolls, suppress the warning.
BUG=417463
TBR=stip@chromium.org
Review URL: https://codereview.chromium.org/627163002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292319 0039d316-1c4b-4281-b951-d872f2087c98
'Add a description' is not enough to know what the test is about. Added
some more info.
BUG=
Review URL: https://codereview.chromium.org/313523002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@274393 0039d316-1c4b-4281-b951-d872f2087c98
This method is necessary to allow good code organization (i.e., unittests live
adjacent to the source files they test) in the new infra.git repository.
R=maruel@chromium.org
Review URL: https://codereview.chromium.org/281013002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@270564 0039d316-1c4b-4281-b951-d872f2087c98
I want to run Python unit tests as part of my presubmit, and those unit tests require PYTHONPATH to be set the way it would be set when the modules they test are running.
Review URL: https://codereview.chromium.org/250693002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@265998 0039d316-1c4b-4281-b951-d872f2087c98
2006-2009/10 was a valid copyright date format before 2011 (when we decided to
use only one year, not a range).
The current directive with copyright is in fact to not change old copyright
dates, so there is no plan to go back and "fix" old files. As such, the
presubmit checks should not be complaining about old files.
BUG=none
Review URL: https://codereview.chromium.org/200643002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@257181 0039d316-1c4b-4281-b951-d872f2087c98
For simplicity, the canned presubmit check just calls git cl format
directly with a new option --dry-run which does not change the files on
disk.
Because some users may not have git or clang-format might somehow have
errors, this canned check is a warning. Additionally, if git cl format
fails with an error then the presubmit check will silently report
success to avoid spamming misconfigured users during upload.
BUG=none
Review URL: https://codereview.chromium.org/141493002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@246066 0039d316-1c4b-4281-b951-d872f2087c98
This is useful to diagnose slow tests. Make this multiprocessing aware. Stop
printing when the commands are added but use the proper message system instead.
R=iannucci@chromium.org
BUG=
Review URL: https://codereview.chromium.org/99453003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@238388 0039d316-1c4b-4281-b951-d872f2087c98
I've unified the checks for language specific exceptions to line length. There is a behavior change where cpp compiler directives no longer have a maximum line length (previously they were restricted to 150% of max line length). This change was made to match the behavior for java files.
BUG=NONE
Review URL: https://codereview.chromium.org/100163002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@238268 0039d316-1c4b-4281-b951-d872f2087c98
Previously we would suggest owners for the whole CL, rather than just
the files currently missing owner approval.
R=maruel@chromium.org
BUG=130978
Review URL: https://chromiumcodereview.appspot.com/23591064
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@224183 0039d316-1c4b-4281-b951-d872f2087c98
Currently the output is:
Found line ending with white spaces in:
cc/trees/layer_tree_host_impl_unittest.cc, line 379
This format is extremely annoying because you must combine the
file and line number together by hand. A more standard format
for expressing a position in a file is filename:lineno. This
changes the format to match:
Found line ending with white spaces in:
cc/trees/layer_tree_host_impl_unittest.cc:379
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/22837004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@217405 0039d316-1c4b-4281-b951-d872f2087c98
Greatly improves presubmit performance on Blink where there may be rule violations (my test case went from 50s to 5s).
BUG=236206
TEST=new test in presubmit_unittest.py; manual performance test on Mac and Windows after touching hundreds of files in webkit; testing included add/move/edit/delete on both binary and text files.
Review URL: https://chromiumcodereview.appspot.com/15898005
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@205275 0039d316-1c4b-4281-b951-d872f2087c98
This will let clients that don't use the heavily filtered canned cpplint
check and want to behave more strictly (aka cc/) reuse any common
settings applied to the cpplint module.
R=maruel@chromium.org
BUG=none
Review URL: https://codereview.chromium.org/13866044
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@194264 0039d316-1c4b-4281-b951-d872f2087c98
For hash_map and hash_set, we should include "base/hashtables.h", not
the <hash_set> or <hash_map> standard headers.
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/14139002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@194047 0039d316-1c4b-4281-b951-d872f2087c98
Blink wants to override the maxlen which is currently configurable in CheckLongLines but not in PanProjectChecks. This exposes the parameter on PanProjectChecks.
Review URL: https://chromiumcodereview.appspot.com/13403004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@192394 0039d316-1c4b-4281-b951-d872f2087c98
If we uploaded a CL w/o any reviewers specified, and then
ran the owners check on it (so that we had a Rietveld
issue number) we would crash in a Python assertion. This
wasn't caught in unit testing because of a limitation
in the unit test scaffolding.
TBR=maruel@chromium.org
BUG=None
Review URL: https://chromiumcodereview.appspot.com/12546003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@186454 0039d316-1c4b-4281-b951-d872f2087c98
Until an issue is uploaded to Rietveld, we don't know the official
email address to use for an owners check. There are three ways to fix
this: we could attempt to log in to rietveld prior to doing the check
and extract the address to use, or we could use ~/.last_codereview_email_address,
or we can use the email address we can determine from the checkout.
All three options have flaws; the first is particularly awkward since there
doesn't seem to be a good way to fetch the email without posting an issue.
The second is flawed if we use different addresses for different repos,
and the third is flawed if the checkout's email address is different from
the rietveld address, or if it is anonymous.
However, since this is only being used for owners checks (in this case),
anonymous checkouts probably don't matter, and hopefully the cases where
the email addresses differ are rare.
R=maruel@chromium.org
BUG=118388, 150049
Review URL: https://chromiumcodereview.appspot.com/12377023
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@186259 0039d316-1c4b-4281-b951-d872f2087c98
Right now we require approval from someone, and we require an owner
approval, but we don't require an approval from an owner *other than
the patch other*. It's conceivable that we might want this, so
I am making this a configurable argument to the presubmit check.
This will also be needed to ensure that we don't suggest you as an
owner for your own patches, when we actually know who you are.
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/12326151
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@185294 0039d316-1c4b-4281-b951-d872f2087c98
Currently, when we run the OWNERS check, we print the list of directories
that contain the relevant OWNERS files for any modified files in a change
still needing approval.
This has two problems:
1) if we bubble all the way up to the top level OWNERS, we print "" instead of
"src/" or something more useful (bug 157191)
2) for OWNERS files that contain per-file set-noparent entries (like changes to IPC messages), this can be really confusing because an owner of other stuff in the directory might've approved things already.
This change will now print the list of files in the CL that are still unapproved.
This might be a lot more verbose (since you get N lines rather than 1 for N files in a given directory), but hopefully it'll be clearer in the two cases above.
Also, this change takes care of some lingering clean-up in the code to rename some methods to be clearer.
R=maruel@chromium.org
BUG=157191
Review URL: https://chromiumcodereview.appspot.com/12314044
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@184219 0039d316-1c4b-4281-b951-d872f2087c98
Printing the list of all the files examined can produce a lot
out of output and bury the interesting output.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12315031
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@183813 0039d316-1c4b-4281-b951-d872f2087c98
We were passing a unicode string in the env block to subprocess.Popen which makes it unhappy. This forces the string to ascii first.
BUG=NONE
TEST=NONE
Review URL: https://chromiumcodereview.appspot.com/12255083
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@183569 0039d316-1c4b-4281-b951-d872f2087c98
The Android WebView code includes several manually-maintained .mk files
that are included by the Android build system. These contain some fairly
long lines as they need to refer to deep pathnames, which means the CQ
cannot be used as it always triggers a presubmit warning.
Allow .mk files to have 200 columns per line instead, which should be
enough for the long paths that are used. Add a test case for this.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12252067
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@182733 0039d316-1c4b-4281-b951-d872f2087c98
r176777 introduced a change that relied on os.path.samepath, which does not exist on Windows.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12021013
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@177701 0039d316-1c4b-4281-b951-d872f2087c98
pylint checks were erronously excluded for PRESUBMIT.py files in
root directory of repos because the regex was getting a './' added.
R=iannucci@chromium.org
TBR=M-A
Review URL: https://chromiumcodereview.appspot.com/11880040
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@176777 0039d316-1c4b-4281-b951-d872f2087c98
This fixes a potential issue with RunPylint in windows where paths
can contain special characters.
TBR=iannucci@chromium.org
Review URL: https://chromiumcodereview.appspot.com/11786007
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@175481 0039d316-1c4b-4281-b951-d872f2087c98
The AffectedSourceFiles query matches against files with paths relative
to repository root. On the other hand _FindAllFiles checks white_lists
and black_lists against files paths relative to PRESUBMIT.py
As a result, when a PRESUBMIT.py is not at RepositoryRoot, RunPylint
can misbehave.
I fix this by modifying the white_lists and black_list regexes to
include the relative directory of PRESUBMIT.py.
Also adding some logging to pylint and simplified misc logic.
Review URL: https://chromiumcodereview.appspot.com/11776016
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@175364 0039d316-1c4b-4281-b951-d872f2087c98
We use the fact the dcommit adds a "Committed:" link to the
description. If we wanted to be more strict, we could make
git cl dcommit post a comment on the CL (which would be nice
since it would notify reviewers that the CL was committed).
Note that this is untested as written, because it doesn't appear
that we have either git-cl integration tests, nor a test for
git cl dcommit adding the word "Committed:" so as written, this
presubmit check is prone to failure. Would like suggestions to
improve.
Also adding DoNotSubmit checks and a couple others to
PanPresubmitChecks.
R=maruel@chromium.org,dpranke@chromium.org
BUG=161702
Review URL: https://chromiumcodereview.appspot.com/11421050
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@171158 0039d316-1c4b-4281-b951-d872f2087c98
This can come up when CQ closed an issue but the user's
local branch is still tied to the issue.
BUG=161702
Review URL: https://chromiumcodereview.appspot.com/11348122
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@171153 0039d316-1c4b-4281-b951-d872f2087c98