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

@@ -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

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)

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.
+option, or it can be 
-In that case local branch will get -patch[patchsetNumber] suffix.
+.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