On Thu, Nov 07, 2019 at 01:58:04PM +0100, Johannes Schindelin wrote:
> 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
