From: Drew Northup <drew.northup@maine.edu>
Date: Fri, 22 Oct 2010 15:24:24 GMT
Subject: Re: [PATCH] make pack-objects a bit more resilient to repo corruption
Message-ID: <1287761064.31218.37.camel@drew-northup.unet.maine.edu>
In-Reply-To: <20101022144600.GA5554@sigill.intra.peff.net>


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:
> 
> > -		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