Re: [PATCH 2/2] pack-refs: add fully-peeled trait
- From
Jeff King <peff@peff.net>
- Date
- Mar 17, 2013, 05:55 UTC
- Message-ID
- <20130317055520.GC16070@sigill.intra.peff.net>
- In-Reply-To
- <7vvc8qshgm.fsf@alter.siamese.dyndns.org>
On Sat, Mar 16, 2013 at 10:50:17PM -0700, Junio C Hamano wrote:
Show 26 quoted lines
> I however wonder if the above implies it may make sense to add this
> on top? Perhaps it is not worth it, because it makes a difference
> only to a repository with annotated tags outside refs/tags hierarchy
> and still has the packed-refs file that was created with an older
> version of Git, so we can just tell "repack with new Git" to users
> with such a repository.
>
> refs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 7f84efd..afc4dde 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -847,8 +847,10 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
> refline[0] == '^' &&
> strlen(refline) == 42 &&
> refline[41] == '\n' &&
> - !get_sha1_hex(refline + 1, sha1))
> + !get_sha1_hex(refline + 1, sha1)) {
> hashcpy(last->u.value.peeled, sha1);
> + last->flag |= REF_KNOWS_PEELED;
> + }
> }
> }
> Almost. The older version of Git would not have written those peel lines in the first place. So yes, if we saw such a file, we could assume the peel lines are valid. But nobody has ever generated that (with the except of git between my two patches).
I do think it may be worth doing, though, just because it makes the handling of the flag more obvious; somebody reading it later would wonder "hey, shouldn't we be setting REF_KNOWS_PEELED here?", and it is simple and harmless to just do it, rather than confusing a later reader.
I'll re-roll in a second to incorporate the comments from Michael.
-Peff