Re: [PATCH 1/2] am: handle stray $dotest directory

Junio C Hamano [off-list ref] writes:

Ramkumar Ramachandra [off-list ref] writes:

quoted

Junio C Hamano wrote:

quoted

And then the message triggers at the second test in t3420 when
applied on top of 587947750bd7 (rebase: implement --[no-]autostash
and rebase.autostash, 2013-05-12) or 45acb7592825 (Merge branch
'rr/rebase-autostash', 2013-06-11).

What was triggered?  (I didn't understand what you said)

The patch applied on top of either of these commits will *BREAK* the
second test in t3420.

Here is how "sh -x t3420-*.sh -i" ends:

    ...
    ++ git rebase unrelated-onto-branch
    Created autostash: cdca6ca
    HEAD is now at 0c4d2f1 third commit
    First, rewinding head to replay your work on top of it...
    Stray /srv/project/git/git.git/t/trash
    directory.t3420-rebase-autostash/.git/rebase-apply directory found.
    Use "git am --abort" to remove it.
    + eval_ret=1
    + test -z t
    + test 1 = 0
    + test -n ''
    + test t = t
    + test -n ''
    + return 1
    + test_failure_ 'rebase: dirty worktree, non-conflicting rebase' '
    ...

I _think_ the new check you added may be too loose.

A fix-up may look like this.


 git-am.sh | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 37edfae..3f89cf6 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -505,9 +505,9 @@ then
 		exit ;;
 	esac
 	rm -f "$dotest/dirtyindex"
-else
-	# Possible stray $dotest directory
-	if test -d "$dotest"; then
+elif test -d "$dotest" && ! test -f "$dotest/autostash"
+then
+	# stray $dotest directory
 	case "$skip,$resolved,$abort" in
 	,,t)
 		rm -fr "$dotest"

@@ -518,8 +518,7 @@ else
 Use \"git am --abort\" to remove it.")"
 		;;
 	esac
-	fi
-
+else
 	# Make sure we are not given --skip, --resolved, nor --abort
 	test "$skip$resolved$abort" = "" ||
 		die "$(gettext "Resolve operation not in progress, we are not resuming.")"

Junio C Hamano wrote:

I _think_ the new check you added may be too loose.

Yep, I totally forgot about the case when git-am.sh is called from an
existing script.  In that case, it is upto the caller to handle
whatever stray directories; we have no business meddling with that.

A fix-up may look like this.

No, don't leak autostash detail.

Just change that condition to

  if test -d "$dotest" && test -z "$rebasing"

and we're done.  I'll send in a re-roll.