On Thu, 13 Jun 2013, Jeff King wrote:

> When we try to load an object from disk and fail, our
> general strategy is to see if we can get it from somewhere
> else (e.g., a loose object). That lets users fix corruption
> problems by copying known-good versions of objects into the
> object database.
> 
> We already handle the case where we were not able to read
> the delta from disk. However, when we find that the delta we
> read does not apply, we simply die.  This case is harder to
> trigger, as corruption in the delta data itself would
> trigger a crc error from zlib.  However, a corruption that
> pointed us at the wrong delta base might cause it.
> 
> We can do the same "fail and try to find the object
> elsewhere" trick instead of dying. This not only gives us a
> chance to recover, but also puts us on code paths that will
> alert the user to the problem (with the current message,
> they do not even know which sha1 caused the problem).
> 
> Signed-off-by: Jeff King <p...@peff.net>

That makes sense.

Could you produce a test case to go along with this change?

> ---
> I needed this earlier today to recover from a corrupted packfile (I
> fortunately had an older version of the repo in backups). Still tracking
> down the exact nature of the corruption.
> 
>  sha1_file.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/sha1_file.c b/sha1_file.c
> index 5c08701..d458708 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2135,8 +2135,17 @@ void *unpack_entry(struct packed_git *p, off_t 
> obj_offset,
>               data = patch_delta(base, base_size,
>                                  delta_data, delta_size,
>                                  &size);
> +
> +             /*
> +              * We could not apply the delta; warn the user, but keep going.
> +              * Our failure will be noticed either in the next iteration of
> +              * the loop, or if this is the final delta, in the caller when
> +              * we return NULL. Those code paths will take care of making
> +              * a more explicit warning and retrying with another copy of
> +              * the object.
> +              */
>               if (!data)
> -                     die("failed to apply delta");
> +                     error("failed to apply delta");
>  
>               free(delta_data);
>       }
> -- 
> 1.8.3.rc2.14.g7eee6b3
> 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to