I didn't read your email until now. It may sound strange, but I can't
read my personal email from work.

Also, looks like I inadvertantly took this message off-list.

On Fri, May 9, 2008 at 4:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
quoted text
> "Brandon Casey" <drafnel@gmail.com> writes:
>
>> Is this invocation of pack-objects that you commented on necessary
>> though? This is the section that is replacing an existing pack file
>> with a newly created pack file of the same name...
>
> Yuck, I failed to see it in the context.  You are right.  This won't do
> anything I suspect.

Now I'm confused. I hope you realized I said "pack-objects" when I
meant "unpack-objects".

It was only the first invocation of unpack-objects that I thought may
be unnecessary. This is because pack-objects (yes, pack) created a new
pack with
the same name as an existing pack which means (if I'm thinking about things
correctly) that none of the objects inside the pack are unreachable. So trying
to unpack-objects is a waste of time.

There is a second invocation of unpack-objects which is run on packs which
will be deleted. This one should unpack any unreferenced objects from each pack.

-brandon


The remainder of your email is below since the list has never seen it.


quoted text
> Perhaps we would want a new option --eject that causes pack-object write
> out unreachable packed objects in the loose format?  This would require a
> minor surgery to write_sha1_file() so that it won't pay attention to the
> return value of has_sha1_file(sha1), perhaps like this.
>
> This is a lunch-time hack and needs more serious work, such as fixing
> horrible "write_sha1_file_1" to something more sensible.
>
> Also it should add --eject as an independent and incompatible option
> instead of dropping --keep-unpacked support.
>
> ---
>
>  builtin-pack-objects.c |   56 ++++++++++++-----------------------------------
>  sha1_file.c            |    9 ++++++-
>  2 files changed, 22 insertions(+), 43 deletions(-)
>
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 777f272..8240a4b 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -1836,38 +1836,25 @@ struct in_pack {
>        struct in_pack_object *array;
>  };
>
> -static void mark_in_pack_object(struct object *object, struct packed_git *p, struct in_pack *in_pack)
> +static void eject_object(struct object *obj)
>  {
> -       in_pack->array[in_pack->nr].offset = find_pack_entry_one(object->sha1, p);
> -       in_pack->array[in_pack->nr].object = object;
> -       in_pack->nr++;
> -}
> -
> -/*
> - * Compare the objects in the offset order, in order to emulate the
> - * "git-rev-list --objects" output that produced the pack originally.
> - */
> -static int ofscmp(const void *a_, const void *b_)
> -{
> -       struct in_pack_object *a = (struct in_pack_object *)a_;
> -       struct in_pack_object *b = (struct in_pack_object *)b_;
> +       enum object_type type;
> +       unsigned long size;
> +       void *data;
> +       unsigned char sha1[20];
>
> -       if (a->offset < b->offset)
> -               return -1;
> -       else if (a->offset > b->offset)
> -               return 1;
> -       else
> -               return hashcmp(a->object->sha1, b->object->sha1);
> +       data = read_sha1_file(obj->sha1, &type, &size);
> +       assert(data && type == obj->type);
> +       if (write_sha1_file_1(data, size, typename(type), sha1, 1))
> +               die("cannot eject packed object %s", sha1_to_hex(obj->sha1));
> +       assert(!memcmp(sha1, obj->sha1));
>  }
>
> -static void add_objects_in_unpacked_packs(struct rev_info *revs)
> +static void eject_objects_in_unpacked_packs(struct rev_info *revs)
>  {
>        struct packed_git *p;
> -       struct in_pack in_pack;
>        uint32_t i;
>
> -       memset(&in_pack, 0, sizeof(in_pack));
> -
>        for (p = packed_git; p; p = p->next) {
>                const unsigned char *sha1;
>                struct object *o;
> @@ -1881,28 +1868,15 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
>                if (open_pack_index(p))
>                        die("cannot open pack index");
>
> -               ALLOC_GROW(in_pack.array,
> -                          in_pack.nr + p->num_objects,
> -                          in_pack.alloc);
> -
>                for (i = 0; i < p->num_objects; i++) {
>                        sha1 = nth_packed_object_sha1(p, i);
>                        o = lookup_unknown_object(sha1);
> -                       if (!(o->flags & OBJECT_ADDED))
> -                               mark_in_pack_object(o, p, &in_pack);
> +                       if (o->flags & OBJECT_ADDED)
> +                               continue;
>                        o->flags |= OBJECT_ADDED;
> +                       eject_object(o);
>                }
>        }
> -
> -       if (in_pack.nr) {
> -               qsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),
> -                     ofscmp);
> -               for (i = 0; i < in_pack.nr; i++) {
> -                       struct object *o = in_pack.array[i].object;
> -                       add_object_entry(o->sha1, o->type, "", 0);
> -               }
> -       }
> -       free(in_pack.array);
>  }
>
>  static void get_object_list(int ac, const char **av)
> @@ -1938,7 +1912,7 @@ static void get_object_list(int ac, const char **av)
>        traverse_commit_list(&revs, show_commit, show_object);
>
>        if (keep_unreachable)
> -               add_objects_in_unpacked_packs(&revs);
> +               eject_objects_in_unpacked_packs(&revs);
>  }
>
>  static int adjust_perm(const char *path, mode_t mode)
> diff --git a/sha1_file.c b/sha1_file.c
> index 3516777..ce3a661 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2102,7 +2102,7 @@ int hash_sha1_file(const void *buf, unsigned long len, const char *type,
>        return 0;
>  }
>
> -int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
> +int write_sha1_file_1(void *buf, unsigned long len, const char *type, unsigned char *returnsha1, int refresh)
>  {
>        int size, ret;
>        unsigned char *compressed;
> @@ -2120,7 +2120,7 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
>        filename = sha1_file_name(sha1);
>        if (returnsha1)
>                hashcpy(returnsha1, sha1);
> -       if (has_sha1_file(sha1))
> +       if (!refresh && has_sha1_file(sha1))
>                return 0;
>        fd = open(filename, O_RDONLY);
>        if (fd >= 0) {
> @@ -2185,6 +2185,11 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
>        return move_temp_to_file(tmpfile, filename);
>  }
>
> +int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
> +{
> +       return write_sha1_file_1(buf, len, type, returnsha1, 0);
> +}
> +
>  /*
>  * We need to unpack and recompress the object for writing
>  * it out to a different file.
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html