Re: [PATCH] pack-objects: protect against disappearing packs

[Nicolas Pitre <nico@fluxnic.net>]

On Thu, 13 Oct 2011, Jeff King wrote:

> On Thu, Oct 13, 2011 at 09:23:20PM -0400, Jeff King wrote:
> 
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index 2b18de5..8681ccd 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -804,6 +804,10 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
> >  		off_t offset = find_pack_entry_one(sha1, p);
> >  		if (offset) {
> >  			if (!found_pack) {
> > +				if (!is_pack_valid(p)) {
> > +					error("packfile %s cannot be accessed", p->pack_name);
> > +					continue;
> > +				}
> 
> This message is modeled after the one in find_pack_entry. However,
> they're not really errors, since we will try to find the object
> elsewhere (and generally succeed). So the messages could just go away.
> Though they can also alert you to something fishy going on (like a
> packfile with bad permissions). But perhaps we should downgrade them
> like this:
> 
> -- >8 --
> Subject: [PATCH] downgrade "packfile cannot be accessed" errors to warnings
> 
> These can happen if another process simultaneously prunes a
> pack. But that is not usually an error condition, because a
> properly-running prune should have repacked the object into
> a new pack. So we will notice that the pack has disappeared
> unexpectedly, print a message, try other packs (possibly
> after re-scanning the list of packs), and find it in the new
> pack.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Acked-by: Nicolas Pitre <nico@fluxnic.net>


> ---
>  builtin/pack-objects.c |    2 +-
>  sha1_file.c            |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 8681ccd..ba3705d 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -805,7 +805,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
>  		if (offset) {
>  			if (!found_pack) {
>  				if (!is_pack_valid(p)) {
> -					error("packfile %s cannot be accessed", p->pack_name);
> +					warning("packfile %s cannot be accessed", p->pack_name);
>  					continue;
>  				}
>  				found_offset = offset;
> diff --git a/sha1_file.c b/sha1_file.c
> index a22c5b4..27f3b9b 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2038,7 +2038,7 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
>  			 * was loaded!
>  			 */
>  			if (!is_pack_valid(p)) {
> -				error("packfile %s cannot be accessed", p->pack_name);
> +				warning("packfile %s cannot be accessed", p->pack_name);
>  				goto next;
>  			}
>  			e->offset = offset;
> -- 
> 1.7.6.4.37.g43b58b
>