Re: [PATCH 6/6] pack-objects: reuse on-disk deltas for thin "have" objects

From: Jeff King <peff@peff.net>
Date: Aug 21, 2018, 20:07 UTC
Message-ID: <20180821200747.GA21955@sigill.intra.peff.net>
In-Reply-To: <xmqqbm9v8pou.fsf@gitster-ct.c.googlers.com>

On Tue, Aug 21, 2018 at 12:50:09PM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> > Sorry for commenting on something completely off-topic, but when
> > applied with "git am -s", I get a resulting commit with 3 S-o-b (the
> > above two, plus the one added by "-s"), with a blank line in between
> > them.  I can understand the first blank line (the one between your
> > two S-o-b), as the first S-o-b does not even appear to be part of
> > the trailer block, but cannot explain why I get an extra one before
> > the one added by "-s".  Puzzled...
> 
> I think your original "two s-o-b with a blank line in between" was
> caused by the same problem, and "git commit --amend -s" perhaps
> added an extra one at the end, and added a blank line before the
> last "paragraph" while at it?
> 
> My suspicion is the long horizontal line at the beginning of the
> table, triggers it.  I haven't followed the code closely yet,
> though.

Ah, yeah, I think you're right. We call find_patch_start(), which thinks the "---" line is the end of the commit message. That makes sense when parsing trailers out of "format-patch" output, but not when we know we have just the commit message.

So one obvious fix is a new option for the trailer code to tell it we have _just_ a commit message. That would still leave this obvious false positive for the format-patch case, but I'm not sure it can be helped.

-Peff