On Fri, 22 Oct 2010, 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;
Well, this get called repeatedly, being within the inner part of the
delta search loop. So you might get that warning as many times as the
delta window which is not that nice. If anything a static flag to
display the warning only once would be needed. But you're pretty likely
to have met that warning/error already from other operations, which is
why I didn't bother.
> Or will some other part of the code already complained to stderr?
Some other part is likely to already have complained, through
check_object() -> sha1_object_info(). But not necessarily in all cases.
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