From 03d7758ada7017b44e243a5e008ed89f3d9f1a05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20Sedl=C3=A1k?= <psedlak@redhat.com>
Date: Mon, 18 Feb 2013 11:14:00 +0100
Subject: [PATCH] Compare different Patch Sets of Review
Adds ability to fetch/checkout branches and launch git diff
for different review patchsets with one command.
Change-Id: I5815eb0cb6025978a39d74ddb31eb179f06154de
---
README.md | 4 +++
git-review | 90 ++++++++++++++++++++++++++++++++++++++++++++++++----
git-review.1 | 45 ++++++++++++++++++++++++--
3 files changed, 129 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index 3f35064..538a872 100644
--- a/README.md
+++ b/README.md
@@ -53,6 +53,10 @@ If you want to download patchset 4 for change 781 from gerrit to review it::
git review -d 781,4
+If you want to compare patchset 4 with patchset 10 of change 781 from gerrit::
+
+ git review -m 781,4-10
+
If you just want to do the commit message and remote setup steps::
git review -s
diff --git a/git-review b/git-review
index 05a1bea..8e758ee 100755
--- a/git-review
+++ b/git-review
@@ -92,6 +92,19 @@ class ChangeSetException(GitReviewException):
return self.__doc__ % self.e
+def parse_review_number(review):
+ parts = review.split(',')
+ if len(parts) < 2:
+ parts.append(None)
+ return parts
+
+
+def build_review_number(review, patchset):
+ if patchset is not None:
+ return '%s,%s' % (review, patchset)
+ return review
+
+
def run_command_status(*argv, **env):
if VERBOSE:
print(datetime.datetime.now(), "Running:", " ".join(argv))
@@ -442,18 +455,23 @@ def check_remote(branch, remote, hostname, port, project):
raise
-def rebase_changes(branch, remote):
+def rebase_changes(branch, remote, interactive=True):
remote_branch = "remotes/%s/%s" % (remote, branch)
if not update_remote(remote):
return False
- cmd = "git rebase -i %s" % remote_branch
+ if interactive:
+ cmd = "git rebase -i %s" % remote_branch
+ else:
+ cmd = "git rebase %s" % remote_branch
+
(status, output) = run_command_status(cmd, GIT_EDITOR='true')
if status != 0:
print("Errors running %s" % cmd)
- print(output)
+ if interactive:
+ print(output)
return False
return True
@@ -716,10 +734,10 @@ def fetch_review(review, masterbranch, remote):
userhost = "%s@%s" % (username, hostname)
review_arg = review
- patchset_number = None
patchset_opt = '--current-patch-set'
- if ',' in review:
- review, patchset_number = review.split(',')
+
+ review, patchset_number = parse_review_number(review)
+ if patchset_number is not None:
patchset_opt = '--patch-sets'
review_info = None
@@ -823,6 +841,52 @@ class DeleteBranchFailed(CommandFailed):
EXIT_CODE = 68
+class InvalidPatchsetsToCompare(GitReviewException):
+ def __init__(self, patchsetA, patchsetB):
+ Exception.__init__(
+ self,
+ "Invalid patchsets for comparison specified (old=%s,new=%s)" % (
+ patchsetA,
+ patchsetB))
+ EXIT_CODE = 39
+
+
+def compare_review(review_spec, branch, remote, rebase=False):
+ new_ps = None # none means latest
+
+ if '-' in review_spec:
+ review_spec, new_ps = review_spec.split('-')
+ review, old_ps = parse_review_number(review_spec)
+
+ if old_ps is None or old_ps == new_ps:
+ raise InvalidPatchsetsToCompare(old_ps, new_ps)
+
+ old_review = build_review_number(review, old_ps)
+ new_review = build_review_number(review, new_ps)
+
+ old_branch = fetch_review(old_review, branch, remote)
+ checkout_review(old_branch)
+
+ if rebase:
+ print('Rebasing %s' % old_branch)
+ rebase = rebase_changes(branch, remote, False)
+ if not rebase:
+ print('Skipping rebase because of conflicts')
+ run_command_exc(CommandFailed, 'git', 'rebase', '--abort')
+
+ new_branch = fetch_review(new_review, branch, remote)
+ checkout_review(new_branch)
+
+ if rebase:
+ print('Rebasing also %s' % new_branch)
+ if not rebase_changes(branch, remote, False):
+ print("Rebasing of the new branch failed, "
+ "diff can be messed up (use -R to not rebase at all)!")
+ run_command_exc(CommandFailed, 'git', 'rebase', '--abort')
+
+ subprocess.check_call(['git', 'diff', old_branch])
+
+
def finish_branch(target_branch):
local_branch = get_branch_name(target_branch)
if VERBOSE:
@@ -896,7 +960,7 @@ def main():
help="Force rebase even when not needed.")
fetch = parser.add_mutually_exclusive_group()
- fetch.set_defaults(download=False, cherrypickcommit=False,
+ fetch.set_defaults(download=False, compare=False, cherrypickcommit=False,
cherrypickindicate=False, cherrypickonly=False)
fetch.add_argument("-d", "--download", dest="changeidentifier",
action=DownloadFlag, metavar="CHANGE",
@@ -922,6 +986,14 @@ def main():
help="Apply the contents of an existing gerrit "
"review to the working directory and prepare "
"for commit")
+ fetch.add_argument("-m", "--compare", dest="changeidentifier",
+ action=DownloadFlag, metavar="CHANGE,PS[-NEW_PS]",
+ const="compare",
+ help="Download specified and latest (or NEW_PS) "
+ "patchsets of an existing gerrit review into "
+ "a branches, rebase on master "
+ "(skipped on conflicts or when -R is specified) "
+ "and show their differences")
parser.add_argument("-u", "--update", dest="update", action="store_true",
help="Force updates from remote locations")
@@ -977,6 +1049,10 @@ def main():
config['hostname'], config['port'], config['project'])
if options.changeidentifier:
+ if options.compare:
+ compare_review(options.changeidentifier,
+ branch, remote, options.rebase)
+ return
local_branch = fetch_review(options.changeidentifier, branch, remote)
if options.download:
checkout_review(local_branch)
diff --git a/git-review.1 b/git-review.1
index b2d381d..0a1e8b5 100644
--- a/git-review.1
+++ b/git-review.1
@@ -28,6 +28,12 @@
.Op Ar branch
.Nm
.Op Fl r Ar remote
+.Op Fl uv
+.Fl m
+.Ar change-ps-range
+.Op Ar branch
+.Nm
+.Op Fl r Ar remote
.Op Fl fnuv
.Fl s
.Op Ar branch
@@ -47,10 +53,14 @@ users that have recently switched to Git from another version
control system.
.Pp
.Ar change
-can be changeNumber as obtained using
+can be
+.Ar changeNumber
+as obtained using
.Fl --list
-option, or it can be changeNumber,patchsetNumber for fetching exact patchset from the change.
-In that case local branch will get -patch[patchsetNumber] suffix.
+option, or it can be
+.Ar changeNumber,patchsetNumber
+for fetching exact patchset from the change.
+In that case local branch name will have a -patch[patchsetNumber] suffix.
.Pp
The following options are available:
.Bl -tag -width indent
@@ -93,6 +103,28 @@ creating a local branch for it.
.Pp
If the current branch is different enough, the change may not apply at all
or produce merge conflicts that need to be resolved by hand.
+.It Fl m Ar change-ps-range , Fl -compare= Ns Ar change-ps-range
+Download the specified patchsets for
+.Ar change
+from Gerrit, rebase both on master and display differences (git-diff).
+.Pp
+.Ar change-ps-range
+can be specified as
+.Ar changeNumber, Ns Ar oldPatchSetNumber Ns Op Ns Ar -newPatchSetNumber
+.Pp
+.Ar oldPatchSetNumber
+is mandatory, and if
+.Ar newPatchSetNumber
+is not specified, the latest patchset will be used.
+.Pp
+This makes it possible to easily compare what has changed from last time you
+reviewed the proposed change.
+.Pp
+If the master branch is different enough, the rebase can produce merge conflicts.
+If that happens rebasing will be aborted and diff displayed for not-rebased branches.
+You can also use
+.Ar --no-rebase ( Ar -R )
+to always skip rebasing.
.It Fl f , Fl -finish
Close down the local branch and switch back to the target branch on
successful submission.
@@ -122,6 +154,10 @@ Do not automatically perform a rebase before submitting the change to
Gerrit.
.Pp
When submitting a change for review, you will usually want it to be based on the tip of upstream branch in order to avoid possible conflicts. When amending a change and rebasing the new patchset, the Gerrit web interface will show a difference between the two patchsets which contains all commits in between. This may confuse many reviewers that would expect to see a much simpler difference.
+.Pp
+Also can be used for
+.Fl --compare
+to skip automatic rebase of fetched reviews.
.It Fl -version
Print the version number and exit.
.El
@@ -205,6 +241,8 @@ Changeset not found.
Particular patchset cannot be fetched from the remote git repository.
.It 38
Specified patchset number not found in the changeset.
+.It 39
+Invalid patchsets for comparison.
.It 64
Cannot checkout downloaded patchset into the new branch.
.It 65
@@ -300,3 +338,4 @@ is maintained by
This manpage has been enhanced by:
.An "Antoine Musso" Aq hashar@free.fr
.An "Marcin Cieslak" Aq saper@saper.info
+.An "Pavel Sedlák" Aq psedlak@redhat.com