What's in git.git (Aug 2009, #01; Wed, 05)

What's in git.git (Aug 2009, #01; Wed, 05)

[Junio C Hamano]

The 1.6.4 release seems to have been quite solid, and there is no
brown-paper-bag bugfixes on 'maint' yet ;-).

A handful of topics have graduated to 'master'.

* The 'maint' branch has these fixes since the last announcement.

Björn Steinbrink (1):
  config: Keep inner whitespace verbatim

Erik Faye-Lund (1):
  send-email: remove debug trace

Jakub Narebski (1):
  gitweb/README: Document $base_url

Jens Lehmann (1):
  Documentation: git submodule: add missing options to synopsis

Matthieu Moy (1):
  Better usage string for reflog.

Miklos Vajna (1):
  hg-to-git: don't import the unused popen2 module


* The 'master' branch has these since the last announcement
  in addition to the above.

André Goddard Rosa (1):
  Fix typos on pt_BR/gittutorial.txt translation

Geoffrey Irving (1):
  git fast-export: add --no-data option

Giuseppe Bilotta (1):
  gitweb: fix 'Use of uninitialized value' error in href()

Jeff King (2):
  show: suppress extra newline when showing annotated tag
  show: add space between multiple items

Johannes Schindelin (1):
  parse-opt: optionally show "--no-" option string

Junio C Hamano (1):
  apply: notice creation/removal patches produced by GNU diff

Michael J Gruber (3):
  t6010-merge-base.sh: Depict the octopus test graph
  git-merge-base/git-show-branch: Cleanup documentation and usage
  git-merge-base/git-show-branch --merge-base: Documentation and test

Michał Kiedrowicz (1):
  init-db: migrate to parse-options

Nanako Shiraishi (1):
  git init: optionally allow a directory argument

Nick Edelen (1):
  Shift object enumeration out of upload-pack

Santi Béjar (2):
  t5520-pull: Test for rebased upstream + fetch + pull --rebase
  pull: support rebased upstream + fetch + pull --rebase

Stephen Boyd (7):
  read-tree: convert unhelpful usage()'s to helpful die()'s
  read-tree: migrate to parse-options
  write-tree: migrate to parse-options
  verify-tag: migrate to parse-options
  verify-pack: migrate to parse-options
  prune-packed: migrate to parse-options
  technical-docs: document tree-walking API

Wesley J. Landaker (2):
  Documentation: git-send-email: fix submission port number
  Documentation: git-send-email: correct statement about standard ports

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Brandon Casey]

Junio C Hamano wrote:
> The 1.6.4 release seems to have been quite solid, and there is no
> brown-paper-bag bugfixes on 'maint' yet ;-).

Found one.

I didn't realize the whole git-am discussion did _not_ result in a
fix being applied.  But git-am will currently refuse to apply any
patch from email that does not have "From " or "From: " in the first
three lines of the email.  For those of us whose mail servers prepend
many lines of the form:

Received: from XXX ([XXX]) by XXX with Microsoft SMTPSVC(6.0.3790.2825);
	 Tue, 14 Jul 2009 07:24:06 -0500
Received: by XXX id n6ECJvlh010405; Tue, 14 Jul 2009 07:24:05 -0500
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751929AbZGNMYD (ORCPT <rfc822;XXX@XXX>);
	Tue, 14 Jul 2009 08:24:03 -0400
Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752103AbZGNMYC
	(ORCPT <rfc822;git-outgoing>); Tue, 14 Jul 2009 08:24:02 -0400
Received: from mail-ew0-f226.google.com ([209.85.219.226]:50485 "EHLO
	mail-ew0-f226.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751626AbZGNMYB (ORCPT <rfc822;git@vger.kernel.org>);
	Tue, 14 Jul 2009 08:24:01 -0400

We can not apply any patches saved from email.


I think we should at least do this to fall back to mbox format:

diff --git a/git-am.sh b/git-am.sh
index d64d997..94fa9c9 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -187,6 +187,7 @@ check_patch_format () {
 				patch_format=stgit
 				;;
 			*)
+				patch_format=mbox
 				;;
 			esac
 			;;

-brandon

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Nicolas Sebrecht]

The 06/08/09, Brandon Casey wrote:

> I think we should at least do this to fall back to mbox format:
> 
> diff --git a/git-am.sh b/git-am.sh
> index d64d997..94fa9c9 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -187,6 +187,7 @@ check_patch_format () {
>  				patch_format=stgit
>  				;;
>  			*)
> +				patch_format=mbox
>  				;;
>  			esac
>  			;;

This is even better that all my crap. But I think we should squash
this:

---
 git-am.sh |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 94fa9c9..e8ec8d7 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -254,9 +254,6 @@ split_patches () {
                this=
                msgnum=
                ;;
-       *)
-               clean_abort "Patch format $patch_format is not supported."
-               ;;
        esac
 }


-- 
Nicolas Sebrecht

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Nicolas Sebrecht]

The 07/08/09, Nicolas Sebrecht wrote:
> 
> This is even better that all my crap. But I think we should squash
> this:
> 
> ---
>  git-am.sh |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/git-am.sh b/git-am.sh
> index 94fa9c9..e8ec8d7 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -254,9 +254,6 @@ split_patches () {
>                 this=
>                 msgnum=
>                 ;;
> -       *)
> -               clean_abort "Patch format $patch_format is not supported."
> -               ;;
>         esac
>  }

Which is wrong as it could be given at the command line. Sorry for the
noise.

-- 
Nicolas Sebrecht

[PATCH 1/3] t4150-am: demonstrate git-am's failure to handle some patch emails

[Brandon Casey]

From: Brandon Casey <drafnel@gmail.com>

Recently git-am gained the ability to detect and apply patches from some
foreign VCS's.  The detection of traditional patch emails though is somewhat
limited and will fail if a "From" field is not detected in the first three
lines of the email header.  Demonstrate the failure by supplying a
perfectly valid email to git-am which it formerly could successfully apply.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 t/t4150-am.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index a12bf84..ad2a85f 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -77,6 +77,12 @@ test_expect_success setup '
 	git commit -s -F msg &&
 	git tag second &&
 	git format-patch --stdout first >patch1 &&
+	{
+		echo "X-Fake-Field: Line One" &&
+		echo "X-Fake-Field: Line Two" &&
+		echo "X-Fake-Field: Line Three" &&
+		git format-patch --stdout first | sed -e "1d"
+	} > patch1.eml &&
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
 	test_tick &&
@@ -108,6 +114,15 @@ test_expect_success 'am applies patch correctly' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_failure 'am correctly applies patch from email lacking "From" in first 3 lines' '
+	git checkout first &&
+	git am patch1.eml &&
+	! test -d .git/rebase-apply &&
+	test -z "$(git diff second)" &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
 GIT_AUTHOR_NAME="Another Thor"
 GIT_AUTHOR_EMAIL="a.thor@example.com"
 GIT_COMMITTER_NAME="Co M Miter"
-- 
1.6.4

[PATCH 2/3] mailinfo: allow individual e-mail files as input

[Brandon Casey]

From: Junio C Hamano <gitster@pobox.com>

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.

This fixes the test in t4150-am.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---


Junio,

You'll notice that I changed your grep -E to an egrep and dropped the -e.
I do not see any other grep which uses -e, and I seem to recall Jeff King
actively removing -e claiming that some greps do not recognize it.  I do not
have a perfect memory though, so apologies to Jeff if I am mistaken.

-brandon


 git-am.sh     |   14 ++++++++++++++
 t/t4150-am.sh |    2 +-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index d64d997..dd60f5d 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" |
+			egrep -v '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
+			patch_format=mbox
+		fi
 	} < "$1" || clean_abort
 }
 
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index ad2a85f..4e8e176 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -114,7 +114,7 @@ test_expect_success 'am applies patch correctly' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
-test_expect_failure 'am correctly applies patch from email lacking "From" in first 3 lines' '
+test_expect_success 'am correctly applies patch from email lacking "From" in first 3 lines' '
 	git checkout first &&
 	git am patch1.eml &&
 	! test -d .git/rebase-apply &&
-- 
1.6.4

[PATCH 3/3] git-am: print fair error message when format detection fails

[Brandon Casey]

From: Nicolas Sebrecht <ni.s@laposte.net>

Avoid git ending with this message:
	"Patch format  is not supported."

With improved error message in the format detection failure case by
Giuseppe Bilotta.

Signed-off-by: Nicolas Sebrecht <ni.s@laposte.net>
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 git-am.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index dd60f5d..f719f6e 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -268,7 +268,11 @@ split_patches () {
 		msgnum=
 		;;
 	*)
-		clean_abort "Patch format $patch_format is not supported."
+		if test -n "$parse_patch" ; then
+			clean_abort "Patch format $patch_format is not supported."
+		else
+			clean_abort "Patch format detection failed."
+		fi
 		;;
 	esac
 }
-- 
1.6.4

Re: [PATCH 2/3] mailinfo: allow individual e-mail files as input

[Jeff King]

On Thu, Aug 06, 2009 at 08:08:12PM -0500, Brandon Casey wrote:

> You'll notice that I changed your grep -E to an egrep and dropped the -e.
> I do not see any other grep which uses -e, and I seem to recall Jeff King
> actively removing -e claiming that some greps do not recognize it.  I do not
> have a perfect memory though, so apologies to Jeff if I am mistaken.

Fortunately git does have a perfect memory:

  $ git log -1 --all-match --author=peff --grep=grep
  commit 759ad19e772a79a2a5ae6b7377d57eb21d29e6a0
  Author: Jeff King <peff@peff.net>
  Date:   Wed Oct 22 15:22:53 2008 -0400

      submodule: fix some non-portable grep invocations

      Not all greps support "-e", but in this case we can easily
      convert it to a single extended regex.

The grep in question is Solaris 8's /usr/bin/grep (which also needs
"egrep" instead of "-E", as you already did).

-Peff

Re: [PATCH 2/3] mailinfo: allow individual e-mail files as input

[Brandon Casey]

Jeff King wrote:
> On Thu, Aug 06, 2009 at 08:08:12PM -0500, Brandon Casey wrote:
> 
>> You'll notice that I changed your grep -E to an egrep and dropped the -e.
>> I do not see any other grep which uses -e, and I seem to recall Jeff King
>> actively removing -e claiming that some greps do not recognize it.  I do not
>> have a perfect memory though, so apologies to Jeff if I am mistaken.
> 
> Fortunately git does have a perfect memory:

:)

[PATCH v2] mailinfo: allow individual e-mail files as input

[Brandon Casey]

From: Junio C Hamano <gitster@pobox.com>

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.

This fixes the test in t4150-am.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---


Maybe the second patch should be replaced with this one.  The original did
not test the first three lines for conformance to RFC2822.

-brandon


 git-am.sh     |   19 +++++++++++++++++++
 t/t4150-am.sh |    2 +-
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index d64d997..49f2be4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -191,6 +191,25 @@ 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...
+			{
+				echo "$l1"
+				echo "$l2"
+				echo "$l3"
+				cat
+			} | sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
+			egrep -v '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
+			patch_format=mbox
+		fi
 	} < "$1" || clean_abort
 }
 
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index ad2a85f..4e8e176 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -114,7 +114,7 @@ test_expect_success 'am applies patch correctly' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
-test_expect_failure 'am correctly applies patch from email lacking "From" in first 3 lines' '
+test_expect_success 'am correctly applies patch from email lacking "From" in first 3 lines' '
 	git checkout first &&
 	git am patch1.eml &&
 	! test -d .git/rebase-apply &&
-- 
1.6.4

[PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Nicolas Sebrecht]

The 06/08/09, Brandon Casey wrote:

> diff --git a/git-am.sh b/git-am.sh
> index d64d997..49f2be4 100755
> --- a/git-am.sh
> +++ b/git-am.sh

<...>

> +			{
> +				echo "$l1"
> +				echo "$l2"
> +				echo "$l3"
> +				cat

UUOC, I guess.

> +			} | sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
                                                   ^^

Is it still needed?

-- 
Nicolas Sebrecht

[PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Nicolas Sebrecht]

The 07/08/09, Nicolas Sebrecht wrote:
> The 06/08/09, Brandon Casey wrote:
> 
> > diff --git a/git-am.sh b/git-am.sh
> > index d64d997..49f2be4 100755
> > --- a/git-am.sh
> > +++ b/git-am.sh
> 
> <...>
> 
> > +			{
> > +				echo "$l1"
> > +				echo "$l2"
> > +				echo "$l3"
> > +				cat
> 
> UUOC, I guess.
> 
> > +			} | sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
>                                                    ^^

Owned by the tabulation, sorry.

Do we still need the "$1"?

-- 
Nicolas Sebrecht

Re: [PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Brandon Casey]

Nicolas Sebrecht wrote:
> The 07/08/09, Nicolas Sebrecht wrote:
>> The 06/08/09, Brandon Casey wrote:
>>
>>> diff --git a/git-am.sh b/git-am.sh
>>> index d64d997..49f2be4 100755
>>> --- a/git-am.sh
>>> +++ b/git-am.sh
>> <...>
>>
>>> +			{
>>> +				echo "$l1"
>>> +				echo "$l2"
>>> +				echo "$l3"
>>> +				cat
>> UUOC, I guess.
>>
>>> +			} | sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
>>                                                    ^^
> 
> Owned by the tabulation, sorry.
> 
> Do we still need the "$1"?

Whoops, I missed that "$1" argument to sed.  That means the v2 followup
patch is unnecessary since the sed is operating on a file argument
and _not_ stdin.  I think it's a little strange like that though...

   {
      sed "$1"
   } < "$1"

-brandon

[PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Nicolas Sebrecht]

The 06/08/09, Brandon Casey wrote:

> Whoops, I missed that "$1" argument to sed.  That means the v2 followup
> patch is unnecessary since the sed is operating on a file argument
> and _not_ stdin.

Yes.

>                   I think it's a little strange like that though...
> 
>    {
>       sed "$1"
>    } < "$1"

I'm not sure why this comment. The former

  sed "$1"

whithout anything else is enough.

-- 
Nicolas Sebrecht

Re: [PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Brandon Casey]

Nicolas Sebrecht wrote:
> The 06/08/09, Brandon Casey wrote:
>>                   I think it's a little strange like that though...
>>
>>    {
>>       sed "$1"
>>    } < "$1"
> 
> I'm not sure why this comment. The former
> 
>   sed "$1"
> 
> whithout anything else is enough.

The "former", or Junio's original patch, effectively has this form:

   {
      sed "$1"
   } < "$1"

Without reading closely enough, I thought it looked like this:

   {
      sed
   } < "$1"

Since I didn't study the sed statement closely enough, I assumed that it was
operating on the remaining portion of the patch email that was redirected to
the block on stdin.  I missed the fact that the file name was supplied to
it.  My comment was that I found it strange (and maybe unintuitive, or maybe
it's just me) that "$1" was piped on stdin and it was supplied as an
argument to sed.

-brandon

[PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Nicolas Sebrecht]

The 06/08/09, Brandon Casey wrote:

> The "former", or Junio's original patch, effectively has this form:
> 
>    {
>       sed "$1"
>    } < "$1"
> 
> Without reading closely enough, I thought it looked like this:
> 
>    {
>       sed
>    } < "$1"
> 
> Since I didn't study the sed statement closely enough, I assumed that it was
> operating on the remaining portion of the patch email that was redirected to
> the block on stdin.  I missed the fact that the file name was supplied to
> it.  My comment was that I found it strange (and maybe unintuitive, or maybe
> it's just me) that "$1" was piped on stdin and it was supplied as an
> argument to sed.

Thinking to this a bit more, I tend to think that your intention to get
rid of the "$1" argument of sed is the right thing to do.

It really seems like the argument has precedence to the redirection

  _but_

I couldn't find any reference to this case in POSIX and I guess that the
behaviour may differ between implementations of sed. I don't know.
Perhaps somebody could tell us if our hesitation is justified (or not)?

Finally and to prevent strange behaviours, I would write

  {
    real l1
    real l2
    real l3
    {
      echo "$l1"
      echo "$l2"
      echo "$l3"
      cat
    } | sed
  } < "$1"

instead of

  {
    real l1
    real l2
    real l3
    sed "$1"
  } < "$1"

because the latter may contain either the content of the whole file
(coming from the argument) or the content of the file _whithout_ the
first three lines (coming from the redirection '<' amputated by the
'read' statements).

Junio?

-- 
Nicolas Sebrecht

Re: [PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Junio C Hamano]

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> It really seems like the argument has precedence to the redirection
> I couldn't find any reference to this case in POSIX and I guess that the
> behaviour may differ between implementations of sed.
> Perhaps somebody could tell us if our hesitation is justified (or not)?

If you give file arguments, standard input is irrelevant to sed.  I bet you
do:

	$ sed -e something file

all the time, and you rely on the command not to get stuck after reading
the file because now it wants to continue reading from its stdin, which is
connected to your terminal.  Have you ever worried about having to type ^D
after every such invocation of sed?  I bet you never have ;-)

Also, your _preferred_ alternative

	{ read l1; read l2; read l3;
          ( echo "$l1"; echo "$l2"; echo "$l3"; cat | sed )
	} <"$1"

is not just simply inefficient, but is actively wrong.  The "read" can
butcher your input.  For example, try:

	$ echo "a b " | ( read l1; echo "<$l1>" )

Re: [PATCH v2] Re: mailinfo: allow individual e-mail files as input

[Brandon Casey]

Nicolas Sebrecht wrote:
> The 06/08/09, Brandon Casey wrote:
> 
>> diff --git a/git-am.sh b/git-am.sh
>> index d64d997..49f2be4 100755
>> --- a/git-am.sh
>> +++ b/git-am.sh
> 
> <...>
> 
>> +			{
>> +				echo "$l1"
>> +				echo "$l2"
>> +				echo "$l3"
>> +				cat
> 
> UUOC, I guess.

I needed to use google to figure out that UUOC means Useless Use Of Cat,
but I think you are mistaken.  Rather than trying to explain it, try this
with and without 'cat' commented out:

#!/bin/sh

{
    {
        echo "line one"
        echo "line two"
        cat
    } | sed -e 's/$/Q/'
} <<-EOF
This is a line of text
Here is another line of text.
And another
EOF

Hopefully you'll see the parallels to the sequence in git-am.sh and understand
that cat was used to send the rest of the email through sed along with the first
three lines that were read explicitly.  git-am.sh looks more like this:

   {
      read l1
      ...
      {
         echo "$l1"
         ...
         cat
      } | sed ...
   } << "$1"

At least, I thought that is how it looked until I read your other email where
you pointed out that "$1" is an argument to sed.

>> +			} | sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
>                                                    ^^
> 
> Is it still needed?

Yes.  The '/^[ 	]/d' portion of the sed statement deletes any lines with
leading space or tab.  This avoids passing continuation fields to the
grep statement which is not designed for them, and so would fail (or
match, depending on how you look at it.  We used -v with grep).

-brandon

[PATCH 2/3] Re: mailinfo: allow individual e-mail files as input

[Nicolas Sebrecht]

The 06/08/09, Brandon Casey wrote:

>  git-am.sh     |   14 ++++++++++++++
>  t/t4150-am.sh |    2 +-
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/git-am.sh b/git-am.sh
> index d64d997..dd60f5d 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" |
> +			egrep -v '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
> +			patch_format=mbox
> +		fi
>  	} < "$1" || clean_abort
>  }

May I ask why you resurrect this "first three lines check for rfc2822"
instead of dumbly falling back to the "mbox" patch_format? Performance?

-- 
Nicolas Sebrecht

Re: [PATCH 2/3] Re: mailinfo: allow individual e-mail files as input

[Brandon Casey]

Nicolas Sebrecht wrote:
> The 06/08/09, Brandon Casey wrote:
> 
>>  git-am.sh     |   14 ++++++++++++++
>>  t/t4150-am.sh |    2 +-
>>  2 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/git-am.sh b/git-am.sh
>> index d64d997..dd60f5d 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" |
>> +			egrep -v '^[A-Za-z]+(-[A-Za-z]+)*:' >/dev/null ||
>> +			patch_format=mbox
>> +		fi
>>  	} < "$1" || clean_abort
>>  }
> 
> May I ask why you resurrect this "first three lines check for rfc2822"
> instead of dumbly falling back to the "mbox" patch_format? Performance?

This at least checks that the header has the correct form for an email.

The dumb fallback to mbox format would just blindly pass the patch to
mailsplit which (I think) would just dump out an improperly formatted
email.  git-am would then start the process of applying the malformed
patch and fail.  With this patch, we can catch the failure earlier
and hopefully provide a better complaint to the user.

-brandon

Re: [PATCH 2/3] mailinfo: allow individual e-mail files as input

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> You'll notice that I changed your grep -E to an egrep and dropped the -e.

That's the right thing to do.  I was re-reading Nicolas Sebrecht's series
and checked with  "git grep -e 'egrep ' -e 'grep .*-E '" to reach the same
conclusion.  Thanks.

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Nanako Shiraishi]

Quoting Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>

> Junio C Hamano wrote:
>> The 1.6.4 release seems to have been quite solid, and there is no
>> brown-paper-bag bugfixes on 'maint' yet ;-).
>
> Found one.
>
> I didn't realize the whole git-am discussion did _not_ result in a
> fix being applied.  But git-am will currently refuse to apply any
> patch from email that does not have "From " or "From: " in the first
> three lines of the email.  For those of us whose mail servers prepend
> many lines of the form:
>
> Received: from XXX ([XXX]) by XXX with Microsoft SMTPSVC(6.0.3790.2825);
> 	 Tue, 14 Jul 2009 07:24:06 -0500

According to an already hashed out discussion, that isn't a mbox format that has been supported, so it isn't even a bug. For details, see e.g.

    http://thread.gmane.org/gmane.comp.version-control.git/123338/focus=123355

And Nicolas Sebrecht has been working with Junio to implement an enhancement to add support for the "individual piece of email" format.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> And Nicolas Sebrecht has been working with Junio to implement an
> enhancement to add support for the "individual piece of email" format.

I'll squash patches 1+2/3 from Brandon into a single change after
retitling it to read "am" not "mailinfo", and queue the result, together
with patch 3/3.

Thanks, Brandon and Nicolas.

Re: What's in git.git (Aug 2009, #01; Wed, 05)

[Brandon Casey]

Nanako Shiraishi wrote:
> Quoting Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
> 
>> Junio C Hamano wrote:
>>> The 1.6.4 release seems to have been quite solid, and there is no
>>> brown-paper-bag bugfixes on 'maint' yet ;-).
>> Found one.
>>
>> I didn't realize the whole git-am discussion did _not_ result in a
>> fix being applied.  But git-am will currently refuse to apply any
>> patch from email that does not have "From " or "From: " in the first
>> three lines of the email.  For those of us whose mail servers prepend
>> many lines of the form:
>>
>> Received: from XXX ([XXX]) by XXX with Microsoft SMTPSVC(6.0.3790.2825);
>> 	 Tue, 14 Jul 2009 07:24:06 -0500
> 
> According to an already hashed out discussion, that isn't a mbox format that has been supported, so it isn't even a bug. For details, see e.g.
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/123338/focus=123355
> 
> And Nicolas Sebrecht has been working with Junio to implement an enhancement to add support for the "individual piece of email" format.

Commit b3f041fb0f7de167dbb6711b0a231d36c4b5de08 titled 'git-am support for
naked email messages (take 2)' by H. Peter Anvin from December 2005 seems
to indicate otherwise.

IMHO, I think something was indeed broken here, but it is a moot point
since it will all be fixed (or "enhanced" depending on your POV) soon. :)

-brandon