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

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

Junio C Hamano wrote:

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:

The 06/08/09, Brandon Casey 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(-)

The 07/08/09, Nicolas Sebrecht wrote:

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

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(-)

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(-)

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(-)

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

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

Jeff King wrote:

:)

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(-)

The 06/08/09, Brandon Casey wrote:

<...>

UUOC, I guess.

                                                   ^^

Is it still needed?

The 07/08/09, Nicolas Sebrecht wrote:

Owned by the tabulation, sorry.

Do we still need the "$1"?

Nicolas Sebrecht 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. I think it's a little strange like that though...

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

-brandon

The 06/08/09, Brandon Casey wrote:

Yes.

I'm not sure why this comment. The former

  sed "$1"

whithout anything else is enough.

Nicolas Sebrecht 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.

-brandon

The 06/08/09, Brandon Casey wrote:

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 <nicolas.s.dev@gmx.fr> writes:

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>" )

Nicolas Sebrecht wrote:

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.

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

The 06/08/09, Brandon Casey wrote:

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 wrote:

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

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

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.

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

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 <nanako3@lavabit.com> writes:

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.

Nanako Shiraishi wrote:

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