On Mon, 11 Aug 2008, Shawn O. Pearce wrote:
> > Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > > If the check is only about a thin delta base that is not going to be
> > > transmit, I'd agree. But I do not see how you are distinguishing that
> > > case and the case where an object you are actually sending is missing (in
> > > which case we would want to error out, wouldn't we?)
>
> Turns out to be pretty simple I think. We just delay the
> error handling for ->type < 0 until write_object(). If we
> get this far we know we wanted to include the object but
> we really don't have it. Up until that point its fine
> for us to get objects which are missing, we'll just wind
> up with a suboptimal pack.
If you're going to die anyway due to an object with unknown type, better
do so _before_ going through the delta search phase and leaving a
partial pack behind. IOW, the type check can be performed in
prepare_pack() instead of write_object() like:
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..01ab49c 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1722,8 +1733,12 @@ static void prepare_pack(int window, int depth)
if (entry->no_try_delta)
continue;
- if (!entry->preferred_base)
+ if (!entry->preferred_base) {
nr_deltas++;
+ if (entry->type < 0)
+ die("unable to get type of object %s",
+ sha1_to_hex(entry->idx.sha1));
+ }
delta_list[n++] = entry;
}
Also a comment in check_object() mentioning where the return value of
sha1_object_info() is verified would be in order.
And I also agree with Junio about a test script for this so the usage is
fully demonstrated, and to ensure it keeps on working as intended
(most people will simply never exercise this otherwise).
Nicolas
--
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
