Patchwork [STABLE] shelve: use rebase instead of merge (issue4068)

login
register
mail settings
Submitter Durham Goode
Date Oct. 25, 2013, 6:59 p.m.
Message ID <ff91f2972263c857ef01.1382727543@dev350.prn1.facebook.com>
Download mbox | patch
Permalink /patch/2823/
State Superseded
Commit 9bfa86746c9c1f6ab51deb8f174ffc482417d09f
Headers show

Comments

Durham Goode - Oct. 25, 2013, 6:59 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1382559168 25200
#      Wed Oct 23 13:12:48 2013 -0700
# Branch stable
# Node ID ff91f2972263c857ef017bd72a195ce0cbe54e6d
# Parent  d51c4d85ec23a67292f07b3121a9353452364f6d
shelve: use rebase instead of merge (issue4068)

Previously, shelve used merge to unshelve things. This meant that if you shelved
changes on one branch, then unshelved on another, all the changes from the first
branch would be present in the second branch, and not just the shelved changes.

The fix is to use rebase to pick the shelve commit off the original branch and
place it on top of the new branch. This means only the shelved changes are
brought across.

This has the side effect of fixing several other issues in shelve:

- you can now unshelve into a file that already has pending changes
- unshelve a mv/cp now has the correct dirstate value (A instead of M)
- you can now unshelve to an ancestor of the shelve
- unshelve now no longer deletes untracked .orig files

Updates tests and adds a new one to cover the issue. The test changes fall into
a few categories:

- I removed some excess output
- The --continue/--abort state is a little different, so the parents and
  dirstate needed updating
- Removed some untracked files at certain points that cluttered the output
Durham Goode - Oct. 25, 2013, 7:02 p.m.
On 10/25/13 11:59 AM, "Durham Goode" <durham@fb.com> wrote:

>
>+                rebase.rebase(ui, repo, **{
>+                    'rev' : [shelvectx.rev()],
>+                    'dest' : str(tmpwctx.rev()),
>+                    'keep' : True,
>+                })
>
>+
>+unshelve should work on an ancestor of the original commit
>+
>+  $ hg shelve
>+  shelved as default
>+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>+  $ hg up 0
>+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
>+  $ hg unshelve
>+  unshelving change 'default'
>+  adding changesets
>+  adding manifests
>+  adding file changes
>+  added 1 changesets with 1 changes to 3 files
>+  $ hg status
>+  A d
>+
>+  $ cd ..


This second version of the patch fixes a bug where it couldn't unshelve
onto rev 0 due to rebase using 'if not dest:' internally.  Converting it
to a string first fixes it.  Also added a test to cover this case and to
cover the unshelving-onto-ancestor case (issue4059).
Matt Mackall - Oct. 25, 2013, 8:37 p.m.
On Fri, 2013-10-25 at 11:59 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1382559168 25200
> #      Wed Oct 23 13:12:48 2013 -0700
> # Branch stable
> # Node ID ff91f2972263c857ef017bd72a195ce0cbe54e6d
> # Parent  d51c4d85ec23a67292f07b3121a9353452364f6d
> shelve: use rebase instead of merge (issue4068)

Queued for stable, thanks.

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -27,6 +27,7 @@ 
 from mercurial import error, hg, mdiff, merge, patch, repair, util
 from mercurial import templatefilters
 from mercurial import lock as lockmod
+from hgext import rebase
 import errno
 
 cmdtable = {}
@@ -95,25 +96,35 @@ 
                 raise util.Abort(_('this version of shelve is incompatible '
                                    'with the version used in this repo'))
             name = fp.readline().strip()
+            wctx = fp.readline().strip()
+            pendingctx = fp.readline().strip()
             parents = [bin(h) for h in fp.readline().split()]
             stripnodes = [bin(h) for h in fp.readline().split()]
+            unknownfiles = fp.readline()[:-1].split('\0')
         finally:
             fp.close()
 
         obj = cls()
         obj.name = name
+        obj.wctx = repo[bin(wctx)]
+        obj.pendingctx = repo[bin(pendingctx)]
         obj.parents = parents
         obj.stripnodes = stripnodes
+        obj.unknownfiles = unknownfiles
 
         return obj
 
     @classmethod
-    def save(cls, repo, name, stripnodes):
+    def save(cls, repo, name, originalwctx, pendingctx, stripnodes,
+             unknownfiles):
         fp = repo.opener(cls._filename, 'wb')
         fp.write('%i\n' % cls._version)
         fp.write('%s\n' % name)
+        fp.write('%s\n' % hex(originalwctx.node()))
+        fp.write('%s\n' % hex(pendingctx.node()))
         fp.write('%s\n' % ' '.join([hex(p) for p in repo.dirstate.parents()]))
         fp.write('%s\n' % ' '.join([hex(n) for n in stripnodes]))
+        fp.write('%s\n' % '\0'.join(unknownfiles))
         fp.close()
 
     @classmethod
@@ -368,44 +379,55 @@ 
     lock = None
     try:
         checkparents(repo, state)
+
+        util.rename(repo.join('unshelverebasestate'),
+                    repo.join('rebasestate'))
+        try:
+            rebase.rebase(ui, repo, **{
+                'abort' : True
+            })
+        except Exception:
+            util.rename(repo.join('rebasestate'),
+                        repo.join('unshelverebasestate'))
+            raise
+
         lock = repo.lock()
-        merge.mergestate(repo).reset()
-        if opts['keep']:
-            repo.setparents(repo.dirstate.parents()[0])
-        else:
-            revertfiles = readshelvedfiles(repo, state.name)
-            wctx = repo.parents()[0]
-            cmdutil.revert(ui, repo, wctx, [wctx.node(), nullid],
-                           *pathtofiles(repo, revertfiles),
-                           **{'no_backup': True})
-            # fix up the weird dirstate states the merge left behind
-            mf = wctx.manifest()
-            dirstate = repo.dirstate
-            for f in revertfiles:
-                if f in mf:
-                    dirstate.normallookup(f)
-                else:
-                    dirstate.drop(f)
-            dirstate._pl = (wctx.node(), nullid)
-            dirstate._dirty = True
+
+        mergefiles(ui, repo, state.wctx, state.pendingctx, state.unknownfiles)
+
         repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
         shelvedstate.clear(repo)
         ui.warn(_("unshelve of '%s' aborted\n") % state.name)
     finally:
         lockmod.release(lock, wlock)
 
+def mergefiles(ui, repo, wctx, shelvectx, unknownfiles):
+    """updates to wctx and merges the changes from shelvectx into the
+    dirstate. drops any files in unknownfiles from the dirstate."""
+    oldquiet = ui.quiet
+    try:
+        ui.quiet = True
+        hg.update(repo, wctx.node())
+        files = []
+        files.extend(shelvectx.files())
+        files.extend(shelvectx.parents()[0].files())
+        cmdutil.revert(ui, repo, shelvectx, repo.dirstate.parents(),
+                       *pathtofiles(repo, files),
+                       **{'no_backup': True})
+    finally:
+        ui.quiet = oldquiet
+
+    # Send untracked files back to being untracked
+    dirstate = repo.dirstate
+    for f in unknownfiles:
+        dirstate.drop(f)
+
 def unshelvecleanup(ui, repo, name, opts):
     """remove related files after an unshelve"""
     if not opts['keep']:
         for filetype in 'hg files patch'.split():
             shelvedfile(repo, name, filetype).unlink()
 
-def finishmerge(ui, repo, ms, stripnodes, name, opts):
-    # Reset the working dir so it's no longer in a merge state.
-    dirstate = repo.dirstate
-    dirstate.setparents(dirstate._pl[0])
-    shelvedstate.clear(repo)
-
 def unshelvecontinue(ui, repo, state, opts):
     """subcommand to continue an in-progress unshelve"""
     # We're finishing off a merge. First parent is our original
@@ -419,9 +441,30 @@ 
             raise util.Abort(
                 _("unresolved conflicts, can't continue"),
                 hint=_("see 'hg resolve', then 'hg unshelve --continue'"))
-        finishmerge(ui, repo, ms, state.stripnodes, state.name, opts)
+
         lock = repo.lock()
+
+        util.rename(repo.join('unshelverebasestate'),
+                    repo.join('rebasestate'))
+        try:
+            rebase.rebase(ui, repo, **{
+                'continue' : True
+            })
+        except Exception:
+            util.rename(repo.join('rebasestate'),
+                        repo.join('unshelverebasestate'))
+            raise
+
+        shelvectx = repo['tip']
+        if not shelvectx in state.pendingctx.children():
+            # rebase was a no-op, so it produced no child commit
+            shelvectx = state.pendingctx
+
+        mergefiles(ui, repo, state.wctx, shelvectx, state.unknownfiles)
+
+        state.stripnodes.append(shelvectx.node())
         repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
+        shelvedstate.clear(repo)
         unshelvecleanup(ui, repo, state.name, opts)
         ui.status(_("unshelve of '%s' complete\n") % state.name)
     finally:
@@ -491,71 +534,96 @@ 
 
     shelvedfiles = readshelvedfiles(repo, basename)
 
-    m, a, r, d = repo.status()[:4]
-    unsafe = set(m + a + r + d).intersection(shelvedfiles)
-    if unsafe:
-        ui.warn(_('the following shelved files have been modified:\n'))
-        for f in sorted(unsafe):
-            ui.warn('  %s\n' % f)
-        ui.warn(_('you must commit, revert, or shelve your changes before you '
-                  'can proceed\n'))
-        raise util.Abort(_('cannot unshelve due to local changes\n'))
-
     wlock = lock = tr = None
     try:
         lock = repo.lock()
+        wlock = repo.wlock()
 
         tr = repo.transaction('unshelve', report=lambda x: None)
         oldtiprev = len(repo)
+
+        wctx = repo['.']
+        tmpwctx = wctx
+        # The goal is to have a commit structure like so:
+        # ...-> wctx -> tmpwctx -> shelvectx
+        # where tmpwctx is an optional commit with the user's pending changes
+        # and shelvectx is the unshelved changes. Then we merge it all down
+        # to the original wctx.
+
+        # Store pending changes in a commit
+        m, a, r, d, u = repo.status(unknown=True)[:5]
+        if m or a or r or d or u:
+            def commitfunc(ui, repo, message, match, opts):
+                hasmq = util.safehasattr(repo, 'mq')
+                if hasmq:
+                    saved, repo.mq.checkapplied = repo.mq.checkapplied, False
+
+                try:
+                    return repo.commit(message, 'shelve@localhost',
+                                       opts.get('date'), match)
+                finally:
+                    if hasmq:
+                        repo.mq.checkapplied = saved
+
+            tempopts = {}
+            tempopts['message'] = "pending changes temporary commit"
+            tempopts['addremove'] = True
+            oldquiet = ui.quiet
+            try:
+                ui.quiet = True
+                node = cmdutil.commit(ui, repo, commitfunc, None, tempopts)
+            finally:
+                ui.quiet = oldquiet
+            tmpwctx = repo[node]
+
         try:
             fp = shelvedfile(repo, basename, 'hg').opener()
             gen = changegroup.readbundle(fp, fp.name)
             repo.addchangegroup(gen, 'unshelve', 'bundle:' + fp.name)
             nodes = [ctx.node() for ctx in repo.set('%d:', oldtiprev)]
             phases.retractboundary(repo, phases.secret, nodes)
-            tr.close()
         finally:
             fp.close()
 
-        tip = repo['tip']
-        wctx = repo['.']
-        ancestor = tip.ancestor(wctx)
+        shelvectx = repo['tip']
 
-        wlock = repo.wlock()
+        # If the shelve is not immediately on top of the commit
+        # we'll be merging with, rebase it to be on top.
+        if tmpwctx.node() != shelvectx.parents()[0].node():
+            try:
+                rebase.rebase(ui, repo, **{
+                    'rev' : [shelvectx.rev()],
+                    'dest' : str(tmpwctx.rev()),
+                    'keep' : True,
+                })
+            except error.InterventionRequired:
+                tr.close()
 
-        if ancestor.node() != wctx.node():
-            conflicts = hg.merge(repo, tip.node(), force=True, remind=False)
-            ms = merge.mergestate(repo)
-            stripnodes = [repo.changelog.node(rev)
-                          for rev in xrange(oldtiprev, len(repo))]
-            if conflicts:
-                shelvedstate.save(repo, basename, stripnodes)
-                # Fix up the dirstate entries of files from the second
-                # parent as if we were not merging, except for those
-                # with unresolved conflicts.
-                parents = repo.parents()
-                revertfiles = set(parents[1].files()).difference(ms)
-                cmdutil.revert(ui, repo, parents[1],
-                               (parents[0].node(), nullid),
-                               *pathtofiles(repo, revertfiles),
-                               **{'no_backup': True})
+                stripnodes = [repo.changelog.node(rev)
+                              for rev in xrange(oldtiprev, len(repo))]
+                shelvedstate.save(repo, basename, wctx, tmpwctx, stripnodes, u)
+
+                util.rename(repo.join('rebasestate'),
+                            repo.join('unshelverebasestate'))
                 raise error.InterventionRequired(
                     _("unresolved conflicts (see 'hg resolve', then "
                       "'hg unshelve --continue')"))
-            finishmerge(ui, repo, ms, stripnodes, basename, opts)
-        else:
-            parent = tip.parents()[0]
-            hg.update(repo, parent.node())
-            cmdutil.revert(ui, repo, tip, repo.dirstate.parents(),
-                           *pathtofiles(repo, tip.files()),
-                           **{'no_backup': True})
 
-        prevquiet = ui.quiet
-        ui.quiet = True
-        try:
-            repo.rollback(force=True)
-        finally:
-            ui.quiet = prevquiet
+            # refresh ctx after rebase completes
+            shelvectx = repo['tip']
+
+            if not shelvectx in tmpwctx.children():
+                # rebase was a no-op, so it produced no child commit
+                shelvectx = tmpwctx
+
+        mergefiles(ui, repo, wctx, shelvectx, u)
+        shelvedstate.clear(repo)
+
+        # The transaction aborting will strip all the commits for us,
+        # but it doesn't update the inmemory structures, so addchangegroup
+        # hooks still fire and try to operate on the missing commits.
+        # Clean up manually to prevent this.
+        repo.changelog.strip(oldtiprev, tr)
 
         unshelvecleanup(ui, repo, basename, opts)
     finally:
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -27,7 +27,6 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 5 changes to 5 files
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
   $ hg commit -q -m 'initial commit'
 
@@ -100,19 +99,19 @@ 
   $ hg shelve -d default
   $ hg qfinish -a -q
 
-local edits should prevent a shelved change from applying
+local edits should not prevent a shelved change from applying
 
-  $ echo e>>a/a
-  $ hg unshelve
+  $ printf "z\na\n" > a/a
+  $ hg unshelve --keep
   unshelving change 'default-01'
-  the following shelved files have been modified:
-    a/a
-  you must commit, revert, or shelve your changes before you can proceed
-  abort: cannot unshelve due to local changes
-  
-  [255]
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 3 changes to 8 files (+1 heads)
+  merging a/a
 
-  $ hg revert -C a/a
+  $ hg revert --all -q
+  $ rm a/a.orig b.rename/b c.copy
 
 apply it and make sure our state is as expected
 
@@ -122,7 +121,6 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 3 changes to 8 files
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg status -C
   M a/a
   A b.rename/b
@@ -201,24 +199,21 @@ 
   merging a/a
   warning: conflicts during merge.
   merging a/a incomplete! (edit conflicts, then use 'hg resolve --mark')
-  2 files updated, 0 files merged, 1 files removed, 1 files unresolved
-  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
   unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
   [1]
 
 ensure that we have a merge with unresolved conflicts
 
-  $ hg heads -q
-  4:cebf2b8de087
-  3:2e69b451d1ea
-  $ hg parents -q
-  3:2e69b451d1ea
-  4:cebf2b8de087
+  $ hg heads -q --template '{rev}\n'
+  5
+  4
+  $ hg parents -q --template '{rev}\n'
+  4
+  5
   $ hg status
   M a/a
   M b.rename/b
   M c.copy
-  A foo/foo
   R b/b
   ? a/a.orig
   $ hg diff
@@ -248,12 +243,6 @@ 
   +++ b/c.copy
   @@ -0,0 +1,1 @@
   +c
-  diff --git a/foo/foo b/foo/foo
-  new file mode 100644
-  --- /dev/null
-  +++ b/foo/foo
-  @@ -0,0 +1,1 @@
-  +foo
   $ hg resolve -l
   U a/a
 
@@ -268,10 +257,10 @@ 
   M a/a
   M b.rename/b
   M c.copy
-  A foo/foo
   R b/b
   ? a/a.orig
   $ hg unshelve -a
+  rebase aborted
   unshelve of 'default' aborted
   $ hg heads -q
   3:2e69b451d1ea
@@ -330,9 +319,9 @@ 
   3:2e69b451d1ea
 
   $ hg status -C
-  M b.rename/b
+  A b.rename/b
     b/b
-  M c.copy
+  A c.copy
     c
   A foo/foo
   R b/b
@@ -372,6 +361,7 @@ 
 set up another conflict between a commit and a shelved change
 
   $ hg revert -q -C -a
+  $ rm a/a.orig b.rename/b c.copy
   $ echo a >> a/a
   $ hg shelve -q
   $ echo x >> a/a
@@ -387,7 +377,6 @@ 
   adding file changes
   added 1 changesets with 1 changes to 6 files (+1 heads)
   merging a/a
-  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
   $ hg parents -q
   4:33f7f61e6c5e
   $ hg shelve -l
@@ -411,7 +400,6 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 7 files
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg shelve --list
   default         (*)    create conflict (glob)
   $ hg shelve --cleanup
@@ -433,7 +421,6 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 7 files
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg bookmark
    * test                      4:33f7f61e6c5e
 
@@ -450,7 +437,6 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 7 files
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
 shelve should leave dirstate clean (issue 4055)
 
@@ -479,8 +465,52 @@ 
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
-  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg status
   M z
 
   $ cd ..
+
+shelve should only unshelve pending changes (issue 4068)
+
+  $ hg init onlypendingchanges
+  $ cd onlypendingchanges
+  $ touch a
+  $ hg ci -Aqm a
+  $ touch b
+  $ hg ci -Aqm b
+  $ hg up -q 0
+  $ touch c
+  $ hg ci -Aqm c
+
+  $ touch d
+  $ hg add d
+  $ hg shelve
+  shelved as default
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg up -q 1
+  $ hg unshelve
+  unshelving change 'default'
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 3 files
+  $ hg status
+  A d
+
+unshelve should work on an ancestor of the original commit
+
+  $ hg shelve
+  shelved as default
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg up 0
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ hg unshelve
+  unshelving change 'default'
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 3 files
+  $ hg status
+  A d
+
+  $ cd ..