Re: [PATCH] contrib/subtree bugfix: Can't `add` annotated tag

From: James Denholm <nod.helm@gmail.com>
Date: May 9, 2014, 07:36 UTC
Message-ID: <CAHYYfeFfo=xVDezAGFyCvuhx=bkzMF6KyDCAjZNKNoGnytXDWA@mail.gmail.com>
In-Reply-To: <xmqqoaz841d3.fsf@gitster.dls.corp.google.com>

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

> The "rev" (not "revs") seems to be used by more things than the
> final commit-tree state.  Are we losing some useful information by
> peeling it too early like this patch does? (...)

You're not wrong, actually, peeling at the last minute (or at least later) would be a better choice. I'd suggest that we aren't losing currently-useful information (as it'd be rare-if-ever that a user would look at a hash in their commit logs and think "Oh, that's that tag!"), but certainly with future development in mind it's more ideal.

Show 5 quoted lines

> I see that add_msg does not use anything useful from latest_new, so
> with the current state of the code, it does not make that much
> difference (except that it says "from commit '$latest_new'", and by
> peeling, the fact that the user wanted to use a tag is lost from the
> result).

Yeah, that might be a worthy thing to porcelain-up in the future with logging the tag name rather than, or in addition to, the hash, as well as a similar change in add_squashed_msg.

Show 5 quoted lines

> Would it be sufficient to do
>
>         git commit-tree $tree $headp -p "$rev^0"
>
> in that "not squashing" codepath instead?

On line 561, sure. Do you want me to do a re-roll?