threads / patch / 4920

patchgit-am: Don't accept an mbox on stdin of we already have a .dotest directory

Subject: [PATCH] git-am: Don't accept an mbox on stdin of we already have a .dotest directory

## tl;dr

3 messages between Jul 19, 2006 and Jul 20, 2006. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Lukas Sandström· Jul 19, 2006, 20:28 UTC · lore
Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
---

It makes no sense to accept an mbox via stdin when we won't accept it on the commandline.

The patch helps the following scenario:

# git init-db "add file1 with content" # git checkout -b apply "edit file1 && commit"

# git checkout -b conflict master "edit file1 && commit"

# git checkout -b ok master "add file2"

# git checkout apply # git format-patch -k -3 master..conflict | git am -k -3 => git-am fails with a conflict message # git reset --hard

# git format-patch -k -3 master..ok | git am -k -3 => git am fails with the same conflict message as above, => since it's trying to apply the old .dotest directory

With the patch it complains about an old .dotest directory instead.

 git-am.sh |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to git-am.sh +4 −2
diff --git a/git-am.sh b/git-am.sh
index 3a129e0..04f0119 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -156,8 +156,10 @@ fi
 
 if test -d "$dotest"
 then
-	test ",$#," = ",0," ||
-	die "previous dotest directory $dotest still exists but mbox given."
+	if test ",$#," != ",0," || ! tty -s
+	then
+		die "previous dotest directory $dotest still exists but mbox given."
+	fi
 	resume=yes
 else
 	# Make sure we are not given --skip nor --resolved
-- 
1.4.1.g59817
Petr Baudis· Jul 20, 2006, 14:33 UTC · re: Lukas Sandström · lore

Re: [PATCH] git-am: Don't accept an mbox on stdin of we already have a .dotest directory

Dear diary, on Wed, Jul 19, 2006 at 10:28:00PM CEST, I got a letter where Lukas Sandström <lukass@etek.chalmers.se> said that...

> Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
> ---

Please don't be afraid of putting stuff to commit message. It can only improve things. :-)

Show 10 quoted lines
> # git format-patch -k -3 master..conflict | git am -k -3
> => git-am fails with a conflict message
> # git reset --hard
> 
> # git format-patch -k -3 master..ok | git am -k -3
> => git am fails with the same conflict message as above,
> => since it's trying to apply the old .dotest directory
> 
> With the patch it complains about an old .dotest
> directory instead.

I think this rather means that git reset --hard should clear the .dotest directory, or something (perhaps just warn)...

> -	test ",$#," = ",0," ||
> +	if test ",$#," != ",0," || ! tty -s

...but this looks like a horrible idea. Does this mean that git-am can't be now ran without a terminal? (E.g. in a cron/at job, inside a procmail rule etc.) That's bad.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam
Lukas Sandström· Jul 20, 2006, 19:35 UTC · re: Petr Baudis · lore

Re: [PATCH] git-am: Don't accept an mbox on stdin of we already have a .dotest directory

Petr Baudis wrote:
Show 21 quoted lines
> Dear diary, on Wed, Jul 19, 2006 at 10:28:00PM CEST, I got a letter
> where Lukas Sandström <lukass@etek.chalmers.se> said that...
>> Signed-off-by: Lukas Sandström <lukass@etek.chalmers.se>
>> ---
> 
> Please don't be afraid of putting stuff to commit message. It can only
> improve things. :-)
> 
>> # git format-patch -k -3 master..conflict | git am -k -3
>> => git-am fails with a conflict message
>> # git reset --hard
>>
>> # git format-patch -k -3 master..ok | git am -k -3
>> => git am fails with the same conflict message as above,
>> => since it's trying to apply the old .dotest directory
>>
>> With the patch it complains about an old .dotest
>> directory instead.
> 
> I think this rather means that git reset --hard should clear the .dotest
> directory, or something (perhaps just warn)...
But the .dotest directory is user-selectable in git-am.
Show 8 quoted lines
> 
>> -	test ",$#," = ",0," ||
>> +	if test ",$#," != ",0," || ! tty -s
> 
> ...but this looks like a horrible idea. Does this mean that git-am can't
> be now ran without a terminal? (E.g. in a cron/at job, inside a procmail
> rule etc.) That's bad.
> 

This would only stop it from running without a terminal if you have an old .dotest directory lying around, ie. you already have a failed am-session. git-am wouldn't work in this case before either, all the patch does is to give a better error-message.

/Lukas

← back to recent threads