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):

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