Submitter | via Mercurial-devel |
---|---|
Date | July 21, 2017, 5:25 a.m. |
Message ID | <CAESOdVCE+hajA17H8p-pmcsCJN_Ti9RDQQvr9JdTJLKfLmhepw@mail.gmail.com> |
Download | mbox | patch |
Permalink | /patch/22538/ |
State | Superseded |
Headers | show |
Comments
Excerpts from Martin von Zweigbergk's message of 2017-07-20 22:25:20 -0700: > On Thu, Jul 20, 2017 at 4:40 PM, Jun Wu <quark@fb.com> wrote: > > # HG changeset patch > > # User Jun Wu <quark@fb.com> > > # Date 1500594002 25200 > > # Thu Jul 20 16:40:02 2017 -0700 > > # Branch stable > > # Node ID 3253102e6e5222a08737e7204eb672ab12e80764 > > # Parent a41e0f1c9b69330a0dd8d6590787faa38d11c0ff > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r 3253102e6e52 > > rebase: move bookmark to destination for commits becoming empty (issue5627) > > > > When rebasing a changeset X and that changeset becomes empty, we should move > > the bookmark on X to rebase destination. > > > > This is a regression caused by scmutil.cleanupnodes refactoring. > > > > diff --git a/hgext/rebase.py b/hgext/rebase.py > > --- a/hgext/rebase.py > > +++ b/hgext/rebase.py > > @@ -438,4 +438,14 @@ class rebaseruntime(object): > > 'to commit\n') % (rev, ctx)) > > self.skipped.add(rev) > > + # Move bookmark to destination. This cannot be handled > > + # by scmutil.cleanupnodes since it will treat rev as > > + # removed (no successor) and move bookmark backwards. > > + destnode = repo[self.dest].node() > > + bmchanges = [(name, destnode) > > + for name in repo.nodebookmarks(ctx.node())] > > + if bmchanges: > > + with repo.transaction('rebase') as tr1: > > + repo._bookmarks.applychanges(repo, tr1, > > + bmchanges) > > > This feels like the wrong place to me. The other bookmark moves are > done later. It also doesn't work with --keep (bookmarks are not > normally moved with --keep). How about something like this on top: This is better. Since you have written the code. Could you send a patch? Otherwise I can shamelessly steal your code in V2. > diff --git a/hgext/rebase.py b/hgext/rebase.py > --- a/hgext/rebase.py > +++ b/hgext/rebase.py > @@ -437,16 +437,6 @@ class rebaseruntime(object): > ui.warn(_('note: rebase of %d:%s created no changes ' > 'to commit\n') % (rev, ctx)) > self.skipped.add(rev) > - # Move bookmark to destination. This cannot be handled > - # by scmutil.cleanupnodes since it will treat rev as > - # removed (no successor) and move bookmark backwards. > - destnode = repo[self.dest].node() > - bmchanges = [(name, destnode) > - for name in > repo.nodebookmarks(ctx.node())] > - if bmchanges: > - with repo.transaction('rebase') as tr1: > - repo._bookmarks.applychanges(repo, tr1, > - bmchanges) > self.state[rev] = p1 > ui.debug('next revision set to %s\n' % p1) > elif self.state[rev] == nullmerge: > @@ -522,7 +512,8 @@ class rebaseruntime(object): > collapsedas = None > if self.collapsef: > collapsedas = newnode > - clearrebased(ui, repo, self.state, self.skipped, collapsedas) > + clearrebased(ui, repo, self.dest, self.state, self.skipped, > + collapsedas) > > clearstatus(repo) > clearcollapsemsg(repo) > @@ -1311,12 +1302,21 @@ def buildstate(repo, dest, rebaseset, co > state[r] = revprecursor > return originalwd, dest.rev(), state > > -def clearrebased(ui, repo, state, skipped, collapsedas=None): > +def clearrebased(ui, repo, dest, state, skipped, collapsedas=None): > """dispose of rebased revision at the end of the rebase > > If `collapsedas` is not None, the rebase was a collapse whose result if the > `collapsedas` node.""" > tonode = repo.changelog.node > + # Move bookmark of skipped nodes to destination. This cannot be handled > + # by scmutil.cleanupnodes since it will treat rev as removed (no successor) > + # and move bookmark backwards. > + bmchanges = [(name, tonode(dest)) > + for rev in skipped > + for name in repo.nodebookmarks(tonode(rev))] > + if bmchanges: > + with repo.transaction('rebase') as tr: > + repo._bookmarks.applychanges(repo, tr, bmchanges) > mapping = {} > for rev, newrev in sorted(state.items()): > if newrev >= 0 and newrev != rev: > diff --git a/tests/test-rebase-emptycommit.t b/tests/test-rebase-emptycommit.t > --- a/tests/test-rebase-emptycommit.t > +++ b/tests/test-rebase-emptycommit.t > @@ -27,6 +27,18 @@ > |/ > o 0 A > > + $ hg rebase -r 2 -d 3 --keep > + rebasing 2:dc0947a82db8 "C" (BOOK) > + note: rebase of 2:dc0947a82db8 created no changes to commit > + $ hg log -G -T '{rev} {desc} {bookmarks}' > + o 3 C > + | > + | o 2 C BOOK > + | | > + o | 1 B > + |/ > + o 0 A > + > $ hg rebase -r 2 -d 3 > rebasing 2:dc0947a82db8 "C" (BOOK) > note: rebase of 2:dc0947a82db8 created no changes to commit
Feel free to take my patch. I can't queue my own patch anyway so it'll be more efficient if you send it and I queue it. On Jul 21, 2017 15:30, "Jun Wu" <quark@fb.com> wrote: > Excerpts from Martin von Zweigbergk's message of 2017-07-20 22:25:20 -0700: > > On Thu, Jul 20, 2017 at 4:40 PM, Jun Wu <quark@fb.com> wrote: > > > # HG changeset patch > > > # User Jun Wu <quark@fb.com> > > > # Date 1500594002 25200 > > > # Thu Jul 20 16:40:02 2017 -0700 > > > # Branch stable > > > # Node ID 3253102e6e5222a08737e7204eb672ab12e80764 > > > # Parent a41e0f1c9b69330a0dd8d6590787faa38d11c0ff > > > # Available At https://bitbucket.org/quark-zju/hg-draft > > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > 3253102e6e52 > > > rebase: move bookmark to destination for commits becoming empty > (issue5627) > > > > > > When rebasing a changeset X and that changeset becomes empty, we > should move > > > the bookmark on X to rebase destination. > > > > > > This is a regression caused by scmutil.cleanupnodes refactoring. > > > > > > diff --git a/hgext/rebase.py b/hgext/rebase.py > > > --- a/hgext/rebase.py > > > +++ b/hgext/rebase.py > > > @@ -438,4 +438,14 @@ class rebaseruntime(object): > > > 'to commit\n') % (rev, ctx)) > > > self.skipped.add(rev) > > > + # Move bookmark to destination. This cannot > be handled > > > + # by scmutil.cleanupnodes since it will treat > rev as > > > + # removed (no successor) and move bookmark > backwards. > > > + destnode = repo[self.dest].node() > > > + bmchanges = [(name, destnode) > > > + for name in > repo.nodebookmarks(ctx.node())] > > > + if bmchanges: > > > + with repo.transaction('rebase') as tr1: > > > + repo._bookmarks.applychanges(repo, > tr1, > > > + > bmchanges) > > > > > > This feels like the wrong place to me. The other bookmark moves are > > done later. It also doesn't work with --keep (bookmarks are not > > normally moved with --keep). How about something like this on top: > > This is better. Since you have written the code. Could you send a patch? > Otherwise I can shamelessly steal your code in V2. > > > diff --git a/hgext/rebase.py b/hgext/rebase.py > > --- a/hgext/rebase.py > > +++ b/hgext/rebase.py > > @@ -437,16 +437,6 @@ class rebaseruntime(object): > > ui.warn(_('note: rebase of %d:%s created no > changes ' > > 'to commit\n') % (rev, ctx)) > > self.skipped.add(rev) > > - # Move bookmark to destination. This cannot be > handled > > - # by scmutil.cleanupnodes since it will treat > rev as > > - # removed (no successor) and move bookmark > backwards. > > - destnode = repo[self.dest].node() > > - bmchanges = [(name, destnode) > > - for name in > > repo.nodebookmarks(ctx.node())] > > - if bmchanges: > > - with repo.transaction('rebase') as tr1: > > - repo._bookmarks.applychanges(repo, tr1, > > - bmchanges) > > self.state[rev] = p1 > > ui.debug('next revision set to %s\n' % p1) > > elif self.state[rev] == nullmerge: > > @@ -522,7 +512,8 @@ class rebaseruntime(object): > > collapsedas = None > > if self.collapsef: > > collapsedas = newnode > > - clearrebased(ui, repo, self.state, self.skipped, > collapsedas) > > + clearrebased(ui, repo, self.dest, self.state, self.skipped, > > + collapsedas) > > > > clearstatus(repo) > > clearcollapsemsg(repo) > > @@ -1311,12 +1302,21 @@ def buildstate(repo, dest, rebaseset, co > > state[r] = revprecursor > > return originalwd, dest.rev(), state > > > > -def clearrebased(ui, repo, state, skipped, collapsedas=None): > > +def clearrebased(ui, repo, dest, state, skipped, collapsedas=None): > > """dispose of rebased revision at the end of the rebase > > > > If `collapsedas` is not None, the rebase was a collapse whose > result if the > > `collapsedas` node.""" > > tonode = repo.changelog.node > > + # Move bookmark of skipped nodes to destination. This cannot be > handled > > + # by scmutil.cleanupnodes since it will treat rev as removed (no > successor) > > + # and move bookmark backwards. > > + bmchanges = [(name, tonode(dest)) > > + for rev in skipped > > + for name in repo.nodebookmarks(tonode(rev))] > > + if bmchanges: > > + with repo.transaction('rebase') as tr: > > + repo._bookmarks.applychanges(repo, tr, bmchanges) > > mapping = {} > > for rev, newrev in sorted(state.items()): > > if newrev >= 0 and newrev != rev: > > diff --git a/tests/test-rebase-emptycommit.t b/tests/test-rebase- > emptycommit.t > > --- a/tests/test-rebase-emptycommit.t > > +++ b/tests/test-rebase-emptycommit.t > > @@ -27,6 +27,18 @@ > > |/ > > o 0 A > > > > + $ hg rebase -r 2 -d 3 --keep > > + rebasing 2:dc0947a82db8 "C" (BOOK) > > + note: rebase of 2:dc0947a82db8 created no changes to commit > > + $ hg log -G -T '{rev} {desc} {bookmarks}' > > + o 3 C > > + | > > + | o 2 C BOOK > > + | | > > + o | 1 B > > + |/ > > + o 0 A > > + > > $ hg rebase -r 2 -d 3 > > rebasing 2:dc0947a82db8 "C" (BOOK) > > note: rebase of 2:dc0947a82db8 created no changes to commit >
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -437,16 +437,6 @@ class rebaseruntime(object): ui.warn(_('note: rebase of %d:%s created no changes ' 'to commit\n') % (rev, ctx)) self.skipped.add(rev) - # Move bookmark to destination. This cannot be handled - # by scmutil.cleanupnodes since it will treat rev as - # removed (no successor) and move bookmark backwards. - destnode = repo[self.dest].node() - bmchanges = [(name, destnode) - for name in repo.nodebookmarks(ctx.node())] - if bmchanges: - with repo.transaction('rebase') as tr1: - repo._bookmarks.applychanges(repo, tr1, - bmchanges) self.state[rev] = p1 ui.debug('next revision set to %s\n' % p1) elif self.state[rev] == nullmerge: @@ -522,7 +512,8 @@ class rebaseruntime(object): collapsedas = None if self.collapsef: collapsedas = newnode - clearrebased(ui, repo, self.state, self.skipped, collapsedas) + clearrebased(ui, repo, self.dest, self.state, self.skipped, + collapsedas) clearstatus(repo) clearcollapsemsg(repo) @@ -1311,12 +1302,21 @@ def buildstate(repo, dest, rebaseset, co state[r] = revprecursor return originalwd, dest.rev(), state -def clearrebased(ui, repo, state, skipped, collapsedas=None): +def clearrebased(ui, repo, dest, state, skipped, collapsedas=None): """dispose of rebased revision at the end of the rebase If `collapsedas` is not None, the rebase was a collapse whose result if the `collapsedas` node.""" tonode = repo.changelog.node + # Move bookmark of skipped nodes to destination. This cannot be handled + # by scmutil.cleanupnodes since it will treat rev as removed (no successor) + # and move bookmark backwards. + bmchanges = [(name, tonode(dest)) + for rev in skipped + for name in repo.nodebookmarks(tonode(rev))] + if bmchanges: + with repo.transaction('rebase') as tr: + repo._bookmarks.applychanges(repo, tr, bmchanges) mapping = {} for rev, newrev in sorted(state.items()): if newrev >= 0 and newrev != rev: diff --git a/tests/test-rebase-emptycommit.t b/tests/test-rebase-emptycommit.t --- a/tests/test-rebase-emptycommit.t +++ b/tests/test-rebase-emptycommit.t @@ -27,6 +27,18 @@ |/ o 0 A + $ hg rebase -r 2 -d 3 --keep + rebasing 2:dc0947a82db8 "C" (BOOK) + note: rebase of 2:dc0947a82db8 created no changes to commit + $ hg log -G -T '{rev} {desc} {bookmarks}' + o 3 C + | + | o 2 C BOOK + | | + o | 1 B + |/ + o 0 A + $ hg rebase -r 2 -d 3 rebasing 2:dc0947a82db8 "C" (BOOK) note: rebase of 2:dc0947a82db8 created no changes to commit