Re: [PATCH] make pack-objects a bit more resilient to repo corruption

On Fri, 2010-10-22 at 10:46 -0400, Jeff King wrote:

On Fri, Oct 22, 2010 at 12:53:32AM -0400, Nicolas Pitre wrote:

quoted

-		if (!src->data)
+		if (!src->data) {
+			if (src_entry->preferred_base) {
+				/* 
+				 * Those objects are not included in the
+				 * resulting pack.  Be resilient and ignore
+				 * them if they can't be read, in case the
+				 * pack could be created nevertheless.
+				 */
+				return 0;
+			}
 			die("object %s cannot be read",
 			    sha1_to_hex(src_entry->idx.sha1));
+		}

By converting this die() into a silent return, are we losing a place
where git might previously have alerted a user to corruption? In this
case, we can continue the operation without the object, but if we have
detected corruption, letting the user know as soon as possible is
probably a good idea.

In other words, should this instead be:

  warning("unable to read preferred base object: %s", ...);
  return 0;

Or will some other part of the code already complained to stderr?

-Peff

Agreed. If it broke we should probably tell the user--even if we can't
do much useful about it other than attempt to recover by continuing.

-- 
-Drew Northup N1XIM
   AKA RvnPhnx on OPN
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59