Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"

Theodore Tso [off-list ref] writes:

While you have "git am" open, how about adding an "git am --abort"
which nukes the .dotest aka .git/rebase directory, and resets HEAD
back to the original position?

This does not seem to have reached the list nor its archives.  I cannot
say I have really looked at it deeply but it may be a good starting
point.  It needs docs ;-)

-- >8 --
From: Nanako Shiraishi <redacted>
Date: Wed, 16 Jul 2008 19:39:10 +0900
Subject: [PATCH] git am --abort

After failing to apply patches in the middle of a series, "git am --abort"
lets you go back to the original commit.

Signed-off-by: Nanako Shiraishi <redacted>
---
 git-am.sh           |   19 +++++++++++++++----
 t/t4151-am-abort.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100755 t/t4151-am-abort.sh

diff --git a/git-am.sh b/git-am.sh
index cc8787b..a44bd7a 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -22,6 +22,7 @@ p=              pass it through git-apply
 resolvemsg=     override error message when patch failure occurs
 r,resolved      to be used after a patch failure
 skip            skip the current patch
+abort           abandon patch application and clear .dotest directory
 rebasing        (internal use for git-rebase)"
 
 . git-sh-setup

@@ -120,7 +121,7 @@ It does not apply to blobs recorded in its index."
 
 prec=4
 dotest="$GIT_DIR/rebase"
-sign= utf8=t keep= skip= interactive= resolved= binary= rebasing=
+sign= utf8=t keep= skip= interactive= resolved= binary= rebasing= abort=
 resolvemsg= resume=
 git_apply_opt=
 

@@ -145,6 +146,8 @@ do
 		resolved=t ;;
 	--skip)
 		skip=t ;;
+	--abort)
+		abort=t ;;
 	--rebasing)
 		rebasing=t threeway=t keep=t binary=t ;;
 	-d|--dotest)

@@ -177,7 +180,7 @@ fi
 
 if test -d "$dotest"
 then
-	case "$#,$skip$resolved" in
+	case "$#,$skip$resolved$abort" in
 	0,*t*)
 		# Explicit resume command and we do not have file, so
 		# we are happy.

@@ -197,9 +200,17 @@ then
 	esac ||
 	die "previous rebase directory $dotest still exists but mbox given."
 	resume=yes
+
+	case "$abort" in
+	t)
+		rm -fr "$dotest" &&
+		git read-tree -m -u ORIG_HEAD &&
+		git reset ORIG_HEAD && :
+		exit ;;
+	esac
 else
-	# Make sure we are not given --skip nor --resolved
-	test ",$skip,$resolved," = ,,, ||
+	# Make sure we are not given --skip, --resolved, nor --abort
+	test "$skip$resolved$abort" = "" ||
 		die "Resolve operation not in progress, we are not resuming."
 
 	# Start afresh.

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
new file mode 100755
index 0000000..96b2cd5
--- /dev/null
+++ b/t/t4151-am-abort.sh

@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='am --abort'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	for i in a b c d e f g
+	do
+		echo $i
+	done >file-1 &&
+	cp file-1 file-2 &&
+	test_tick &&
+	git add file-1 file-2 &&
+	git commit -m initial &&
+	git tag initial &&
+	for i in 2 3 4 5
+	do
+		echo $i >>file-1 &&
+		test_tick &&
+		git commit -a -m $i || break
+	done &&
+	git format-patch initial &&
+	git checkout -b side initial &&
+	echo local change >file-2 &&
+	cp file-2 file-2-expect
+'
+
+test_expect_success 'am stops at a patch that does not apply' '
+	test_must_fail git am 000[124]-*.patch &&
+	git log --pretty=tformat:%s >actual &&
+	for i in 3 2 initial
+	do
+		echo $i
+	done >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'am --abort goes back' '
+	git am --abort &&
+	git rev-parse HEAD >actual &&
+	git rev-parse initial >expect &&
+	test_cmp expect actual &&
+	test_cmp file-2-expect file-2 &&
+	git diff-index --exit-code --cached HEAD
+'
+
+test_done

-- 
1.5.6

Junio C Hamano a écrit :

Theodore Tso [off-list ref] writes:

quoted

While you have "git am" open, how about adding an "git am --abort"
which nukes the .dotest aka .git/rebase directory, and resets HEAD
back to the original position?

This does not seem to have reached the list nor its archives.  I cannot
say I have really looked at it deeply but it may be a good starting
point.  It needs docs ;-)

This can be squashed to Nanako's path: it adds missing docs,
'git rerere clear' and move $dotest deletion at the end in case
'git read-tree' failed (because of a dirty index).

Olivier.

-- >8 --
Subject: [PATCH] git am --abort

To squash.

Signed-off-by: Olivier Marin <redacted>
---
 Documentation/git-am.txt |    5 ++++-
 git-am.sh                |    8 +++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 2d7f162..e010a16 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt

@@ -13,7 +13,7 @@ SYNOPSIS
          [--3way] [--interactive] [--binary]
          [--whitespace=<option>] [-C<n>] [-p<n>]
 	 [<mbox> | <Maildir>...]
-'git am' (--skip | --resolved)
+'git am' (--skip | --resolved | --abort)
 
 DESCRIPTION
 -----------

@@ -99,6 +99,9 @@ default.   You could use `--no-utf8` to override this.
 	or `--skip` to handle the failure.  This is solely
 	for internal use between 'git-rebase' and 'git-am'.
 
+--abort::
+	Restore the original branch and abort the patching operation.
+
 DISCUSSION
 ----------
 

diff --git a/git-am.sh b/git-am.sh
index 5e645e4..04b2e96 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -22,7 +22,7 @@ p=              pass it through git-apply
 resolvemsg=     override error message when patch failure occurs
 r,resolved      to be used after a patch failure
 skip            skip the current patch
-abort           abandon patch application and clear .dotest directory
+abort           restore the original branch and abort the patching operation.
 rebasing        (internal use for git-rebase)"
 
 . git-sh-setup

@@ -55,6 +55,7 @@ stop_here_user_resolve () {
     fi
     echo "When you have resolved this problem run \"$cmdline --resolved\"."
     echo "If you would prefer to skip this patch, instead run \"$cmdline --skip\"."
+    echo "To restore the original branch and stop patching run \"$cmdline --abort\"."
 
     stop_here $1
 }

@@ -203,9 +204,10 @@ then
 
 	case "$abort" in
 	t)
-		rm -fr "$dotest" &&
+		git rerere clear &&
 		git read-tree -m -u ORIG_HEAD &&
-		git reset ORIG_HEAD && :
+		git reset ORIG_HEAD &&
+		rm -fr "$dotest"
 		exit ;;
 	esac
 else

-- 
1.5.6.3.441.g3087

Junio C Hamano a écrit :

Theodore Tso [off-list ref] writes:

quoted

While you have "git am" open, how about adding an "git am --abort"
which nukes the .dotest aka .git/rebase directory, and resets HEAD
back to the original position?

This does not seem to have reached the list nor its archives.  I cannot
say I have really looked at it deeply but it may be a good starting
point.  It needs docs ;-)

Thinking about the way "git am" works:

It tries to apply patches even on a dirty tree which makes difficult
to automatically do a "git reset --hard" with --skip or --abort and
forces the user to clean the index by hand if last patch failed with
unmerged files.

So, do some people still use "git am" with a dirty tree or will a
patch that make it work like "git rebase" be accepted?

I think users can use "git stash" and "git stash pop" today. Did I
miss something?

Note: the version that use "git sequencer" seems to work like I want
      but I do not know if it is a side effect or not.

Olivier.

Hi,

Olivier Marin wrote:

Note: the version that use "git sequencer" seems to work like I want
      but I do not know if it is a side effect or not.

If you mean with "work like I want" that it complains about an unclean
work tree: the check is explicitly done by git-sequencer.

I guess there are situations where you want to apply a patch on a dirty
work tree, e.g. if you fixed some tiny issues so that patches can
apply cleanly.
But I think it does not hurt to commit such tiny changes and perhaps
squash later.

Regards,
  Stephan

-- 
Stephan Beyer [off-list ref], PGP 0x6EDDD207FCC5040F

Stephan Beyer a écrit :

If you mean with "work like I want" that it complains about an unclean
work tree: the check is explicitly done by git-sequencer.

Yes, I mean: refuse to work if the tree is not clean _and_ --skip
and --abort clean the index/tree for me.
With current git version "git am --skip" refuse to continue with a
message: "Dirty index: cannot apply patches (dirty: ...)", if some
files stay unmerged.
As a user I find --skip and --abort useless^Wpainful to use in this
situation.

I guess there are situations where you want to apply a patch on a dirty
work tree, e.g. if you fixed some tiny issues so that patches can
apply cleanly.

Maybe.

Olivier.

Olivier Marin [off-list ref] writes:

It tries to apply patches even on a dirty tree which makes difficult
to automatically do a "git reset --hard" with --skip or --abort and
forces the user to clean the index by hand if last patch failed with
unmerged files.

So, do some people still use "git am" with a dirty tree or will a
patch that make it work like "git rebase" be accepted?

Anything that changes "am" to require a clean working tree will NEVER be
accepted.  I personally rely on the ability for it to run in a dirty tree,
so does Linus.

	Side note.  Anything that changes "merge" to require a clean
	working tree is also unacceptable.  Cf.

	http://thread.gmane.org/gmane.comp.version-control.git/9073/focus=9089

	Linus talks about "patch" in the paragraph second to the last one
	in the message; back then he was talking about "git-applymbox" but
	the same argument there applies to its newer incarnation "git-am".

	Side note #2.  It would have been nice if "rebase" were also
	written in such a way that it can work in a dirty tree as long as
	local changes did not interfere with the operation, but it is a
	lot more involved.

When I looked at the "am --abort" patch briefly, I had an impression (by
reading its test case) that it correctly refrained from doing the
destructive "reset --hard".

[This is git@vger.kernel.org only copy]

Junio C Hamano wrote:

Olivier Marin [off-list ref] writes:

quoted

It tries to apply patches even on a dirty tree which makes difficult
to automatically do a "git reset --hard" with --skip or --abort and
forces the user to clean the index by hand if last patch failed with
unmerged files.

So, do some people still use "git am" with a dirty tree or will a
patch that make it work like "git rebase" be accepted?

Anything that changes "am" to require a clean working tree will NEVER be
accepted.  I personally rely on the ability for it to run in a dirty tree,
so does Linus.

      Side note.  Anything that changes "merge" to require a clean
      working tree is also unacceptable.  Cf.

      http://thread.gmane.org/gmane.comp.version-control.git/9073/focus=9089

      Linus talks about "patch" in the paragraph second to the last one
      in the message; back then he was talking about "git-applymbox" but
      the same argument there applies to its newer incarnation "git-am".

      Side note #2.  It would have been nice if "rebase" were also
      written in such a way that it can work in a dirty tree as long as
      local changes did not interfere with the operation, but it is a
      lot more involved.

When I looked at the "am --abort" patch briefly, I had an impression (by
reading its test case) that it correctly refrained from doing the
destructive "reset --hard".

I guess instead of "git reset --hard" we can use here "git stash save
&& git stash apply --index" to save state (perhaps as "git stash save
--no-reset"), and either "git stash drop" at the the end, or 
"git reset --hard && git stash pop --index" at '--abort'.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Junio C Hamano a écrit :

Anything that changes "am" to require a clean working tree will NEVER be
accepted.  I personally rely on the ability for it to run in a dirty tree,
so does Linus.

	Side note.  Anything that changes "merge" to require a clean
	working tree is also unacceptable.  Cf.

	http://thread.gmane.org/gmane.comp.version-control.git/9073/focus=9089

	Linus talks about "patch" in the paragraph second to the last one
	in the message; back then he was talking about "git-applymbox" but
	the same argument there applies to its newer incarnation "git-am".

	Side note #2.  It would have been nice if "rebase" were also
	written in such a way that it can work in a dirty tree as long as
	local changes did not interfere with the operation, but it is a
	lot more involved.

Thanks for the explanation and the really interesting pointer.

The last thing that still annoy me is the --skip that refuse to skip in 3-way
merge. Perhaps we can use the "git read-tree --reset -u" thing for skip too.

So, what about the following patch?

-- >8 --
From: Olivier Marin <redacted>

git am --skip: clean the index while preserving local changes

In 3-way merge, "am" will let the index with unmerged path waiting
for us to resolve conflicts and continue. But if we want to --skip
instead, "am" refuses to continue because of the dirty index.

With this patch, "am" will clean the index without touching files
locally modified, before continue.

Signed-off-by: Olivier Marin <redacted>
---
 git-am.sh           |   12 +++++++++---
 t/t4151-am-abort.sh |   11 +++++++++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 60aaa4a..864c77e 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -202,8 +202,15 @@ then
 	die "previous rebase directory $dotest still exists but mbox given."
 	resume=yes
 
-	case "$abort" in
-	t)
+	case "$skip,$abort" in
+	t,)
+		git rerere clear
+		git read-tree --reset -u HEAD HEAD
+		orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
+		git reset HEAD
+		git update-ref ORIG_HEAD $orig_head
+		;;
+	,t)
 		git rerere clear
 		git read-tree --reset -u HEAD ORIG_HEAD
 		git reset ORIG_HEAD

@@ -297,7 +304,6 @@ last=`cat "$dotest/last"`
 this=`cat "$dotest/next"`
 if test "$skip" = t
 then
-	git rerere clear
 	this=`expr "$this" + 1`
 	resume=
 fi

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index dda7e2c..249093b 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh

@@ -14,7 +14,7 @@ test_expect_success setup '
 	git add file-1 file-2 &&
 	git commit -m initial &&
 	git tag initial &&
-	for i in 2 3 4 5
+	for i in 2 3 4 5 6
 	do
 		echo $i >>file-1 &&
 		test_tick &&

@@ -32,7 +32,7 @@ do
 		git reset --hard initial &&
 		cp file-2-expect file-2 &&
 
-		test_must_fail git am$with3 000[124]-*.patch &&
+		test_must_fail git am$with3 000[1245]-*.patch &&
 		git log --pretty=tformat:%s >actual &&
 		for i in 3 2 initial
 		do

@@ -41,6 +41,13 @@ do
 		test_cmp expect actual
 	'
 
+	test_expect_success "am$with3 --skip continue after failed am$with3" '
+		test_must_fail git-am$with3 --skip >output &&
+		test "$(grep "^Applying" output)" = "Applying 6" &&
+		test_cmp file-2-expect file-2 &&
+		test ! -f .git/rr-cache/MERGE_RR
+	'
+
 	test_expect_success "am --abort goes back after failed am$with3" '
 		git-am --abort &&
 		git rev-parse HEAD >actual &&

-- 
1.5.6.4.570.g052e.dirty

The git-rerere documentation talks about commands that invoke
"git rerere clear" automatically. git am --abort is added and
a typo is fixed additionally.

Signed-off-by: Stephan Beyer <redacted>
---
 Documentation/git-rerere.txt           |    2 +-
 contrib/completion/git-completion.bash |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index beebd53..89f321b 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt

@@ -37,7 +37,7 @@ its working state.
 'clear'::
 
 This resets the metadata used by rerere if a merge resolution is to be
-is aborted.  Calling 'git-am --skip' or 'git-rebase [--skip|--abort]'
+aborted.  Calling 'git-am [--skip|--abort]' or 'git-rebase [--skip|--abort]'
 will automatically invoke this command.
 
 'diff'::

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2edb341..8fc9145 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -489,7 +489,7 @@ _git_am ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ]; then
-		__gitcomp "--skip --resolved"
+		__gitcomp "--skip --resolved --abort"
 		return
 	fi
 	case "$cur" in

-- 
1.5.6.4.459.gfa44d

Olivier Marin [off-list ref] writes:

The last thing that still annoy me is the --skip that refuse to skip in 3-way
merge. Perhaps we can use the "git read-tree --reset -u" thing for skip too.

Hmm...

We traditionally left that as something the user deliberately should do to
signal --skip that the user knows he is dropping that change (by the way,
so did "git-rebase").  But with fb6e4e1 (Do git reset --hard HEAD when
using git rebase --skip, 2007-11-08), we run the reset upon rebase --skip,
so it probably is a good idea to match it here as well.

Sorry, I do not quite understand what this reset after the read-tree dance
is trying to do; you have already reset the index to the tree in HEAD when
you cleared the change involved in the patch application with that
two-tree form of read-tree.

Junio C Hamano a écrit :

quoted

diff --git a/git-am.sh b/git-am.sh
index 60aaa4a..864c77e 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -202,8 +202,15 @@ then
 	die "previous rebase directory $dotest still exists but mbox given."
 	resume=yes
 
-	case "$abort" in
-	t)
+	case "$skip,$abort" in
+	t,)
+		git rerere clear
+		git read-tree --reset -u HEAD HEAD
+		orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
+		git reset HEAD
+		git update-ref ORIG_HEAD $orig_head
+		;;

Sorry, I do not quite understand what this reset after the read-tree dance
is trying to do; you have already reset the index to the tree in HEAD when
you cleared the change involved in the patch application with that
two-tree form of read-tree.

I add reset to unstage paths with no conflict that are left behind. Perhaps
read-tree can clear those paths as well but I do not know how, the man page
is hard to follow.

-- 
Olivier.

Olivier Marin [off-list ref] writes:

Junio C Hamano a écrit :

quoted

quoted

diff --git a/git-am.sh b/git-am.sh

...

quoted

@@ -202,8 +202,15 @@ then

...

quoted

+	case "$skip,$abort" in
+	t,)
+		git rerere clear
+		git read-tree --reset -u HEAD HEAD
+		orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
+		git reset HEAD
+		git update-ref ORIG_HEAD $orig_head
+		;;

...

I add reset to unstage paths with no conflict that are left behind.

Does removing the last three lines (1) change the behaviour?  (2) break any test?

If the answers are "Yes and No", we would need a few more tests in the
testsuite.

If the answers are "No and naturally no", then let's remove the last three
lines in the hunk.

Junio C Hamano a écrit :

Olivier Marin [off-list ref] writes:

quoted

Junio C Hamano a écrit :

quoted

quoted

diff --git a/git-am.sh b/git-am.sh

...

quoted

@@ -202,8 +202,15 @@ then

...

quoted

+	case "$skip,$abort" in
+	t,)
+		git rerere clear
+		git read-tree --reset -u HEAD HEAD
+		orig_head=$(cat "$GIT_DIR/ORIG_HEAD")
+		git reset HEAD
+		git update-ref ORIG_HEAD $orig_head
+		;;

...

I add reset to unstage paths with no conflict that are left behind.

Does removing the last three lines (1) change the behaviour?  (2) break any test?

If the answers are "Yes and No", we would need a few more tests in the
testsuite.

The answers are "Yes and No" and test case update follow.

-- >8 --
From: Olivier Marin <redacted>
[PATCH] update test case to protect am --skip behaviour

Signed-off-by: Olivier Marin <redacted>
---
 t/t4151-am-abort.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index f45ab0a..7d86cdf 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh

@@ -17,6 +17,8 @@ test_expect_success setup '
 	for i in 2 3 4 5 6
 	do
 		echo $i >>file-1 &&
+		echo $i >otherfile-$i &&
+		git add otherfile-$i &&
 		test_tick &&
 		git commit -a -m $i || break
 	done &&

-- 
1.6.0.rc0.44.gd618.dirty