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

[Yuya Nishihara]

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