diff --git a/git_drover.py b/git_drover.py index 73c92004aa..9b4f6232e1 100755 --- a/git_drover.py +++ b/git_drover.py @@ -5,6 +5,7 @@ """git drover: A tool for merging changes to release branches.""" import argparse +import cPickle import functools import logging import os @@ -20,6 +21,28 @@ class Error(Exception): pass +_PATCH_ERROR_MESSAGE = """Patch failed to apply. + +A workdir for this cherry-pick has been created in + {0} + +To continue, resolve the conflicts there and run + git drover --continue {0} + +To abort this cherry-pick run + git drover --abort {0} +""" + + +class PatchError(Error): + """An error indicating that the patch failed to apply.""" + + def __init__(self, workdir): + super(PatchError, self).__init__(_PATCH_ERROR_MESSAGE.format(workdir)) + + +_DEV_NULL_FILE = open(os.devnull, 'w') + if os.name == 'nt': # This is a just-good-enough emulation of os.symlink for drover to work on # Windows. It uses junctioning of directories (most of the contents of @@ -43,7 +66,7 @@ else: class _Drover(object): - def __init__(self, branch, revision, parent_repo, dry_run): + def __init__(self, branch, revision, parent_repo, dry_run, verbose): self._branch = branch self._branch_ref = 'refs/remotes/branch-heads/%s' % branch self._revision = revision @@ -51,7 +74,60 @@ class _Drover(object): self._dry_run = dry_run self._workdir = None self._branch_name = None - self._dev_null_file = open(os.devnull, 'w') + self._needs_cleanup = True + self._verbose = verbose + self._process_options() + + def _process_options(self): + if self._verbose: + logging.getLogger().setLevel(logging.DEBUG) + + + @classmethod + def resume(cls, workdir): + """Continues a cherry-pick that required manual resolution. + + Args: + workdir: A string containing the path to the workdir used by drover. + """ + drover = cls._restore_drover(workdir) + drover._continue() + + @classmethod + def abort(cls, workdir): + """Aborts a cherry-pick that required manual resolution. + + Args: + workdir: A string containing the path to the workdir used by drover. + """ + drover = cls._restore_drover(workdir) + drover._cleanup() + + @staticmethod + def _restore_drover(workdir): + """Restores a saved drover state contained within a workdir. + + Args: + workdir: A string containing the path to the workdir used by drover. + """ + try: + with open(os.path.join(workdir, '.git', 'drover'), 'rb') as f: + drover = cPickle.load(f) + drover._process_options() + return drover + except (IOError, cPickle.UnpicklingError): + raise Error('%r is not git drover workdir' % workdir) + + def _continue(self): + if os.path.exists(os.path.join(self._workdir, '.git', 'CHERRY_PICK_HEAD')): + self._run_git_command( + ['commit', '--no-edit'], + error_message='All conflicts must be resolved before continuing') + + if self._upload_and_land(): + # Only clean up the workdir on success. The manually resolved cherry-pick + # can be reused if the user cancels before landing. + self._cleanup() def run(self): """Runs this Drover instance. @@ -70,35 +146,29 @@ class _Drover(object): self._run_git_command(['show', '-s', self._revision]), self._branch)): return self._create_checkout() - self._prepare_cherry_pick() - if self._dry_run: - logging.info('--dry_run enabled; not landing.') - return - - self._run_git_command(['cl', 'upload'], - error_message='Upload failed', - interactive=True) + self._perform_cherry_pick() + self._upload_and_land() - if not self._confirm('About to land on %s.' % self._branch): + def _cleanup(self): + if not self._needs_cleanup: return - self._run_git_command(['cl', 'land', '--bypass-hooks'], interactive=True) - def _cleanup(self): - if self._branch_name: - try: - self._run_git_command(['cherry-pick', '--abort']) - except Error: - pass - self._run_git_command(['checkout', '--detach']) - self._run_git_command(['branch', '-D', self._branch_name]) if self._workdir: logging.debug('Deleting %s', self._workdir) if os.name == 'nt': - # Use rmdir to properly handle the junctions we created. - subprocess.check_call(['rmdir', '/s', '/q', self._workdir], shell=True) + try: + # Use rmdir to properly handle the junctions we created. + subprocess.check_call( + ['rmdir', '/s', '/q', self._workdir], shell=True) + except subprocess.CalledProcessError: + logging.error( + 'Failed to delete workdir %r. Please remove it manually.', + self._workdir) else: shutil.rmtree(self._workdir) - self._dev_null_file.close() + self._workdir = None + if self._branch_name: + self._run_git_command(['branch', '-D', self._branch_name]) @staticmethod def _confirm(message): @@ -166,25 +236,63 @@ class _Drover(object): self.FILES_TO_COPY, mk_symlink) self._run_git_command(['config', 'core.sparsecheckout', 'true']) with open(os.path.join(git_dir, 'info', 'sparse-checkout'), 'w') as f: - f.write('codereview.settings') + f.write('/codereview.settings') branch_name = os.path.split(self._workdir)[-1] self._run_git_command(['checkout', '-b', branch_name, self._branch_ref]) self._branch_name = branch_name - def _prepare_cherry_pick(self): - self._run_git_command(['cherry-pick', '-x', self._revision], - error_message='Patch failed to apply') + def _perform_cherry_pick(self): + try: + self._run_git_command(['cherry-pick', '-x', self._revision], + error_message='Patch failed to apply') + except Error: + self._prepare_manual_resolve() + self._save_state() + self._needs_cleanup = False + raise PatchError(self._workdir) + + def _save_state(self): + """Saves the state of this Drover instances to the workdir.""" + with open(os.path.join(self._workdir, '.git', 'drover'), 'wb') as f: + cPickle.dump(self, f) + + def _prepare_manual_resolve(self): + """Prepare the workdir for the user to manually resolve the cherry-pick.""" + # Files that have been deleted between branch and cherry-pick will not have + # their skip-worktree bit set so set it manually for those files to avoid + # git status incorrectly listing them as unstaged deletes. + repo_status = self._run_git_command(['status', '--porcelain']).splitlines() + extra_files = [f[3:] for f in repo_status if f[:2] == ' D'] + if extra_files: + self._run_git_command(['update-index', '--skip-worktree', '--'] + + extra_files) + + def _upload_and_land(self): + if self._dry_run: + logging.info('--dry_run enabled; not landing.') + return True + self._run_git_command(['reset', '--hard']) + self._run_git_command(['cl', 'upload'], + error_message='Upload failed', + interactive=True) + + if not self._confirm('About to land on %s.' % self._branch): + return False + self._run_git_command(['cl', 'land', '--bypass-hooks'], interactive=True) + return True def _run_git_command(self, args, error_message=None, interactive=False): """Runs a git command. Args: args: A list of strings containing the args to pass to git. - interactive: error_message: A string containing the error message to report if the command fails. + interactive: A bool containing whether the command requires user + interaction. If false, the command will be provided with no input and + the output is captured. Raises: Error: The command failed to complete successfully. @@ -195,11 +303,11 @@ class _Drover(object): run = subprocess.check_call if interactive else subprocess.check_output + # Discard stderr unless verbose is enabled. + stderr = None if self._verbose else _DEV_NULL_FILE + try: - return run(['git'] + args, - shell=False, - cwd=cwd, - stderr=self._dev_null_file) + return run(['git'] + args, shell=False, cwd=cwd, stderr=stderr) except (OSError, subprocess.CalledProcessError) as e: if error_message: raise Error(error_message) @@ -207,7 +315,7 @@ class _Drover(object): raise Error('Command %r failed: %s' % (' '.join(args), e)) -def cherry_pick_change(branch, revision, parent_repo, dry_run): +def cherry_pick_change(branch, revision, parent_repo, dry_run, verbose=False): """Cherry-picks a change into a branch. Args: @@ -218,31 +326,64 @@ def cherry_pick_change(branch, revision, parent_repo, dry_run): revision. parent_repo: A string containing the path to the parent repo to use for this cherry-pick. - dry_run: A boolean containing whether to stop before uploading the + dry_run: A bool containing whether to stop before uploading the cherry-pick cl. + verbose: A bool containing whether to print verbose logging. Raises: Error: An error occurred while attempting to cherry-pick |cl| to |branch|. """ - drover = _Drover(branch, revision, parent_repo, dry_run) + drover = _Drover(branch, revision, parent_repo, dry_run, verbose) drover.run() +def continue_cherry_pick(workdir): + """Continues a cherry-pick that required manual resolution. + + Args: + workdir: A string containing the path to the workdir used by drover. + """ + _Drover.resume(workdir) + + +def abort_cherry_pick(workdir): + """Aborts a cherry-pick that required manual resolution. + + Args: + workdir: A string containing the path to the workdir used by drover. + """ + _Drover.abort(workdir) + + def main(): parser = argparse.ArgumentParser( description='Cherry-pick a change into a release branch.') + group = parser.add_mutually_exclusive_group(required=True) parser.add_argument( '--branch', type=str, - required=True, metavar='', help='the name of the branch to which to cherry-pick; e.g. 1234') - parser.add_argument('--cherry-pick', - type=str, - required=True, - metavar='', - help=('the change to cherry-pick; this can be any string ' - 'that unambiguously refers to a revision')) + group.add_argument( + '--cherry-pick', + type=str, + metavar='', + help=('the change to cherry-pick; this can be any string ' + 'that unambiguously refers to a revision not involving HEAD')) + group.add_argument( + '--continue', + type=str, + nargs='?', + dest='resume', + const=os.path.abspath('.'), + metavar='path_to_workdir', + help='Continue a drover cherry-pick after resolving conflicts') + group.add_argument('--abort', + type=str, + nargs='?', + const=os.path.abspath('.'), + metavar='path_to_workdir', + help='Abort a drover cherry-pick') parser.add_argument( '--parent_checkout', type=str, @@ -263,13 +404,19 @@ def main(): default=False, help='show verbose logging') options = parser.parse_args() - if options.verbose: - logging.getLogger().setLevel(logging.DEBUG) try: - cherry_pick_change(options.branch, options.cherry_pick, - options.parent_checkout, options.dry_run) + if options.resume: + _Drover.resume(options.resume) + elif options.abort: + _Drover.abort(options.abort) + else: + if not options.branch: + parser.error('argument --branch is required for --cherry-pick') + cherry_pick_change(options.branch, options.cherry_pick, + options.parent_checkout, options.dry_run, + options.verbose) except Error as e: - logging.error(e.message) + print 'Error:', e.message sys.exit(128) diff --git a/man/html/git-drover.html b/man/html/git-drover.html index 3171d2aeb4..ebe03457fa 100644 --- a/man/html/git-drover.html +++ b/man/html/git-drover.html @@ -755,7 +755,9 @@ git-drover(1) Manual Page

SYNOPSIS

-
git drover --branch <branch> --cherry-pick <commit>
+
git drover --branch <branch>
+           (--cherry-pick <change> | --continue [path_to_workdir] |
+            --abort [path_to_workdir])
            [--parent_checkout <path-to-existing-checkout>]
            [--verbose] [--dry-run]
@@ -767,8 +769,7 @@ git-drover(1) Manual Page

git drover applies a commit to a release branch. It creates a new workdir from an existing checkout to avoid downloading a new checkout without affecting the -existing checkout. Creating a workdir requires symlinks so this does not work on -Windows. See the EXAMPLE section for the equivalent sequence of commands to run.

+existing checkout.

git drover does not support reverts. See the EXAMPLE section for the equivalent sequence of commands to run.

@@ -794,6 +795,25 @@ equivalent sequence of commands to run.

+--continue [path_to_workdir] +
+
+

+ Continue a cherry-pick that required manual resolution. The path to the drover + workdir is optional. If unspecified, the current directory is used. +

+
+
+--abort [path_to_workdir] +
+
+

+ Abort a cherry-pick that required manual resolution and clean up its workdir. + The path to the drover workdir is optional. If unspecified, the current + directory is used. +

+
+
--parent_checkout
@@ -838,7 +858,37 @@ at least once to fetch the branches.

Merge Example

# Here's a commit (from some.committer) that we want to 'drover'.
 $ git log -n 1 --pretty=fuller
-commit 8b79b7b2f7e6e728f9a3c7b385c72efc7c47244a
+commit f7448045de01b54914db8b902ca77fbbf42b3146
+Author:     some.committer <some.committer@chromium.org>
+AuthorDate: Thu Apr 10 08:54:46 2014 +0000
+Commit:     some.committer <some.committer@chromium.org>
+CommitDate: Thu Apr 10 08:54:46 2014 +0000
+
+    This change needs to go to branch 9999
+
+# Now do the 'drover'.
+$ git drover --branch 9999 --cherry-pick f7448045de01b54914db8b902ca77fbbf42b3146
+Going to cherry-pick
+"""
+commit f7448045de01b54914db8b902ca77fbbf42b3146
+Author: some.committer <some.committer@chromium.org>
+Date:   Thu Apr 10 08:54:46 2014 +0000
+
+    This change needs to go to branch 9999
+"""
+to 9999. Continue (y/n)? y
+
+# A cl is uploaded to rietveld, where it can be reviewed before landing.
+
+About to land on 9999. Continue (y/n)? y
+# The cherry-pick cl is landed on the branch 9999.
+

+ +
+

Merge with Conflicts Example

+

# Here's a commit (from some.committer) that we want to 'drover'.
+$ git log -n 1 --pretty=fuller
+commit ca8e437616d853cb10008a252b54cfed928f157c
 Author:     some.committer <some.committer@chromium.org>
 AuthorDate: Thu Apr 10 08:54:46 2014 +0000
 Commit:     some.committer <some.committer@chromium.org>
@@ -847,10 +897,10 @@ CommitDate: Thu Apr 10 08:54:46 2014 +0000
     This change needs to go to branch 9999
 
 # Now do the 'drover'.
-$ git drover --branch 9999 --cherry-pick 8b79b7b2f7e6e728f9a3c7b385c72efc7c47244a
+$ git drover --branch 9999 --cherry-pick ca8e437616d853cb10008a252b54cfed928f157c
 Going to cherry-pick
 """
-commit 8b79b7b2f7e6e728f9a3c7b385c72efc7c47244a
+commit ca8e437616d853cb10008a252b54cfed928f157c
 Author: some.committer <some.committer@chromium.org>
 Date:   Thu Apr 10 08:54:46 2014 +0000
 
@@ -858,6 +908,23 @@ Date:   Thu Apr 10 08:54:46 2014 +0000
 """
 to 9999. Continue (y/n)? y
 
+Error: Patch failed to apply.
+
+A workdir for this cherry-pick has been created in
+  /tmp/drover_9999
+
+To continue, resolve the conflicts there and run
+  git drover --continue /tmp/drover_9999
+
+To abort this cherry-pick run
+  git drover --abort /tmp/drover_9999
+
+$ pushd /tmp/drover_9999
+# Manually resolve conflicts.
+$ git add path/to/file_with_conflicts
+$ popd
+$ git drover --continue /tmp/drover_9999
+
 # A cl is uploaded to rietveld, where it can be reviewed before landing.
 
 About to land on 9999. Continue (y/n)? y
@@ -875,24 +942,24 @@ Branch drover_9999 set up to track remote ref refs/branch-heads/9999.
 
 # Here's the commit we want to revert.
 $ git log -n 1
-commit 33b0e9164d4564eb8a4b4e5b951bba6edeeecacb
+commit 98d544a18e19bb80be9d4a8094efda8ab1d2534b
 Author: some.committer <some.committer@chromium.org>
 Date:   Thu Apr 10 08:54:46 2014 +0000
 
     This change is horribly broken.
 
 # Now do the revert.
-$ git revert 33b0e9164d4564eb8a4b4e5b951bba6edeeecacb
+$ git revert 98d544a18e19bb80be9d4a8094efda8ab1d2534b
 
 # That reverted the change and committed the revert.
 $ git log -n 1
-commit 8a2d2bb98b9cfc9260a9bc86da1eec2a43f43f8b
+commit 0fc5e9101886dcb1aebbb9434d0df9341f0dedfe
 Author: you <you@chromium.org>
 Date:   Thu Apr 10 09:11:36 2014 +0000
 
     Revert "This change is horribly broken."
 
-    This reverts commit 33b0e9164d4564eb8a4b4e5b951bba6edeeecacb.
+    This reverts commit 98d544a18e19bb80be9d4a8094efda8ab1d2534b.
 
 # As with old drover, reverts are generally OK to commit without LGTM.
 $ git cl upload -r some.committer@chromium.org --send-mail
@@ -906,7 +973,7 @@ Date:   Thu Apr 10 09:11:36 2014 +0000
 
 # Here's a commit (from some.committer) that we want to 'drover'.
 $ git log -n 1 --pretty=fuller
-commit 537f446fa3d5e41acab017bb0b082fbd0c9eb043
+commit b27fff7b167964750303f60222e79f3932f438e0
 Author:     some.committer <some.committer@chromium.org>
 AuthorDate: Thu Apr 10 08:54:46 2014 +0000
 Commit:     some.committer <some.committer@chromium.org>
@@ -920,8 +987,8 @@ Branch drover_9999 set up to track remote ref refs/branch-heads/9999.
 
 # Now do the 'drover'.
 # IMPORTANT!!! Do Not leave off the '-x' flag
-$ git cherry-pick -x 537f446fa3d5e41acab017bb0b082fbd0c9eb043
-[drover_9999 b468abc] This change needs to go to branch 9999
+$ git cherry-pick -x b27fff7b167964750303f60222e79f3932f438e0
+[drover_9999 bd8dc1c] This change needs to go to branch 9999
  Author: some.committer <some.committer@chromium.org>
  Date: Thu Apr 10 08:54:46 2014 +0000
  1 file changed, 1 insertion(+)
@@ -930,7 +997,7 @@ Branch drover_9999 set up to track remote ref refs/branch-heads/9999.
 # That took the code authored by some.committer and committed it to
 # the branch by the person who drovered it (i.e. you).
 $ git log -n 1 --pretty=fuller
-commit b468abc42ddd4fd9aecc48c3eda172265306d2b4
+commit bd8dc1c0678913e36b2b06855810803a2e0ab906
 Author:     some.committer <some.committer@chromium.org>
 AuthorDate: Thu Apr 10 08:54:46 2014 +0000
 Commit:     you <you@chromium.org>
@@ -938,7 +1005,7 @@ CommitDate: Thu Apr 10 09:11:36 2014 +0000
 
     This change needs to go to branch 9999
 
-    (cherry picked from commit 537f446fa3d5e41acab017bb0b082fbd0c9eb043)
+    (cherry picked from commit b27fff7b167964750303f60222e79f3932f438e0)
 
 # Looks good. Ship it!
 $ git cl upload
@@ -968,7 +1035,7 @@ from 
 

diff --git a/man/man1/git-drover.1 b/man/man1/git-drover.1 index 2041fa18ec..82744a2dd1 100644 --- a/man/man1/git-drover.1 +++ b/man/man1/git-drover.1 @@ -2,12 +2,12 @@ .\" Title: git-drover .\" Author: [FIXME: author] [see http://docbook.sf.net/el/author] .\" Generator: DocBook XSL Stylesheets v1.78.1 -.\" Date: 09/23/2015 +.\" Date: 10/20/2015 .\" Manual: Chromium depot_tools Manual -.\" Source: depot_tools 4549a59 +.\" Source: depot_tools 704d890 .\" Language: English .\" -.TH "GIT\-DROVER" "1" "09/23/2015" "depot_tools 4549a59" "Chromium depot_tools Manual" +.TH "GIT\-DROVER" "1" "10/20/2015" "depot_tools 704d890" "Chromium depot_tools Manual" .\" ----------------------------------------------------------------- .\" * Define some portability stuff .\" ----------------------------------------------------------------- @@ -32,14 +32,16 @@ git-drover \- Apply a commit from the trunk to a release branch, or from one rel .SH "SYNOPSIS" .sp .nf -\fIgit drover\fR \-\-branch \-\-cherry\-pick +\fIgit drover\fR \-\-branch + (\-\-cherry\-pick | \-\-continue [path_to_workdir] | + \-\-abort [path_to_workdir]) [\-\-parent_checkout ] [\-\-verbose] [\-\-dry\-run] .fi .sp .SH "DESCRIPTION" .sp -git drover applies a commit to a release branch\&. It creates a new workdir from an existing checkout to avoid downloading a new checkout without affecting the existing checkout\&. Creating a workdir requires symlinks so this does not work on Windows\&. See the EXAMPLE section for the equivalent sequence of commands to run\&. +git drover applies a commit to a release branch\&. It creates a new workdir from an existing checkout to avoid downloading a new checkout without affecting the existing checkout\&. .sp git drover does not support reverts\&. See the EXAMPLE section for the equivalent sequence of commands to run\&. .SH "OPTIONS" @@ -54,6 +56,16 @@ The branch to cherry\-pick the commit to\&. The commit to cherry\-pick\&. .RE .PP +\-\-continue [path_to_workdir] +.RS 4 +Continue a cherry\-pick that required manual resolution\&. The path to the drover workdir is optional\&. If unspecified, the current directory is used\&. +.RE +.PP +\-\-abort [path_to_workdir] +.RS 4 +Abort a cherry\-pick that required manual resolution and clean up its workdir\&. The path to the drover workdir is optional\&. If unspecified, the current directory is used\&. +.RE +.PP \-\-parent_checkout .RS 4 The path to the chromium checkout to use as the source for a creating git\-new\-workdir workdir to use for cherry\-picking\&. If unspecified, the current directory is used\&. @@ -89,7 +101,54 @@ Before working with branches, you must \fIgclient sync \-\-with_branch_heads\fR .nf # Here\*(Aqs a commit (from some\&.committer) that we want to \*(Aqdrover\*(Aq\&. \fB$ git log \-n 1 \-\-pretty=fuller\fR -commit b5a049e34297f22a4ea63567b32e3290bb3f244c +commit 9b111fcda69cb6bf8a38d1e77867c298d80ca9d1 +Author: some\&.committer +AuthorDate: Thu Apr 10 08:54:46 2014 +0000 +Commit: some\&.committer +CommitDate: Thu Apr 10 08:54:46 2014 +0000 + + This change needs to go to branch 9999 + +# Now do the \*(Aqdrover\*(Aq\&. +\fB$ git drover \-\-branch 9999 \-\-cherry\-pick 9b111fcda69cb6bf8a38d1e77867c298d80ca9d1\fR +Going to cherry\-pick +""" +commit 9b111fcda69cb6bf8a38d1e77867c298d80ca9d1 +Author: some\&.committer +Date: Thu Apr 10 08:54:46 2014 +0000 + + This change needs to go to branch 9999 +""" +to 9999\&. Continue (y/n)? y + +# A cl is uploaded to rietveld, where it can be reviewed before landing\&. + +About to land on 9999\&. Continue (y/n)? y +# The cherry\-pick cl is landed on the branch 9999\&. +.fi +.if n \{\ +.RE +.\} +.sp +.RE +.sp +.it 1 an-trap +.nr an-no-space-flag 1 +.nr an-break-flag 1 +.br +.ps +1 +\fBMerge with Conflicts Example\fR +.RS 4 +.sp + +.sp +.if n \{\ +.RS 4 +.\} +.nf +# Here\*(Aqs a commit (from some\&.committer) that we want to \*(Aqdrover\*(Aq\&. +\fB$ git log \-n 1 \-\-pretty=fuller\fR +commit 89ce5bb7bdf7149754b94e4d9fc5413435e6680d Author: some\&.committer AuthorDate: Thu Apr 10 08:54:46 2014 +0000 Commit: some\&.committer @@ -98,10 +157,10 @@ CommitDate: Thu Apr 10 08:54:46 2014 +0000 This change needs to go to branch 9999 # Now do the \*(Aqdrover\*(Aq\&. -\fB$ git drover \-\-branch 9999 \-\-cherry\-pick b5a049e34297f22a4ea63567b32e3290bb3f244c\fR +\fB$ git drover \-\-branch 9999 \-\-cherry\-pick 89ce5bb7bdf7149754b94e4d9fc5413435e6680d\fR Going to cherry\-pick """ -commit b5a049e34297f22a4ea63567b32e3290bb3f244c +commit 89ce5bb7bdf7149754b94e4d9fc5413435e6680d Author: some\&.committer Date: Thu Apr 10 08:54:46 2014 +0000 @@ -109,6 +168,23 @@ Date: Thu Apr 10 08:54:46 2014 +0000 """ to 9999\&. Continue (y/n)? y +Error: Patch failed to apply\&. + +A workdir for this cherry\-pick has been created in + /tmp/drover_9999 + +To continue, resolve the conflicts there and run + git drover \-\-continue /tmp/drover_9999 + +To abort this cherry\-pick run + git drover \-\-abort /tmp/drover_9999 + +\fB$ pushd /tmp/drover_9999\fR +# Manually resolve conflicts\&. +\fB$ git add path/to/file_with_conflicts\fR +\fB$ popd\fR +\fB$ git drover \-\-continue /tmp/drover_9999\fR + # A cl is uploaded to rietveld, where it can be reviewed before landing\&. About to land on 9999\&. Continue (y/n)? y @@ -143,24 +219,24 @@ Branch drover_9999 set up to track remote ref refs/branch\-heads/9999\&. # Here\*(Aqs the commit we want to revert\&. \fB$ git log \-n 1\fR -commit 215689406a8ca5813412becc6258509be903db59 +commit d5efb50dcc76ea29ad30d1015fb291a9ba5fc0db Author: some\&.committer Date: Thu Apr 10 08:54:46 2014 +0000 This change is horribly broken\&. # Now do the revert\&. -\fB$ git revert 215689406a8ca5813412becc6258509be903db59\fR +\fB$ git revert d5efb50dcc76ea29ad30d1015fb291a9ba5fc0db\fR # That reverted the change and committed the revert\&. \fB$ git log \-n 1\fR -commit 1efaf0e8b1c6c6afadfb37e15023b52b960ac2fd +commit 1b6ccf84e2c13c6f6a2b69b29626aed8fa0ae24d Author: you Date: Thu Apr 10 09:11:36 2014 +0000 Revert "This change is horribly broken\&." - This reverts commit 215689406a8ca5813412becc6258509be903db59\&. + This reverts commit d5efb50dcc76ea29ad30d1015fb291a9ba5fc0db\&. # As with old drover, reverts are generally OK to commit without LGTM\&. \fB$ git cl upload \-r some\&.committer@chromium\&.org \-\-send\-mail\fR @@ -191,7 +267,7 @@ Date: Thu Apr 10 09:11:36 2014 +0000 # Here\*(Aqs a commit (from some\&.committer) that we want to \*(Aqdrover\*(Aq\&. \fB$ git log \-n 1 \-\-pretty=fuller\fR -commit 640f962733bfd2b9c44539a0d65952643750957e +commit 2655f8d9d48f18dbfed237c67b8c65134050b2e3 Author: some\&.committer AuthorDate: Thu Apr 10 08:54:46 2014 +0000 Commit: some\&.committer @@ -205,8 +281,8 @@ Branch drover_9999 set up to track remote ref refs/branch\-heads/9999\&. # Now do the \*(Aqdrover\*(Aq\&. # IMPORTANT!!! Do Not leave off the \*(Aq\-x\*(Aq flag -\fB$ git cherry\-pick \-x 640f962733bfd2b9c44539a0d65952643750957e\fR -[drover_9999 5f1ae97] This change needs to go to branch 9999 +\fB$ git cherry\-pick \-x 2655f8d9d48f18dbfed237c67b8c65134050b2e3\fR +[drover_9999 589e8ad] This change needs to go to branch 9999 Author: some\&.committer Date: Thu Apr 10 08:54:46 2014 +0000 1 file changed, 1 insertion(+) @@ -215,7 +291,7 @@ Branch drover_9999 set up to track remote ref refs/branch\-heads/9999\&. # That took the code authored by some\&.committer and committed it to # the branch by the person who drovered it (i\&.e\&. you)\&. \fB$ git log \-n 1 \-\-pretty=fuller\fR -commit 5f1ae978a8d05c16d8ed812163b7aa927f028bf9 +commit 589e8adbb095944100d029724b4038094120f23b Author: some\&.committer AuthorDate: Thu Apr 10 08:54:46 2014 +0000 Commit: you @@ -223,7 +299,7 @@ CommitDate: Thu Apr 10 09:11:36 2014 +0000 This change needs to go to branch 9999 - (cherry picked from commit 640f962733bfd2b9c44539a0d65952643750957e) + (cherry picked from commit 2655f8d9d48f18dbfed237c67b8c65134050b2e3) # Looks good\&. Ship it! \fB$ git cl upload\fR diff --git a/man/src/git-drover.demo.4.sh b/man/src/git-drover.demo.4.sh new file mode 100755 index 0000000000..2e36330dd1 --- /dev/null +++ b/man/src/git-drover.demo.4.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +. git-drover.demo.common.sh + +drover_c "This change needs to go to branch 9999" + +echo "# Here's a commit (from some.committer) that we want to 'drover'." +run git log -n 1 --pretty=fuller +echo +echo "# Now do the 'drover'." +pcommand git drover --branch 9999 \ + --cherry-pick $(git show-ref -s pick_commit) + +echo "Going to cherry-pick" +echo '"""' +output git log -n 1 +echo '"""' +echo "to 9999. Continue (y/n)? y" +echo +echo "Error: Patch failed to apply." +echo +echo "A workdir for this cherry-pick has been created in" +echo " /tmp/drover_9999" +echo +echo "To continue, resolve the conflicts there and run" +echo " git drover --continue /tmp/drover_9999" +echo +echo "To abort this cherry-pick run" +echo " git drover --abort /tmp/drover_9999" +echo +pcommand pushd /tmp/drover_9999 +echo "# Manually resolve conflicts." +pcommand git add path/to/file_with_conflicts +pcommand popd +pcommand git drover --continue /tmp/drover_9999 +echo +echo "# A cl is uploaded to rietveld, where it can be reviewed before landing." +echo +echo "About to land on 9999. Continue (y/n)? y" +echo "# The cherry-pick cl is landed on the branch 9999." diff --git a/man/src/git-drover.txt b/man/src/git-drover.txt index 98eb92d557..a69459410a 100644 --- a/man/src/git-drover.txt +++ b/man/src/git-drover.txt @@ -9,7 +9,9 @@ include::_git-drover_desc.helper.txt[] SYNOPSIS -------- [verse] -'git drover' --branch --cherry-pick +'git drover' --branch + (--cherry-pick | --continue [path_to_workdir] | + --abort [path_to_workdir]) [--parent_checkout ] [--verbose] [--dry-run] @@ -18,8 +20,7 @@ DESCRIPTION `git drover` applies a commit to a release branch. It creates a new workdir from an existing checkout to avoid downloading a new checkout without affecting the -existing checkout. Creating a workdir requires symlinks so this does not work on -Windows. See the EXAMPLE section for the equivalent sequence of commands to run. +existing checkout. `git drover` does not support reverts. See the EXAMPLE section for the equivalent sequence of commands to run. @@ -32,6 +33,15 @@ OPTIONS --cherry-pick :: The commit to cherry-pick. +--continue [path_to_workdir]:: + Continue a cherry-pick that required manual resolution. The path to the drover + workdir is optional. If unspecified, the current directory is used. + +--abort [path_to_workdir]:: + Abort a cherry-pick that required manual resolution and clean up its workdir. + The path to the drover workdir is optional. If unspecified, the current + directory is used. + --parent_checkout:: The path to the chromium checkout to use as the source for a creating git-new-workdir workdir to use for cherry-picking. If unspecified, the current @@ -63,6 +73,10 @@ Merge Example ^^^^^^^^^^^^^ demo:1[] +Merge with Conflicts Example +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +demo:4[] + Revert Example ^^^^^^^^^^^^^^ demo:2[] diff --git a/tests/git_drover_test.py b/tests/git_drover_test.py index c9f1f8fe28..98f9352ede 100755 --- a/tests/git_drover_test.py +++ b/tests/git_drover_test.py @@ -30,8 +30,8 @@ class GitDroverTest(auto_stub.TestCase): with open(os.path.join(self._parent_repo, '.git', 'HEAD'), 'w') as f: f.write('HEAD') os.mkdir(os.path.join(self._parent_repo, '.git', 'info')) - with open(os.path.join(self._parent_repo, '.git', 'info', 'refs'), - 'w') as f: + with open( + os.path.join(self._parent_repo, '.git', 'info', 'refs'), 'w') as f: f.write('refs') self.mock(tempfile, 'mkdtemp', self._mkdtemp) self.mock(__builtins__, 'raw_input', self._get_input) @@ -40,9 +40,6 @@ class GitDroverTest(auto_stub.TestCase): self._commands = [] self._input = [] self._fail_on_command = None - self._has_os_symlink = hasattr(os, 'symlink') - if not self._has_os_symlink: - os.symlink = lambda source, dest: None self.REPO_CHECK_COMMANDS = [ (['git', '--help'], self._parent_repo), @@ -59,22 +56,34 @@ class GitDroverTest(auto_stub.TestCase): (['git', 'checkout', '-b', 'drover_branch_123', 'refs/remotes/branch-heads/branch'], self._target_repo), (['git', 'cherry-pick', '-x', 'cl'], self._target_repo), + ] + self.UPLOAD_COMMANDS = [ (['git', 'reset', '--hard'], self._target_repo), + (['git', 'cl', 'upload'], self._target_repo), ] - self.UPLOAD_COMMAND = [(['git', 'cl', 'upload'], self._target_repo)] self.LAND_COMMAND = [ (['git', 'cl', 'land', '--bypass-hooks'], self._target_repo), ] - self.BRANCH_CLEANUP_COMMANDS = [ - (['git', 'cherry-pick', '--abort'], self._target_repo), - (['git', 'checkout', '--detach'], self._target_repo), - (['git', 'branch', '-D', 'drover_branch_123'], self._target_repo), + if os.name == 'nt': + self.BRANCH_CLEANUP_COMMANDS = [ + (['rmdir', '/s', '/q', self._target_repo], None), + (['git', 'branch', '-D', 'drover_branch_123'], self._parent_repo), + ] + else: + self.BRANCH_CLEANUP_COMMANDS = [ + (['git', 'branch', '-D', 'drover_branch_123'], self._parent_repo), + ] + self.MANUAL_RESOLVE_PREPARATION_COMMANDS = [ + (['git', 'status', '--porcelain'], self._target_repo), + (['git', 'update-index', '--skip-worktree', '--', 'foo', 'bar'], + self._target_repo), + ] + self.FINISH_MANUAL_RESOLVE_COMMANDS = [ + (['git', 'commit', '--no-edit'], self._target_repo), ] def tearDown(self): shutil.rmtree(self._temp_directory) - if not self._has_os_symlink: - del os.symlink super(GitDroverTest, self).tearDown() def _mkdtemp(self, prefix='tmp'): @@ -89,20 +98,27 @@ class GitDroverTest(auto_stub.TestCase): return result def _check_call(self, args, stderr=None, stdout=None, shell='', cwd=None): - self.assertFalse(shell) + if args[0] == 'rmdir': + subprocess.call(args, shell=shell) + else: + self.assertFalse(shell) self._commands.append((args, cwd)) if (self._fail_on_command is not None and self._fail_on_command == len(self._commands)): + self._fail_on_command = None raise subprocess.CalledProcessError(1, args[0]) if args == ['git', 'rev-parse', '--git-dir']: return os.path.join(self._parent_repo, '.git') + if args == ['git', 'status', '--porcelain']: + return ' D foo\nUU baz\n D bar\n' + return '' def testSuccess(self): self._input = ['y', 'y'] git_drover.cherry_pick_change('branch', 'cl', self._parent_repo, False) self.assertEqual( self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + - self.UPLOAD_COMMAND + self.LAND_COMMAND + self.BRANCH_CLEANUP_COMMANDS, + self.UPLOAD_COMMANDS + self.LAND_COMMAND + self.BRANCH_CLEANUP_COMMANDS, self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) @@ -110,9 +126,8 @@ class GitDroverTest(auto_stub.TestCase): def testDryRun(self): self._input = ['y'] git_drover.cherry_pick_change('branch', 'cl', self._parent_repo, True) - self.assertEqual( - self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + - self.BRANCH_CLEANUP_COMMANDS, self._commands) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + + self.BRANCH_CLEANUP_COMMANDS, self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) @@ -134,7 +149,7 @@ class GitDroverTest(auto_stub.TestCase): self._input = ['y', 'n'] git_drover.cherry_pick_change('branch', 'cl', self._parent_repo, False) self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + - self.UPLOAD_COMMAND + self.BRANCH_CLEANUP_COMMANDS, + self.UPLOAD_COMMANDS + self.BRANCH_CLEANUP_COMMANDS, self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) @@ -153,29 +168,105 @@ class GitDroverTest(auto_stub.TestCase): self._fail_on_command = 8 self.assertRaises(git_drover.Error, git_drover.cherry_pick_change, 'branch', 'cl', self._parent_repo, False) - self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:2], - self._commands) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:2] + + self.BRANCH_CLEANUP_COMMANDS[:-1], self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) - def testFailDuringCherryPick(self): + def testFailDuringCherryPickAndAbort(self): self._input = ['y'] self._fail_on_command = 10 self.assertRaises(git_drover.Error, git_drover.cherry_pick_change, 'branch', 'cl', self._parent_repo, False) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:4] + + self.MANUAL_RESOLVE_PREPARATION_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + self._commands = [] + git_drover.abort_cherry_pick(self._target_repo) + self.assertEqual(self.BRANCH_CLEANUP_COMMANDS, self._commands) + self.assertFalse(os.path.exists(self._target_repo)) + + def testFailDuringCherryPickAndContinue(self): + self._input = ['y'] + self._fail_on_command = 10 + self.assertRaises(git_drover.PatchError, git_drover.cherry_pick_change, + 'branch', 'cl', self._parent_repo, False) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:4] + + self.MANUAL_RESOLVE_PREPARATION_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + + self._commands = [] + self._input = ['n'] + git_drover.continue_cherry_pick(self._target_repo) + self.assertEqual(self.UPLOAD_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + + self._commands = [] + self._input = ['y'] + git_drover.continue_cherry_pick(self._target_repo) self.assertEqual( - self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:4] + - self.BRANCH_CLEANUP_COMMANDS, self._commands) + self.UPLOAD_COMMANDS + self.LAND_COMMAND + self.BRANCH_CLEANUP_COMMANDS, + self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) + def testFailDuringCherryPickAndContinueWithoutCommitting(self): + self._input = ['y'] + self._fail_on_command = 10 + self.assertRaises(git_drover.PatchError, git_drover.cherry_pick_change, + 'branch', 'cl', self._parent_repo, False) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:4] + + self.MANUAL_RESOLVE_PREPARATION_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + self._commands = [] + with open(os.path.join(self._target_repo, '.git', 'CHERRY_PICK_HEAD'), 'w'): + pass + + self._commands = [] + self._input = ['y'] + git_drover.continue_cherry_pick(self._target_repo) + self.assertEqual(self.FINISH_MANUAL_RESOLVE_COMMANDS + self.UPLOAD_COMMANDS + + self.LAND_COMMAND + self.BRANCH_CLEANUP_COMMANDS, + self._commands) + self.assertFalse(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + + def testFailDuringCherryPickAndContinueWithoutResolving(self): + self._input = ['y'] + self._fail_on_command = 10 + self.assertRaises(git_drover.PatchError, git_drover.cherry_pick_change, + 'branch', 'cl', self._parent_repo, False) + self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS[:4] + + self.MANUAL_RESOLVE_PREPARATION_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + self.assertFalse(self._input) + self._commands = [] + self._fail_on_command = 1 + with open(os.path.join(self._target_repo, '.git', 'CHERRY_PICK_HEAD'), 'w'): + pass + self.assertRaisesRegexp(git_drover.Error, + 'All conflicts must be resolved before continuing', + git_drover.continue_cherry_pick, self._target_repo) + self.assertEqual(self.FINISH_MANUAL_RESOLVE_COMMANDS, self._commands) + self.assertTrue(os.path.exists(self._target_repo)) + + self._commands = [] + git_drover.abort_cherry_pick(self._target_repo) + self.assertEqual(self.BRANCH_CLEANUP_COMMANDS, self._commands) + self.assertFalse(os.path.exists(self._target_repo)) + def testFailAfterCherryPick(self): self._input = ['y'] self._fail_on_command = 11 self.assertRaises(git_drover.Error, git_drover.cherry_pick_change, 'branch', 'cl', self._parent_repo, False) self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + - self.BRANCH_CLEANUP_COMMANDS, self._commands) + self.UPLOAD_COMMANDS[:1] + self.BRANCH_CLEANUP_COMMANDS, + self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) @@ -185,19 +276,26 @@ class GitDroverTest(auto_stub.TestCase): self.assertRaises(git_drover.Error, git_drover.cherry_pick_change, 'branch', 'cl', self._parent_repo, False) self.assertEqual(self.REPO_CHECK_COMMANDS + self.LOCAL_REPO_COMMANDS + - self.UPLOAD_COMMAND + self.BRANCH_CLEANUP_COMMANDS, + self.UPLOAD_COMMANDS + self.BRANCH_CLEANUP_COMMANDS, self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) def testInvalidParentRepoDirectory(self): - self.assertRaises( - git_drover.Error, git_drover.cherry_pick_change, 'branch', 'cl', - os.path.join(self._parent_repo, 'fake'), False) + self.assertRaises(git_drover.Error, git_drover.cherry_pick_change, 'branch', + 'cl', os.path.join(self._parent_repo, 'fake'), False) self.assertFalse(self._commands) self.assertFalse(os.path.exists(self._target_repo)) self.assertFalse(self._input) + def testContinueInvalidWorkdir(self): + self.assertRaises(git_drover.Error, git_drover.continue_cherry_pick, + self._parent_repo) + + def testAbortInvalidWorkdir(self): + self.assertRaises(git_drover.Error, git_drover.abort_cherry_pick, + self._parent_repo) + if __name__ == '__main__': unittest.main()