[PATCH 6 of 7 shelve-ext v5] shelve: add obs-based unshelve functionality
Ryan McElroy
rm at fb.com
Thu Mar 30 05:24:09 EDT 2017
On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1490790691 25200
> # Wed Mar 29 05:31:31 2017 -0700
> # Node ID 9ce0a366a6316d1cffc2a875e7dc6b4a3227c851
> # Parent 743fea249a852994c5bd3e47cdfb617719f19a0a
> 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.
s/fro/for
> 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.
> 5. As per sprint discussions, hiding commits with obsmarkers is
> not ideal. This is okay for now, as long as obsshelve is an
> experimental feature. In future, once a better approach to
s/In future/In the future
> hiding things is implemented, we can change the way obsshelve
> works (and also change its name to not refer to obsolescense).
This is a great comment.
>
> Tests for obs-shelve are coming in a separate series.
Why?
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -25,6 +25,7 @@ from __future__ import absolute_import
> import collections
> import errno
> import itertools
> +import time
>
> from mercurial.i18n import _
> from mercurial import (
> @@ -254,8 +255,13 @@ class shelvedstate(object):
>
> def prunenodes(self, ui, repo):
> """Cleanup temporary nodes from the repo"""
> - repair.strip(ui, repo, self.nodestoprune, backup=False,
> - topic='shelve')
> + if self.obsshelve:
> + unfi = repo.unfiltered()
> + relations = [(unfi[n], ()) for n in self.nodestoprune]
> + obsolete.createmarkers(repo, relations)
> + else:
> + repair.strip(ui, repo, self.nodestoprune, backup=False,
> + topic='shelve')
>
> def cleanupoldbackups(repo):
> vfs = vfsmod.vfs(repo.vfs.join(backupdir))
> @@ -675,9 +681,14 @@ def unshelvecontinue(ui, repo, state, op
>
> repo.vfs.rename('unshelverebasestate', '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:
> repo.vfs.rename('rebasestate', 'unshelverebasestate')
> raise
> @@ -717,31 +728,59 @@ 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):
> """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 = shelvedfile(repo, basename, 'oshelve').readobsshelveinfo()
> + shelvenode = nodemod.bin(md['node'])
> + repo = repo.unfiltered()
> + try:
> + shelvectx = repo[shelvenode]
> + except error.RepoLookupError:
> + m = _("shelved node %s not found in repo")
> + raise error.Abort(m % md['node'])
> + else:
> + shelvedfile(repo, basename, 'hg').applybundle()
> + shelvectx = repo['tip']
> return repo, shelvectx
>
> def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
> tmpwctx, shelvectx, branchtorestore,
> - activebookmark):
> + activebookmark, 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()
> @@ -749,7 +788,8 @@ 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'), activebookmark)
> + branchtorestore, opts.get('keep'),
> + activebookmark, obsshelve)
>
> repo.vfs.rename('rebasestate', 'unshelverebasestate')
> raise error.InterventionRequired(
> @@ -775,8 +815,11 @@ def _forgetunknownfiles(repo, shelvectx,
> toforget = (addedafter & shelveunknown) - addedbefore
> repo[None].forget(toforget)
>
> -def _finishunshelve(repo, oldtiprev, tr, activebookmark):
> +def _finishunshelve(repo, oldtiprev, tr, activebookmark, obsshelve):
> _restoreactivebookmark(repo, activebookmark)
> + 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.
> @@ -795,6 +838,20 @@ def _checkunshelveuntrackedproblems(ui,
> hint = _("run hg status to see which files are missing")
> raise error.Abort(m, hint=hint)
>
> +def _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx):
Maybe call this pruneredundantnodes?
Comment should clarify what is considered "redundant".
> + # 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])
> + seen = set()
> + relations = []
> + for nto in nodestoobsolete:
> + if nto in seen:
> + continue
> + seen.add(nto)
> + relations.append((unfi[nto.rev()], ()))
> + obsolete.createmarkers(unfi, relations)
> +
> @command('unshelve',
> [('a', 'abort', None,
> _('abort an incomplete unshelve operation')),
> @@ -907,6 +964,14 @@ 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
> + ui.note(_("falling back to traditional unshelve since "
> + "shelve was traditional"))
> try:
> lock = repo.lock()
> tr = repo.transaction('unshelve', report=lambda x: None)
> @@ -921,28 +986,31 @@ def _dounshelve(ui, repo, *shelved, **op
> # to the original pctx.
>
> activebookmark = _backupactivebookmark(repo)
> - overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
> - with ui.configoverride(overrides, 'unshelve'):
> - tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
> - tmpwctx)
> - repo, shelvectx = _unshelverestorecommit(ui, repo, basename,
> - oldquiet)
> - _checkunshelveuntrackedproblems(ui, repo, shelvectx)
> - branchtorestore = ''
> - if shelvectx.branch() != shelvectx.p1().branch():
> - branchtorestore = shelvectx.branch()
> + tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
> + tmpwctx)
> + repo, shelvectx = _unshelverestorecommit(ui, repo, basename, obsshelve)
> + _checkunshelveuntrackedproblems(ui, repo, shelvectx)
> + branchtorestore = ''
> + if shelvectx.branch() != shelvectx.p1().branch():
> + branchtorestore = shelvectx.branch()
>
> + 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,
> - activebookmark)
> + activebookmark, obsshelve)
> mergefiles(ui, repo, pctx, shelvectx)
> restorebranch(ui, repo, branchtorestore)
> _forgetunknownfiles(repo, shelvectx, addedbefore)
>
> - shelvedstate.clear(repo)
> - _finishunshelve(repo, oldtiprev, tr, activebookmark)
> - unshelvecleanup(ui, repo, basename, opts)
> + if obsshelve:
> + _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx)
> +
> + shelvedstate.clear(repo)
> + _finishunshelve(repo, oldtiprev, tr, activebookmark, obsshelve)
> + unshelvecleanup(ui, repo, basename, opts)
> finally:
> if tr:
> tr.release()
Again, I think that this would be clearer with more separated logic
rather than interspersing the two. It also seems that there are a lot of
not-obviously-related changes here, such as renaming overrides, which
make it more taxing to review.
More information about the Mercurial-devel
mailing list