Re: RFC: Allow missing objects during packing

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

quoted

Junio C Hamano [off-list ref] wrote:

quoted

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