Junio C Hamano <gitster@xxxxxxxxx> wrote:
> This adds "--repack-unpacked" option to pack-objects to help
> combining small packs into one, without losing unreferenced
> objects that are in the packs. When this option is given in
> addition to the above command line, we also make sure all the
> objects in the named existing packs are included in the result.
What about calling it "--keep-unreachable" instead?
> static void show_object(struct object_array_entry *p)
> {
> + struct object *o = lookup_unknown_object(p->item->sha1);
Can't you just say "o = p->item" here?
> +struct in_pack_object {
> + off_t offset;
> + const unsigned char *sha1;
> +};
I can't help but think it would be better to store a "struct object*"
here instead of the "const unsigned char *sha1". Both are of type
pointer but we only need the struct object* when are working with
the in_pack_object elements.
> +static void mark_in_pack_object(const unsigned char *sha1, struct packed_git *p, struct in_pack *in_pack)
> +{
> + in_pack->array[in_pack->nr].offset = find_pack_entry_one(sha1, p);
> + in_pack->array[in_pack->nr].sha1 = sha1;
> + in_pack->nr++;
> +}
Per above just pass the "struct object*" into this function and
store that into the array, instead of the sha1. Of course you now
need to use "o->sha1" to perform the find_pack_entry_one().
> +/*
> + * 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_;
> +
> + if (a->offset < b->offset)
> + return -1;
> + else if (a->offset > b->offset)
> + return 1;
> + else
> + return hashcmp(a->sha1, b->sha1);
> +}
Really? It is not possible for two objects to be placed at the
same offset within the same packfile and yet have two different
SHA-1 values. The final else condition above is just "return 0".
> +static void add_objects_in_unpacked_packs(struct rev_info *revs)
...
> + qsort(in_pack.array, in_pack.nr, sizeof(in_pack.array[0]),
> + ofscmp);
> + for (i = 0; i < in_pack.nr; i++) {
> + sha1 = in_pack.array[i].sha1;
> + o = lookup_unknown_object(sha1);
> + add_object_entry(sha1, o->type, "", 0);
If you save the "struct object*" into the in_pack_object (instead
of the SHA-1) as I described above you can avoid this second
lookup_unknown_object() call when we decide that we do need to pack
the object.
Otherwise I think it looks quite sane.
--
Shawn.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
