Submitter | via Mercurial-devel |
---|---|
Date | July 28, 2018, 12:11 a.m. |
Message ID | <CAESOdVCaAo6TN8RF51Fj3GHgxg3hD4kQ+3o+rSaJRv81W6iesw@mail.gmail.com> |
Download | mbox | patch |
Permalink | /patch/32957/ |
State | New |
Headers | show |
Comments
On 28/07/2018 02:11, Martin von Zweigbergk via Mercurial-devel wrote: > > > On Fri, Jul 27, 2018 at 3:26 PM Boris FELD <boris.feld@octobus.net > <mailto:boris.feld@octobus.net>> wrote: > > > > On 27/07/2018 16:00, Yuya Nishihara 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? > > `checkout in destrepo` no longer works because 'repo[checkout]' no > longer works. This whole series is about resolving `checkout` to a > node > before calling `checkout in destrepo`. > > > I downloaded hg-git and Dulwich and tried it out. Lots of things > broken by 670eb4fa1b86 (demandimport: make module ignores a set (API), > 2018-05-05). If I check out 91618801d5c3 (context: raise > ProgrammingError on repo['my-tag'], 2018-07-06) and back out that > commit, I can reproduce the failures I assume you've seen. All the > tests pass again with the below patch. Looks good? I'm running hg > tests now. I will update this thread if they fail. Could you see if it > passes the hg-subversion tests too? If it does, can you add in > comments there and send it as a proper patch? Thanks. The patch makes hg-subversion tests happy and looks fine, I will send it as a proper patch, thank you.
Patch
diff --git a/mercurial/hg.py b/mercurial/hg.py --- a/mercurial/hg.py +++ b/mercurial/hg.py @@ -731,8 +731,10 @@ def clone(ui, peeropts, source, dest=Non uprev = None status = None if checkout is not None: - if checkout in destrepo: + if len(checkout) == 20 and checkout in destrepo: uprev = checkout + elif scmutil.isrevsymbol(destrepo, checkout): + uprev = scmutil.revsymbol(destrepo, checkout).node() else: if update is not True: