Submitter | Mads Kiilerich |
---|---|
Date | Dec. 2, 2014, 4:13 a.m. |
Message ID | <63e92a66482bb5af8b9b.1417493628@localhost.localdomain> |
Download | mbox | patch |
Permalink | /patch/6939/ |
State | Accepted |
Commit | ffef6d503ab2b6c732ae6c786066b6ac9f6e858d |
Headers | show |
Comments
On Mon, 01 Dec 2014 23:13:48 -0500, Mads Kiilerich <mads@kiilerich.com> wrote: > # HG changeset patch > # User Mads Kiilerich <madski@unity3d.com> > # Date 1417493579 -3600 > # Tue Dec 02 05:12:59 2014 +0100 > # Node ID 63e92a66482bb5af8b9bdd95dfb0fff500eef582 > # Parent 9a8d248290a4e634a6bdc0a1d07514fe8e971ad9 > rebase: avoid redundant repo[rev].rev() - just keep working in rev space > > diff --git a/hgext/rebase.py b/hgext/rebase.py > --- a/hgext/rebase.py > +++ b/hgext/rebase.py > @@ -540,20 +540,20 @@ def rebasenode(repo, rev, p1, state, col > 'Rebase a single revision' > # Merge phase > # Update to target and merge it with local > - if repo['.'].rev() != repo[p1].rev(): > - repo.ui.debug(" update to %d:%s\n" % (repo[p1].rev(), repo[p1])) > + if repo['.'].rev() != p1: > + repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) This crashed when running evolve 8d28bb4fc127, but the Windows test suite runs clean. Maybe it needs to be "%s:%s", but I don't have time to chase down if p1 is always a string right now, and didn't want to throw out a patch that guesses. There are a couple of debug lines below this change too. $ ../hg evolve move:[23950] addremove: restore the relative path printing when files are named atop:[23951] match: introduce uipath() to properly style a file path ** Unknown exception encountered with possibly-broken third-party extension evolve ** which supports versions 3.2 of Mercurial. ** Please disable evolve and try your action again. ** If that fixes the bug please report it to http://bz.selenic.com/ ** Python 2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit (Intel)] ** Mercurial Distributed SCM (version 3.2.2+44-c237499a7fba+20141205) ** Extensions loaded: eol, convert, graphlog, patchbomb, progress, extdiff, strip, mq, evolve, rebase Traceback (most recent call last): File "../hg", line 43, in <module> mercurial.dispatch.run() ... File "c:\Users\Matt\Projects\hg\mercurial\util.py", line 679, in check return func(*args, **kwargs) File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1225, in evolve progresscb=progresscb) File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1251, in _evolveany return _solveunstable(ui, repo, tro, dryrunopt, confirmopt, progresscb) File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1355, in _solveunstable relocate(repo, orig, target, keepbranch) File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 811, in relocate orig.p1().node()) File "c:\Users\Matt\Projects\hg\hgext\rebase.py", line 547, in rebasenode repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) TypeError: %d format: a number is required, not str --Matt
On 12/05/2014 05:39 AM, Matt Harbison wrote: > > On Mon, 01 Dec 2014 23:13:48 -0500, Mads Kiilerich > <mads@kiilerich.com> wrote: > >> # HG changeset patch >> # User Mads Kiilerich <madski@unity3d.com> >> # Date 1417493579 -3600 >> # Tue Dec 02 05:12:59 2014 +0100 >> # Node ID 63e92a66482bb5af8b9bdd95dfb0fff500eef582 >> # Parent 9a8d248290a4e634a6bdc0a1d07514fe8e971ad9 >> rebase: avoid redundant repo[rev].rev() - just keep working in rev space >> >> diff --git a/hgext/rebase.py b/hgext/rebase.py >> --- a/hgext/rebase.py >> +++ b/hgext/rebase.py >> @@ -540,20 +540,20 @@ def rebasenode(repo, rev, p1, state, col >> 'Rebase a single revision' >> # Merge phase >> # Update to target and merge it with local >> - if repo['.'].rev() != repo[p1].rev(): >> - repo.ui.debug(" update to %d:%s\n" % (repo[p1].rev(), >> repo[p1])) >> + if repo['.'].rev() != p1: >> + repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) > > This crashed when running evolve 8d28bb4fc127, but the Windows test > suite runs clean. Maybe it needs to be "%s:%s", but I don't have time > to chase down if p1 is always a string right now, and didn't want to > throw out a patch that guesses. There are a couple of debug lines > below this change too. > > $ ../hg evolve > move:[23950] addremove: restore the relative path printing when files > are named > atop:[23951] match: introduce uipath() to properly style a file path > ** Unknown exception encountered with possibly-broken third-party > extension evolve > ** which supports versions 3.2 of Mercurial. > ** Please disable evolve and try your action again. > ** If that fixes the bug please report it to http://bz.selenic.com/ > ** Python 2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit > (Intel)] > ** Mercurial Distributed SCM (version 3.2.2+44-c237499a7fba+20141205) > ** Extensions loaded: eol, convert, graphlog, patchbomb, progress, > extdiff, strip, mq, evolve, rebase > Traceback (most recent call last): > File "../hg", line 43, in <module> > mercurial.dispatch.run() > ... > File "c:\Users\Matt\Projects\hg\mercurial\util.py", line 679, in check > return func(*args, **kwargs) > File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1225, > in evolve > progresscb=progresscb) > File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1251, > in _evolveany > return _solveunstable(ui, repo, tro, dryrunopt, confirmopt, > progresscb) > File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1355, > in _solveunstable > relocate(repo, orig, target, keepbranch) > File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 811, > in relocate > orig.p1().node()) > File "c:\Users\Matt\Projects\hg\hgext\rebase.py", line 547, in > rebasenode > repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) > TypeError: %d format: a number is required, not str So evolve calls rebasenode directly, passing nodeids where Mercurial itself only passes revs. Fortunately it is caught early - http://selenic.com/hg/rev/ffef6d503ab2 removed the unused support for nodeids. Calling rebasenode directly hints that evolve duplicates some parts of rebase internals - that seems unfortunate. If rebasenode has to stay a stable API then it should be documented as such ... but I doubt that is how it is. /Mads
On 12/05/2014 07:16 AM, Mads Kiilerich wrote: > On 12/05/2014 05:39 AM, Matt Harbison wrote: >> >> On Mon, 01 Dec 2014 23:13:48 -0500, Mads Kiilerich >> <mads@kiilerich.com> wrote: >> >>> # HG changeset patch >>> # User Mads Kiilerich <madski@unity3d.com> >>> # Date 1417493579 -3600 >>> # Tue Dec 02 05:12:59 2014 +0100 >>> # Node ID 63e92a66482bb5af8b9bdd95dfb0fff500eef582 >>> # Parent 9a8d248290a4e634a6bdc0a1d07514fe8e971ad9 >>> rebase: avoid redundant repo[rev].rev() - just keep working in rev space >>> >>> diff --git a/hgext/rebase.py b/hgext/rebase.py >>> --- a/hgext/rebase.py >>> +++ b/hgext/rebase.py >>> @@ -540,20 +540,20 @@ def rebasenode(repo, rev, p1, state, col >>> 'Rebase a single revision' >>> # Merge phase >>> # Update to target and merge it with local >>> - if repo['.'].rev() != repo[p1].rev(): >>> - repo.ui.debug(" update to %d:%s\n" % (repo[p1].rev(), >>> repo[p1])) >>> + if repo['.'].rev() != p1: >>> + repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) >> >> This crashed when running evolve 8d28bb4fc127, but the Windows test >> suite runs clean. Maybe it needs to be "%s:%s", but I don't have time >> to chase down if p1 is always a string right now, and didn't want to >> throw out a patch that guesses. There are a couple of debug lines >> below this change too. >> >> $ ../hg evolve >> move:[23950] addremove: restore the relative path printing when files >> are named >> atop:[23951] match: introduce uipath() to properly style a file path >> ** Unknown exception encountered with possibly-broken third-party >> extension evolve >> ** which supports versions 3.2 of Mercurial. >> ** Please disable evolve and try your action again. >> ** If that fixes the bug please report it to http://bz.selenic.com/ >> ** Python 2.7.2 (default, Jun 12 2011, 15:08:59) [MSC v.1500 32 bit >> (Intel)] >> ** Mercurial Distributed SCM (version 3.2.2+44-c237499a7fba+20141205) >> ** Extensions loaded: eol, convert, graphlog, patchbomb, progress, >> extdiff, strip, mq, evolve, rebase >> Traceback (most recent call last): >> File "../hg", line 43, in <module> >> mercurial.dispatch.run() >> ... >> File "c:\Users\Matt\Projects\hg\mercurial\util.py", line 679, in check >> return func(*args, **kwargs) >> File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1225, >> in evolve >> progresscb=progresscb) >> File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1251, >> in _evolveany >> return _solveunstable(ui, repo, tro, dryrunopt, confirmopt, >> progresscb) >> File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 1355, >> in _solveunstable >> relocate(repo, orig, target, keepbranch) >> File "C:/Users/Matt/Projects/hg-evolve/hgext/evolve.py", line 811, >> in relocate >> orig.p1().node()) >> File "c:\Users\Matt\Projects\hg\hgext\rebase.py", line 547, in >> rebasenode >> repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) >> TypeError: %d format: a number is required, not str > > So evolve calls rebasenode directly, passing nodeids where Mercurial > itself only passes revs. Fortunately it is caught early - > http://selenic.com/hg/rev/ffef6d503ab2 removed the unused support for > nodeids. Yes, I need to migrate it to the new API matt made for 3.2 but I did not come to it yet > > Calling rebasenode directly hints that evolve duplicates some parts of > rebase internals - that seems unfortunate. If rebasenode has to stay a > stable API then it should be documented as such ... but I doubt that is > how it is. Feel free to break evolve.
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -389,7 +389,7 @@ def rebase(ui, repo, **opts): finally: ui.setconfig('ui', 'forcemerge', '', 'rebase') if not collapsef: - merging = repo[p2].rev() != nullrev + merging = p2 != nullrev editform = cmdutil.mergeeditform(merging, 'rebase') editor = cmdutil.getcommiteditor(editform=editform, **opts) newnode = concludenode(repo, rev, p1, p2, extrafn=extrafn, @@ -540,20 +540,20 @@ def rebasenode(repo, rev, p1, state, col 'Rebase a single revision' # Merge phase # Update to target and merge it with local - if repo['.'].rev() != repo[p1].rev(): - repo.ui.debug(" update to %d:%s\n" % (repo[p1].rev(), repo[p1])) + if repo['.'].rev() != p1: + repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) merge.update(repo, p1, False, True, False) else: repo.ui.debug(" already in target\n") repo.dirstate.write() - repo.ui.debug(" merge against %d:%s\n" % (repo[rev].rev(), repo[rev])) - if repo[rev].rev() == repo[min(state)].rev(): + repo.ui.debug(" merge against %d:%s\n" % (rev, repo[rev])) + if rev == min(state): # Case (1) initial changeset of a non-detaching rebase. # Let the merge mechanism find the base itself. base = None elif not repo[rev].p2(): # Case (2) detaching the node with a single parent, use this parent - base = repo[rev].p1().node() + base = repo[rev].p1().rev() else: # In case of merge, we need to pick the right parent as merge base. # @@ -580,8 +580,8 @@ def rebasenode(repo, rev, p1, state, col # Which does not represent anything sensible and creates a lot of # conflicts. for p in repo[rev].parents(): - if state.get(p.rev()) == repo[p1].rev(): - base = p.node() + if state.get(p.rev()) == p1: + base = p.rev() break else: # fallback when base not found base = None @@ -590,7 +590,7 @@ def rebasenode(repo, rev, p1, state, col raise AssertionError('no base found to rebase on ' '(rebasenode called wrong)') if base is not None: - repo.ui.debug(" detach base %d:%s\n" % (repo[base].rev(), repo[base])) + repo.ui.debug(" detach base %d:%s\n" % (base, repo[base])) # When collapsing in-place, the parent is the common ancestor, we # have to allow merging with it. stats = merge.update(repo, rev, True, True, False, base, collapse, @@ -832,7 +832,7 @@ def abort(repo, originalwd, target, stat if cleanup: # Update away from the rebase if necessary if inrebase(repo, originalwd, state): - merge.update(repo, repo[originalwd].rev(), False, True, False) + merge.update(repo, originalwd, False, True, False) # Strip from the first rebased revision rebased = filter(lambda x: x > -1 and x != target, state.values())