Aborting cherry-pick

Hi,

I'd like to suggest improving cherry-pick messages/documentation in case of conflicts.

Example of cherry-pick conflict:

   $ git cherry-pick  c64e8caa56fd76577bbaea37592f4a9df10ab1b8
   error: could not apply c64e8ca... <<commit message>>
   hint: after resolving the conflicts, mark the corrected paths
   hint: with 'git add <paths>' or 'git rm <paths>'

   $ git status
   # On branch master
   # Unmerged paths:
   #   (use "git reset HEAD <file>..." to unstage)
   #   (use "git add/rm <file>..." as appropriate to mark resolution)
   #
   #	both modified:      <<file>>
   #
   no changes added to commit (use "git add" and/or "git commit -a")

I see following possibilities:

1. add hint to cherry-pick how to abort merge, for example:
hint: use 'git reset --merge ORIG_HEAD' to abort merge

2. add hint to status how to abort merge, for example:
# (use 'git reset --merge ORIG_HEAD' to abort merge)

3. add "--abort" to cherry-pick (same option as for git-merge)

The ORIG_HEAD is specified in man page example. There is also a possibility of omitting ORIG_HEAD in which case local changes are retained. Maybe this is a better suggesting for the user? Or maybe describe both cases somewhere and point user to this documentation in status/cherry-pick output?

git-cherry-pick uses word "cancel" in man page (in example) and git-merge uses word "abort" for (as I understand) the same action.

I would suggest using either same word for both cases, or using both words in both cases. Otherwise it's confusing if you read about "aborting" merges and when you want to abort cherry-pick you don't find it - because it's called "canceling".

What is your opinion on this?

Piotrek

If you tried a cherry-pick or revert resulting in complex conflicts and want to start over, "git reset --merge HEAD" might help.

---
(+cc some relevant people. Johan wrote merge --abort, Jay is adding a status hint during cherry-pick, Christian wrote reset --merge and much related cherry-pick functionality)

[...]

Good idea. "git cherry-pick" does not set ORIG_HEAD, so I put "git reset --merge HEAD" instead.

"git reset --merge" will remove local changes marked with "git add", under the assumption that they were part of the conflict resolution and thus should be cleared away.

This leaves me afraid of the following scenario:

	git pull
	# conflict?  agh!

	# ... two days later ...
	... hack hack hack ...
	... add add add ...
	git commit;	# fails because of unmerged files

	# whoops, forgot about that merge.
	# Let's do what it says to do:
	git reset --merge ORIG_HEAD

It seems safest to only recommend "reset --merge" immediately after a mergy operation.

I assume you mean on the git-reset page? It might be nice to add more examples and to update the current ones with current best practice (e.g., using reset --keep more often).

The use of ORIG_HEAD there also allows the following:

	git pull
	# success?  But I didn't want that
	git reset --merge ORIG_HEAD

and protects against situations in which ORIG_HEAD is not present (though merge --abort does that better, I think).

Hmm, where is this example in the cherry-pick manpage?

Thanks for the suggestions. Generally speaking, I like them.

 builtin/revert.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

index dc1b702..78baae7 100644
@@ -267,11 +267,13 @@ static void print_advice(void)
 	}
 
 	advise("after resolving the conflicts, mark the corrected paths");
-	advise("with 'git add <paths>' or 'git rm <paths>'");
+	advise("with 'git add <paths>' and 'git rm <paths>'");
 
 	if (action == CHERRY_PICK)
 		advise("and commit the result with 'git commit -c %s'",
 		       find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
+
+	advise("or use 'git reset --merge HEAD' to abort");
 }
 
 static void write_message(struct strbuf *msgbuf, const char *filename)

1.7.4.1

W dniu 18.02.2011 02:24, Jonathan Nieder pisze:

You're right. I was sure I have tested it, but when I try to use ORIG_HEAD now it does not work.

I took it from Documentation/git-cherry-pick.txt, which seem to be wrong:

   $ git cherry-pick topic^             <1>
   $ git diff                           <2>
   $ git reset --merge ORIG_HEAD        <3>
   [...]
   <3> cancel the cherry-pick.  In other words, return to the
   pre-cherry-pick state, preserving any local modifications you had in
   the working tree.

Didn't know that (one probably shouldn't merge with uncommitted/local changes anyway). git-reset.txt mentions it, but there's no explicit warning.

Is it possible to recognize that you have something more than what was cause by the merge/cherry-pick?

Yes, it says:

Running "git reset --hard ORIG_HEAD" will let you go back to where you were, but it will discard your local changes, which you do not want. "git reset --merge" keeps your local changes.

It's in the Documentation/cherry-pick.txt (in current master). I assume man pages are generated from those text files. I have also cited it at the beginning of this mail.

Piotr Krukowiecki

The example that uses "reset --merge" to bail out from a failed cherry-pick is dangerously wrong. For example,

	git reset --keep master
	git cherry-pick HEAD@{1};	# conflicts!
	git reset --merge ORIG_HEAD;	# let's try that again
	git cherry-pick --strategy=more-careful HEAD@{1}

would not reset the topic to the pre cherry-pick state (master) but to whatever branch we were on before then.

The example used ORIG_HEAD by habit from aborting merges. "git reset --merge HEAD" is more appropriate here.

---

Good catch. The manpage I was looking at was stale.

Care to write a patch?

[...]

[...]

I suppose it depends what you mean. I see at least two distinct problems to solve:

A. "undo" facility to recover from an ugly cherry-pick

This one is what reset --merge is for. The idea is that after spending a little while trying to make something good out of a mess, you say, "oh, bother, let me get back to where I started".

So in this case it really does make sense to back out any changes you marked with "git add" after the cherry-pick, since they were part of the messy resolution process. If there had been any changes registered before the cherry-pick, the cherry-pick would have just failed without making a mess.

A patch in flight makes "git cherry-pick" print advice for this case when it encounters conflicts (thanks!).

B. clearing away a forgotten merge from long ago

If you had been doing work without remembering that there was a merge in progress, the best way to recover is probably a plain "git reset HEAD -- .".

This is a case people might be looking for advice about when reading "git status" ("where was I?") output.

Thanks.

 Documentation/git-cherry-pick.txt |    6 +-
 t/t3404-rebase-interactive.sh     |    6 --
 t/t3510-cherry-pick-doc.sh        |  137 +++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                     |    9 +++
 4 files changed, 149 insertions(+), 9 deletions(-)
 create mode 100755 t/t3510-cherry-pick-doc.sh

index 749d68a..abedcb7 100644
@@ -137,7 +137,7 @@ again, this time exercising more care about matching up context lines.
 ------------
 $ git cherry-pick topic^             <1>
 $ git diff                           <2>
-$ git reset --merge ORIG_HEAD        <3>
+$ git reset --merge HEAD             <3>
 $ git cherry-pick -Xpatience topic^  <4>
 ------------
 <1> apply the change that would be shown by `git show topic^`.

@@ -146,8 +146,8 @@ information about the conflict is written to the index and
 working tree and no new commit results.
 <2> summarize changes to be reconciled
 <3> cancel the cherry-pick.  In other words, return to the
-pre-cherry-pick state, preserving any local modifications you had in
-the working tree.
+pre-cherry-pick state, preserving any local modifications you have
+in the working tree.
 <4> try to apply the change introduced by `topic^` again,
 spending extra time to avoid mistakes based on incorrectly matching
 context lines.

index 7d8147b..88f1192 100755
@@ -29,12 +29,6 @@ Initial setup:
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
-test_cmp_rev () {
-	git rev-parse --verify "$1" >expect.rev &&
-	git rev-parse --verify "$2" >actual.rev &&
-	test_cmp expect.rev actual.rev
-}
-
 set_fake_editor
 
 # WARNING: Modifications to the initial repository can change the SHA ID used

index 0fdc541..0c053b3 100644
@@ -673,6 +673,15 @@ test_line_count () {
 	fi
 }
 
+# test_cmp_rev checks that two revision specifiers refer to the
+# same object.  Uses expect.rev and actual.rev files in the
+# current directory as scratch space.
+test_cmp_rev () {
+	git rev-parse --verify "$1" >expect.rev &&
+	git rev-parse --verify "$2" >actual.rev &&
+	test_cmp expect.rev actual.rev
+}
+
 # This is not among top-level (test_expect_success | test_expect_failure)
 # but is a prefix that can be used in the test script, like:
 #

1.7.4.1

W dniu 19.02.2011 00:17, Jonathan Nieder pisze:

I tried but gave up :(

That's what I meant. There's a 'git merge --abort', the same should be available for cherry-pick.

If I understand correctly, this requires changes to cherry-pick to set ORIG_HEAD?

Maybe 'git cherry-pick --abort' could do 'reset --merge' internally. You would not have to remember "ok, for merge I can do --abort, but for cherry pick I must do reset --merge".

Indeed, cherry-pick won't start if there are any stashed changes. I wasn't aware about this.

Which is a bit strange: why cherry-pick behaves differently than merge?
(merge allows staged changes when merging, cherry-pick doesn't). Shouldn't they work the same, i.e. allow merge or cherry-pick if the changes are not conflicting?

Anyway, cherry-pick refuses to work if you have staged changes, but still doesn't mind local unstaged changes. Which makes possible following case:

do some local hacking, do not stage yet cherry-pick commit with merge conflicts
try to resolve them, adding local changes in meantime give up and try to abort with 'git reset --merge'

In result your local changes will be lost. I don't know how real such situation is, but maybe git could/should prevent it?

For example by saying "you're trying to abort merge, but it will also revert your local changes from before the merge".

Nice :)

Piotr Krukowiecki