Jeff King <p...@peff.net> writes:

> On Thu, Mar 14, 2013 at 01:24:48AM -0400, Jeff King wrote:
>
>> So the only question is how much work we want to put into making sure
>> the new reader handles the old writer correctly. Doing 2c is obviously
>> more rigorous, and it is not that much work to add the fully-packed
>> flag, but I kind of wonder if anybody even cares. We can just say "it's
>> a bug fix; run `git pack-refs` again if you care" and call it a day
>> (i.e., 1b).
>
> Urgh, for some reason I kept writing "fully-packed" but obviously I
> meant "fully-peeled". Hopefully you figured it out.
>
> Just as a quick sketch of how much work is in involved in 2c, I think
> the complete solution would look like this (but note I haven't tested
> this at all):

Looks reasonable from a cursory look, and I think we cannot avoid
going with the route to add "fully-peeled" token, because we do have
to express "we know this ref outside refs/tags/ area is not an
annotated tag" by marking them as REF_KNOWS_PEELED for the peel_ref()
optimization to work.



> diff --git a/pack-refs.c b/pack-refs.c
> index f09a054..261a6a6 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -27,6 +27,7 @@ static int handle_one_ref(const char *path, const unsigned 
> char *sha1,
>                         int flags, void *cb_data)
>  {
>       struct pack_refs_cb_data *cb = cb_data;
> +     struct object *o;
>       int is_tag_ref;
>  
>       /* Do not pack the symbolic refs */
> @@ -39,14 +40,12 @@ static int handle_one_ref(const char *path, const 
> unsigned char *sha1,
>               return 0;
>  
>       fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
> -     if (is_tag_ref) {
> -             struct object *o = parse_object(sha1);
> -             if (o->type == OBJ_TAG) {
> -                     o = deref_tag(o, path, 0);
> -                     if (o)
> -                             fprintf(cb->refs_file, "^%s\n",
> -                                     sha1_to_hex(o->sha1));
> -             }
> +     o = parse_object(sha1);
> +     if (o->type == OBJ_TAG) {
> +             o = deref_tag(o, path, 0);
> +             if (o)
> +                     fprintf(cb->refs_file, "^%s\n",
> +                             sha1_to_hex(o->sha1));
>       }
>  
>       if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) {
> @@ -128,7 +127,7 @@ int pack_refs(unsigned int flags)
>               die_errno("unable to create ref-pack file structure");
>  
>       /* perhaps other traits later as well */
> -     fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
> +     fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n");
>  
>       for_each_ref(handle_one_ref, &cbdata);
>       if (ferror(cbdata.refs_file))
> diff --git a/refs.c b/refs.c
> index 175b9fc..770abf4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -808,6 +808,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
>       struct ref_entry *last = NULL;
>       char refline[PATH_MAX];
>       int flag = REF_ISPACKED;
> +     int fully_peeled = 0;
>  
>       while (fgets(refline, sizeof(refline), f)) {
>               unsigned char sha1[20];
> @@ -818,13 +819,18 @@ static void read_packed_refs(FILE *f, struct ref_dir 
> *dir)
>                       const char *traits = refline + sizeof(header) - 1;
>                       if (strstr(traits, " peeled "))
>                               flag |= REF_KNOWS_PEELED;
> +                     if (strstr(traits, " fully-peeled "))
> +                             fully_peeled = 1;
>                       /* perhaps other traits later as well */
>                       continue;
>               }
>  
>               refname = parse_ref_line(refline, sha1);
>               if (refname) {
> -                     last = create_ref_entry(refname, sha1, flag, 1);
> +                     int this_flag = flag;
> +                     if (!fully_peeled && prefixcmp(refname, "refs/tags/"))
> +                             this_flag &= ~REF_KNOWS_PEELED;
> +                     last = create_ref_entry(refname, sha1, this_flag, 1);
>                       add_ref(dir, last);
>                       continue;
>               }
>
> So it's really not that much code. The bigger question is whether we
> want to have to carry the "fully-peeled" tag forever, and how other
> implementations would treat it.
>
> -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