Re: [PATCH v3] git-am: fix maildir support regression: accept email file as patch

Junio C Hamano [off-list ref] writes:

Please describe what that new something is.
...
Running three independent printf piped to two processes in a loop is
quite silly.
...

That is, something like this.

-- >8 --
Subject: mailinfo: allow individual e-mail files as input

We traditionally allowed a mbox file or a directory name of a maildir (but
never an individual file inside a maildir) to be given to "git am".  Even
though an individual file in a maildir (or more generally, a piece of
RFC2822 e-mail) is not a mbox file, it contains enough information to
create a commit out of it, so there is no reason to reject one.  Running
mailsplit on such a file feels stupid, but it does not hurt.

This builds on top of a5a6755 (git-am foreign patch support: introduce
patch_format, 2009-05-27) that introduced mailbox format detection.  The
codepath to deal with a mbox requires it to begin with "From " line and
also allows it to begin with "From: ", but a random piece of e-mail can
and often do begin with any valid RFC2822 header lines.

Instead of checking the first line, we extract all the lines up to the
first empty line, and make sure they look like e-mail headers.

Signed-off-by: Junio C Hamano <redacted>
---
 git-am.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index d64d997..617ca2f 100755
--- a/git-am.sh
+++ b/git-am.sh

@@ -191,6 +191,20 @@ check_patch_format () {
 			esac
 			;;
 		esac
+		if test -z "$patch_format" &&
+			test -n "$l1" &&
+			test -n "$l2" &&
+			test -n "$l3"
+		then
+			# This begins with three non-empty lines.  Is this a
+			# piece of e-mail a-la RFC2822?  Grab all the headers,
+			# discarding the indented remainder of folded lines,
+			# and see if it looks like that they all begin with the
+			# header field names...
+			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
+			grep -v -E -e '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
+			patch_format=mbox
+		fi
 	} < "$1" || clean_abort
 }
 

The 15/07/09, Junio C Hamano wrote:

We traditionally allowed a mbox file or a directory name of a maildir (but
never an individual file inside a maildir) to be given to "git am".  Even
though an individual file in a maildir (or more generally, a piece of
RFC2822 e-mail) is not a mbox file, it contains enough information to
create a commit out of it, so there is no reason to reject one.  Running
mailsplit on such a file feels stupid, but it does not hurt.

Junio, I'm sorry but you missed the point.

It is not about adding a new feature. It's about keeping compatibility
with maildir. The current version _rejects_ good patches.

But, as it's very easy to move emails from a maildir _or_ have a symlink
which links to "maildir/cur" or whatever, we really should rely on the
content of the files.

-- 
Nicolas Sebrecht

The 16/07/09, Nicolas Sebrecht wrote:

It is not about adding a new feature. It's about keeping compatibility
with maildir. The current version _rejects_ good patches.

But, as it's very easy to move emails from a maildir _or_ have a symlink
which links to "maildir/cur" or whatever, we really should rely on the
content of the files.

Hum, it's not true. 

Symlink as parameter didn't work before. So, it breaks things like:

	$ git am symlink/*
	$ git am directory/{anything relying on shell globbing}
	$ git am patch1 patch2

This was traditionally permitted even if git-am was not designed to. I
wonder if we should add this feature as it could break end-user
workflows.

-- 
Nicolas Sebrecht