Over last two weeks of running tryjobs on CLs of others, I found
this very useful. This CL also simplifies code.
R=agable@chromium.org
Change-Id: I5c7ce0d311b1ca024b92227dbec54e5197205c62
Reviewed-on: https://chromium-review.googlesource.com/930742
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@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>
Context:
https://bugs.chromium.org/p/chromium/issues/detail?id=800355#c22
Prepare git-cl-try to a new format of builders-map.appspot.com response
where each entry contains buckets, as opposed to masters.
Bug: 800355
Change-Id: I5a90c6c4860a7e1fca843067c477a272d782cba1
Reviewed-on: https://chromium-review.googlesource.com/885100
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
The default description in the editor when you invoke "git cl upload"
includes recent commit descriptions, some blank lines and comments, and
"Bug: ". The upload script properly strips out the comments but was
leaving in the "Bug:" line. This leads to CLs with blank Bug: lines,
which is sloppy. It also means that if a user changes their mind and
closes the upload editor without adding any content... they would still
have a CL created.
This change removes the "Bug: " line if nothing has been added to it,
thus avoiding uploading a change with an empty description, and avoiding
having meaningless Bug: lines.
Change-Id: I18c2094e5712d31729899c0ebd0a52de97dc4e7f
Reviewed-on: https://chromium-review.googlesource.com/861920
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This patch adds the standard issue option (-i/--issue) to
"git cl try" command.
This is useful for maintaining FlagExpectations with fyi bots.
Runtime-flag features often see new failures, and a good number
of tests are flaky. Maintainers can upload a CL and use cron to
run try jobs to collect multiple tests results to find new
failures and flaky tests.
Change-Id: I069593950023ddae814faf3074c33de4c98cb70e
Reviewed-on: https://chromium-review.googlesource.com/850792
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
git cl split previously had no way to find out what it would do before
it went ahead and uploaded CLs. Now you can use --dry-run to get it
to print info about each CL it would upload.
Change-Id: If2ac74a89b0a8b30d86819e16ece46f487b1e7c7
Reviewed-on: https://chromium-review.googlesource.com/821336
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Chris Watkins <watk@chromium.org>
This includes a minor refactor so that some gclient_scm methods
can all share the same core.quotePath specifier.
R=iannucci
Bug: 792302
Change-Id: Iaadf190f5c0666787cf7c2ccda88d6dba9aace9b
Reviewed-on: https://chromium-review.googlesource.com/823131
Reviewed-by: Robbie Iannucci <iannucci@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>
We don't set the branch.<name>.gerritpatchset git config value
during "git cl upload" because it isn't returned from the
server either during push or during any of our subsequent RPCs,
and adding another RPC just for that metadata would be slower
that we would like.
But calling "git cl status" already makes an RPC per branch, so
take advantage of that to cache the gerritpatchset value during
that operation.
R=robertma
Bug: 792611
Change-Id: I37b09f3c4cfced86668b457eff82f2424012b14b
Reviewed-on: https://chromium-review.googlesource.com/815137
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
While ripping SVN support out of git-cl, the Changelist() constructor
changed to require that the branch name start with "refs/heads/",
but the CMDissue --reverse code was never updated to feed it the
fully-qualified refname.
R=thestig
Bug: 791176
Change-Id: Ia7fee0f77fec080d91f34ad43bfd9f7c6a4bf64b
Reviewed-on: https://chromium-review.googlesource.com/806239
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This will let developers run PRESUBMIT against all files in the
repo, rather than just those their CL touches. This will be
particularly helpful when modifying PRESUBMIT.py itself.
R=maruel@chromium.org
Bug: 519046
Change-Id: I5775eef8aecefe0cd358537c9218afc7480a7619
Reviewed-on: https://chromium-review.googlesource.com/798056
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@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>
If you name your branches "bug-123-fix-foobar" or "123-fix-foobar", git cl
upload will automatically populate the "Bug: " line in the cl description with
123.
Bug: 778252
Change-Id: I5bf570d5d44e8681c552a52a8b5ed0c64ce82284
Reviewed-on: https://chromium-review.googlesource.com/738237
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Sometimes I want to get try bot results without checking out the
relevant branch.
R=agable@chromium.org
Bug:
Change-Id: I89d118962ee37b10d95cf4044b61a0f2234b7e60
Reviewed-on: https://chromium-review.googlesource.com/710500
Commit-Queue: Stefan Zager <szager@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
If -m option value starts with "luci.", most probably the user intends
to schedule a build on a LUCI bucket. Print an error message asking
the user to use -B option.
Bug: 749696
Change-Id: If7a038a1f629ee808e3261ae1c753e388bfea893
Reviewed-on: https://chromium-review.googlesource.com/707534
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@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>
This allows a script to generate a sufficient list of owners (and
reviewers) for a given CL. Unlike the owners presubmit check and
the normal, interactive mode of `git-cl owners`, we do not print
the comments associated with an owner for each file, since
presumably a script doesn't care about them (and the comments
aren't in a particularly machine-parsable form, anyway).
This could be used by the WPT import scripts, and presumably
by roll scripts and other such things.
I didn't add a test for this because there appears to be no good
mechanism for testing top-level CMD functions in git_cl that
didn't end up just repeating the code mock-for-mock :(.
R: phajdan.jr@chromium.org
Change-Id: I3a9e6d117d6f4bafe938b07b028f119835c3e575
Reviewed-on: https://chromium-review.googlesource.com/648653
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
This adds support for the new urls (which contain the project) to
git-cl [description|checkout|issue|patch] and others.
Bug: 757602
Change-Id: I6e854a85e655e14daa7e4172ca938953778bf514
Reviewed-on: https://chromium-review.googlesource.com/634518
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This allows us to skip 'git cl format' for histograms.xml, which in
turn allows us to issue a presubmit error rather than a warning when
that file is malformatted, without running the formatting code twice.
The formatter is somewhat slow for this file, so it's helpful to avoid
duplicating that work.
BUG=none
TEST=none
R=iannucci@chromium.org
Change-Id: Ia86d45c7c56a9f89db34ca7175ef686589084f85
Reviewed-on: https://chromium-review.googlesource.com/569105
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@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>
The actual flag is --send-mail. Also includes a fix
to publish these changes if a comment is included.
Bug: 740950
Change-Id: I38ca0a35c1364c8364eb0ef301137c04daede40b
Reviewed-on: https://chromium-review.googlesource.com/572033
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>
There's no reason not to, since it is pretty easy, and other
commands like "git cl patch" already do so.
Bug: 733715
Change-Id: I3396153ef11bfdbbf09b8a7f54f9a5b1047e290f
Reviewed-on: https://chromium-review.googlesource.com/537113
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@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>
The logic for handling --title, --message, and the
commit message of the most recent commit was accidentally
doing this differently from how Rietveld had done them.
Bug: 728391
Change-Id: I70a46ccb470d790103f5d6bb745902595be28eee
Reviewed-on: https://chromium-review.googlesource.com/527339
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>
The private mode is a better fit for how we want private
(e.g. security-critical) changes to be reviewed. Draft
was simply a placeholder until private was ready for use,
which it now is.
Bug: 721880, 722627
Change-Id: Ib7b76c555437f4ddc7ab2b0e7ce5a9f9ee8be825
Reviewed-on: https://chromium-review.googlesource.com/513243
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrew Bonventre <andybons@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>
Many WATCHLIST and codereview.settings files contain
large public email lists, which defeats the purpose
of private reviews.
R=tandrii@chromium.org
Bug: 721880
Change-Id: Ibb3f314f735b783e628f8cc1fc22c74e0df59f1c
Reviewed-on: https://chromium-review.googlesource.com/506489
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Before, git cl creds-check expected `http.cookiefile` of git config
to have path without '~' which git understands on Linux/Mac.
R=machenbach@chromium.org
Bug: 719728
Change-Id: Ica1308b776b62bda7a8b85113c0c0df837c7d933
Reviewed-on: https://chromium-review.googlesource.com/501828
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@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>
Previously, if the branch structure was
origin/master
master [ 3 commits behind origin/master ]
my-branch [ 1 commit ahead of master ]
then git-cl would see that the upstream branch is master,
and decide to diff against origin/master instead. But that
diff would contain the contents of the three commits that
the local master branch hasn't caught up with yet. That's
not what we want.
This CL makes it so that basing on top of local master is
the same as basing on top of remote branches. Just like
when based on top of origin/master, it simply calculates
the merge-base with the parent commit (which is assumed
to be present in the remote), and uploads that diff.
Change-Id: I91caa30f6256a6e019e613dc5ff624734bca027e
Reviewed-on: https://chromium-review.googlesource.com/499307
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This will allow git cl format to handle histograms.xml being
split into several files, and also adds support for UKM.
Bug:699328
Change-Id: I2bfbd333b78b679f69d7cb298f5adaf02f6a4dec
Reviewed-on: https://chromium-review.googlesource.com/483979
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Steven Holte <holte@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>
We want to assert that if add_owners_to is provided, change is also
provided.
Bug:
Change-Id: I0b9674dcd1cdd5e42a8ab92583570ecc02ba1dcd
Reviewed-on: https://chromium-review.googlesource.com/481040
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@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 will be useful for tools like the recipe autoroller to
semi-intelligently choose a collection of reviewers.
Bug:
Change-Id: I7a238127e126340c033fc8e80523ef2408ebf970
Reviewed-on: https://chromium-review.googlesource.com/479679
Commit-Queue: Andrii Shyshkalov <tandrii@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>
The loop over the messages didn't early-exit when it found
a non-CQ message, so it would continue looking until it
found any message by a non-owner. So all CLs with any comments
by anyone other than the uploader would be in 'reply' state
forever.
This CL fixes that, and also makes some other tweaks to
make the gerrit status code simpler and faster.
Bug: 706460
Change-Id: I5cc06962f5121fe042a315e5e2e205e556eb85da
Reviewed-on: https://chromium-review.googlesource.com/470586
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@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>
PolyGerrit used to surround emails with invisible unicode char, which
resulted in reviewers not being found by Gerrit. It has since been
fixed (handled in Google internal bug b/34702620).
R=sergiyb@chromium.org
Change-Id: I43ade0ef23d88611081744eff57129a69b5176c8
Reviewed-on: https://chromium-review.googlesource.com/472808
Reviewed-by: Sergiy Byelozyorov <sergiyb@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 a hack to support the most common git workflow: keeping
a local branch named 'master' which is a replica of 'origin/master'
with no local changes, and basing all feature branches off of
that.
Test:
* created master branch, created feature branch, uploaded CL correctly
* created master branch, landed change on master branch, created
feature branch, uploaded CL, CL contains change on master too
* created non-master branch, created feature branch, failed to
upload CL
Bug: 682104
Change-Id: I8481b787e6dcab162d2846c07f1ddad950f491e0
Reviewed-on: https://chromium-review.googlesource.com/464107
Reviewed-by: Andrew Bonventre <andybons@google.com>
Commit-Queue: Aaron Gable <agable@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>
The issue is with directory separators:
"Check that c:\src\chromium\src or
c:\src\chromium\src\tools/metrics/histograms/pretty_print.py
exist and have execution permission."
BUG=679514
Change-Id: I88a57783ca3bf8aa5af4d83fb34901c2d2385fe9
Reviewed-on: https://chromium-review.googlesource.com/457800
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Pierre-Antoine Manzagol <manzagop@chromium.org>
Note that because it is now a gerrit footer, it both appears in the same block
as the Change-Id footer (no blank line between them), and isn't guaranteed to be
above the Change-Id footer. This doesn't matter during "git cl upload", when
a Change-Id hasn't been allocated yet, but will show up during "git cl
description".
Bug: 681184
Change-Id: I2ab6fc13be8e992709618a666012410b1a7c02de
Reviewed-on: https://chromium-review.googlesource.com/446660
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
These methods are used to produce report in the next CL.
BUG=689543
Change-Id: I71b2705ac8b046103b4982d47f7ec97f8ef7818b
Reviewed-on: https://chromium-review.googlesource.com/455838
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
If the commit or cherry-pick command fails, git automatically drops
the user into a state where they can try to manually fix it. That's
great. But with the old call order, it would also leave the metadata
unset. This sets the issue and patchset number before doing the
potentially-failing heavy lifting, so that the branch will be in a
consistent state after the user finishes fixing up the failed command.
BUG=701130
Change-Id: I792b9fb9e61ba62626c19aa1837d21f8cd8f594e
Reviewed-on: https://chromium-review.googlesource.com/456039
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
There will be many other methods that will need to share state.
BUG=689543
R=machenbach@chromium.org
Change-Id: I4ce6fe173ff7c92e57737417c65125156606a664
Reviewed-on: https://chromium-review.googlesource.com/455780
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
The default BUG_LINE_FORMAT is the existing BUG=%s. Projects that wish
to begin using Gerrit-style footers like Bug: %s can now set this in
codereview.settings.
BUG=616753
Change-Id: I4470311a86db228eab2a1655ae884736cce8c380
Reviewed-on: https://chromium-review.googlesource.com/451565
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
When git-cl doesn’t see a BUG= line, it introduces a new one. The
eventual goal is to switch away from THIS=TYPE of metadata to the footer
field style that’s more natural to git and, most importantly, Gerrit.
With this change, git-cl will treat an existing Bug: line identically to
a BUG= line, and not introduce a new BUG= line if either are present.
This stops short of switching the default format to Bug:, but it allows
projects that wish to switch to the new format now to not be pestered by
git-cl introducing BUG= every time it edits a change description.
This is similar to the bug line parsing done by Bugdroid, which already
undersatnds the Bug: format. See
https://chromium.googlesource.com/infra/infra/+/ba095768d776/infra/services/bugdroid/log_parser.py.
BUG=616753
Change-Id: I23faa55c8275de44b71b031e603f2f55778d7f25
Reviewed-on: https://chromium-review.googlesource.com/451123
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This makes them accessible to presubmit scripts' PostUploadHook.
Fixed bugs where caching needed to be bypassed in order for
sequential PostUploadHooks to see each others' results.
BUG=688765
Change-Id: I56c0c6b6419e2474f4b7f701be036fb2a524f8e3
Reviewed-on: https://chromium-review.googlesource.com/439877
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
(edited, so that next next line gets u200 char
between R= and reviewer email)
R=mstarzinger@chromium.org
TBR=agable@chromium.org
BUG=b/34702620 (internal)
Change-Id: I58f2528dd28e3a43d8eb2d7d4f5c046460d8a9ee
Reviewed-on: https://chromium-review.googlesource.com/440085
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Also fixes git cl description <url> by propagating codereview host information
in all cases.
BUG=681704
R=agable@chromium.org
Change-Id: Ia79c10b19d72b5a8797a1428ad8a79c8f4480901
Reviewed-on: https://chromium-review.googlesource.com/431036
Reviewed-by: Aaron Gable <agable@chromium.org>
This CL moves JS formatting behind the --js flag for git cl format
due to issue with LayoutTests. It also adds an option to check
javascript for the presubmit canned formatting check.
BUG=567770
Change-Id: I9c080b1136f6ffef9fb1b08d3bfc97ce5b3185dc
Reviewed-on: https://chromium-review.googlesource.com/430526
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Christopher Lam <calamity@chromium.org>
Robert Iannucci
Andrii Shyshkalov
Nodir Turakulov
Aaron Gable
Edward Lemur
Bruce Dawson
Koji Ishii
Lei Zhang
Chris Watkins
Quinten Yearsley
Daniel Cheng
Gabriel Charette
Nick Carter
Dominic Battre
Stefan Zager
Yoshisato Yanagisawa
Dirk Pranke
Ilya Sherman
Anna Henningsen
Quinten Yearsley
Francois Doray
Steven Holte
Jochen Eisinger
Pierre-Antoine Manzagol
Mark Mentovai
Alexei Svitkine
Kenneth Russell
Christopher Lam