This check is making any script which runs `git cl presubmit` locally fail.
In particular, `jj upload` runs `git cl presubmit`, and then uploads
only if it succeeds (we cannot run `git cl upload`, because that can
specifically only upload the currently checked out commit).
According to the bug I linked, "I think that historically it was possible
for developers to try to commit directly from their Chromium repo,
without uploading, and this aims to stop that. It also warns users that
their change has not been uploaded. Whether it still needs to be an
error is not clear, but I'm not sure it's causing any problems so I'm
inclined to leave it."
To sum things up:
* Since presubmits can be bypassed, this is not a security measure
* These days I don't believe you can commit directly from your chromium
repo
* It's now starting to cause problems.
Bug: 40253731
Change-Id: I0d83fa68d0e65dcdd8932c7d3053750dcc16d3fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6875404
Reviewed-by: Scott Lee <ddoman@chromium.org>
Commit-Queue: Matt Stark <msta@google.com>
In https://crrev.com/c/6697693, presubmit's canned formatting checks
started looking at *.star files. This included enforcing a max line
length of 80 chars in such files.
Most (all?) *.star files found in gclient-managed repos are lucicfg
client repos, which generally have their own formatter and use 4-space
indents. This can lead to some friction with presubmit's 80 char limit.
So this uncaps that limit for all *.star files.
Bug: None
Change-Id: Ice68676c5d097d7951e83cf81ca9ac8adc8151ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6862147
Reviewed-by: Scott Lee <ddoman@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
This reverts commit eb60ab38de.
Reason for revert: re-landing with an additional patch
-------------
* Problem
Browser infra runs ci.*-presubmit builders, such as linux-presubmit,
with --all to ensure that the entire chromium/src passes
presubmit checks.
crrev.com/c/6842238 changed the finding type for License Check
from warning to error, but the CI presubmit builders failed because
there are many files without valid CopyRight.
Not only the existing files, all the new files that are added with
`Bypass-Check-License: <reason> footer could also cause the presubmit
builder to fail.
* This CL
In addition to the original patch from crrev.com/c/6842238,
this CL makes additional patches to turn the CopyRight errors into
warnings, if --file or --all is given.
The same approach is used in
https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_canned_checks.py;l=982-987;drc=eb60ab38deeda6975c9b0fef883978f2a9f69120
Bug: 435696543,40237859
Original change's description:
> Revert "presubmit: emit errors instead of warnings for bad copyright headers"
>
> This reverts commit fa62515ecb.
>
> Reason for revert: it seems that the existing files without valid copyright headers are causing linux-presubmit builds to fail. b/438791294
>
> Bug: 435696543,40237859
> Original change's description:
> > presubmit: emit errors instead of warnings for bad copyright headers
> >
> > This is an effective revert of https://crrev.com/c/4895337 with
> > additional patches to support a footer.
> >
> > https://crrev.com/c/3887721 updated CheckLicense() to emit errors
> > for bad copyright headers. However, https://crrev.com/c/4895337
> > was changed the finding type from error to warning, claiming that
> > the check is N/A for moved third files, but it's not so easy
> > to programatiically distinguish moved third-party files.
> >
> > After discussions, it was decided to change the finding type back
> > to error to prevent accidental submissions for new files without
> > correct CopyRight headers.
> >
> > To mitigate moved, third-party files, this CL adds support for
> > "Bypass-Check-License: <reason>" footer.
> >
> > If the check should be ignored in a given CL, CL authors should
> > use the footer instead.
> >
> > Bug: 435696543,40237859
> > Change-Id: I177915c65932a3d76ea60ee6a0e396f726bc400d
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6842238
> > Reviewed-by: Gavin Mak <gavinmak@google.com>
> > Commit-Queue: Scott Lee <ddoman@chromium.org>
>
> Bug: 435696543,40237859
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Change-Id: Ibedf8d13e3742249947e29e625a14cceaf89879c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6852377
> Commit-Queue: Scott Lee <ddoman@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>
Bug: 435696543,40237859
Change-Id: Iafdb29b928c016eb3949e29fd43a2ba5f53e0ba0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6852108
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
This reverts commit fa62515ecb.
Reason for revert: it seems that the existing files without valid copyright headers are causing linux-presubmit builds to fail. b/438791294
Bug: 435696543,40237859
Original change's description:
> presubmit: emit errors instead of warnings for bad copyright headers
>
> This is an effective revert of https://crrev.com/c/4895337 with
> additional patches to support a footer.
>
> https://crrev.com/c/3887721 updated CheckLicense() to emit errors
> for bad copyright headers. However, https://crrev.com/c/4895337
> was changed the finding type from error to warning, claiming that
> the check is N/A for moved third files, but it's not so easy
> to programatiically distinguish moved third-party files.
>
> After discussions, it was decided to change the finding type back
> to error to prevent accidental submissions for new files without
> correct CopyRight headers.
>
> To mitigate moved, third-party files, this CL adds support for
> "Bypass-Check-License: <reason>" footer.
>
> If the check should be ignored in a given CL, CL authors should
> use the footer instead.
>
> Bug: 435696543,40237859
> Change-Id: I177915c65932a3d76ea60ee6a0e396f726bc400d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6842238
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Scott Lee <ddoman@chromium.org>
Bug: 435696543,40237859
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Ibedf8d13e3742249947e29e625a14cceaf89879c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6852377
Commit-Queue: Scott Lee <ddoman@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
This is an effective revert of https://crrev.com/c/4895337 with
additional patches to support a footer.
https://crrev.com/c/3887721 updated CheckLicense() to emit errors
for bad copyright headers. However, https://crrev.com/c/4895337
was changed the finding type from error to warning, claiming that
the check is N/A for moved third files, but it's not so easy
to programatiically distinguish moved third-party files.
After discussions, it was decided to change the finding type back
to error to prevent accidental submissions for new files without
correct CopyRight headers.
To mitigate moved, third-party files, this CL adds support for
"Bypass-Check-License: <reason>" footer.
If the check should be ignored in a given CL, CL authors should
use the footer instead.
Bug: 435696543,40237859
Change-Id: I177915c65932a3d76ea60ee6a0e396f726bc400d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6842238
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
The existing licence regex assumed each line would start with an unknown
and optional comment delimiter, followed by a required space. However
the standard way to write the licence in XML files uses a multi-line
comment, each individual line simply starts with the licence text, no
delimiter or space. The license check thus needed to be updated to make
the space optional as well.
Bug: 407060754
Change-Id: I4a83dd9408de890593952e441ba6b650f726907c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6409392
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Andy Perelson <ajp@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Reciprocal licenses can only be used in open source projects.
This change updates the presubmit validation checks to accept an
optional flag `allow_reciprocal_licenses`. When True, the allowlist is
extended to include reciprocal licenses.
Bug: 385020146
Change-Id: I0374658207bc87ffd74e033762ee4973c6e83b3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6107863
Reviewed-by: Jordan Brown <rop@google.com>
Auto-Submit: Jordan Brown <rop@google.com>
Reviewed-by: Rachael Newitt <renewitt@google.com>
Commit-Queue: Rachael Newitt <renewitt@google.com>
This is achieved by duplicating vpython spec in tests directory and
changing the python version to 3.8. The reason for this change is that
even though depot_tools support 3.11, the entire infra is still on
3.8 so any new 3.8+ feature used in depot_tools may break infra test and
it has already happened couple of times.
Bug: 346784198
Change-Id: I7dbd0d926da21854abf47af2c2f6edfeec2c5c4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5625090
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
git cl upload prompts a user if they upload a particularly large change,
determined by the number of CCs added on the change. With the watchlists
analyzer and eventual removal of CCing watchers from git cl, this check
needs to be updated to work without knowing the number of CCs.
This change does two things:
1. Moves the check from git cl upload to a pan-project presubmit check.
2. Update the heuristic of a LSC from > 100 CCs to > 100 files modified.
Bug: 324562917
Change-Id: I6bd0db9fde942273acebc320c8b52e81b02bd42f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5296199
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
