On Fri, 22 Oct 2010, Drew Northup wrote:
>
> 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.
Please don't misinterpret this case. As far as this change is
concerned, nothing is actually "broken". The operation _will_ still
succeed. The repository may be broken, but in this case we can do
without the broken object. In those cases where the object is really
needed the original die() is still in place.
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