This reverts commit c87ed60623.
Reason for revert:
Causes a regressions for CL descriptions on Windows
Original change's description:
> git-cl: Fix some python3 compatibility errors.
>
> Also, fix bug in git cl status where the 'updated' field was used to compare messages, even though
> it doesn't exist (see [1]). This CL modifies it to use 'date', which does exist.
>
> [1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-message-info
>
> Bug: 1002209
> Change-Id: I5a5e1193b8502c3ad35d94808ea178cad7f44ac6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1891259
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Anthony Polito <apolito@google.com>
TBR=ehmaldonado@chromium.org,apolito@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 1002209
Change-Id: I004f202b12c6b99cb6b24cb12a14fba7898569e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1898547
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Lets the client repository make 'git cl format' use the
'--full' option by default. This solves issues when using less
common clang-format options that don't behave well with small
diffs. For example, AlignConsecutiveAssignments.
This is a replacement for the 'diff-lines-of-context' setting.
Bug: angleproject:4003
Change-Id: I81dc3b4992a7420e7235da88ec78e51ec4c0d24f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1879148
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
This reverts commit 812ac228e3.
Reason for revert: The fix was not sufficient to solve the git cl
format difference between 'git cl format' and 'git cl format --full'.
Looks like we'll need a different solution.
Original change's description:
> git-cl: Add diff lines of context setting.
>
> Clients using some Clang-Format options can run into bugs when using
> 0 lines of context. AlignConsecutiveAssignments in particular is picky
> and behaves differently with 0 lines of context.
>
> This behaviour lead to 'git cl format' behaving differently from
> 'git cl format --full'. That could also break presubmit format checks.
> Using >0 lines of context for 'git diff' fixes the inconsistency.
>
> Add a setting 'DIFF_LINES_OF_CONTEXT' that can be controlled via
> codereview.settings. Defaults to zero to preserve the old behaviour.
> The setting allows the client to control the number of lines of context
> to use when running 'git cl format'.
>
> Bug: angleproject:4003
> Change-Id: Ied2ebf23df4c41ba19bfbd5b8ddf526b56a20b31
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1864309
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
TBR=agable@chromium.org,ehmaldonado@chromium.org,jmadill@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: angleproject:4003
Change-Id: Ib6b70969509a1e5446922d488f11d13a0c59c536
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1877003
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Clients using some Clang-Format options can run into bugs when using
0 lines of context. AlignConsecutiveAssignments in particular is picky
and behaves differently with 0 lines of context.
This behaviour lead to 'git cl format' behaving differently from
'git cl format --full'. That could also break presubmit format checks.
Using >0 lines of context for 'git diff' fixes the inconsistency.
Add a setting 'DIFF_LINES_OF_CONTEXT' that can be controlled via
codereview.settings. Defaults to zero to preserve the old behaviour.
The setting allows the client to control the number of lines of context
to use when running 'git cl format'.
Bug: angleproject:4003
Change-Id: Ied2ebf23df4c41ba19bfbd5b8ddf526b56a20b31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1864309
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
The latter form is deprecated, and can generate extraneous output when
running the tests:
DeprecationWarning: Please use assertEqual instead.
Bug: 984182, 1007872
Change-Id: Ibac81178e719a739560bbc65e0f6b9eda40b313a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1859995
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Adds support for .yapfignore files to "git cl format" when formatting
Python files. yapf is supposed to parse the .yapfignore file in the
current working directory, but this appears to not work when files
are explicitly passed to yapf for formatting like they are in git cl
format. Instead, parse the .yapfignore file ourselves and skip over any
matching files.
Bug: angleproject:3985
Change-Id: I5e8469470fb8ddbaa914005b012ac1f39dfdd223
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1849807
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
This option take the same format as -b / --bug but
spits out "Fixed: #,#" instead of "Bug: #,#".
This CL also looks for fix-, fixed-, and fixes-
branch prefixes and uses those as cue to add "Fixed:"
to the CL description when editing. This is a slight
behavioral change for fix- specifically, as it used
to produce "Bug: " instead.
Bug: monorail:4470
Change-Id: Ib24a1ff33ca3674e53fc5437f459ea5708988290
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1846785
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Dan Beam <dbeam@chromium.org>
Tag is the same as used by Gerrit plugin for the same functionality:
retry_failed:1
R=ehmaldonado
Change-Id: I14ef435b676088c86ae7cc0536c6a68fa58bd89f
Bug: 445502
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1848258
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Auto-Submit: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
This reverts commit f0faf48ddb.
Reason for revert:
Legacy properties are no longer used by Skia.
Original change's description:
> git-cl: Set legacy properties.
>
> Will be removed back again on Oct 9, 2019.
>
> Bug: 1004447
> Change-Id: I7003fac9ae8b184e2a7073b516535e8fd599f4f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1825446
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
TBR=tandrii@google.com,ehmaldonado@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 1004447
Change-Id: I4e1fb31b6179bf10c3d9554cd313f7040aaf1b1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1847959
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Make it work the same way as "git cl try --retry-failed".
R=ehmaldonado
Change-Id: Id3a709dc40b6b6d068acbba0c610da254ced8ff3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1845478
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Auto-Submit: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Get rid of _process_codereview_select_options and detected_codereview_from_url and simplify issue parsing.
Change-Id: I4200fd83ee868587c8627d6771c64f886b34a88b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1838384
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
This will prevent moving reviewers to cc when uploading patches.
Bug: 1010499
Change-Id: Ida16a4686f46350ba009bdaec1542895208c3f2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1835016
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
The expected behavior is that for CLs that were already uploaded
before, where some tryjobs were run and failed, git cl upload
--retry-failed will be kind of like git cl upload --cq-dry-run
except it will only trigger tryjobs that failed.
Bug: 985887
Change-Id: I6371bca3ba501b1ea2cd7160e2f933530d7e633f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1828322
Auto-Submit: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@google.com>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
There are many functions in git_cl that appear unused, although
it's possible that some of them are used in other places, where
git_cl may be imported as a library.
This CL just removes some of the private ones, which at least
were not supposed to be used from other places.
Things removed here:
- _get_commiter_timestamp
- _git_amend_head
- GitCookiesChecker._get_usernames_of_domain
- constants MERGE_BRANCH, CHERRY_PICK_BRANCH;
the comment says that these are supposed to
be used in CMDland, but apparently they aren't.
contextlib which is only used by this, is also
removed
- CodereviewSettingsFileMock in git_cl_test;
it is not used anywhere in the test, and I
assume that nobody's importing and using
git_cl_test...
git_cl_test still passes, git cl seems to still work
Change-Id: Ib8eee0dfb181c8cda04c0894d2daea52b3ad42ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1818621
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
With certain git remote problems an error message is emitted that
has the place of the url and the place of the branch wrong.
Example:
Remote "opera" for branch "git@github.com:operasoftware/gn-opera.git"
points to "bratell/no_jumbo_for_s", but it doesn't exist.
Should be:
Remote "opera" for branch "bratell/no_jumbo_for_s" points to
"git@github.com:operasoftware/gn-opera.git", but it doesn't exist.
R=dpranke@chromium.org
Change-Id: I484ac8af226903a49df1f62e837d01c345fb5f02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1806595
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
CQ will soon start canceling tryjobs it triggered on no-longer-latest
patchsets if these tryjobs won't be re-used. New option can be used by
developers who upload new patchsets but actually care for prior
CQ-triggered tryjobs to complete.
This footer is effectively sticky, unless someone removes it by hand later.
We need to have the footer one way or another to tell the CQ what to do
(and provide a visible audit trail).
We may revisit the stickiness later based on accumulated usage data s.t.
the following uploads remove the footer unless the flag is passed.
Bug: 909895
Test: git cl upload --preserve-tryjobs
Change-Id: Ibbc6e917504c31eab8ab85296b3ecafd3add46df
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1701506
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
There are some Google-internal repos that use a custom scheme. Having
any remotes pointing to these URLs breaks git cl archive.
Change-Id: Ibd686c5cd11e090f0821ff2b412540138252dfce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1614946
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
In crbug.com/955206 they pointed out that:
- It's not clear when to upload the issue
- It's not clear where to report it, or where to look for existing issues.
- It's not made clear that the traces can contain information the user might not want to share.
I've reworded the message to address those concerns
Bug: 955206
Change-Id: Ia1426007d389328eac52babeee1ec3782c2b104e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1606608
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
This is a reland of dc8e23d356
Original change's description:
> git-cl: Keep git push traces
>
> Keep the last N git push traces.
> Name them after the time when they were collected, and add a
> README file to each one to provide some context to developers.
>
> Bug: 955206
> Change-Id: Ib5fcf2f78fb65f6ddd80a93619c14e1ef70c5564
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595108
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Bug: 955206
Change-Id: Ie63a3ebfe3024d48ad7e0c8492a2939e635371aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1600246
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
This reverts commit dc8e23d356.
Reason for revert: Breaking chrome releases crbug.com/960638
Original change's description:
> git-cl: Keep git push traces
>
> Keep the last N git push traces.
> Name them after the time when they were collected, and add a
> README file to each one to provide some context to developers.
>
> Bug: 955206
> Change-Id: Ib5fcf2f78fb65f6ddd80a93619c14e1ef70c5564
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595108
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
TBR=dpranke@chromium.org,ehmaldonado@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 955206
Change-Id: Id56dfb1b32a54070d761ef75bc26bbb9081f86fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1600535
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>
Keep the last N git push traces.
Name them after the time when they were collected, and add a
README file to each one to provide some context to developers.
Bug: 955206
Change-Id: Ib5fcf2f78fb65f6ddd80a93619c14e1ef70c5564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595108
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@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>
For instance, if the bug-prefix is set to "b/", git cl upload will offer a
line with "Bug: b/", however it won't get stripped away if it's left blank,
unlike the default "Bug:" line. This change fixes that inconsistency by
also taking the bug-prefix into account when stripping.
Change-Id: Ib6e4d18c1ff52ec77cd1422be15b1e6920332238
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1528972
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Jonas Termansen <sortie@google.com>
In particular, only show robot comments from the latest patchset.
Bug: 924780
Change-Id: I12038ddd2d90a5cb561b248de3fd37908d5b927e
Reviewed-on: https://chromium-review.googlesource.com/c/1457282
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Commit a185e2e3c8 removed the Rietveld code review
implementation, albeit hasn't expunged all references to Rietveld. This leads
to an assertion error when running |git cl format| on a non-Gerrit change.
There appears to have been a recent attempt to articulate this error better in
a185e2e3c8, albeit not fix the underlying issue.
This change addresses the immediate issue by defaulting the code review
implementation to Gerrit rather than Reitveld.
R=joenotcharles@google.com, tandrii@chromium.org
Bug: 914150
Change-Id: I64d33e5a172cc43339ec417f7f0a7820e0337772
Reviewed-on: https://chromium-review.googlesource.com/c/1372928
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bryce Thomas <bryct@amazon.com>
Except when TBR is active, because it's not clear how to
compute max score for Code-Review label before uploading a change.
Tested with this very own CL:
To https://chromium.googlesource.com/chromium/tools/depot_tools.git
* [new branch] c78de165c7 ->
refs/for/refs/heads/master%ready,notify=ALL,m=Initial_upload,r=ehmaldonado,
cc=ajp,cc=chromium-reviews@chromium.org,cc=iannucci+depot_tools@chromium.org,
l=Commit-Queue+1,hashtag=git-cl-upload
R=ehmaldonado
Bug: 877717
Change-Id: I464d9df2814bd830dd608bb881040e2dd3c41dbb
Reviewed-on: https://chromium-review.googlesource.com/c/1307052
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
For other hosts, behavior is not changed.
Tested on this very CL:
To https://chromium.googlesource.com/chromium/tools/depot_tools.git
* [new branch] 9057c2235b ->
refs/for/refs/heads/master%ready,notify=ALL,m=Initial_upload,r=ehmaldonado,cc=chromium-reviews@chromium.org,cc=iannucci+depot_tools@chromium.org,hashtag=git-cl-upload
R=ehmaldonado
Bug: 877717
Change-Id: I951fc576105211590c6c303ce0ed2fe142628224
Reviewed-on: https://chromium-review.googlesource.com/c/1307051
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Also fix a typo in ValidateEmail function which didn't support
email addresses with '-' in them, e.g., infra-dev@chromium.org.
R=ehmaldonado
Bug: 875089
Change-Id: I2d73c1473527c9bf62e25e9f88250196b783fcb0
Reviewed-on: https://chromium-review.googlesource.com/c/1242849
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Also deletes lots of tests which provided coverage for Rietveld only,
and hence no longer useful.
R=ehmaldonado
Bug: 770408
Change-Id: I31195f7819a52d1063ed28064a74fd70fbc39357
Reviewed-on: https://chromium-review.googlesource.com/c/1279133
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
This adds O(reviewers+cc) RPC to Gerrit before upload,
though done concurrently in up to 10 threads. This information isn't
used yet, but will be in follow up CLs.
R=ehmaldonado
Bug: 877717
Change-Id: I2374f249ee874a71089244309e50e8e88a3dee7d
Reviewed-on: https://chromium-review.googlesource.com/c/1242847
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This removes 1 RPC when tbr-ing second or later patchset.
Since tests cover TBR only in first upload, there are
no test changes. Since adding test is real PITA here,
keep them as is.
R=ehmaldonado
Change-Id: I90801b06727ce2131a703d6cd6c24ffad798bc8a
Reviewed-on: https://chromium-review.googlesource.com/1242212
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Print "git cl comments" json to stdout when the output file is "-"
Change-Id: I91021317e52019a0d499cccc4f40f062127b300e
Reviewed-on: https://chromium-review.googlesource.com/1228094
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
This fixes Gerrit project detection based on remote URL,
accounting for potential 'a/' prefix in the URL path component,
which isn't part of the Gerrit project name.
R=ehmaldonado, mmoss
Bug: 876964, 876910
Change-Id: I473ae8c6c9e0f2034b350901abd67db151e0a3d3
Reviewed-on: https://chromium-review.googlesource.com/1187573
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Sadly, this makes maintaining one test handling covering
exceptional circumstance very complex, so it was removed.
I've decoupled test that ensures that GetRemoteUrl works from
upload codepath.
R=ehmaldonado@chromium.org
Bug: 876910
Change-Id: I39de410c72d893e73492d5c3fc8f60a6ebc4f11f
Reviewed-on: https://chromium-review.googlesource.com/1186142
Reviewed-by: Ryan Tseng <hinoka@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
When using non-squashed uploads, there usually isn't an associated
issue with a branch. Thus setting WIP when the issue tag wasn't
present would continually set WIP even on trivial rebases. Not
adding the WIP tag when uploading allows the user to set WIP manually,
while not forcing the WIP flag after every new upload.
Also updates the unit tests that use no-squash.
Bug: 767439
Change-Id: I990a82139fefe1a0c5c7b149d39045cf985539b7
Reviewed-on: https://chromium-review.googlesource.com/1033187
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Unfortunately, w/o rewrite of gerrit_util, one can't take advantage
of existing auth.Authenticator (which also doesn't know about ~/.netrc
or .gitcookies, but that's fixable). This rewrite, however, will likely
cause issues for nasty scripts outside of depot_tools which use
gerrit_util as a Python library. So, I followed outdated way of
gerrit_util :(
Also contains refactoring of auth library, which was kept backwards
compatible for the same reasons as above.
R=vadimsh@chromium.org
Test-with: led tool
Test-artifact: https://ci.chromium.org/swarming/task/3cf4687dfd26ca10?server=chromium-swarm.appspot.com
Bug: 834536
Change-Id: I6b84b8b5732aee5d345e2b2ba44f47aaecc0f6c5
Reviewed-on: https://chromium-review.googlesource.com/1018488
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This allows people to configure their gitcookies file to be
whereever they want. As long as it actually exists and has
credentials in it, we'll accept it, just like git itself.
Change-Id: I4aa4806ddca0e61b28b003b0d3bc486407c13ab4
Reviewed-on: https://chromium-review.googlesource.com/951917
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
The purpose of this CL is to remove confusion around whether or
not git-cl support the similarity flags when talking to Gerrit.
It does not. However, because Rietveld support has not been
fully ripped out of git-cl, this change does modify some of the
Rietveld-specific code to remove similarity support from everywhere.
Bug: 800902
Change-Id: I8703ed60d6889c7a36400b8a9f8f26de4669020a
Reviewed-on: https://chromium-review.googlesource.com/912389
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
I don't know of any use-case where someone would want to
use git-cl-patch to pull a (e.g.) chromium change into
their (e.g.) v8 checkout.
Bug: 803918
Change-Id: Id53f1cc3ab97e623d0098bb366a573b18b54e91a
Reviewed-on: https://chromium-review.googlesource.com/876930
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This is a reland of 9219d35688
The original was reverted due to a typo (core,quotePath instead
of core.quotePath). This version is fixed.
Original change's description:
> Use core.quotePath=false when git is listing files
>
> This prevents git from putting quotes around some file names
> (those that have astral-plane characters) and not around others.
>
> R=maruel
>
> Bug: 792302
>
> Change-Id: I3b6a6b36c4720116de811b40177b59aa25c263db
> Reviewed-on: https://chromium-review.googlesource.com/815454
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Bug: 792302
Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: build_limited_scripts_slave
Change-Id: I28d2260948aaf63bd865888c2f60e4cdee9aea48
Reviewed-on: https://chromium-review.googlesource.com/822990
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Context: The WPT importer consumes the JSON produced by
git cl try-results is consumed and uses it to determine
when the CQ has passed (whether all non-experimental cq
jobs were successful) and where layout test results are
stored.
Added fields:
created_ts: This could be used to sort builds by time.
tags: Includes user_agent tag and experimental tag, as well as
experimental: whether the job is experimental
Bug: 792652
Change-Id: Ic6099a21b21db668b88b1a8e54aefd84529a2a91
Reviewed-on: https://chromium-review.googlesource.com/819573
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This reverts commit 9219d35688.
Reason for revert: unfortunately this says "core,quotePath" and since it includes recipe changes, we need something that the roller can munch on :(
Original change's description:
> Use core.quotePath=false when git is listing files
>
> This prevents git from putting quotes around some file names
> (those that have astral-plane characters) and not around others.
>
> R=maruel
>
> Bug: 792302
> Recipe-Nontrivial-Roll: build
> Recipe-Nontrivial-Roll: build_limited_scripts_slave
> Change-Id: I3b6a6b36c4720116de811b40177b59aa25c263db
> Reviewed-on: https://chromium-review.googlesource.com/815454
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
TBR=maruel@chromium.org,agable@chromium.org
Change-Id: I226388f19024403240a1443eb2b878b9293220e1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 792302
Reviewed-on: https://chromium-review.googlesource.com/821671
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
This prevents git from putting quotes around some file names
(those that have astral-plane characters) and not around others.
R=maruel
Bug: 792302
Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: build_limited_scripts_slave
Change-Id: I3b6a6b36c4720116de811b40177b59aa25c263db
Reviewed-on: https://chromium-review.googlesource.com/815454
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
If a commit subject contains [hashtags], include them in Gerrit CL.
Replace consecutive non-alphanums with dash.
Also add --hashtag flag for explicit hashtagging.
Bug:
Change-Id: I25aed286013043263f959ff340a5b5478faa0f27
Reviewed-on: https://chromium-review.googlesource.com/764974
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Previously, git-cl-diff went through a dance where it would create
a new branch, download the uploaded patch onto that branch, and
then diff against that. This had all sorts of problems: if you
aborted the command, it might leave you on that branch; if you have
local changes, they might get clobbered or the command would refuse
to run.
Now that we're in a Gerrit-only world, and patchsets are by definition
equivalent to commits, we can simply diff against whatever local commit
was last uploaded or, in a pinch, fetch the uploaded commit and diff
against that.
Bug: 759893
Change-Id: Ia4b93dcfb9b8aba85817e62731f68d6450026e75
Reviewed-on: https://chromium-review.googlesource.com/639915
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Since git-cl TBR always sets +1, it does not allow auto-submit where
using +2 score as approval. Let me make git-cl automatically adjust
to proper score.
Bug: 762425
Change-Id: I71fe1af1b8bf5e68d2509c60e8bf05024b6bdbb7
Reviewed-on: https://chromium-review.googlesource.com/680717
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Using self.GetDescription() uses the description from the web.
Given that the user is purposefully disassociating their current
branch from the review on the web, they may have already changed
the commit message. We shouldn't set it back.
Bug: 742730
Change-Id: I0545cb6288c332fd475d1de7fb302f71ee41a415
Reviewed-on: https://chromium-review.googlesource.com/578229
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
In Rietveld, adding a comment to a change automatically
published it no matter what. In Gerrit, we need to explicitly
mark the change as Ready for Review. This CL adds a new
parameter to the wrapper methods around the SetReview
API so that they can mark changes as Ready.
Bug: 740950
Change-Id: Icb2ad7c5beb03a4760657a761841745f0d75514e
Reviewed-on: https://chromium-review.googlesource.com/572031
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Although git-cl-upload warns when uploading a new patchset
to a change owned by someone else, if the uploader has run
'git cl issue 0', then git-cl believes they'll be uploading
a new change, so it doesn't bother checking. However, once
the upload begins, Gerrit notices the Change-Id in the commit
message, and instead adds a new patchset to someone else's
review (if the uploader is a committer).
This change introduces some logic to git-cl-issue to also
remove any Change-Id from the commit message when a user
tries to clear the metadata about their branch.
Bug: 741648
Change-Id: I6c7c3b24a7fc09c68220c8200b732fbdf9cf1fd3
Reviewed-on: https://chromium-review.googlesource.com/568267
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Due to relaxation of when last paragraph of commit message is
is consider as containing footers, `git cl land` started removing
non-canonic footer lines from last paragraph if Git Numberer is enabled
on a repo. This only manifests in manual lands of Rietveld CLs or
bypassing code review entirely.
R=agable@chromium.org
Bug: 736852
Change-Id: I3972c590c3959974157ada9de9891a3c08bd385a
Reviewed-on: https://chromium-review.googlesource.com/562278
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
It seems like some folks are confused by additional patchsets
after the first putting the change back into WIP mode. This
confusion is honestly understandable. Maybe we try only setting
it on the very first upload, and just controlling the notify
parameter for future patchsets.
Bug: 721836, 737675
Change-Id: If56e5c71e0c6b3b46c2e30ac0b6d80b878218181
Reviewed-on: https://chromium-review.googlesource.com/552779
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: smut <smut@google.com>
Commit-Queue: Aaron Gable <agable@chromium.org>
The previous CL forgot that the lack of '%wip' doesn't
mark a change ready-to-review, you have to explicitly pass
'%ready' in the refspec to do that.
TBR=tandrii@chromium.org
Bug: 721836
Change-Id: Iea82222d64edf1b73fefa9bca3feec4188e35ab3
Reviewed-on: https://chromium-review.googlesource.com/551005
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Work-In-Progress is a new change flag that can be set on
Gerrit changes. While a change is in WIP mode, certain things
are different:
* It doesn't send emails except to the change owner
* The "Reply" button becomes "Start Review"
* When a change is moved out of WIP, it sends a special
"ready for review" message to any new reviewers
This is much more similar to the Rietveld model, where users
would "Publish" their changes for the reviewers to look at.
Bug: 721836
Change-Id: I3b9697e311fa176cb679ecefbfead9bb32b6afaf
Reviewed-on: https://chromium-review.googlesource.com/549015
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This is a partial revert of https://chromium-review.googlesource.com/c/527345/
Turns out more people were confused by the new behavior than
expected, so we're returning to "cherry-pick" being the default,
but supporting the collaboration workflow is important, so we're
adding a warning message and support for "reset --hard" behind a
pre-existing flag.
Bug: 723787
Change-Id: Ib6038a42e3bdcc0db93c1f32d759e9ff0e91a065
Reviewed-on: https://chromium-review.googlesource.com/538137
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This CL changes the way that "git cl patch" behaves for Gerrit changes.
Previously, git-cl-patch behaved just like it did for Rietveld:
make sure you're on a branch, download the diff, apply it on top
of your branch. However, this causes problems with Gerrit. Namely,
when you upload a change to Gerrit, git-cl has to make sure that all
parents of your local change have previously been uploaded as well,
either as other changes or as commits already landed on the target
branch. But the method for "applying a patch" from Gerrit was to
cherry-pick it, and that changes the commit hash. So the resulting
commit would *not* have been uploaded to Gerrit. Thus, the
following routine didn't work with Gerrit:
$ git checkout -t origin/master -b your-work
$ git cl patch 123456
$ git checkout -tb my-work
$ #hack and commit
$ git cl upload
This would fail during the upload with a message saying that the
contents of 'your-work' hadn't been uploaded.
This CL fixes the situation by replacing the cherry-pick with
a hard reset. This means that the contents of the 'your-work'
branch will be *exactly* what was downloaded from Gerrit. Uploads
based on top of that commit will work just fine.
Finally, in a concession to some people who want 'git cl patch'
to actually apply a patch instead of performing a hard reset, if
the current branch contains local work, then rather than leaving
that work behind with a hard reset, we fall back to the old
cherry-pick behavior with a confirmation dialog and warning that
uploading will be hard.
Bug: 723787
Change-Id: I3ad164f6d3078bff00139d446bb8ce97738a1344
Reviewed-on: https://chromium-review.googlesource.com/527345
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This prevents TBR (self code-review) permissions from blocking
the CL from being uploaded at all. Instead, it will fully
upload, and then show a better error message after upload is
complete.
Bug: 729967
Change-Id: I55e3e98e200143076afcaab858064d9f5c62f8ef
Reviewed-on: https://chromium-review.googlesource.com/527325
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This brings the gerrit version of "git cl comments" into line
with the Rietveld implementation by including file- and line-level
comments as well as top-level review comments. It requires an
extra API call to do so, so this may result in some slow-down, but
the result is worth it.
It formats the comments to match the formatting used in the
PolyGerrit UI, with the addition of visible URLs linking to
the comment since we can't hyperlink text in the terminal.
This CL also causes it to ignore messages and comments with
the 'autogenerated' tag, which are generally less interesting
and clutter the output.
Bug: 726514
Change-Id: I1fd939d90259b43886ddc209c0e727eab36cc9c9
Reviewed-on: https://chromium-review.googlesource.com/520722
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
For a bit of context, see the TODO in the code --
I think that the original intent of that TODO was that we to make
the way that CQ dry runs are triggered consistent, and also make
the behavior of dry runs consistent across different commands.
Change-Id: I80dfc31ade302a6af7fa84011e2871d416ea9c96
Reviewed-on: https://chromium-review.googlesource.com/518930
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This information is redundant when using Gerrit,
and is well known to anyone still using Rietveld.
Change-Id: I03119a84edb67fd20fbe5e2a8e0f0975e69558ed
Reviewed-on: https://chromium-review.googlesource.com/510923
Reviewed-by: Andrew Bonventre <andybons@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
The old system had two faults:
* It set reviewers and ccs via different mechanisms, which is confusing
* It set CCs with a single call for each, resulting in N separate emails,
with each email going to the uploader and all reviewers and all previous
CCs.
This new system just collects all reviewers and CCs, and sets them
in a single call. That call will fail if *any* of the individual
reviewers or ccs fail, so it also parses the response and retries with
only the ones which would have succeeded on their own. If that second
call fails, or the first fails in an unexpected way, it raises an
exception like normal
Bug: 710028
Change-Id: I1be508487a41f0b68f9c41908229b8f5342830a3
Reviewed-on: https://chromium-review.googlesource.com/479712
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This makes the library behave more like what we'd expect,
while still allowing certain function to specify that
returning 204 or 404 is expected/acceptable behavior.
Change-Id: If3ce5598d1603819ee97aaeab0072a9e786ed96d
Reviewed-on: https://chromium-review.googlesource.com/481043
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Gerrit sometimes returns a full response json object at
the same time as returning a non-200 status code. This
refactor makes it easier for calling code to request
access to that object and handle error cases on its own.
The original version of this commit had a bug where
ReadHttpResponse properly set the default value for
accept_statuses, but all calls which came through
ReadHttpJsonResponse were setting None instead.
Bug: 710028
Change-Id: I8cee435d8acd487fb777b3fd69b5e48e19d2e5a3
Reviewed-on: https://chromium-review.googlesource.com/481060
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This reverts commit 6d7ab1bfe5.
Reason for revert: Stacktrace:
File "/s/depot_tools/gerrit_util.py", line 816, in GetAccountDetails
return ReadHttpJsonResponse(conn)
File "/s/depot_tools/gerrit_util.py", line 376, in ReadHttpJsonResponse
fh = ReadHttpResponse(conn, accept_statuses)
File "/s/depot_tools/gerrit_util.py", line 365, in ReadHttpResponse
if response.status not in accept_statuses:
TypeError: argument of type 'NoneType' is not iterable
Original change's description:
> Refactor ReadHttpResponse to be error-friendlier
>
> Gerrit sometimes returns a full response json object at
> the same time as returning a non-200 status code. This
> refactor makes it easier for calling code to request
> access to that object and handle error cases on its own.
>
> Bug: 710028
> Change-Id: Id1017d580d2fb843d5ca6287efcfed8775c52cd6
> Reviewed-on: https://chromium-review.googlesource.com/479450
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
>
TBR=agable@chromium.org,tandrii@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: Ia9d9ce835e207a32e7cc8ee35c0cf40c823c7b78
Reviewed-on: https://chromium-review.googlesource.com/481059
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Gerrit sometimes returns a full response json object at
the same time as returning a non-200 status code. This
refactor makes it easier for calling code to request
access to that object and handle error cases on its own.
Bug: 710028
Change-Id: Id1017d580d2fb843d5ca6287efcfed8775c52cd6
Reviewed-on: https://chromium-review.googlesource.com/479450
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This CL propagates <diff_base> all the way to become parent commit
of the syntentic commit generated by squashing the current branch.
BUG=649846
R=agable@chromium.org
Change-Id: Ided7ebbb5c3a1114cac18adb62b3a9c27610018c
Reviewed-on: https://chromium-review.googlesource.com/475229
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
While working on fixing git-cl-status for gerrit, I realized
it would be really easy to bring the Rietveld version up to
parity and simplify it at the same time.
Bug: 706460
Change-Id: Icff32b532fa29f8869205111cd117176e0d34b8f
Reviewed-on: https://chromium-review.googlesource.com/470448
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
* if --rietveld or --gerrit is given, parse only using appropriate
parser.
* if url has '$HOST-review' in the beginning, assume it's Gerrit
* if type of codereview is detected based on URL, then print this
information for the user.
This also applies to `git cl description` but message is logged instead,
because in '-d|--display' option git cl is supposed to print only description,
and some tooling likely relies on this :(
R=jochen@chromium.org
BUG=706406
Change-Id: I21c9355c5334fd71db27618cda11287f75168b59
Reviewed-on: https://chromium-review.googlesource.com/473186
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
This is temporary, and will be changed in subsequent CLs. For now,
it suffices to stop relying on pseudo-random dictionary order in tests
and prod.
R=jochen@chromium.org
BUG=706406
Change-Id: I26c467a28bc63b5f81d20fc222a2b6f0511c507f
Reviewed-on: https://chromium-review.googlesource.com/472750
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This reverts commit 0267fd2c29.
Reason for revert: seems to have changed some behavior as reported by SKIA.
Original change's description:
> git cl upload for Gerrit: use push options instead of refspec.
>
> This removes limitation of no special chars in patchset titles.
>
> BUG=chromium:663787,chromium:707963,gerrit:5184
> R=sergiyb@chromium.org,agable@chromium.org
> TEST=uploaded this CL using depot_tools with this patch :)
>
> Change-Id: I5d684d0a0aa286a45ff99cca6d57aefa8436cd0f
> Reviewed-on: https://chromium-review.googlesource.com/468926
> Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Sergiy Byelozyorov <sergiyb@google.com>
>
TBR=agable@chromium.org,sergiyb@google.com,tandrii@chromium.org,sergiyb@chromium.org,borenet@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:663787,chromium:707963,gerrit:5184
Change-Id: I3306091b14b97a200150389d0480b69120af8c61
Reviewed-on: https://chromium-review.googlesource.com/469006
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This removes limitation of no special chars in patchset titles.
BUG=chromium:663787,chromium:707963,gerrit:5184
R=sergiyb@chromium.org,agable@chromium.org
TEST=uploaded this CL using depot_tools with this patch :)
Change-Id: I5d684d0a0aa286a45ff99cca6d57aefa8436cd0f
Reviewed-on: https://chromium-review.googlesource.com/468926
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Sergiy Byelozyorov <sergiyb@google.com>
This allows for having some global comments such as timezones or
long-term unavailability.
The comments go into build/OWNERS.status (that way, they should be
available in all repos that map in build/ for the gn config files).
The local can be overwritten in codereview.settings.
The format is
email: status
Comments (starting with #) are allowed in that file, but they're ignored.
BUG=694222
R=dpranke@chromium.org
Change-Id: I49f58be87497d1ccaaa74f0a2f3d373403be44e7
Reviewed-on: https://chromium-review.googlesource.com/459542
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
This is needed for making PostUploadHooks that don't have to re-invent
the git footers library.
Bug:
Change-Id: I0a0ccf3dffd25152c2c273487ddbd9b279d80678
Reviewed-on: https://chromium-review.googlesource.com/461729
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>