From: Jeff King <peff@peff.net>
Date: Thu, 13 Mar 2008 12:59:20 GMT
Subject: Re: [PATCH 08/16] t4200: use cut instead of sed
Message-ID: <20080313125920.GF19485@coredump.intra.peff.net>
In-Reply-To: <7vejaf1b0d.fsf@gitster.siamese.dyndns.org>

On Wed, Mar 12, 2008 at 09:52:18PM -0700, Junio C Hamano wrote:

> > Some versions of sed (like the one on Solaris) don't like to
> > match literal tabs, and simply print nothing. Instead, let's
> > use cut.
> 
> > -sha1=$(sed -e 's/	.*//' .git/rr-cache/MERGE_RR)
> 
> This is a bit hard to believe.  On one of my ancient Sun box:

Ah, sorry. I tested this line by hand, found it didn't work, and
stupidly jumped to the assumption that it was the literal tab (that
being the only interesting thing in the input).

But the actual problem is that MERGE_RR lacks a trailing newline. I
don't see any code to add newlines, even though it seems possible that
we will write out several paths. So I think we need a newline here:

diff --git a/builtin-rerere.c b/builtin-rerere.c
index c607aad..e4a1dc1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
 		int length = strlen(path) + 1;
 		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
 		    write_in_full(out_fd, "\t", 1) != 1 ||
-		    write_in_full(out_fd, path, length) != length)
+		    write_in_full(out_fd, path, length) != length ||
+		    write_in_full(out_fd, "\n", 1) != 1)
 			die("unable to write rerere record");
 	}
 	if (commit_lock_file(&write_lock) != 0)

And unless I am missing something, rerere on multiple paths is very
broken (but that seems weird, since this code is so old).

-Peff