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>
Jamie Madill
Andrii Shyshkalov
Olivier Robin
Robert Iannucci
Aaron Gable
Nodir Turakulov
Lei Zhang
Quinten Yearsley
Daniel Cheng
Nick Carter
Dirk Pranke
Stefan Zager
Yoshisato Yanagisawa
Quinten Yearsley