[PATCH 3 of 3 STABLE] clone: process 'lookup' return as an arbitrary symbol

[Martin von Zweigbergk]

On Fri, Jul 27, 2018 at 7:02 AM Yuya Nishihara <yuya at tcha.org> wrote:

> On Fri, 27 Jul 2018 09:30:26 +0200, Boris FELD wrote:
> > On 26/07/2018 17:24, Martin von Zweigbergk wrote:
> > >     >> +                if checkout is not None and not
> > >     node.isnode(checkout):
> > >     > IIUC, hggit/hgsubversion may pass in a 20-length symbolic name?
> > >     Yes, this is unfortunate. Having everything working but 20-lenght
> > >     names
> > >     is a first step, we can follow-up with more logic to check if the
> > >     symbol
> > >     is a valid name.
> > >
> > >
> > > I think Yuya is suggesting that you simply remove the "and not
> > > node.isnode(checkout)",
>
> Yes.
>
> > We need to detect valid node because they cannot go through revsymbol,
> > it breaks the tests without that check. The previous API was able to
> > handle any cases, it is now hard to get the same result when needed.
> >
> > If a 20 chars string is a valid node (ie: exists in the repo) it should
> > probably take precedence over the name. It seems extremely unlikely to
> > not care about that case.
>
> Yes. So we need to check if 'checkout in destrepo' instead of
> isnode(checkout),
> right?
>
>   if checkout in destrepo:
>       # binary nodeid
>   elif isrevsymbol(destrepo, checkout):
>       # work around for hggit/hgsubversion
>


Oh, the destrepo is available here? Then yes, it's definitely worth
checking. I suggested not worrying about the risk of conflicts because I
assumed it was awkward to check if the node was in the destination repo at
this point in the code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180727/53648f69/attachment.html>