Re: [BUG] "fatal: bad object .alternate" during fetch with alternates

From: Jeff King <peff@peff.net>
Date: Nov 8, 2019, 08:54 UTC
Message-ID: <20191108085413.GA4356@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1911071349590.46@tvgsbejvaqbjf.bet>

On Thu, Nov 07, 2019 at 01:58:04PM +0100, Johannes Schindelin wrote:

Show 19 quoted lines

> So what about the idea of ignoring (with a warning) them instead,
> without bothering to try saying much about the alternate itself? I.e.
> something like this patch (which is admittedly a bit verbose because it
> _also_ has to update a signature):
>
> [...]
> @@ -1569,7 +1569,11 @@ static void add_one_alternate_ref(const struct object_id *oid,
>  	struct add_alternate_refs_data *data = vdata;
>  	struct object *obj;
> 
> -	obj = get_reference(data->revs, name, oid, data->flags);
> +	if (!(obj = get_reference(data->revs, name, oid, data->flags, 1))) {
> +		warning("ignoring stale alternate reference to '%s'",
> +			oid_to_hex(oid));
> +		return;
> +	}
>  	add_rev_cmdline(data->revs, obj, name, REV_CMD_REV, data->flags);
>  	add_pending_object(data->revs, obj, name);
>  }

I don't think it makes sense to tie this to "--alternate-refs" in this way. I think there are two ways to look at resolving this:

  - for the UNINTERESTING side of a traversal, we might be willing to
    ignore a missing object. But then it would apply equally to
    non-alternates, too. (And we do this already, but not for ref tips,
    since those aren't just "stale" but rather indicative of a
    corruption).

  - any alternate-ref borrowing is an optimization and thus best-effort.
    So we should treat them specially, whether UNINTERESTING or not. But
    then I think the right place to do that is not inside rev-list, but
    in for_each_alternate_ref(), or even in the for-each-ref run inside
    the alternate repository. Then it would help rev-list, but also
    avoid advertising the bogus object across a push/fetch.

-Peff