On Mon, 11 Aug 2008, Shawn O. Pearce wrote:

quoted text
> > Junio C Hamano <gitster@pobox.com> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html