On Sat, Mar 16, 2013 at 10:50:17PM -0700, Junio C Hamano wrote:

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to