[PATCH 6 of 7 shelve-ext v5] shelve: add obs-based unshelve functionality
Kostia Balytskyi
kobalyts at outlook.com
Thu Mar 30 06:22:11 EDT 2017
On 30/03/2017 10:24, Ryan McElroy wrote:
> 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?
It is not yet clear what is the best way to send them. It can either be
a single >1000 line
file or ~30 separate patches. In any case, I want to have series as
short as possible.
>
>>
>> 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.
See my reply to the previous patch for the logic separation. I do not
think there's a lot of obsolescense/traditional branching in this
function. Sorry about the overrides renaming, but I feel like wrapping
smaller pieces of code is good.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list