[PATCH 10 of 10 shelve-ext] shelve: add logic to preserve active bookmarks
Jun Wu
quark at fb.com
Tue Nov 29 16:36:50 UTC 2016
General direction looks good to me. I have a quick scan and commented on
some nits.
It seems the unshelve process is a different from the traditional one so
a lot of methods were changed to accept new "obsshelve" (bool), "tr",
"shfile" parameters. In this case, it may be cleaner to have two different
"unshelver" classes that implement different approaches and keep internal
states like "shfile" etc.
That said, the above is only a suggestion, not a request-change. We can
always do refactoring later.
Excerpts from Kostia Balytskyi's message of 2016-11-29 07:23:04 -0800:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1480431519 28800
> # Tue Nov 29 06:58:39 2016 -0800
> # Node ID 5c55e23c067998adf36b9a2c6eb028ba1a7fc643
> # Parent 533d99eca3bf11c4aac869674e0abb16b74ed670
> shelve: add logic to preserve active bookmarks
>
> This adds an explicit active-bookmark-handling logic
> to *both* traditional and obs-based shelve. Although it
> is possible to only add it to obs-based, I think it would
> be ugly and I see no harm in explicitly handling bookmarks
> in addition to reliance on trasnactions.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -30,6 +30,7 @@ import time
>
> from mercurial.i18n import _
> from mercurial import (
> + bookmarks,
> bundle2,
> bundlerepo,
> changegroup,
> @@ -200,6 +201,8 @@ class shelvedstate(object):
> _nokeep = 'nokeep'
> _obsbased = 'obsbased'
> _traditional = 'traditional'
> + # colon is essential to differentiate from a real bookmark name
> + _noactivebook = ':no-active-bookmark'
>
> def __init__(self, ui, repo):
> self.ui = ui
> @@ -222,6 +225,7 @@ class shelvedstate(object):
> branchtorestore = fp.readline().strip()
> keep = fp.readline().strip() == cls._keep
> obsshelve = fp.readline().strip() == cls._obsbased
> + activebook = fp.readline().strip()
> except (ValueError, TypeError) as err:
> raise error.CorruptedState(str(err))
> finally:
> @@ -237,6 +241,9 @@ class shelvedstate(object):
> obj.branchtorestore = branchtorestore
> obj.keep = keep
> obj.obsshelve = obsshelve
> + obj.activebookmark = ''
> + if activebook != cls._noactivebook:
> + obj.activebookmark = activebook
> except error.RepoLookupError as err:
> raise error.CorruptedState(str(err))
>
> @@ -244,7 +251,7 @@ class shelvedstate(object):
>
> @classmethod
> def save(cls, repo, name, originalwctx, pendingctx, nodestoprune,
> - branchtorestore, keep=False, obsshelve=False):
> + branchtorestore, keep=False, obsshelve=False, activebook=''):
> fp = repo.vfs(cls._filename, 'wb')
> fp.write('%i\n' % cls._version)
> fp.write('%s\n' % name)
> @@ -257,6 +264,7 @@ class shelvedstate(object):
> fp.write('%s\n' % branchtorestore)
> fp.write('%s\n' % (cls._keep if keep else cls._nokeep))
> fp.write('%s\n' % (cls._obsbased if obsshelve else cls._traditional))
> + fp.write('%s\n' % (activebook or cls._noactivebook))
> fp.close()
>
> @classmethod
> @@ -295,6 +303,16 @@ def cleanupoldbackups(repo):
> if err.errno != errno.ENOENT:
> raise
>
> +def _backupactivebookmark(repo):
> + activebookmark = repo._activebookmark
> + if activebookmark:
> + bookmarks.deactivate(repo)
> + return activebookmark
> +
> +def _restoreactivebookmark(repo, mark):
> + if mark:
> + bookmarks.activate(repo, mark)
> +
> def _aborttransaction(repo):
> '''Abort current transaction for shelve/unshelve, but keep dirstate
> '''
> @@ -410,7 +428,7 @@ def _includeunknownfiles(repo, pats, opt
> extra['shelve_unknown'] = '\0'.join(s.unknown)
> repo[None].add(s.unknown)
>
> -def _finishshelve(ui, repo, tr):
> +def _finishshelve(ui, repo, tr, activebookmark):
> if isobsshelve(repo, ui):
> tr.close()
> tr.release()
> @@ -433,7 +451,7 @@ def _docreatecmd(ui, repo, pats, opts):
> if not opts.get('message'):
> opts['message'] = desc
>
> - lock = tr = None
> + lock = tr = activebookmark = None
> try:
> lock = repo.lock()
>
> @@ -449,6 +467,7 @@ def _docreatecmd(ui, repo, pats, opts):
> not opts.get('addremove', False))
>
> name = getshelvename(repo, parent, opts)
> + activebookmark = _backupactivebookmark(repo)
> extra = {}
> if includeunknown:
> _includeunknownfiles(repo, pats, opts, extra)
> @@ -463,7 +482,8 @@ def _docreatecmd(ui, repo, pats, opts):
> node = cmdutil.commit(ui, repo, commitfunc, pats, opts)
> else:
> node = cmdutil.dorecord(ui, repo, commitfunc, None,
> - False, cmdutil.recordfilter, *pats, **opts)
> + False, cmdutil.recordfilter, *pats,
> + **opts)
> if not node:
> _nothingtoshelvemessaging(ui, repo, pats, opts)
> return 1
> @@ -477,8 +497,9 @@ def _docreatecmd(ui, repo, pats, opts):
> if origbranch != repo['.'].branch() and not _isbareshelve(pats, opts):
> repo.dirstate.setbranch(origbranch)
>
> - _finishshelve(ui, repo, tr)
> + _finishshelve(ui, repo, tr, activebookmark)
> finally:
> + _restoreactivebookmark(repo, activebookmark)
> lockmod.release(tr, lock)
>
> def _isbareshelve(pats, opts):
> @@ -701,6 +722,7 @@ def unshelvecontinue(ui, repo, state, op
> restorebranch(ui, repo, state.branchtorestore)
>
> state.prunenodes()
> + _restoreactivebookmark(repo, state.activebookmark)
> shelvedstate.clear(repo)
> unshelvecleanup(ui, repo, state.name, opts)
> ui.status(_("unshelve of '%s' complete\n") % state.name)
> @@ -741,7 +763,8 @@ def _unshelverestorecommit(ui, repo, bas
> return repo, shelvectx
>
> def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
> - tmpwctx, shelvectx, branchtorestore, obsshelve):
> + tmpwctx, shelvectx, branchtorestore, obsshelve,
> + activebookmark):
> """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.
> @@ -778,7 +801,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'), obsshelve)
> + branchtorestore, opts.get('keep'), obsshelve,
> + activebookmark)
>
> util.rename(repo.join('rebasestate'),
> repo.join('unshelverebasestate'))
> @@ -805,7 +829,8 @@ def _forgetunknownfiles(repo, shelvectx,
> toforget = (addedafter & shelveunknown) - addedbefore
> repo[None].forget(toforget)
>
> -def _finishunshelve(repo, oldtiprev, tr, obsshelve):
> +def _finishunshelve(repo, oldtiprev, tr, obsshelve, activebookmark):
> + _restoreactivebookmark(repo, activebookmark)
> if obsshelve:
> tr.close()
> return
> @@ -954,6 +979,7 @@ def _dounshelve(ui, repo, *shelved, **op
> # and shelvectx is the unshelved changes. Then we merge it all down
> # to the original pctx.
>
> + activebookmark = _backupactivebookmark(repo)
> tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
> tmpwctx)
>
> @@ -970,7 +996,7 @@ def _dounshelve(ui, repo, *shelved, **op
> shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
> basename, pctx, tmpwctx,
> shelvectx, branchtorestore,
> - obsshelve)
> + obsshelve, activebookmark)
> mergefiles(ui, repo, pctx, shelvectx)
> restorebranch(ui, repo, branchtorestore)
> _forgetunknownfiles(repo, shelvectx, addedbefore)
> @@ -979,7 +1005,7 @@ def _dounshelve(ui, repo, *shelved, **op
> _obsoleteredundantnodes(repo, tr, pctx, shelvectx, tmpwctx)
>
> shelvedstate.clear(repo)
> - _finishunshelve(repo, oldtiprev, tr, obsshelve)
> + _finishunshelve(repo, oldtiprev, tr, obsshelve, activebookmark)
> unshelvecleanup(ui, repo, basename, opts)
> finally:
> if tr:
More information about the Mercurial-devel
mailing list