Patchwork [09,of,10,shelve-ext] shelve: add obs-based unshelve functionality

# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1480431173 28800
#      Tue Nov 29 06:52:53 2016 -0800
# Node ID 533d99eca3bf11c4aac869674e0abb16b74ed670
# Parent  85c9c651887915733feb3d385866955741f28ec0
shelve: add obs-based unshelve functionality

Obsolescense-based unshelve works as follows:
1. Instead of stripping temporary nodes, markers are created to
obsolete them.
2. Restoring commit is just finding it in an unfiltered repo.
3. '--keep' is only passed to rebase on traditional unshelves
(and thus traditional rebases), becuase we want markers to be
created fro obsolete-based rebases.
4. 'hg unshelve' uses unfiltered repo to perform rebases
because we want rebase to be able to create markers between original
and new commits. 'rebaseskipobsolete' is disabled to make rebase not
skip the commit altogether.

Excerpts from Kostia Balytskyi's message of 2016-11-29 07:23:03 -0800:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1480431173 28800
> #      Tue Nov 29 06:52:53 2016 -0800
> # Node ID 533d99eca3bf11c4aac869674e0abb16b74ed670
> # Parent  85c9c651887915733feb3d385866955741f28ec0
> shelve: add obs-based unshelve functionality
> 
> Obsolescense-based unshelve works as follows:
> 1. Instead of stripping temporary nodes, markers are created to
> obsolete them.
> 2. Restoring commit is just finding it in an unfiltered repo.
> 3. '--keep' is only passed to rebase on traditional unshelves
> (and thus traditional rebases), becuase we want markers to be
> created fro obsolete-based rebases.
> 4. 'hg unshelve' uses unfiltered repo to perform rebases
> because we want rebase to be able to create markers between original
> and new commits. 'rebaseskipobsolete' is disabled to make rebase not
> skip the commit altogether.
> 
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -26,6 +26,7 @@ import collections
>  import errno
>  import itertools
>  import json
> +import time
>  
>  from mercurial.i18n import _
>  from mercurial import (
> @@ -264,8 +265,13 @@ class shelvedstate(object):
>  
>      def prunenodes(self):
>          """Cleanup temporary nodes from the repo"""
> -        repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> -                     topic='shelve')
> +        if self.obsshelve:
> +            unfi = self.repo.unfiltered()
> +            relations = [(unfi[n], ()) for n in self.nodestoprune]
> +            obsolete.createmarkers(self.repo, relations)
> +        else:
> +            repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> +                         topic='shelve')
>  
>  def cleanupoldbackups(repo):
>      vfs = scmutil.vfs(repo.join(backupdir))
> @@ -670,9 +676,14 @@ def unshelvecontinue(ui, repo, state, op
>          util.rename(repo.join('unshelverebasestate'),
>                      repo.join('rebasestate'))
>          try:
> -            rebase.rebase(ui, repo, **{
> -                'continue' : True
> -            })
> +            # if shelve is obs-based, we want rebase to be able
> +            # to create markers to already-obsoleted commits
> +            _repo = repo.unfiltered() if state.obsshelve else repo
> +            with ui.configoverride({('experimental', 'rebaseskipobsolete'):
> +                                    'off'}, 'unshelve'):
> +                rebase.rebase(ui, _repo, **{
> +                    'continue' : True,
> +                    })
>          except Exception:
>              util.rename(repo.join('rebasestate'),
>                          repo.join('unshelverebasestate'))
> @@ -712,30 +723,54 @@ def _commitworkingcopychanges(ui, repo, 
>      with ui.configoverride({('ui', 'quiet'): True}):
>          node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)
>      tmpwctx = repo[node]
> +    ui.debug("temporary working copy commit: %s:%s\n" %
> +             (tmpwctx.rev(), nodemod.short(node)))
>      return tmpwctx, addedbefore
>  
> -def _unshelverestorecommit(ui, repo, basename):
> +def _unshelverestorecommit(ui, repo, basename, obsshelve, shfile):

"basename" and "shfile" looks duplicated. Maybe just keep one of them.
(pass "file=shelvedfile(repo, basename, 'hg')", or just pass basename)

>      """Recreate commit in the repository during the unshelve"""
>      with ui.configoverride({('ui', 'quiet'): True}):
> -        shelvedfile(repo, basename, 'hg').applybundle()
> -        shelvectx = repo['tip']
> +        if obsshelve:
> +            md = shfile.readjson()
> +            shelvenode = nodemod.bin(md['node'])
> +            repo = repo.unfiltered()
> +            shelvectx = repo[shelvenode]
> +        else:
> +            shelvedfile(repo, basename, 'hg').applybundle()
> +            shelvectx = repo['tip']
>      return repo, shelvectx
>  
>  def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
> -                          tmpwctx, shelvectx, branchtorestore):
> +                          tmpwctx, shelvectx, branchtorestore, obsshelve):
>      """Rebase restored commit from its original location to a destination"""
>      # 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():
> +        # shelvectx is immediately on top of the tmpwctx
>          return shelvectx
>  
> +    # we need a new commit extra every time we perform a rebase to ensure
> +    # that "nothing to rebase" does not happen with obs-based shelve
> +    # "nothing to rebase" means that tip does not point to a "successor"
> +    # commit after a rebase and we have no way to learn which commit
> +    # should be a "shelvectx". this is a dirty hack until we implement
> +    # some way to learn the results of rebase operation, other than
> +    # text output and return code
> +    def extrafn(ctx, extra):
> +        extra['unshelve_time'] = str(time.time())
> +
>      ui.status(_('rebasing shelved changes\n'))
>      try:
> +        # we only want keep to be true if shelve is traditional, since
> +        # for obs-based shelve, rebase will also be obs-based and
> +        # markers created help us track the relationship between shelvectx
> +        # and its new version
>          rebase.rebase(ui, repo, **{
>              'rev': [shelvectx.rev()],
>              'dest': str(tmpwctx.rev()),
> -            'keep': True,
> +            'keep': not obsshelve,
>              'tool': opts.get('tool', ''),
> +            'extrafn': extrafn if obsshelve else None
>          })

This is another case why we need an in-core "lightweight", "smart" rebase
method badly. See https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090907.html

>      except error.InterventionRequired:
>          tr.close()
> @@ -743,7 +778,7 @@ def _rebaserestoredcommit(ui, repo, opts
>          nodestoprune = [repo.changelog.node(rev)
>                          for rev in xrange(oldtiprev, len(repo))]
>          shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoprune,
> -                          branchtorestore, opts.get('keep'))
> +                          branchtorestore, opts.get('keep'), obsshelve)
>  
>          util.rename(repo.join('rebasestate'),
>                      repo.join('unshelverebasestate'))
> @@ -770,7 +805,10 @@ def _forgetunknownfiles(repo, shelvectx,
>      toforget = (addedafter & shelveunknown) - addedbefore
>      repo[None].forget(toforget)
>  
> -def _finishunshelve(repo, oldtiprev, tr):
> +def _finishunshelve(repo, oldtiprev, tr, obsshelve):
> +    if obsshelve:
> +        tr.close()
> +        return

This looks a bit weird. Could we use "with" or "finally" ?

>      # 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.
> @@ -778,6 +816,18 @@ def _finishunshelve(repo, oldtiprev, tr)
>      repo.unfiltered().changelog.strip(oldtiprev, tr)
>      _aborttransaction(repo)
>  
> +def _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx):
> +    # order is important in the list of [shelvectx, tmpwctx] below
> +    # some nodes may already be obsolete
> +    unfi = repo.unfiltered()
> +    nodestoobsolete = filter(lambda x: x != pctx, [shelvectx, tmpwctx])
> +    obsoleted = set()
> +    for nts in nodestoobsolete:
> +        if nts in obsoleted:
> +            continue
> +        obsoleted.add(nts)
> +        obsolete.createmarkers(unfi, [(unfi[nts.node()], ())])

It's better to calculate the "relations" in the loop and call
"createmarkers" a single time to create all of them.

> +
>  @command('unshelve',
>           [('a', 'abort', None,
>             _('abort an incomplete unshelve operation')),
> @@ -885,6 +935,12 @@ def _dounshelve(ui, repo, *shelved, **op
>          raise error.Abort(_("shelved change '%s' not found") % basename)
>  
>      lock = tr = None
> +    obsshelve = isobsshelve(repo, ui)
> +    obsshelvedfile = shelvedfile(repo, basename, 'oshelve')
> +    if obsshelve and not obsshelvedfile.exists():
> +        # although we can unshelve a obs-based shelve technically,
> +        # this particular shelve was created using a traditional way

Maybe print something here.

> +        obsshelve = False
>      try:
>          lock = repo.lock()
>          tr = repo.transaction('unshelve', report=lambda x: None)
> @@ -901,23 +957,29 @@ def _dounshelve(ui, repo, *shelved, **op
>          tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
>                                                           tmpwctx)
>  
> -        repo, shelvectx = _unshelverestorecommit(ui, repo, basename)
> +        repo, shelvectx = _unshelverestorecommit(ui, repo, basename,
> +                                                 obsshelve, obsshelvedfile)
>  
>          branchtorestore = ''
>          if shelvectx.branch() != shelvectx.p1().branch():
>              branchtorestore = shelvectx.branch()
>  
> -        with ui.configoverride({('ui', 'forcemerge'): opts.get('tool', '')},
> -                               'unshelve'):
> +        rebaseconfigoverrides = {('ui', 'forcemerge'): opts.get('tool', ''),
> +                                 ('experimental', 'rebaseskipobsolete'): 'off'}
> +        with ui.configoverride(rebaseconfigoverrides, 'unshelve'):
>              shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
>                                                basename, pctx, tmpwctx,
> -                                              shelvectx, branchtorestore)
> +                                              shelvectx, branchtorestore,
> +                                              obsshelve)
>              mergefiles(ui, repo, pctx, shelvectx)
>          restorebranch(ui, repo, branchtorestore)
>          _forgetunknownfiles(repo, shelvectx, addedbefore)
>  
> +        if obsshelve:
> +            _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx)
> +
>          shelvedstate.clear(repo)
> -        _finishunshelve(repo, oldtiprev, tr)
> +        _finishunshelve(repo, oldtiprev, tr, obsshelve)
>          unshelvecleanup(ui, repo, basename, opts)
>      finally:
>          if tr:

On Tue, Nov 29, 2016, 08:23 Kostia Balytskyi <ikostia@fb.com> wrote:

> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1480431173 28800
> #      Tue Nov 29 06:52:53 2016 -0800
> # Node ID 533d99eca3bf11c4aac869674e0abb16b74ed670
> # Parent  85c9c651887915733feb3d385866955741f28ec0
> shelve: add obs-based unshelve functionality
>
> Obsolescense-based unshelve works as follows:
> 1. Instead of stripping temporary nodes, markers are created to
> obsolete them.
> 2. Restoring commit is just finding it in an unfiltered repo.
>

What happens if it's been stripped? Speaking of that, I didn't see any
tests added in this series. Seems like there should be some. Did I just
miss them?

3. '--keep' is only passed to rebase on traditional unshelves
> (and thus traditional rebases), becuase we want markers to be
> created fro obsolete-based rebases.
> 4. 'hg unshelve' uses unfiltered repo to perform rebases
> because we want rebase to be able to create markers between original
> and new commits. 'rebaseskipobsolete' is disabled to make rebase not
> skip the commit altogether.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -26,6 +26,7 @@ import collections
>  import errno
>  import itertools
>  import json
> +import time
>
>  from mercurial.i18n import _
>  from mercurial import (
> @@ -264,8 +265,13 @@ class shelvedstate(object):
>
>      def prunenodes(self):
>          """Cleanup temporary nodes from the repo"""
> -        repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> -                     topic='shelve')
> +        if self.obsshelve:
> +            unfi = self.repo.unfiltered()
> +            relations = [(unfi[n], ()) for n in self.nodestoprune]
> +            obsolete.createmarkers(self.repo, relations)
> +        else:
> +            repair.strip(self.ui, self.repo, self.nodestoprune,
> backup=False,
> +                         topic='shelve')
>
>  def cleanupoldbackups(repo):
>      vfs = scmutil.vfs(repo.join(backupdir))
> @@ -670,9 +676,14 @@ def unshelvecontinue(ui, repo, state, op
>          util.rename(repo.join('unshelverebasestate'),
>                      repo.join('rebasestate'))
>          try:
> -            rebase.rebase(ui, repo, **{
> -                'continue' : True
> -            })
> +            # if shelve is obs-based, we want rebase to be able
> +            # to create markers to already-obsoleted commits
> +            _repo = repo.unfiltered() if state.obsshelve else repo
> +            with ui.configoverride({('experimental',
> 'rebaseskipobsolete'):
> +                                    'off'}, 'unshelve'):
> +                rebase.rebase(ui, _repo, **{
> +                    'continue' : True,
> +                    })
>          except Exception:
>              util.rename(repo.join('rebasestate'),
>                          repo.join('unshelverebasestate'))
> @@ -712,30 +723,54 @@ def _commitworkingcopychanges(ui, repo,
>      with ui.configoverride({('ui', 'quiet'): True}):
>          node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)
>      tmpwctx = repo[node]
> +    ui.debug("temporary working copy commit: %s:%s\n" %
> +             (tmpwctx.rev(), nodemod.short(node)))
>      return tmpwctx, addedbefore
>
> -def _unshelverestorecommit(ui, repo, basename):
> +def _unshelverestorecommit(ui, repo, basename, obsshelve, shfile):
>      """Recreate commit in the repository during the unshelve"""
>      with ui.configoverride({('ui', 'quiet'): True}):
> -        shelvedfile(repo, basename, 'hg').applybundle()
> -        shelvectx = repo['tip']
> +        if obsshelve:
> +            md = shfile.readjson()
> +            shelvenode = nodemod.bin(md['node'])
> +            repo = repo.unfiltered()
> +            shelvectx = repo[shelvenode]
> +        else:
> +            shelvedfile(repo, basename, 'hg').applybundle()
> +            shelvectx = repo['tip']
>      return repo, shelvectx
>
>  def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
> -                          tmpwctx, shelvectx, branchtorestore):
> +                          tmpwctx, shelvectx, branchtorestore, obsshelve):
>      """Rebase restored commit from its original location to a
> destination"""
>      # 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():
> +        # shelvectx is immediately on top of the tmpwctx
>          return shelvectx
>
> +    # we need a new commit extra every time we perform a rebase to ensure
> +    # that "nothing to rebase" does not happen with obs-based shelve
> +    # "nothing to rebase" means that tip does not point to a "successor"
> +    # commit after a rebase and we have no way to learn which commit
> +    # should be a "shelvectx". this is a dirty hack until we implement
> +    # some way to learn the results of rebase operation, other than
> +    # text output and return code
> +    def extrafn(ctx, extra):
> +        extra['unshelve_time'] = str(time.time())
> +
>      ui.status(_('rebasing shelved changes\n'))
>      try:
> +        # we only want keep to be true if shelve is traditional, since
> +        # for obs-based shelve, rebase will also be obs-based and
> +        # markers created help us track the relationship between shelvectx
> +        # and its new version
>          rebase.rebase(ui, repo, **{
>              'rev': [shelvectx.rev()],
>              'dest': str(tmpwctx.rev()),
> -            'keep': True,
> +            'keep': not obsshelve,
>              'tool': opts.get('tool', ''),
> +            'extrafn': extrafn if obsshelve else None
>          })
>      except error.InterventionRequired:
>          tr.close()
> @@ -743,7 +778,7 @@ def _rebaserestoredcommit(ui, repo, opts
>          nodestoprune = [repo.changelog.node(rev)
>                          for rev in xrange(oldtiprev, len(repo))]
>          shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoprune,
> -                          branchtorestore, opts.get('keep'))
> +                          branchtorestore, opts.get('keep'), obsshelve)
>
>          util.rename(repo.join('rebasestate'),
>                      repo.join('unshelverebasestate'))
> @@ -770,7 +805,10 @@ def _forgetunknownfiles(repo, shelvectx,
>      toforget = (addedafter & shelveunknown) - addedbefore
>      repo[None].forget(toforget)
>
> -def _finishunshelve(repo, oldtiprev, tr):
> +def _finishunshelve(repo, oldtiprev, tr, obsshelve):
> +    if obsshelve:
> +        tr.close()
> +        return
>      # 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.
> @@ -778,6 +816,18 @@ def _finishunshelve(repo, oldtiprev, tr)
>      repo.unfiltered().changelog.strip(oldtiprev, tr)
>      _aborttransaction(repo)
>
> +def _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx):
> +    # order is important in the list of [shelvectx, tmpwctx] below
> +    # some nodes may already be obsolete
> +    unfi = repo.unfiltered()
> +    nodestoobsolete = filter(lambda x: x != pctx, [shelvectx, tmpwctx])
> +    obsoleted = set()
> +    for nts in nodestoobsolete:
> +        if nts in obsoleted:
> +            continue
> +        obsoleted.add(nts)
> +        obsolete.createmarkers(unfi, [(unfi[nts.node()], ())])
> +
>  @command('unshelve',
>           [('a', 'abort', None,
>             _('abort an incomplete unshelve operation')),
> @@ -885,6 +935,12 @@ def _dounshelve(ui, repo, *shelved, **op
>          raise error.Abort(_("shelved change '%s' not found") % basename)
>
>      lock = tr = None
> +    obsshelve = isobsshelve(repo, ui)
> +    obsshelvedfile = shelvedfile(repo, basename, 'oshelve')
> +    if obsshelve and not obsshelvedfile.exists():
> +        # although we can unshelve a obs-based shelve technically,
> +        # this particular shelve was created using a traditional way
> +        obsshelve = False
>      try:
>          lock = repo.lock()
>          tr = repo.transaction('unshelve', report=lambda x: None)
> @@ -901,23 +957,29 @@ def _dounshelve(ui, repo, *shelved, **op
>          tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
>                                                           tmpwctx)
>
> -        repo, shelvectx = _unshelverestorecommit(ui, repo, basename)
> +        repo, shelvectx = _unshelverestorecommit(ui, repo, basename,
> +                                                 obsshelve,
> obsshelvedfile)
>
>          branchtorestore = ''
>          if shelvectx.branch() != shelvectx.p1().branch():
>              branchtorestore = shelvectx.branch()
>
> -        with ui.configoverride({('ui', 'forcemerge'): opts.get('tool',
> '')},
> -                               'unshelve'):
> +        rebaseconfigoverrides = {('ui', 'forcemerge'): opts.get('tool',
> ''),
> +                                 ('experimental', 'rebaseskipobsolete'):
> 'off'}
> +        with ui.configoverride(rebaseconfigoverrides, 'unshelve'):
>              shelvectx = _rebaserestoredcommit(ui, repo, opts, tr,
> oldtiprev,
>                                                basename, pctx, tmpwctx,
> -                                              shelvectx, branchtorestore)
> +                                              shelvectx, branchtorestore,
> +                                              obsshelve)
>              mergefiles(ui, repo, pctx, shelvectx)
>          restorebranch(ui, repo, branchtorestore)
>          _forgetunknownfiles(repo, shelvectx, addedbefore)
>
> +        if obsshelve:
> +            _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx)
> +
>          shelvedstate.clear(repo)
> -        _finishunshelve(repo, oldtiprev, tr)
> +        _finishunshelve(repo, oldtiprev, tr, obsshelve)
>          unshelvecleanup(ui, repo, basename, opts)
>      finally:
>          if tr:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

On 11/29/16 4:26 PM, Jun Wu wrote:
> Excerpts from Kostia Balytskyi's message of 2016-11-29 07:23:03 -0800:

>> # HG changeset patch

>> # User Kostia Balytskyi <ikostia@fb.com>

>> # Date 1480431173 28800

>> #      Tue Nov 29 06:52:53 2016 -0800

>> # Node ID 533d99eca3bf11c4aac869674e0abb16b74ed670

>> # Parent  85c9c651887915733feb3d385866955741f28ec0

>> shelve: add obs-based unshelve functionality

>>

>> Obsolescense-based unshelve works as follows:

>> 1. Instead of stripping temporary nodes, markers are created to

>> obsolete them.

>> 2. Restoring commit is just finding it in an unfiltered repo.

>> 3. '--keep' is only passed to rebase on traditional unshelves

>> (and thus traditional rebases), becuase we want markers to be

>> created fro obsolete-based rebases.

>> 4. 'hg unshelve' uses unfiltered repo to perform rebases

>> because we want rebase to be able to create markers between original

>> and new commits. 'rebaseskipobsolete' is disabled to make rebase not

>> skip the commit altogether.

>>

>> diff --git a/hgext/shelve.py b/hgext/shelve.py

>> --- a/hgext/shelve.py

>> +++ b/hgext/shelve.py

>> @@ -26,6 +26,7 @@ import collections

>>   import errno

>>   import itertools

>>   import json

>> +import time

>>   

>>   from mercurial.i18n import _

>>   from mercurial import (

>> @@ -264,8 +265,13 @@ class shelvedstate(object):

>>   

>>       def prunenodes(self):

>>           """Cleanup temporary nodes from the repo"""

>> -        repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,

>> -                     topic='shelve')

>> +        if self.obsshelve:

>> +            unfi = self.repo.unfiltered()

>> +            relations = [(unfi[n], ()) for n in self.nodestoprune]

>> +            obsolete.createmarkers(self.repo, relations)

>> +        else:

>> +            repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,

>> +                         topic='shelve')

>>   

>>   def cleanupoldbackups(repo):

>>       vfs = scmutil.vfs(repo.join(backupdir))

>> @@ -670,9 +676,14 @@ def unshelvecontinue(ui, repo, state, op

>>           util.rename(repo.join('unshelverebasestate'),

>>                       repo.join('rebasestate'))

>>           try:

>> -            rebase.rebase(ui, repo, **{

>> -                'continue' : True

>> -            })

>> +            # if shelve is obs-based, we want rebase to be able

>> +            # to create markers to already-obsoleted commits

>> +            _repo = repo.unfiltered() if state.obsshelve else repo

>> +            with ui.configoverride({('experimental', 'rebaseskipobsolete'):

>> +                                    'off'}, 'unshelve'):

>> +                rebase.rebase(ui, _repo, **{

>> +                    'continue' : True,

>> +                    })

>>           except Exception:

>>               util.rename(repo.join('rebasestate'),

>>                           repo.join('unshelverebasestate'))

>> @@ -712,30 +723,54 @@ def _commitworkingcopychanges(ui, repo,

>>       with ui.configoverride({('ui', 'quiet'): True}):

>>           node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)

>>       tmpwctx = repo[node]

>> +    ui.debug("temporary working copy commit: %s:%s\n" %

>> +             (tmpwctx.rev(), nodemod.short(node)))

>>       return tmpwctx, addedbefore

>>   

>> -def _unshelverestorecommit(ui, repo, basename):

>> +def _unshelverestorecommit(ui, repo, basename, obsshelve, shfile):

> "basename" and "shfile" looks duplicated. Maybe just keep one of them.

> (pass "file=shelvedfile(repo, basename, 'hg')", or just pass basename)

Will fix.
>>       """Recreate commit in the repository during the unshelve"""

>>       with ui.configoverride({('ui', 'quiet'): True}):

>> -        shelvedfile(repo, basename, 'hg').applybundle()

>> -        shelvectx = repo['tip']

>> +        if obsshelve:

>> +            md = shfile.readjson()

>> +            shelvenode = nodemod.bin(md['node'])

>> +            repo = repo.unfiltered()

>> +            shelvectx = repo[shelvenode]

>> +        else:

>> +            shelvedfile(repo, basename, 'hg').applybundle()

>> +            shelvectx = repo['tip']

>>       return repo, shelvectx

>>   

>>   def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,

>> -                          tmpwctx, shelvectx, branchtorestore):

>> +                          tmpwctx, shelvectx, branchtorestore, obsshelve):

>>       """Rebase restored commit from its original location to a destination"""

>>       # 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():

>> +        # shelvectx is immediately on top of the tmpwctx

>>           return shelvectx

>>   

>> +    # we need a new commit extra every time we perform a rebase to ensure

>> +    # that "nothing to rebase" does not happen with obs-based shelve

>> +    # "nothing to rebase" means that tip does not point to a "successor"

>> +    # commit after a rebase and we have no way to learn which commit

>> +    # should be a "shelvectx". this is a dirty hack until we implement

>> +    # some way to learn the results of rebase operation, other than

>> +    # text output and return code

>> +    def extrafn(ctx, extra):

>> +        extra['unshelve_time'] = str(time.time())

>> +

>>       ui.status(_('rebasing shelved changes\n'))

>>       try:

>> +        # we only want keep to be true if shelve is traditional, since

>> +        # for obs-based shelve, rebase will also be obs-based and

>> +        # markers created help us track the relationship between shelvectx

>> +        # and its new version

>>           rebase.rebase(ui, repo, **{

>>               'rev': [shelvectx.rev()],

>>               'dest': str(tmpwctx.rev()),

>> -            'keep': True,

>> +            'keep': not obsshelve,

>>               'tool': opts.get('tool', ''),

>> +            'extrafn': extrafn if obsshelve else None

>>           })

> This is another case why we need an in-core "lightweight", "smart" rebase

> method badly. See https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090907.html

>

>>       except error.InterventionRequired:

>>           tr.close()

>> @@ -743,7 +778,7 @@ def _rebaserestoredcommit(ui, repo, opts

>>           nodestoprune = [repo.changelog.node(rev)

>>                           for rev in xrange(oldtiprev, len(repo))]

>>           shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoprune,

>> -                          branchtorestore, opts.get('keep'))

>> +                          branchtorestore, opts.get('keep'), obsshelve)

>>   

>>           util.rename(repo.join('rebasestate'),

>>                       repo.join('unshelverebasestate'))

>> @@ -770,7 +805,10 @@ def _forgetunknownfiles(repo, shelvectx,

>>       toforget = (addedafter & shelveunknown) - addedbefore

>>       repo[None].forget(toforget)

>>   

>> -def _finishunshelve(repo, oldtiprev, tr):

>> +def _finishunshelve(repo, oldtiprev, tr, obsshelve):

>> +    if obsshelve:

>> +        tr.close()

>> +        return

> This looks a bit weird. Could we use "with" or "finally" ?

_finishunshelve is currently a wrapper for _aborttransaction() which 
used to be invoked not from finally, but from try. I want to keep it 
this way for obsshelve as well. Also, finally seems like a wrong place 
for tr.close().
>>       # 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.

>> @@ -778,6 +816,18 @@ def _finishunshelve(repo, oldtiprev, tr)

>>       repo.unfiltered().changelog.strip(oldtiprev, tr)

>>       _aborttransaction(repo)

>>   

>> +def _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx):

>> +    # order is important in the list of [shelvectx, tmpwctx] below

>> +    # some nodes may already be obsolete

>> +    unfi = repo.unfiltered()

>> +    nodestoobsolete = filter(lambda x: x != pctx, [shelvectx, tmpwctx])

>> +    obsoleted = set()

>> +    for nts in nodestoobsolete:

>> +        if nts in obsoleted:

>> +            continue

>> +        obsoleted.add(nts)

>> +        obsolete.createmarkers(unfi, [(unfi[nts.node()], ())])

> It's better to calculate the "relations" in the loop and call

> "createmarkers" a single time to create all of them.

Will fix.
>

>> +

>>   @command('unshelve',

>>            [('a', 'abort', None,

>>              _('abort an incomplete unshelve operation')),

>> @@ -885,6 +935,12 @@ def _dounshelve(ui, repo, *shelved, **op

>>           raise error.Abort(_("shelved change '%s' not found") % basename)

>>   

>>       lock = tr = None

>> +    obsshelve = isobsshelve(repo, ui)

>> +    obsshelvedfile = shelvedfile(repo, basename, 'oshelve')

>> +    if obsshelve and not obsshelvedfile.exists():

>> +        # although we can unshelve a obs-based shelve technically,

>> +        # this particular shelve was created using a traditional way

> Maybe print something here.

Will print ui.note, don't think this deserves a ui.status.
>

>> +        obsshelve = False

>>       try:

>>           lock = repo.lock()

>>           tr = repo.transaction('unshelve', report=lambda x: None)

>> @@ -901,23 +957,29 @@ def _dounshelve(ui, repo, *shelved, **op

>>           tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,

>>                                                            tmpwctx)

>>   

>> -        repo, shelvectx = _unshelverestorecommit(ui, repo, basename)

>> +        repo, shelvectx = _unshelverestorecommit(ui, repo, basename,

>> +                                                 obsshelve, obsshelvedfile)

>>   

>>           branchtorestore = ''

>>           if shelvectx.branch() != shelvectx.p1().branch():

>>               branchtorestore = shelvectx.branch()

>>   

>> -        with ui.configoverride({('ui', 'forcemerge'): opts.get('tool', '')},

>> -                               'unshelve'):

>> +        rebaseconfigoverrides = {('ui', 'forcemerge'): opts.get('tool', ''),

>> +                                 ('experimental', 'rebaseskipobsolete'): 'off'}

>> +        with ui.configoverride(rebaseconfigoverrides, 'unshelve'):

>>               shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,

>>                                                 basename, pctx, tmpwctx,

>> -                                              shelvectx, branchtorestore)

>> +                                              shelvectx, branchtorestore,

>> +                                              obsshelve)

>>               mergefiles(ui, repo, pctx, shelvectx)

>>           restorebranch(ui, repo, branchtorestore)

>>           _forgetunknownfiles(repo, shelvectx, addedbefore)

>>   

>> +        if obsshelve:

>> +            _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx)

>> +

>>           shelvedstate.clear(repo)

>> -        _finishunshelve(repo, oldtiprev, tr)

>> +        _finishunshelve(repo, oldtiprev, tr, obsshelve)

>>           unshelvecleanup(ui, repo, basename, opts)

>>       finally:

>>           if tr:

> _______________________________________________

> Mercurial-devel mailing list

> Mercurial-devel@mercurial-scm.org

> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel