D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)

Yuya Nishihara yuya at tcha.org
Sun Dec 23 22:07:48 EST 2018


Queued, thanks.

I have a concern about compatibility with ancient Mercurial. Can you check it
and send a follow-up as needed?

>   I had to change the handling of the case where the server doesn't
>   support the lookup query, because if it fails, it would otherwise make
>   fremotebookmark.result() block forever. This is due to
>   wireprotov1peer.peerexecutor.sendcommands's behavior (it fills a
>   single future if any query fails synchronously and leaves all other
>   futures unchanged), but I don't know if the fix is to cancel all other
>   futures, or to keep going with the other queries.

@indygreg

> +        if opts['bookmark'] or revs:
> +            # The list of bookmark used here is the same used to actually update
> +            # the bookmark names, to avoid the race from issue 4689 and we do
> +            # all lookup and bookmark queries in one go so they see the same
> +            # version of the server state (issue 4700).
> +            nodes = []
> +            fnodes = []
> +            revs = revs or []
> +            if revs and not other.capable('lookup'):
> +                err = _("other repository doesn't support revision lookup, "
> +                        "so a rev cannot be specified.")
> +                raise error.Abort(err)
> +            with other.commandexecutor() as e:
> +                fremotebookmarks = e.callcommand('listkeys', {
> +                    'namespace': 'bookmarks'
> +                })
> +                for r in revs:
> +                    fnodes.append(e.callcommand('lookup', {'key': r}))

IIRC, `listkeys` is a newer command than `lookup`. If the peer doesn't support
`listkeys`, I suspect this batch query would fail. In that case, maybe `listkeys`
has to be skipped if the peer doesn't support it and if --bookmark is not
specified.


More information about the Mercurial-devel mailing list