Commit Graph

2455 Commits (1fe11ccfe0ee780a0807ec8e2c023a30e3709316)

Author SHA1 Message Date
Gavin Mak 9c0db48c84 Make sure _UpdateMirrorIfNotContains fetches the specified revision
_UpdateMirrorIfNotContains populates the mirror if a given rev
isn't found in the mirror, but if this rev is invalid, it's never
fetched at all. Check that we've actually fetched it after
populating the mirror.

Bug: 407864212
Change-Id: Id66695636e749c4f7372aa522ab03ec4ec8feb52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6441962
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Gavin Mak <gavinmak@google.com>
3 weeks ago
Allen Li 848643d8b5 [git_cl_test] Fix test mocking for git_auth.AutoConfigure
Bug: 404613530
Change-Id: I307637ccd5ac5e59ca13588572dd0cb0bf691d03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6431247
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Allen Li <ayatane@chromium.org>
3 weeks ago
Allen Li 79b74c9be8 [git_cl_test] Delete broken mocked tests
These tests get broken by If0a214fe074d7e7591b5d37417ca447525c983b6 so
delete them.

Now, you might raise your eyebrows at that, but hear me out.

These tests are so heavily mocked that I argue that they aren't
providing any value.  What's the point of tests?  To save time by
catching regressions early.

Being dumped a large list of expected mocked calls like:

  @10: (['os.path.isfile', 'TEMP_DIR/trace-packet'],)
  @11: (['FileRead', 'TEMP_DIR/trace-packet'],)
  @12: (['FileWrite', 'TEMP_DIR/trace-packet', 'git-hash: 012345\ngit-hash: abcdea\n'],)
  @13: (['make_archive', 'TRACES_DIR/20170316T200041.000000-traces', 'zip', 'TEMP_DIR'],)
  @14: (['FileWrite', 'TEMP_DIR/git-config', <ANY>],)

provides little value.  Like, what the heck is this even supposed to
be testing?

I argue that:

the cost of potentially having a regression in the future, fixing it,
and adding a new, sane test for that case

is LESS than:

the cost of someone looking at one of these tests failures and
trying to figure out what exactly broke, whether it is an actual
breakage or do they just need to wade into the mess of expected mocked
calls and figure out how to update the mocked calls to reflect a
perfectly fine change that they are trying to make

Bug: 404613530
Change-Id: Id8e2f5ebb9ad55ee32291c2f70b2d26fb479b195
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6428049
Reviewed-by: Gregory Nisbet <gregorynisbet@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Allen Li <ayatane@chromium.org>
3 weeks ago
Allen Li a71f255604 [git_cl_test] Fix getAccountDetailsMock
Bug: 404613530
Change-Id: Iefc2f0456182e4e6eb816be2b3605685528d82f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6428048
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Allen Li <ayatane@chromium.org>
3 weeks ago
Jordan Brown 4373d97c21 roll_dep: Update README.chromium when rolling submodules
Attempt to update the README.chromium file with the new submodule
revision after rolling it in a DEPS update. This update is flag-guarded
and will only run when explicitly specified.

Currently, only README.chromium files with a single "Revision:" line are
supported. Multiple lines and files with the divider are not handled.
These are left as future TODOs.

Bug: 390067679
Change-Id: Ib776564ae94360cc72dd633fc7ed7b3f84b5b9d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173767
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Jordan Brown <rop@google.com>
Reviewed-by: Jiewei Qian <qjw@chromium.org>
4 weeks ago
Allen Li 5dcf92389b Fix git_map_test.py
I think the latest Git version changed this behavior.

Change-Id: Id10c3ec41daf4e6700fc8168373cae9482502231
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6422996
Commit-Queue: Allen Li <ayatane@chromium.org>
Feels: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
4 weeks ago
Sky Malice c214cd9faf Add android resource files to DEFAULT_FILES_TO_CHECK.
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>
1 month ago
Allen Li a0d13c9549 [git_auth] Use pathless URL for cred helper
The credential helper rules should not have a path, while the URL
rewrite rules should.

Reland of I79b52ab4af62367363617b2a9afa03a67f5ea0b9

Bug: b/401338175
Bug: b/403635929
Change-Id: Ib89d9e855ca5eba29cc67f8846bb7ca0cb3622ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6363080
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
1 month ago
Allen Li 8fecc592a2 Add auth config wizard
Make this available for testing first.

This should be functionally roughly analogous with the current auto
configuration logic, and it prints all actions it performs so it
should be pretty safe to play with, even if it hypothetically
misbehaves.

Bug: b/401338175
Change-Id: I803c7e167e355ec8cca1f5959099138c2fee305e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6334614
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
1 month ago
Josip Sokcevic 948edc4382 Revert "[git_auth] Use pathless URL for cred helper"
This reverts commit ce47e785fa.

Reason for revert: fatal: --local can only be used inside a git
repository.

Original change's description:
> [git_auth] Use pathless URL for cred helper
>
> The credential helper rules should not have a path, while the URL
> rewrite rules should.
>
> Bug: b/401338175
> Change-Id: I79b52ab4af62367363617b2a9afa03a67f5ea0b9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6345631
> Commit-Queue: Allen Li <ayatane@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>

Bug: b/401338175
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Ifa887eeb7a7049665570e865431b41ac18649b90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6357165
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>
1 month ago
Allen Li ce47e785fa [git_auth] Use pathless URL for cred helper
The credential helper rules should not have a path, while the URL
rewrite rules should.

Bug: b/401338175
Change-Id: I79b52ab4af62367363617b2a9afa03a67f5ea0b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6345631
Commit-Queue: Allen Li <ayatane@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
1 month ago
Anton Bershanskyi ab918ca0bc Reland "Add AffectedFile.Extension() in presubmit_support"
This is a reland of commit d447f41294

This reland excludes the change to PRESUBMIT.py which originally
caused regression and because of which the original commit was
reverted.

Original change's description:
> Add AffectedFile.Extension() in presubmit_support
>
> Add convenience method AffectedFile.Extension() for extracting file
> extension in a standard way. Add a test for this new functionality.
> Add a similar test for AffectedFile.UnixLocalPath().
>
> Bug: None
> Change-Id: If591e751fb2e4fb5355ad0b6f93f310667849d68
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6273341
> Commit-Queue: Anton Bershanskyi <bershanskyi@gmail.com>
> Reviewed-by: Dirk Pranke <dpranke@google.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>

Bug: None
Change-Id: Ie5fc551b9c0818dfd9a1c571cc247cc157dbc8b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6335512
Commit-Queue: Anton Bershanskyi <bershanskyi@gmail.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
2 months ago
Josip Sokcevic f61e11c19f roll_dep_test: fix tests
This partially reverts commit 1221440015.

Change-Id: Ib2e9d41b3871004376a952e643e13a7e67eb750e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6340427
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Allen Li <ayatane@chromium.org>
2 months ago
Allen Li b0d4f7c473 [gerrit_util] Add CheckShouldUseSSO function
This exposes the reason for the SSO detection, to be used in the auth
config wizard.

There are some minor changes to fix pyright (type checking) errors on
these lines.

Bug: b/401338175
Change-Id: Ibf63a1a078a9ae82598796e329d075f19de58ea7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6334613
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
2 months ago
Peter Wen 5d5ea66f01 git_cl: Add flag for owners override
This allows setting OO+1 with `git cl upload --enable-owners-override`.

R=sokcevic@chromium.org

Bug: None
Change-Id: Iddac69653a539a15beee38ea4957be1a9b19b57a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6340467
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Auto-Submit: Peter Wen <wnwen@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Devon Loehr 9dc0551d52 [git cl split] Add new cluster-based splitting algorithm
This CL adds a new way of generating splittings, which clusters files
based on their directory structure (while respecting `set noparent` in
OWNERS files). The user provides a minimum and maximum acceptable
number of files per CL, and the algorithm attempts to create CLs in
that range with a single unique reviewer for each. I've tested it on
some example CLs, and it seems to work well -- certainly better than
the existing algorithm.

Usage of the new algorithm is triggered by passing the
`--target-range` option to `git cl split`. A second new argument,
`--expect-owners-override`, may also be used to ignore OWNERS during
the clustering algorithm.

Bug: 389069356
Change-Id: I321e6424b2bdb7624b370af23b73759fda261161
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6324373
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Yiwei Zhang cbf61c7044 CheckNewDEPSHooksHasRequiredReviewers checks approval from required
reviewers

The check mandates approval from required reviewers during submission.
Without it, the CL author could land the CL without explicitly
approval from required reviewers.

Bug: 396736534
Change-Id: I4671f123419b8f94a969b6caccf13a2064511a0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6313625
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Devon Loehr 5825f91d8c [git cl split] Add trie structure for clustering
This CL defines a trie-based datastructure for representing files based
on their path. It directly mirrors the structure of a file system,
keeping track of directories and the files inside them. It also stores
some information about OWNERS files, for use during clustering (we
won't cluster files together if there's a "break" in ownership due to
`set noparent). Optionally, the ownership information can be overridden;
this will be done via a command-line flag when the algorithm is fully
implemented.

Bug: 335797528
Change-Id: I5dcdf36695a1da5714ec021e5e18b6c36855a4f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6321290
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Devon Loehr c48f866fcf Select a minimal number of owners for a set of files
This CL adds a function which takes a set of files, and attempts to
select a single owner for all of them. If it cannot, it falls back to
the standard owner selection algorithm, which may result in more owners
being chosen than necessary, but guarantees that a valid set of owners
is always returned.

Bug: 389069356
Change-Id: I985804040f149a02bfb5b3c6b946602a81334e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6321289
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Devon Loehr 20193073f8 [git cl split] Make Emit and EmitWarning variadic
This better matches the print function which they replace, and prevents
errors from passing the wrong number of arguments. We also change
printing in one function to be more user-friendly.

Bug: 389069356
Change-Id: Ia41d256b441bffa063f5d0b5ab4eb20725aaeaf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6322690
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Yiwei Zhang 191c73e7a2 CheckNewDEPSHooksHasRequiredReviewers computes reviewers from OWNERS
Instead of taking reviewers as input args, it computes the reviewers
from the top-level OWNERS file because otherwise, all callers will
basically do the same thing at the callsite.

Bug: 396736534
Change-Id: I3ea83b8285fab8b75cb77b40b10414c6a3dea6b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6313624
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Yiwei Zhang 1221440015 roll_dep_test: remove file:// scheme in the git url
roll_dep_test is currently using file:///path/to/repo as git url.
Interestingly, the test is not failing in presubmit bot but
failing locally because it is trying to clone using sso protocol
(see: b/391455129#comment11). I think the presubmit bot is not
equipped with git-remote-sso so the tests are able to pass. However,
on a local workstation, git-remote-sso is available but it should
not be used as the file:// scheme should indicate using local
transport protocol. Quoted from
https://git-scm.com/docs/git-clone#_git_urls
> When Git doesn’t know how to handle a certain transport
protocol, it attempts to use the remote-<transport> remote helper,
if one exists.

Removing the file:// scheme appears to fix the issue even though
the only difference is the usage of --local flag according to the
doc.

fwiw, I did try to inject `--local` flag but it gives me a warning
`--local is ignored`. I haven't dig deep enough to figure out why
the flag is ignored.

Bug: 391455129
Change-Id: I8fab9aee5198b2eedfacdb60161197b58f7533ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6313297
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
2 months ago
Yiwei Zhang e5159abea6 Add CheckNewDEPSHooksHasRequiredReviewers canned check
This will later be used in chromium PRESUBMIT to ensure newly added DEPS
hook MUST be reviewed by a dedicated reviewer.

Bug: 396736534
Change-Id: I3859814c6316d4e576d12114277671ed5db80e33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6284565
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
2 months ago
Yiwei Zhang 53f5a2d969 stop running test in python 3.8
chromium[1] and cros[2] have both moved to 3.11 which is the version
depot_tools use by default right now. Therefore, we should stop running
tests using python 3.8 to ensure compatibility and start using features
introduced after python 3.8

[1]: https://issues.chromium.org/40942322
[2]: http://b/318531403

Change-Id: I2bac00421a46793717387cec1898035b10b13b9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6306285
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Auto-Submit: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Devon Loehr f0b8c3b16c [git cl split]: Allow users to override reviewer assignment
This CL adds a --reviewers flag, which allows users to specify that CLs
should all be sent to the same (possibly empty) set of reviewers. This
can be useful for LSCs where OO+1 is available.

Bug: 389069356, 40269201
Change-Id: Ic6202b280bb676f75a71c4ccf0c41b32483d5c6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6298664
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Devon Loehr 4629c3474e [git cl split]: Deprecate $directory in favor of $description
`git cl split` allows users to parameterize their CL descriptions
by including the string $directory, which is replaced by the list of
directories covered by each generated CL. However, as we add more ways
of generating splittings, a list of directories may not always be the
most appropriate description. For example, the --from-file option lets
users write arbitrary description strings.

This CL renames the $directory string to $description, to reflect this
new generality, and refactors all places in the code that expected a
directory list instead of a string. Since users may beused to the
current version, it does not remove $directory, but instead notes that
it is deprecated and emits a warning when it is used. After some time,
we can remove it entirely.

Bug: 389069356
Change-Id: If8c947fdcbbb4897675b015a377cf21123e51467
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6299688
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Devon Loehr 158b6837dc [git cl split] Add summarize option during dry runs
Currently, the dry run setting causes the script to print out extremely
detailed information about the splitting it generated. While this is
sometimes useful, often users only care about the overall shape of the
splitting. This CL adds a flag which lets them see the same brief
summary that's printed during full runs.

Bug: 389069356
Change-Id: I473740e50f382e63b1289101f3253ff8ae9e6bbe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6298096
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Devon Loehr 33522b8bbe Fix Python SyntaxError
Python has started warning about invalid escape sequences in strings.
This fixes the warning by escaping the backslash in question. It seems
Python was already (correctly) printing the literal backslash despite
the warning, so no behavioral change.

Bug: None
Change-Id: I2d5d8d56b989d9a1bb87aba69af0ebac0209a7a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6299490
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Brian Ryner <bryner@google.com>
2 months ago
Devon Loehr 6e0fc6aaa1 [git cl split]: Refactor print function
This CL replaces the builtin print function with an alias `Emit`.
This is symmetric with `EmitWarning`, and primarily serves to allow
easier mocking during tests and debugging. No functional change.

Bug: 389069356
Change-Id: Ib2f5a7648458b6b1181606a3dccd9829542c5521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6299489
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
2 months ago
Devon Loehr c994f00dee [git cl split] Codify and enforce auto-resumption
`git cl split` claims to have built-in auto-resume capabilities, but
the nondeterminism in its splitting algorithm means they don't work
in practice. Now that we have the ability to load splittings from
files, we can ensure the user is using the same splitting as before,
and resume safely.

This CL does two things:
- It checks that any existing split branches match the splitting we're
  about to upload, and complains if they don't match. This relies on
  the fact that our branch names are unique and deterministic.
- It changes the auto-resume message to mention that --from-file is
  required, and includes the relevant filename. This requires tracking
  that filename from earlier in the program.

Bug: 389069356
Change-Id: Ic1d8964e96193ca93e05a9a39e286b84ffb61b06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6280953
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
2 months ago
Paul Irish 237b84e76c Colorize branch names for rebase-update
To ease the UX of finding the branch name when there's a conflict to
resolve.

Change-Id: I34d16f2f7bd897a60e96b0391772b5d8985f3cb6
Bug: none
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6278647
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Auto-Submit: Paul Irish <paulirish@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>
2 months ago
Gavin Mak 9f715f8017 Revert "Add AffectedFile.Extension() in presubmit_support"
This reverts commit d447f41294.

Reason for revert: "AttributeError: 'str' object has no attribute 'Extension'"

Original change's description:
> Add AffectedFile.Extension() in presubmit_support
>
> Add convenience method AffectedFile.Extension() for extracting file
> extension in a standard way. Add a test for this new functionality.
> Add a similar test for AffectedFile.UnixLocalPath().
>
> Bug: None
> Change-Id: If591e751fb2e4fb5355ad0b6f93f310667849d68
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6273341
> Commit-Queue: Anton Bershanskyi <bershanskyi@gmail.com>
> Reviewed-by: Dirk Pranke <dpranke@google.com>
> Reviewed-by: Gavin Mak <gavinmak@google.com>

Bug: None
Change-Id: Ic51b0942e3a3267194fb73af5e92c902b2098c4c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6279837
Auto-Submit: Gavin Mak <gavinmak@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2 months ago
Anton Bershanskyi d447f41294 Add AffectedFile.Extension() in presubmit_support
Add convenience method AffectedFile.Extension() for extracting file
extension in a standard way. Add a test for this new functionality.
Add a similar test for AffectedFile.UnixLocalPath().

Bug: None
Change-Id: If591e751fb2e4fb5355ad0b6f93f310667849d68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6273341
Commit-Queue: Anton Bershanskyi <bershanskyi@gmail.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
2 months ago
Gavin Mak 4734cfabf4 Fix roll_dep_test.py
4 tests consistently error with "FileNotFoundError". Ensure that both
DEPS and gclient config use the same "file://" URI format so gclient
recognizes the repo as identical and doesn't move src/ to _bad_scm/.

Bug: 391455129
Change-Id: I507750754adc5e83b8a8ce853013a20f60102096
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6259135
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
2 months ago
Mason Freed a155deff42 Revert "Remove "attempt to squash" behavior from `git rebase-update`"
This reverts commit 58625e82c6.

Reason for revert: Several comments on crbug.com/40264739 that this broke their workflow. I plan to revert this and put patchset #2 back up for review, with an opt-in instead of a change to the default behavior.

Original change's description:
> Remove "attempt to squash" behavior from `git rebase-update`
>
> In common workflows, this step only succeeds ~5% of the time, and
> in the other 95% of cases, simply adds ~30 seconds of wasted time
> to the execution time of `rebase-update`. Therefore, this CL removes
> the automated functionality, and leaves squashing as a manual option
> to the user.
>
> Fixed: 40264739
> Bug: 40390274
> Change-Id: Ib2a3ffe4b0e5d0b74323a2a0d34ab0e1b7fafb08
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6104282
> Auto-Submit: Mason Freed <masonf@chromium.org>
> Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>

Bug: 40390274
Change-Id: I91210a5a21b3751695553433389f346ba443bb1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6254237
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
3 months ago
Devon Loehr 0822fcc141 `git cl split`: Save and load splittings from files
This CL implements the proposal (from the design doc in the cited bug)
to allow `git cl split` to save the generated splitting to a
human-readable and editable text file. This allows users to save the
splitting in case something goes wrong and they need to retry, and also
lets them tweak a generated splitting if they get something mostly, but
not quite, usable.

It also allows users to edit the generated splitting interactively, like
they do for e.g. CL descriptions during `git cl upload`.

Bug: 389069356
Change-Id: I8ae21f2eb7c022ba181ae5af273ce09605b5acec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6235727
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
3 months ago
Devon Loehr ff733a6496 `git cl split`: Include dirname as part of branch names
This CL changes the way `git cl split` generates branch names during
upload. Specifically, it attempts to derive a common directory for all
the files in each CL, and includes that dirname as part of the branch
name, in addition to a hash of the files in the CL. This makes it a bit
easier to guess what's in a branch, given its name.

Since the hash is still included, branch names should remain unique.
Finding a common directory should be deterministic, so we can still rely
on branches having the same name if the script is re-run with the same
splitting.

Bug: 389069356
Change-Id: I70490258755f13962f3db5c835619caa24176af9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6243377
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
3 months ago
Devon Loehr 5a3c9438e6 `git cl split`: Name sub-branches by hash
`git cl split` operates by creating a new git branch for each CL
it uploads. It names this branch based on the directory of the files
in the CL. It uses this convention when resuming an interrupted upload:
it skips branches that already exist.

However, this approach is brittle for several reasons:
- The splitting is nondeterministic, so there's no guarantee that CLs
with the same directories will be generated when re-run. Even if they
are, there's no guarantee they'll have exactly the same files.
- For CLs with multiple directories, it uses an arbitrary one, so it
may not choose the same name a second time
- It bakes in an assumption that every CL is defined by the directories
it draws files from.

This CL alleviates this issues by instead naming branches using the hash
of their contained files. This ensures that, with extremely high
probability, branches will have the same name if and only if they have
the same (sorted) list of files. Thus resumed uploads can safely skip
existing branches.

Bug: 389069356
Change-Id: I19bf7cd6ff8b0127f5906384f7f43b3e40ac8fee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6236622
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
3 months ago
Dirk Pranke 7cffe56af4 Add UnixLocalPath*() in presubmit_support.
This adds AffectedFile.UnixLocalPath(), Change.UnixLocalPaths(), and
input_api.UnixLocalPaths() to the presubmit support APIs, so that we can
check paths in a platform-independent manner (i.e., not have to worry
about whether we're running on Windows and might need to handle
backslashes).

Change-Id: I1a424825384cb950ef2d4d3008c9c6e60028cf07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6236404
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
3 months ago
Alexander Cooper 682a6e1194 Add git cl squash-closed behavior
Introduces a new git cl command `squash-closed`. Similar to archive,
this will operate on all closed branches. Unlike archive, rather than
removing these branches and potentially leaving their downstreams in a
wedged state, this will squash the branch and appropriately reparent the
downstream. This makes this a good function to run in preparation of a
`git rebase-update`, especially if there are chained git branches, as
this will then shrink any merged branches to a single commit which
should cleanly apply and drop off.

Bug: 40264739
Change-Id: I1836f944d43f5f3dcebbebf06437bef890da96ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6230362
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
3 months ago
Alexander Cooper 380df04b62 Modify git squash-branch to perform reparenting
Bug: 40264739
Change-Id: I4ad7f4f8a670334b32c239458048e56c6af44098
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6227541
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
3 months ago
Scott Lee c60f8194a5 lucicfg: format star files if --lucicfg
Bug: 390409288
Change-Id: Ida238f3a903c4f1dd31c8fa007e0a8823aeaf1aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6219826
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
3 months ago
Gavin Mak 77a800d729 Print branch name when running presubmits
This implements the proposed solution B in https://crbug.com/380877175#comment5
and will help distinguish presubmit results when working with stacked changes.

Bug: 380877175
Change-Id: I7f0807f7ce03109288976069c57c28488d6cadfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6196130
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
3 months ago
Devon Loehr 36427aab02 split_cl: Always ask for confirmation before uploading
`git cl split` has the potential to do a lot of operations at once,
which is problematic if it didn't end up splitting things in a way
the user approves of. This changes it to print a summary of the
splitting, and asking the user to review it briefly before proceeding
with the upload.

Bug: 389069356, 40642562, 40269201
Change-Id: I90bead0147badbaa3afcce2264d805ae498f101c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6163905
Commit-Queue: Devon Loehr <dloehr@google.com>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Andy Perelson <ajp@google.com>
3 months ago
Allen Li 8eff9d233f [auth] Add test for overwriting global SSO rewrite rules
Sanity check for attached bug

Bug: b/390219533
Change-Id: I788e6beaa7f838568714b98b6f7923d455e6e3a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6178580
Commit-Queue: Allen Li <ayatane@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
3 months ago
Allen Li 877b1e3cbd [auth] Hide global SSO rewrite in local repo
If the user uses SSO (and thus has SSO rewrite rules) for their global
config, a repo which locally uses an email that doesn't use SSO needs
an override.

Bug: b/390219533
Change-Id: I639dae4c1a45bbd2c17180855c22260979b0dbc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6177780
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Allen Li <ayatane@chromium.org>
3 months ago
Scott Lee 80d1969422 presubmit_diff: add support for -U
This is a reland of https://crrev.com/c/6168707 with fixes.
The CL adds a new param, unified, to git_diff() and create_diffs()
as a required param.

The functions themselves are tested in presubmit_diffs_test.py, but
their usages in presubmit_support.py were not, and the missing param
caused a failure in CiderG (b/389876151)

This CL basically carries the same patch, but makes the params
optional with a default value and adds unit tests for the usage
in presubmit_support.py

Tested with CiderG: https://screenshot.googleplex.com/PYEDZ3NGn5cYGtV


Change-Id: Ic747c6a133c016dbcd80034e521c76a3db3a3f60
Bug: 389876151, 379902295
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6182703
Commit-Queue: Scott Lee <ddoman@chromium.org>
Reviewed-by: Gary Tong <gatong@chromium.org>
3 months ago
Scott Lee 7b9ced4220 Revert "Reland "add support for -U in presubmit_diff.py""
This reverts commit 9a9142793a.

Reason for revert: failed again

Original change's description:
> Reland "add support for -U in presubmit_diff.py"
>
> This reverts commit 4c54361841.
>
> Reason for revert: Reland with a fix.
>
> Find https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173762/1..2
> Tested by running presubmit_support.py --generate_diff
>
>
> Original change's description:
> > Revert "add support for -U in presubmit_diff.py"
> >
> > This reverts commit b576ab3b78.
> >
> > Reason for revert: http://b/389876151
> >
> > Original change's description:
> > > add support for -U in presubmit_diff.py
> > >
> > > presubmit_diff.py is going to be used to compute the changes to be
> > > formatted, and -U helps minimize the number of irrelevant lines
> > > from formatting.
> > >
> > > Bug: 379902295
> > > Change-Id: I9c0a2ee6b5ffa6b9fe4427362556020d525f1105
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6168707
> > > Reviewed-by: Gavin Mak <gavinmak@google.com>
> > > Commit-Queue: Scott Lee <ddoman@chromium.org>
> >
> > Bug: 379902295
> > Change-Id: I82dd707e5ae3d4b1760e632506ee0e1bc1d76e09
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173760
> > Reviewed-by: Scott Lee <ddoman@chromium.org>
> > Commit-Queue: Gavin Mak <gavinmak@google.com>
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
>
> Bug: 379902295
> Change-Id: Icbc4aa98bbfaa816143be064217fb2d992b48baf
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173762
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Scott Lee <ddoman@chromium.org>

Bug: 379902295
Change-Id: I84875f6667689e1a9085876555bc6aef4ea2d7b4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6177776
Commit-Queue: Scott Lee <ddoman@chromium.org>
Reviewed-by: Gary Tong <gatong@chromium.org>
3 months ago
Mike Frysinger 232ffd8df3 gerrit: addmessage: support attention set control
Sometimes I want to add a message without touching the attention set.
The default review call will automatically update it regardless.  Add
a flag to explicitly control behavior.

Test:
	# No attention set option.
	$ ./gerrit_client.py ...
	-> ignore_automatic_attention_set_rules not included in body of request
	$ ./gerrit_client.py ... --automatic-attention
	-> body has {'ignore_automatic_attention_set_rules': False}
	$ ./gerrit_client.py ... --no-automatic-attention
	-> body has {'ignore_automatic_attention_set_rules': True}

Change-Id: Idcf7975ba691942cfe385a2156ef3a3ec064a0b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6172486
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Auto-Submit: Mike Frysinger <vapier@chromium.org>
3 months ago
Scott Lee 9a9142793a Reland "add support for -U in presubmit_diff.py"
This reverts commit 4c54361841.

Reason for revert: Reland with a fix.

Find https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173762/1..2
Tested by running presubmit_support.py --generate_diff


Original change's description:
> Revert "add support for -U in presubmit_diff.py"
>
> This reverts commit b576ab3b78.
>
> Reason for revert: http://b/389876151
>
> Original change's description:
> > add support for -U in presubmit_diff.py
> >
> > presubmit_diff.py is going to be used to compute the changes to be
> > formatted, and -U helps minimize the number of irrelevant lines
> > from formatting.
> >
> > Bug: 379902295
> > Change-Id: I9c0a2ee6b5ffa6b9fe4427362556020d525f1105
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6168707
> > Reviewed-by: Gavin Mak <gavinmak@google.com>
> > Commit-Queue: Scott Lee <ddoman@chromium.org>
>
> Bug: 379902295
> Change-Id: I82dd707e5ae3d4b1760e632506ee0e1bc1d76e09
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173760
> Reviewed-by: Scott Lee <ddoman@chromium.org>
> Commit-Queue: Gavin Mak <gavinmak@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>

Bug: 379902295
Change-Id: Icbc4aa98bbfaa816143be064217fb2d992b48baf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6173762
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Scott Lee <ddoman@chromium.org>
3 months ago