Commit Graph

409 Commits (6aed4f5a0c67ad028c9e8e08a4e8dbc98bccd07b)

Author SHA1 Message Date
Joanna Wang 6aed4f5a0c Clarify that gclient gitmodules' changes need to be committed.
Bug: 1483198
Change-Id: I1dfae06bfc273b01d168b507861984327ad0d629
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4892468
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
2 years ago
Yuanjun Huang 5dffbd43c4 [config] change to use luci-config v2 in presubmit
In `CheckChangedLUCIConfigs`, it will call luci-config to list all
config sets. Change that call to v2.

Change-Id: I7a00d157fa631174b2124f0920026fa6b1fea04c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4900076
Commit-Queue: Yuanjun Huang <yuanjunh@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
2 years ago
Maggie Chen 34e0ecf20e Change from PresubmitError to PresubmitWarning for bad license
We cannot distinguish if this is a moved third-party file. So just
do warnings for a license header from a third-party company.

Bug: 1462922
Change-Id: I78394dcccb9028bce6c535dfce625364dd29157b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4895337
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
2 years ago
Gavin Mak f936d540e1 Remove __future__ imports
All __future__ imports (unicode_literals, print_function) are already
mandatory in py3. Also remove an outdated py2 comment in
presubmit_canned_checks.py

Bug: 1475402
Change-Id: I27cf6a8268f6dd1081f22af782c4c29a975376ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4867135
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
2 years ago
Yiwei Zhang 7a69b031d5 presubmit: support checking new TODO format
R=sokcevic

Bug: 1479023
Change-Id: I6eabb16447526fbc8de83b823763888aff1a8249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4847314
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
2 years ago
Mike Frysinger 124bb8e53c switch to 4 space indent
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>
2 years ago
Gavin Mak a94d8feecb Drop py2 support in presubmit files
python3 is the only supported version of python in depot_tools.

Bug: 1475402
Change-Id: I62ef9979a57bedc3be6886239068de5cd73f0504
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4814439
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
2 years ago
Josip Sokcevic 6308837868 Reland "Check if DEPS git is not in git submodules"
This is a reland of commit e6f40ea034

Original change's description:
> Check if DEPS git is not in git submodules
>
> If a new git dependency is added to DEPS file, presubmit check should
> fail if there's no corresponding git submodule entry if git_dependencies
> is set to SYNC.
>
> R=jojwang@google.com
>
> Bug: 1476115
> Change-Id: I0fdebb036c129c2f97524b86ee4d70c07e5b0091
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818792
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>

R=jojwang@google.com

Bug: 1476115
Change-Id: I4cd6e541aabb5d8be883e15e5693c4ad9085bcad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4825584
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
Anne Redulla b9d7c85582 [ssci] Added CheckChromiumDependencyMetadata in presubmit_canned_checks
This CL adds a new function `CheckChromiumDependencyMetadata` in
`presubmit_canned_checks.py`. It can be used to check that files satisfy
the format defined by `README.chromium.template`
(https://chromium.googlesource.com/chromium/src/+/main/third_party/README.chromium.template).

The code for metadata validation can be found in `//metadata`. Note that
all metadata validation issues will be returned as warnings only for
now, while the quality of metadata is being uplifted.


Bug: b:277147404
Change-Id: Iacf1b3a11219ab752549f6dc6e882c93c0fbe780
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4812578
Commit-Queue: Anne Redulla <aredulla@google.com>
Reviewed-by: Rachael Newitt <renewitt@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Josip Sokcevic 6c8d2a9ce5 Revert "Check if DEPS git is not in git submodules"
This reverts commit e6f40ea034.

Reason for revert: infra and angle have git dependencies with
no gitlink.

Original change's description:
> Check if DEPS git is not in git submodules
>
> If a new git dependency is added to DEPS file, presubmit check should
> fail if there's no corresponding git submodule entry if git_dependencies
> is set to SYNC.
>
> R=jojwang@google.com
>
> Bug: 1476115
> Change-Id: I0fdebb036c129c2f97524b86ee4d70c07e5b0091
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818792
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>

Bug: 1476115
Change-Id: Id990407f1738ac1e3d3950e0a85e9e39f8f1b624
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4820455
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2 years ago
Josip Sokcevic e6f40ea034 Check if DEPS git is not in git submodules
If a new git dependency is added to DEPS file, presubmit check should
fail if there's no corresponding git submodule entry if git_dependencies
is set to SYNC.

R=jojwang@google.com

Bug: 1476115
Change-Id: I0fdebb036c129c2f97524b86ee4d70c07e5b0091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818792
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
2 years ago
Josip Sokcevic 0c6534f852 Reland "Use git_dependencies variable in gitlink check"
This is a reland of commit a83906be85

Original change's description:
> Use git_dependencies variable in gitlink check
>
> The old implementation was written before gclient got git_dependencies
> property. Instead of relying on arbitrary string, use actual content of
> git_dependencies.
>
> R=jojwang@google.com
>
> Change-Id: Iea90e531ac6e9a9aaf58ca1dc73347692448107a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818788
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>

Change-Id: Iea92f74a489977804685c8b0ec4ba4eb4687185a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818795
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
Josip Sokcevic f69f58e2c8 Revert "Use git_dependencies variable in gitlink check"
This reverts commit a83906be85.

Reason for revert: may break presubmit checks in some cases

Original change's description:
> Use git_dependencies variable in gitlink check
>
> The old implementation was written before gclient got git_dependencies
> property. Instead of relying on arbitrary string, use actual content of
> git_dependencies.
>
> R=jojwang@google.com
>
> Change-Id: Iea90e531ac6e9a9aaf58ca1dc73347692448107a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818788
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>

Change-Id: I526a633b0bda647684ed87888ead9d02e24e9da4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818794
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2 years ago
Josip Sokcevic a83906be85 Use git_dependencies variable in gitlink check
The old implementation was written before gclient got git_dependencies
property. Instead of relying on arbitrary string, use actual content of
git_dependencies.

R=jojwang@google.com

Change-Id: Iea90e531ac6e9a9aaf58ca1dc73347692448107a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818788
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
2 years ago
Anne Redulla 760f8bcfb9 Revert "[ssci] Added CheckChromiumMetadataFiles in presubmit_canned_checks"
This reverts commit a1cfc693af.

Reason for revert: causing presubmit errors downstream

Original change's description:
> [ssci] Added CheckChromiumMetadataFiles in presubmit_canned_checks
>
> Bug: b:277147404
> Change-Id: I14a2f11b256bc85fdfe225443ef533c38463ca3e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4796694
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Reviewed-by: Rachael Newitt <renewitt@google.com>
> Commit-Queue: Anne Redulla <aredulla@google.com>

Bug: b:277147404
Change-Id: I83f52494bc1a3a786505b8b74b2053269baa6e8e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4803286
Commit-Queue: Anne Redulla <aredulla@google.com>
Auto-Submit: Anne Redulla <aredulla@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Anne Redulla <aredulla@google.com>
2 years ago
Anne Redulla a1cfc693af [ssci] Added CheckChromiumMetadataFiles in presubmit_canned_checks
Bug: b:277147404
Change-Id: I14a2f11b256bc85fdfe225443ef533c38463ca3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4796694
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Rachael Newitt <renewitt@google.com>
Commit-Queue: Anne Redulla <aredulla@google.com>
2 years ago
Josip Sokcevic de6bc6620a Improve presubmit check messaging
Also add oneline status on gclient gitmodules.

R=jojwang@google.com

Change-Id: I05c9f856ce6fd1c3ebf1dc7da672d25196a4cb67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4771975
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
mark a. foltz 90fed30866 [depot_tools] Clarify error message for license headers.
The license header check error message can't distinguish between new and
moved files.  We don't want to update the year for moved files, so
clarify instructions for that case.

Bug: 1457007
Change-Id: I47471e56a5d9889b7315d9ad6dd8a5dd5cfe956f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4641350
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
2 years ago
Yiwei Zhang fbb06cdb23 config: output warnings for validation error/warnings in non-affected files
R=iannucci, sokcevic, yuanjunh

Bug: 1449933
Change-Id: I863c55f5a12da103b4167c8cf9a1aac02962838f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4602126
Reviewed-by: Yuanjun Huang <yuanjunh@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
Yiwei Zhang 422f4d59a4 config: use lucicfg validate to validate the config files
The new flow is that if any affected file is in the config directory
of a config set, the script will call `lucicfg validate` to validate
the whole config directory and then filter out the validation result
that does not belong to affected file(s). This will introduce a bit
more latency in valdiation because before, only affected files are
sent to LUCI Config for validation. However, this is not latency
senstively operation and always validating the full snapshot will
faciliate us breaking down the config as it's possible in order to
validate the affected file, LUCI Config will need an unaffected file.

R=iannucci, yuanjunh

Bug: 1449933
Change-Id: Id330973f59b882569a8ece36edb4b6a8ff0ed2d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4591233
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Yuanjun Huang <yuanjunh@google.com>
2 years ago
Gavin Mak df2f11113c Update code-owners documentation links
Bug: 1223715
Change-Id: Ia4cfaf6c421432f0b64662ab02bc4041f32173d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4585553
Auto-Submit: Gavin Mak <gavinmak@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
2 years ago
Joanna Wang cb0a55b733 Add exact git update-index command in presubmit mismatch message.
Bug:1417051
Change-Id: Iaadced083d49de2e76c04649ab71a4b0534c0706
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4574568
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
2 years ago
Josip Sokcevic 04afb4b256 Skip owners file format check on skip_owners
CheckOwnersFormat requires network and code owners plugin to be enabled.
This moves that check under condition.

R=jojwang@google.com

Change-Id: Ic9cfd5e33987cac9aae93d744ebfb2482eb309be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4571106
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
Bruce Dawson 05ebc9e3e0 Allow calling tag functions from PostUploadHook
Calling CheckChangeHasNoUnwantedTags and
CheckChangeHasBugFieldFromChange from a PostUploadHook is useful because
it ensures that users will see a message if they add a bad tag when
creating the commit message during upload. Allowing this requires
creating entry points for these functions that take a change parameter
instead of an input_api parameter. It also requires fixing our use of
the deprecated inspect.getargspec function.

In order to not tell users twice (once before and once after upload)
that they haven't used a Bug: tag (it is optional, after all) a flag
to make these suggestions optional is also added.

This change is required in support of crrev.com/c/4490087.

Bug: 1434702
Change-Id: I3fd909a71c6c7d7671d6f986ad60d5375143a2f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4519532
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Bruce Dawson b6cb9e0b9a Remove Python 2 support for presubmit Commands
PRESUBMIT.py files can invoke other scripts, and those scripts may be
run under Python 2 or Python 3. There are multiple mechanisms that
govern which version will be used, sometimes requiring several flags
to be passed.

This change removes support for Python 2 from this system, thus making
it simpler to invoke Python 3 scripts.

The function parameters that are used to select Python versions are
passed in from multiple places so they still need to be supported, but
they are now ignored. The parameters are deleted to prevent accidental
use.

This change was tested by running this command in a Chromium repo:

  git cl presubmit --force --all

Bug: 1207012
Change-Id: I4adc7164417e155ff80d3a039cf36ed030756456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4328470
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Josip Sokcevic c333c3424e Drop pylint-1.5
R=gavinmak@google.com

Bug: 1353487
Change-Id: Ifbe09b60f891d585edbd5831e4cb3c5a97f1fe8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4491423
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
2 years ago
Gavin Mak d3568a42bf Handle windows paths correctly in CheckInclusiveLanguage
Exclude file paths always use forward slash but tests currently change
based on which OS is running them, so this bug never got caught.

Bug: 1440473
Change-Id: Iafd7e57c2dedd0c9990a1a620abd3ae38631a4f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4481518
Reviewed-by: Sean McCullough <seanmccullough@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
2 years ago
Bruce Dawson 407e5e7b66 Show error on Fixes: and suggest Fixed:
The Bug: syntax is well known as a way of tagging bugs in a commit
message, at least partially because when it is missing there is a
presubmit warning suggesting that it be used.

The Fixed: syntax is less well known because there is no such reminder.
And, if somebody types "Fixes: " instead of "Fixed: " there is no error.
Therefore it is not surprising that "Fixes: " was used seven times in
the last 10,000 commits. See crrev.com/c/4416218 for one example.

This change adds both a reminder that Fixed: exists as well as an error
if Fixes: is used.

Fixed: 1434702
Change-Id: Id940d0af5cfea206ee923fbb779bf686b34c5f33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4436316
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Yuanjun Huang 257ad53176 Compress the request body if it's too large to send to luci-config
Sometimes, users may send very large config files to luci-config service
to validate (e.g chromeos). The GFE has 32 MiB limit. Therefore,
compress the json body and set a custom `application/json-zlib` content
type header that luci-config service is able to recognize.

Bug: 1417047
Change-Id: I61ee50125bab5746f8094d81cab484d70002ac53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4296563
Commit-Queue: Yuanjun Huang <yuanjunh@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
2 years ago
Collin Baker 97f6d11cef Set presubmit line length check to 100 for Rust
Rust style dictates 100 columns instead of 80. Update presubmit check
accordingly.

Bug: chromium:1292073
Change-Id: I4f0c0cc3b84ebb4559f29a6500ffd843febc8f93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4295201
Auto-Submit: Collin Baker <collinbaker@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Josip Sokcevic a6898e71ab Add presubmit check for git submodule
This change allows repos that use depot_tools to have submodules. It
relies on a "magic" text `SUBMODULE_MIGRATION` and
`use_git_submodules`in DEPS file.

SUBMODULE_MIGRATION is placed in DEPS file when git submodules should be
accepted but parity with DEPS file is expected. We don't rely on gclient
getdep as it's too slow for a presubmit check. Instead, we just naively
check presence of git submodule commit hash in the DEPS file.

use_git_submodules (to be a proper variable in DEPS file) indicates the
project migrated to git submodules and no git dependencies are specified
in DEPS file.

R=aravindvasudev@google.com, jojwang@google.com

Bug: 1417051
Change-Id: I03fcd98a001ee8513740108e58bc1dfe3c9a460e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4262472
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 years ago
Yiwei Zhang e636a25853 split the config validation request into smaller requests
R=iannucci, yuanjunh

Bug: b/268065448
Change-Id: I6d700c39bdea8438815e3bf5a462038b05619a9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4240362
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Yuanjun Huang <yuanjunh@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
2 years ago
Brian Ryner 790a0c522d Fix CheckCIPDManifest for python 3.
Change-Id: Icf7b8e7ad732a731520550ff9e276f13016effb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4149964
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Brian Ryner <bryner@google.com>
2 years ago
Gavin Mak d83509c0c3 Move presubmit_canned_checks tests under tests/ and fix
Only tests under tests/ are run on presubmit, so
presubmit_canned_checks_test.py is not run. Move these there and fix
them.

Bug: 1385693
Change-Id: Ib01a816b0a81919722ea49e5551010251c0e3287
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4038514
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
2 years ago
Victor Hugo Vianna Silva fe46d759b5 Add presubmit warning to update references to (re)moved OWNERS files
file:// references in OWNERS files can become invalid when the file
is moved or deleted.

Bug: 1385205
Change-Id: Icf1a65b3d96d6ad298eac6645d8f692cb09dc75a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4029143
Auto-Submit: Victor Vianna <victorvianna@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
2 years ago
mark a. foltz fadcbfdb27 [depot_tools] Adds a better error message for copyright header checks.
The current message prints a regular expression.
This new message prints the correct copyright header which can be
copy-pasted into new files.

Bug: 1376752
Change-Id: Ifb03924641ad75d2b58158a752e0a62648436edc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4032368
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
2 years ago
Bruce Dawson 25cf78395c Fix two CheckLicense bugs and improve tests
Some recent "improvements" to CheckLicense were buggy, at least
partially because I did not increase test coverage when adding new code
paths. This change fixes two known mistakes and greatly increases test
coverage.

One bug was that the code assumed that match.groups()[0] would always be
the current year, but that assumption is invalid when a license regex is
passed in.

Another bug is that the new_license_re took away the possibility of the
license ending with */ but that is used in .css files.

The increased test coverage catches these and validates other
assumptions around new files.

The failures were reproduced and then the fixes validated using this
command:

  vpython3 tests/presubmit_unittest.py

Bug: 1098010
Change-Id: Ic91ca9f7a182660aef7b1eead79e841f58e15a3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4032366
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
2 years ago
Gavin Mak ff5d25bb10 Add canned presubmit checks for corp links
Only googlesource links should be used publicly.

Bug:b/253074243
Change-Id: Id972821acbfde635717e21bba879880e5312f92c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3955706
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
3 years ago
Vadim Shtayura 8e2da91cbb Fix UnboundLocalError in CheckLicense.
`current_year` wasn't initialized if `licence_re` is set.
See e.g. https://ci.chromium.org/b/8799737896715431809

R=brucedawson@chromium.org, iannucci@chromium.org
BUG=1098010

Change-Id: Idb1a320af7f4790f2dac0fba7a62e2990a3664e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3969027
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
3 years ago
Bruce Dawson 3b9552584d Fix year comparison
Change crrev.com/c/3967210 compared a regex result (str) to current_year
(int), which will never match. This fixes the comparison by converting
current_year to str. Unit tests to validate the behavior will be added
in a follow-up.

Bug: 1098010
Change-Id: I79d0adc6d6566df71425bfb448091445f57ad2c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3968797
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
3 years ago
Bruce Dawson a7949c2545 Fix license checks
In crrev.com/c/3955701 the license checks were made stricter on new
files. Unfortunately moved and new files are indistinguishable in the
presubmit system, and moved files are not supposed to update their
copyright year.

So, CheckLicense() now emits three different types of presubmit results:
  1) If a file is new/moved and the license is malformed then an error
is reported.
  2) If a file is new/moved and the year is not current then a warning
is issued, with advice to ignore the warning if the file was moved.
  3) If a file is not new/moved and it has a bad license then a warning
is issued.

This will ensure that new files do not have bad licenses, and will
usually get the year correct.

The new output looks like this (for one moved file with an old date,
one file with a bad license, and one new file with a bad license):

** Presubmit Warnings: 2 **
License doesn't list the current year. If this is a new file, use the current year. If this is a moved file then ignore this warning.
  base\win\moved.cc

License must match:
.*? Copyright (\(c\) )?(2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors(\. All rights reserved\.)?\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  base\win\bad_license.cc

** Presubmit ERRORS: 1 **
License on new files must match:
.*? Copyright (2022|2021|2020|2019|2018|2017|2016|2015|2014|2013|2012|2011|2010|2009|2008|2007|2006|2006-2008|2006-2009|2006-2010) The Chromium Authors\n.*? Use of this source code is governed by a BSD-style license that can be\n.*? found in the LICENSE file\.\n

Found a bad license header in these new files:
  base\win\new_file.cc

Bug: 1098010
Change-Id: Ia62b0591ee416c55566427bba9fdd91d74a26349
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3967210
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
3 years ago
Bruce Dawson dbaca22bdf Make license check stricter on new files
The Chromium license check is quite forgiving because it has to handle
variations that have accumulated over the years. But, there is no need
for it to be forgiving on new files. This change requires that new files
have a license that _exactly_ matches what we want, including having the
current year. This will catch lots of errors that would otherwise
require an observant reviewer.

This does mean that if an old file is copied that it might require some
updates.

This was tested with presubmit --all where it gave identical results to
the old check. It was also tested with new files with bad licenses to
make sure that the new check behaved as designed.

This change also switches from f.OldContents() to f.Action() == 'A'.
f.OldContents() is very expensive (about _40_ minutes if invoked on all
files during presubmit --all) and f.Action() == 'A'dded better indicates
what we are actually checking.

This change also switches from checking for the presence of key_line
before doing the regex search to just doing the regex search on the
start of the file. Both techniques avoid the extreme cost of a regex
search on large files with no license, but the new technique is simpler.

Bug: 1098010

Change-Id: I028d72cd31f5d0f787a522c54683de32f2a98867
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3955701
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Bruce Dawson 5e4d74983e Support a tag to ignore long lines
Long lines in source files are bad, but sometimes unavoidable. Since
ignoring warnings is even worse than long lines it is wise to have a way
to disable the warning. This change adds support for a multi-language
tag that can be put in unavoidably long lines.

Bug: 1309977
Change-Id: I205086050b5aa5b4a02a651c06615c82ec0e1a38
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3896084
Reviewed-by: Dmitrii Kuragin <kuragin@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Fabrice de Gans 9ebcfa6be1 [code-health] Run CheckVPythonSpec under python3
This change also makes CheckVPythonSpec() look for .vpython3 files.

Bug: 1336295
Change-Id: I4bbec48debe42748811a8cfcef6da9602017d4bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3891974
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
3 years ago
Bruce Dawson 9623d88e8f Enforce license requirement on new files
We have long had a presubmit that warns when a file lacks a suitable
license, but because this is a warning it can be ignored by the author
and is not visible to reviewers. Every failure that is not prevented
will occur, and indeed newly added files from this year (seen in
crrev.com/c/3706888 and crrev.com/c/3437860, and possibly others) are
missing valid licenses.

This change turns the warning into an error on newly added files.

Bug: 1361031
Change-Id: Ie17bd6e591918affd9d865a3bf2be6c2bf62d1ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3887721
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
3 years ago
Gavin Mak 0f1addcbe9 Reland "Remove old DepotToolsOwners implementation"
This is a reland of commit 10dbd7babd

Original change's description:
> Remove old DepotToolsOwners implementation
>
> code-owners should have been enabled for most hosts that depot_tools
> supports by now. Remove our own implementation and rely on code-owners.
>
> Change-Id: Iaf0d3db65b2e5063b67d42b92188c4ec51d2cd9a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3783475
> Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
> Reviewed-by: Joanna Wang <jojwang@chromium.org>
> Commit-Queue: Gavin Mak <gavinmak@google.com>

Change-Id: Ic87f34411bb38b91fcf49adb07601ae10244e828
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3881043
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
3 years ago
Avi Drissman 863438746a Adjust copyright regex
As per the lawyers, the preferred style has no period after "Authors"
when the "all rights" sentence is dropped. Adjust the regex to not
match a period when there is no "all rights" sentence.

Bug: 1098010
Change-Id: Ib4103c09f9d02d58b9294643bbedd72fc895bbe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3876526
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Avi Drissman <avi@chromium.org>
3 years ago
Josip Sokcevic 6efb3ed962 Reland "Use io.open for opening files"
This is a reland of commit eb16430cf9

Original change's description:
> Use io.open for opening files
>
> Use io.open which is consistent in py2 and py3.
>
> Bug: 1357152
> Change-Id: I49a3df503026bc6918362a9b5600f57111111111
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872429
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>

Bug: 1357152
Change-Id: I58881c040ca7333f0247875a0c4abe31cf793b08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3877645
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
3 years ago
Gavin Mak a3a95d439b Revert "Use io.open for opening files"
This reverts commit eb16430cf9.

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

Original change's description:
> Use io.open for opening files
>
> Use io.open which is consistent in py2 and py3.
>
> Bug: 1357152
> Change-Id: I49a3df503026bc6918362a9b5600f57111111111
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872429
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@google.com>

Bug: 1357152
Change-Id: Iec18fe8fd37f28887507d47d7b35b93ccee72375
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872487
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Gavin Mak <gavinmak@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
3 years ago
Josip Sokcevic eb16430cf9 Use io.open for opening files
Use io.open which is consistent in py2 and py3.

Bug: 1357152
Change-Id: I49a3df503026bc6918362a9b5600f57111111111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3872429
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
3 years ago