* [PATCH v3 0/2] Fix am with stray $dotest directory
@ 2013-06-15 12:43 Ramkumar Ramachandra
2013-06-15 12:43 ` [PATCH v3 1/2] am: handle " Ramkumar Ramachandra
2013-06-15 12:43 ` [PATCH v3 2/2] t/am: use test_path_is_missing() where appropriate Ramkumar Ramachandra
0 siblings, 2 replies; 8+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-15 12:43 UTC (permalink / raw
To: Git List; +Cc: Junio C Hamano
The test in [1/2] was too loose in the previous iteration: guard it
with "test -z $rebasing". Also fix a couple of minor problems pointed
out by Junio (extra indentation, $-unescaped).
Thanks.
Ramkumar Ramachandra (2):
am: handle stray $dotest directory
t/am: use test_path_is_missing() where appropriate
git-am.sh | 17 +++++++++++++++++
t/t4150-am.sh | 40 +++++++++++++++++++++++-----------------
2 files changed, 40 insertions(+), 17 deletions(-)
--
1.8.3.1.383.g8881048.dirty
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-15 12:43 [PATCH v3 0/2] Fix am with stray $dotest directory Ramkumar Ramachandra
@ 2013-06-15 12:43 ` Ramkumar Ramachandra
2013-06-17 2:45 ` Junio C Hamano
2013-06-15 12:43 ` [PATCH v3 2/2] t/am: use test_path_is_missing() where appropriate Ramkumar Ramachandra
1 sibling, 1 reply; 8+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-15 12:43 UTC (permalink / raw
To: Git List; +Cc: Junio C Hamano
The following bug has been observed:
$ git am # no input file
^C
$ git am --abort
Resolve operation not in progress, we are not resuming.
This happens because the following test fails:
test -d "$dotest" && test -f "$dotest/last" && test -f "$dotest/next"
and the codepath for an "am in-progress" is not executed. It falls back
to the codepath that treats this as a "fresh execution". Before
rr/rebase-autostash, this condition was
test -d "$dotest"
It would incorrectly execute the "normal" am --abort codepath:
git read-tree --reset -u HEAD ORIG_HEAD
git reset ORIG_HEAD
by incorrectly assuming that an am is "in progress" (i.e. ORIG_HEAD
etc. was written during the previous execution).
Notice that
$ git am
^C
executes nothing of significance, is equivalent to
$ mkdir .git/rebase-apply
Therefore, the correct solution is to treat .git/rebase-apply as a
"stray directory" and remove it on --abort in the fresh-execution
codepath. Also ensure that we're not called with --rebasing from
git-rebase--am.sh; in that case, it is the responsibility of the caller
to handle and stray directories.
While at it, tell the user to run "git am --abort" to get rid of the
stray $dotest directory, if she attempts anything else.
Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
git-am.sh | 17 +++++++++++++++++
t/t4150-am.sh | 6 ++++++
2 files changed, 23 insertions(+)
diff --git a/git-am.sh b/git-am.sh
index 1cf3d1d..91a2bcc 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -506,6 +506,23 @@ then
esac
rm -f "$dotest/dirtyindex"
else
+ # Possible stray $dotest directory in the independent-run
+ # case; in the --rebasing case, it is upto the caller
+ # (git-rebase--am) to take care of stray directories.
+ if test -d "$dotest" && test -z "$rebasing"
+ then
+ case "$skip,$resolved,$abort" in
+ ,,t)
+ rm -fr "$dotest"
+ exit 0
+ ;;
+ *)
+ die "$(eval_gettext "Stray \$dotest directory found.
+Use \"git am --abort\" to remove it.")"
+ ;;
+ esac
+ fi
+
# 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.")"
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 12f6b02..6c2cc3e 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -363,6 +363,12 @@ test_expect_success 'am --skip works' '
test_cmp expected another
'
+test_expect_success 'am --abort removes a stray directory' '
+ mkdir .git/rebase-apply &&
+ git am --abort &&
+ test_path_is_missing .git/rebase-apply
+'
+
test_expect_success 'am --resolved works' '
echo goodbye >expected &&
rm -fr .git/rebase-apply &&
--
1.8.3.1.383.g8881048.dirty
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] t/am: use test_path_is_missing() where appropriate
2013-06-15 12:43 [PATCH v3 0/2] Fix am with stray $dotest directory Ramkumar Ramachandra
2013-06-15 12:43 ` [PATCH v3 1/2] am: handle " Ramkumar Ramachandra
@ 2013-06-15 12:43 ` Ramkumar Ramachandra
1 sibling, 0 replies; 8+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-15 12:43 UTC (permalink / raw
To: Git List; +Cc: Junio C Hamano
Replace instances of ! test -d with test_path_is_missing.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t4150-am.sh | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 6c2cc3e..5edb79a 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -147,7 +147,7 @@ test_expect_success 'am applies patch correctly' '
git checkout first &&
test_tick &&
git am <patch1 &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code second &&
test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
@@ -158,7 +158,7 @@ test_expect_success 'am applies patch e-mail not in a mbox' '
git reset --hard &&
git checkout first &&
git am patch1.eml &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code second &&
test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
@@ -169,7 +169,7 @@ test_expect_success 'am applies patch e-mail not in a mbox with CRLF' '
git reset --hard &&
git checkout first &&
git am patch1-crlf.eml &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code second &&
test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
@@ -180,7 +180,7 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' '
git reset --hard &&
git checkout first &&
git am patch1-ws.eml &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code second &&
test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
@@ -206,7 +206,7 @@ test_expect_success 'am changes committer and keeps author' '
git reset --hard &&
git checkout first &&
git am patch2 &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
test "$(git rev-parse master^^)" = "$(git rev-parse HEAD^^)" &&
git diff --exit-code master..HEAD &&
git diff --exit-code master^..HEAD^ &&
@@ -258,7 +258,7 @@ test_expect_success 'am --keep really keeps the subject' '
git reset --hard &&
git checkout HEAD^ &&
git am --keep patch4 &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git cat-file commit HEAD >actual &&
grep "Re: Re: Re: \[PATCH 1/5 v2\] \[foo\] third" actual
'
@@ -268,7 +268,7 @@ test_expect_success 'am --keep-non-patch really keeps the non-patch part' '
git reset --hard &&
git checkout HEAD^ &&
git am --keep-non-patch patch4 &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git cat-file commit HEAD >actual &&
grep "^\[foo\] third" actual
'
@@ -283,7 +283,7 @@ test_expect_success 'am -3 falls back to 3-way merge' '
test_tick &&
git commit -m "copied stuff" &&
git am -3 lorem-move.patch &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code lorem
'
@@ -297,7 +297,7 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' '
test_tick &&
git commit -m "copied stuff" &&
git am -3 -p0 lorem-zero.patch &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code lorem
'
@@ -307,7 +307,7 @@ test_expect_success 'am can rename a file' '
git reset --hard &&
git checkout lorem^0 &&
git am rename.patch &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git update-index --refresh &&
git diff --exit-code rename
'
@@ -318,7 +318,7 @@ test_expect_success 'am -3 can rename a file' '
git reset --hard &&
git checkout lorem^0 &&
git am -3 rename.patch &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git update-index --refresh &&
git diff --exit-code rename
'
@@ -329,7 +329,7 @@ test_expect_success 'am -3 can rename a file after falling back to 3-way merge'
git reset --hard &&
git checkout lorem^0 &&
git am -3 rename-add.patch &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git update-index --refresh &&
git diff --exit-code rename
'
@@ -358,7 +358,7 @@ test_expect_success 'am pauses on conflict' '
test_expect_success 'am --skip works' '
echo goodbye >expected &&
git am --skip &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code lorem2^^ -- file &&
test_cmp expected another
'
@@ -379,7 +379,7 @@ test_expect_success 'am --resolved works' '
echo resolved >>file &&
git add file &&
git am --resolved &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
test_cmp expected another
'
@@ -388,7 +388,7 @@ test_expect_success 'am takes patches from a Pine mailbox' '
git reset --hard &&
git checkout first &&
cat pine patch1 | git am &&
- ! test -d .git/rebase-apply &&
+ test_path_is_missing .git/rebase-apply &&
git diff --exit-code master^..HEAD
'
@@ -397,7 +397,7 @@ test_expect_success 'am fails on mail without patch' '
git reset --hard &&
test_must_fail git am <failmail &&
git am --abort &&
- ! test -d .git/rebase-apply
+ test_path_is_missing .git/rebase-apply
'
test_expect_success 'am fails on empty patch' '
@@ -406,7 +406,7 @@ test_expect_success 'am fails on empty patch' '
echo "---" >>failmail &&
test_must_fail git am <failmail &&
git am --skip &&
- ! test -d .git/rebase-apply
+ test_path_is_missing .git/rebase-apply
'
test_expect_success 'am works from stdin in subdirectory' '
--
1.8.3.1.383.g8881048.dirty
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-15 12:43 ` [PATCH v3 1/2] am: handle " Ramkumar Ramachandra
@ 2013-06-17 2:45 ` Junio C Hamano
2013-06-17 17:54 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2013-06-17 2:45 UTC (permalink / raw
To: Ramkumar Ramachandra; +Cc: Git List
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> else
> + # Possible stray $dotest directory in the independent-run
> + # case; in the --rebasing case, it is upto the caller
> + # (git-rebase--am) to take care of stray directories.
> + if test -d "$dotest" && test -z "$rebasing"
The $rebasing variable is set only when the command line of "git am"
has --rebasing, i.e. only when it is driven by "git rebase". So
this will not affect what happens to "git am --abort" that is run by
the end user. That sounds like a safe thing to do as far as "am" is
concerned.
Will replace what has been queued on 'pu'. Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-17 2:45 ` Junio C Hamano
@ 2013-06-17 17:54 ` Junio C Hamano
2013-06-18 7:05 ` Ramkumar Ramachandra
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2013-06-17 17:54 UTC (permalink / raw
To: Ramkumar Ramachandra; +Cc: Git List
Junio C Hamano <gitster@pobox.com> writes:
> Will replace what has been queued on 'pu'.
... after fixing an indentation error, that is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-17 17:54 ` Junio C Hamano
@ 2013-06-18 7:05 ` Ramkumar Ramachandra
2013-06-18 14:31 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-18 7:05 UTC (permalink / raw
To: Junio C Hamano; +Cc: Git List
Junio C Hamano wrote:
>> Will replace what has been queued on 'pu'.
>
> ... after fixing an indentation error, that is.
Where did the error occur? I thought I taught my Emacs to always
indent properly.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-18 7:05 ` Ramkumar Ramachandra
@ 2013-06-18 14:31 ` Junio C Hamano
2013-06-18 14:44 ` Ramkumar Ramachandra
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2013-06-18 14:31 UTC (permalink / raw
To: Ramkumar Ramachandra; +Cc: Git List
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Junio C Hamano wrote:
>>> Will replace what has been queued on 'pu'.
>>
>> ... after fixing an indentation error, that is.
>
> Where did the error occur?
I think you can compare what is in 'pu' and what you sent out
without asking.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] am: handle stray $dotest directory
2013-06-18 14:31 ` Junio C Hamano
@ 2013-06-18 14:44 ` Ramkumar Ramachandra
0 siblings, 0 replies; 8+ messages in thread
From: Ramkumar Ramachandra @ 2013-06-18 14:44 UTC (permalink / raw
To: Junio C Hamano; +Cc: Git List
Junio C Hamano wrote:
>> Where did the error occur?
>
> I think you can compare what is in 'pu' and what you sent out
> without asking.
Ah, I sent out the series before I taught Emacs to indent properly.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-06-18 14:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-15 12:43 [PATCH v3 0/2] Fix am with stray $dotest directory Ramkumar Ramachandra
2013-06-15 12:43 ` [PATCH v3 1/2] am: handle " Ramkumar Ramachandra
2013-06-17 2:45 ` Junio C Hamano
2013-06-17 17:54 ` Junio C Hamano
2013-06-18 7:05 ` Ramkumar Ramachandra
2013-06-18 14:31 ` Junio C Hamano
2013-06-18 14:44 ` Ramkumar Ramachandra
2013-06-15 12:43 ` [PATCH v3 2/2] t/am: use test_path_is_missing() where appropriate Ramkumar Ramachandra
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).