This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6579

abort: added support for unshelve
ClosedPublic

Authored by taapas1128 on Jun 26 2019, 12:45 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG3fb0493812c0: abort: added support for unshelve
Summary

This patch adds the support for shelve in hg abort plan.

For this the logic to load a shelvedstate and the error
handling for it had been shifted to a seperate function
_loadunshelvedstate(). This returns a tuple with state file
and opts.

hgabortunshelve() has been created for independent calls.
In case abortion of unshelve is called via hg abort the
shelvedstate needs to be loaded seperately. This has been
ensured by _loadunshelvedstate()

hgabortunshelve() is then registered as abortfunc for state
detection API.

Results are shown as tests.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

taapas1128 created this revision.Jun 26 2019, 12:45 PM
pulkit added a subscriber: pulkit.Jun 30 2019, 3:17 PM

This patch needs to be rebased on tip of hg-committed as shelve is in core now.

taapas1128 updated this revision to Diff 15734.Jul 1 2019, 5:50 PM
taapas1128 added a comment.Jul 1 2019, 5:57 PM

I have rebased it.

pulkit added inline comments.Jul 2 2019, 5:45 PM
mercurial/shelve.py
653

Let's take this code out into a new function and reuse that function at both the places instead of duplicating code.

taapas1128 edited the summary of this revision. (Show Details)Jul 3 2019, 7:01 PM
taapas1128 updated this revision to Diff 15753.
taapas1128 marked an inline comment as done.Jul 3 2019, 7:07 PM
taapas1128 updated this revision to Diff 15756.Jul 4 2019, 2:53 AM
taapas1128 updated this revision to Diff 15768.Jul 4 2019, 5:07 PM
taapas1128 updated this revision to Diff 15781.Jul 6 2019, 2:54 PM
mharbison72 added a subscriber: mharbison72.Jul 8 2019, 1:09 PM
mharbison72 added inline comments.
tests/test-shelve2.t
728

I think that you can eliminate the duplication with this:

abort: no unshelve in progress (abortflag !)
abort: merge does not support 'hg abort' (no-abortflag !)
(use 'hg commit' or 'hg merge --abort') (no-abortflag !)
mharbison72 added inline comments.Jul 8 2019, 1:27 PM
tests/test-shelve2.t
1–2

Shouldn't these be 2 lines, to maximize the test coverage? Presumably --abort and hg abort can work with either phase or strip based shelving. See test-narrow.t, or some of the lfs tests.

#testcases stripbased phasebased
#testcases abortflag abortcommand
taapas1128 edited the summary of this revision. (Show Details)Jul 8 2019, 2:18 PM
taapas1128 updated this revision to Diff 15806.
taapas1128 added a comment.Jul 8 2019, 2:22 PM

@mharbison72 Thanks for the suggestions. I have amended the patch accordingly.

taapas1128 edited the summary of this revision. (Show Details)Jul 9 2019, 8:25 AM
taapas1128 updated this revision to Diff 15833.
pulkit added inline comments.Jul 10 2019, 1:10 PM
mercurial/shelve.py
627–650

continuef and abortf can be read from opts, no need to pass them separately.

Also, opts should be pass as a dictionary here, no need to pass that as kwargs.

647

I think we can convert this ui.warn() to error.Abort() and hence get rid of sys.exit(). Send a standalone patch before this one which make this an error instead of warn.

653

nit: state, opts = _loadshelvedstate(ui, repo, {'abort'=True})

655

let's take wlock() only again when if we are processing hg abort. We can define this function as unshelveabort(ui, repo, state, abortcommand=False) and pass state as None, and abortcommand as True. When abortcommand is True, reassign state to new one and take wlock. Also make sure to release the wlock later in case of abortcommand.

894–895

same as a comment above related to directly assigning it to state, opts

taapas1128 updated this revision to Diff 15859.Jul 10 2019, 2:26 PM
pulkit added a comment.Jul 10 2019, 2:54 PM

The commit message is outdated.

taapas1128 edited the summary of this revision. (Show Details)Jul 10 2019, 3:00 PM
taapas1128 updated this revision to Diff 15860.
taapas1128 added a comment.Jul 10 2019, 3:02 PM

updated that.

pulkit added inline comments.Jul 10 2019, 3:06 PM
mercurial/shelve.py
648

we can prevent returning opts here. One user does not need it and other one is passing by reference.

taapas1128 updated this revision to Diff 15861.Jul 10 2019, 3:18 PM
pulkit accepted this revision.Jul 10 2019, 3:18 PM
This revision is now accepted and ready to land.Jul 10 2019, 3:18 PM
taapas1128 added a commit: rHG3fb0493812c0: abort: added support for unshelve.Jul 10 2019, 5:10 PM
Closed by commit rHG3fb0493812c0: abort: added support for unshelve (authored by taapas1128). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (6 lines)
Mmercurial/shelve.py (56 lines)
Mmercurial/state.py (4 lines)
Mtests/test-shelve2.t (29 lines)

Diff 15882

mercurial/commands.py

Timestamp in seconds is used to decide order of backups. More Timestamp in seconds is used to decide order of backups. More
than ``maxbackups`` backups are kept, if same timestamp than ``maxbackups`` backups are kept, if same timestamp
prevents from deciding exact order of them, for safety. prevents from deciding exact order of them, for safety.
""" """
with repo.wlock(): with repo.wlock():
return shelvemod.dounshelve(ui, repo, *shelved, **opts) return shelvemod.dounshelve(ui, repo, *shelved, **opts)
statemod.addunfinished(
'unshelve', fname='shelvedstate', continueflag=True,
abortfunc=shelvemod.hgabortunshelve,
cmdmsg=_('unshelve already in progress'),
)
@command('update|up|checkout|co', @command('update|up|checkout|co',
[('C', 'clean', None, _('discard uncommitted changes (no backup)')), [('C', 'clean', None, _('discard uncommitted changes (no backup)')),
('c', 'check', None, _('require clean working directory')), ('c', 'check', None, _('require clean working directory')),
('m', 'merge', None, _('merge uncommitted changes')), ('m', 'merge', None, _('merge uncommitted changes')),
('d', 'date', '', _('tipmost revision matching date'), _('DATE')), ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
('r', 'rev', '', _('revision'), _('REV')) ('r', 'rev', '', _('revision'), _('REV'))
] + mergetoolopts, ] + mergetoolopts,
_('[-C|-c|-m] [-d DATE] [[-r] REV]'), _('[-C|-c|-m] [-d DATE] [[-r] REV]'),

mercurial/shelve.py

listcmd(ui, repo, pats, opts) listcmd(ui, repo, pats, opts)
def checkparents(repo, state): def checkparents(repo, state):
"""check parent while resuming an unshelve""" """check parent while resuming an unshelve"""
if state.parents != repo.dirstate.parents(): if state.parents != repo.dirstate.parents():
raise error.Abort(_('working directory parents do not match unshelve ' raise error.Abort(_('working directory parents do not match unshelve '
'state')) 'state'))
def unshelveabort(ui, repo, state, opts): def _loadshelvedstate(ui, repo, opts):
try:
state = shelvedstate.load(repo)
if opts.get('keep') is None:
opts['keep'] = state.keep
except IOError as err:
if err.errno != errno.ENOENT:
raise
cmdutil.wrongtooltocontinue(repo, _('unshelve'))
except error.CorruptedState as err:
ui.debug(pycompat.bytestr(err) + '\n')
if opts.get('continue'):
msg = _('corrupted shelved state file')
hint = _('please run hg unshelve --abort to abort unshelve '
'operation')
raise error.Abort(msg, hint=hint)
elif opts.get('abort'):
shelvedstate.clear(repo)
raise error.Abort(_('could not read shelved state file, your '
'working copy may be in an unexpected state\n'
'please update to some commit\n'))
pulkitUnsubmitted
Done

I think we can convert this ui.warn() to error.Abort() and hence get rid of sys.exit(). Send a standalone patch before this one which make this an error instead of warn.

pulkit: I think we can convert this `ui.warn()` to `error.Abort()` and hence get rid of `sys.exit()`.
return state
pulkitUnsubmitted
Not Done

we can prevent returning opts here. One user does not need it and other one is passing by reference.

pulkit: we can prevent returning opts here. One user does not need it and other one is passing by…
def unshelveabort(ui, repo, state):
pulkitUnsubmitted
Done

continuef and abortf can be read from opts, no need to pass them separately.

Also, opts should be pass as a dictionary here, no need to pass that as kwargs.

pulkit: continuef and abortf can be read from opts, no need to pass them separately. Also, `opts`…
"""subcommand that abort an in-progress unshelve""" """subcommand that abort an in-progress unshelve"""
with repo.lock(): with repo.lock():
try: try:
pulkitUnsubmitted
Done

Let's take this code out into a new function and reuse that function at both the places instead of duplicating code.

pulkit: Let's take this code out into a new function and reuse that function at both the places instead…
pulkitUnsubmitted
Done

nit: state, opts = _loadshelvedstate(ui, repo, {'abort'=True})

pulkit: nit: `state, opts = _loadshelvedstate(ui, repo, {'abort'=True})`
checkparents(repo, state) checkparents(repo, state)
pulkitUnsubmitted
Done

let's take wlock() only again when if we are processing hg abort. We can define this function as unshelveabort(ui, repo, state, abortcommand=False) and pass state as None, and abortcommand as True. When abortcommand is True, reassign state to new one and take wlock. Also make sure to release the wlock later in case of abortcommand.

pulkit: let's take `wlock()` only again when if we are processing `hg abort`. We can define this…
merge.update(repo, state.pendingctx, branchmerge=False, force=True) merge.update(repo, state.pendingctx, branchmerge=False, force=True)
if (state.activebookmark if (state.activebookmark
and state.activebookmark in repo._bookmarks): and state.activebookmark in repo._bookmarks):
bookmarks.activate(repo, state.activebookmark) bookmarks.activate(repo, state.activebookmark)
mergefiles(ui, repo, state.wctx, state.pendingctx) mergefiles(ui, repo, state.wctx, state.pendingctx)
if not phases.supportinternal(repo): if not phases.supportinternal(repo):
repair.strip(ui, repo, state.nodestoremove, backup=False, repair.strip(ui, repo, state.nodestoremove, backup=False,
topic='shelve') topic='shelve')
finally: finally:
shelvedstate.clear(repo) shelvedstate.clear(repo)
ui.warn(_("unshelve of '%s' aborted\n") % state.name) ui.warn(_("unshelve of '%s' aborted\n") % state.name)
def hgabortunshelve(ui, repo):
"""logic to abort unshelve using 'hg abort"""
with repo.wlock():
state = _loadshelvedstate(ui, repo, {'abort' : True})
return unshelveabort(ui, repo, state)
def mergefiles(ui, repo, wctx, shelvectx): def mergefiles(ui, repo, wctx, shelvectx):
"""updates to wctx and merges the changes from shelvectx into the """updates to wctx and merges the changes from shelvectx into the
dirstate.""" dirstate."""
with ui.configoverride({('ui', 'quiet'): True}): with ui.configoverride({('ui', 'quiet'): True}):
hg.update(repo, wctx.node()) hg.update(repo, wctx.node())
ui.pushbuffer(True) ui.pushbuffer(True)
cmdutil.revert(ui, repo, shelvectx, repo.dirstate.parents()) cmdutil.revert(ui, repo, shelvectx, repo.dirstate.parents())
ui.popbuffer() ui.popbuffer()
def restorebranch(ui, repo, branchtorestore): def restorebranch(ui, repo, branchtorestore):
if branchtorestore and branchtorestore != repo.dirstate.branch(): if branchtorestore and branchtorestore != repo.dirstate.branch():
repo.dirstate.setbranch(branchtorestore) repo.dirstate.setbranch(branchtorestore)
ui.status(_('marked working directory as branch %s\n') ui.status(_('marked working directory as branch %s\n')
% branchtorestore) % branchtorestore)
def unshelvecleanup(ui, repo, name, opts): def unshelvecleanup(ui, repo, name, opts):
"""remove related files after an unshelve""" """remove related files after an unshelve"""
if not opts.get('keep'): if not opts.get('keep'):
for filetype in shelvefileextensions: for filetype in shelvefileextensions:
shfile = shelvedfile(repo, name, filetype) shfile = shelvedfile(repo, name, filetype)
if shfile.exists(): if shfile.exists():
shfile.movetobackup() shfile.movetobackup()
cleanupoldbackups(repo) cleanupoldbackups(repo)
def unshelvecontinue(ui, repo, state, opts): def unshelvecontinue(ui, repo, state, opts):
"""subcommand to continue an in-progress unshelve""" """subcommand to continue an in-progress unshelve"""
# We're finishing off a merge. First parent is our original # We're finishing off a merge. First parent is our original
# parent, second is the temporary "fake" commit we're unshelving. # parent, second is the temporary "fake" commit we're unshelving.
with repo.lock(): with repo.lock():
checkparents(repo, state) checkparents(repo, state)
ms = merge.mergestate.read(repo) ms = merge.mergestate.read(repo)
if list(ms.unresolved()): if list(ms.unresolved()):
if abortf or continuef: if abortf or continuef:
if abortf and continuef: if abortf and continuef:
raise error.Abort(_('cannot use both abort and continue')) raise error.Abort(_('cannot use both abort and continue'))
if shelved: if shelved:
raise error.Abort(_('cannot combine abort/continue with ' raise error.Abort(_('cannot combine abort/continue with '
'naming a shelved change')) 'naming a shelved change'))
if abortf and opts.get('tool', False): if abortf and opts.get('tool', False):
ui.warn(_('tool option will be ignored\n')) ui.warn(_('tool option will be ignored\n'))
try: state = _loadshelvedstate(ui, repo, opts)
pulkitUnsubmitted
Done

same as a comment above related to directly assigning it to state, opts

pulkit: same as a comment above related to directly assigning it to `state, opts`
state = shelvedstate.load(repo)
if opts.get('keep') is None:
opts['keep'] = state.keep
except IOError as err:
if err.errno != errno.ENOENT:
raise
cmdutil.wrongtooltocontinue(repo, _('unshelve'))
except error.CorruptedState as err:
ui.debug(pycompat.bytestr(err) + '\n')
if continuef:
msg = _('corrupted shelved state file')
hint = _('please run hg unshelve --abort to abort unshelve '
'operation')
raise error.Abort(msg, hint=hint)
elif abortf:
shelvedstate.clear(repo)
raise error.Abort(_('could not read shelved state file, your '
'working copy may be in an unexpected state\n'
'please update to some commit\n'))
if abortf: if abortf:
return unshelveabort(ui, repo, state, opts) return unshelveabort(ui, repo, state)
elif continuef: elif continuef:
return unshelvecontinue(ui, repo, state, opts) return unshelvecontinue(ui, repo, state, opts)
elif len(shelved) > 1: elif len(shelved) > 1:
raise error.Abort(_('can only unshelve one change at a time')) raise error.Abort(_('can only unshelve one change at a time'))
elif not shelved: elif not shelved:
shelved = listshelves(repo) shelved = listshelves(repo)
if not shelved: if not shelved:
raise error.Abort(_('no shelved changes to apply!')) raise error.Abort(_('no shelved changes to apply!'))

mercurial/state.py

reportonly, continueflag, stopflag, cmdmsg, reportonly, continueflag, stopflag, cmdmsg,
cmdhint, statushint, abortfunc) cmdhint, statushint, abortfunc)
if opname == 'merge': if opname == 'merge':
_unfinishedstates.append(statecheckobj) _unfinishedstates.append(statecheckobj)
else: else:
_unfinishedstates.insert(0, statecheckobj) _unfinishedstates.insert(0, statecheckobj)
addunfinished( addunfinished(
'unshelve', fname='shelvedstate', continueflag=True,
cmdmsg=_('unshelve already in progress')
)
addunfinished(
'update', fname='updatestate', clearable=True, 'update', fname='updatestate', clearable=True,
cmdmsg=_('last update was interrupted'), cmdmsg=_('last update was interrupted'),
cmdhint=_("use 'hg update' to get a consistent checkout"), cmdhint=_("use 'hg update' to get a consistent checkout"),
statushint=_("To continue: hg update") statushint=_("To continue: hg update")
) )
addunfinished( addunfinished(
'bisect', fname='bisect.state', allowcommit=True, reportonly=True, 'bisect', fname='bisect.state', allowcommit=True, reportonly=True,
statushint=_('To mark the changeset good: hg bisect --good\n' statushint=_('To mark the changeset good: hg bisect --good\n'

tests/test-shelve2.t

#testcases stripbased phasebased #testcases stripbased phasebased
#testcases abortflag abortcommand
mharbison72Unsubmitted
Not Done

Shouldn't these be 2 lines, to maximize the test coverage? Presumably --abort and hg abort can work with either phase or strip based shelving. See test-narrow.t, or some of the lfs tests.

#testcases stripbased phasebased
#testcases abortflag abortcommand
mharbison72: Shouldn't these be 2 lines, to maximize the test coverage? Presumably `--abort` and `hg abort`…
$ cat <<EOF >> $HGRCPATH $ cat <<EOF >> $HGRCPATH
> [extensions] > [extensions]
> mq = > mq =
> [defaults] > [defaults]
> diff = --nodates --git > diff = --nodates --git
> qnew = --date '0 0' > qnew = --date '0 0'
> [shelve] > [shelve]
> maxbackups = 2 > maxbackups = 2
> EOF > EOF
#if phasebased #if phasebased
$ cat <<EOF >> $HGRCPATH $ cat <<EOF >> $HGRCPATH
> [format] > [format]
> internal-phase = yes > internal-phase = yes
> EOF > EOF
#endif #endif
#if abortflag
$ cat >> $HGRCPATH <<EOF
> [alias]
> abort = unshelve --abort
> EOF
#endif
shelve should leave dirstate clean (issue4055) shelve should leave dirstate clean (issue4055)
$ hg init shelverebase $ hg init shelverebase
$ cd shelverebase $ cd shelverebase
$ printf 'x\ny\n' > x $ printf 'x\ny\n' > x
$ echo z > z $ echo z > z
$ hg commit -Aqm xy $ hg commit -Aqm xy
$ echo z >> x $ echo z >> x
$ cat f $ cat f
<<<<<<< shelve: 6b563750f973 - test: intermediate other change <<<<<<< shelve: 6b563750f973 - test: intermediate other change
g g
======= =======
f f
>>>>>>> working-copy: aef214a5229c - shelve: changes to: commit stuff >>>>>>> working-copy: aef214a5229c - shelve: changes to: commit stuff
$ cat f.orig $ cat f.orig
g g
$ hg unshelve --abort
#if abortcommand
when in dry-run mode
$ hg abort --dry-run
unshelve in progress, will be aborted
#endif
$ hg abort
unshelve of 'default' aborted unshelve of 'default' aborted
$ hg st $ hg st
? f.orig ? f.orig
$ cat f.orig $ cat f.orig
g g
$ hg shelve --delete default $ hg shelve --delete default
$ cd .. $ cd ..
Unshelve --continue fails with appropriate message if shelvedstate is corrupted Unshelve --continue fails with appropriate message if shelvedstate is corrupted
$ hg unshelve --continue $ hg unshelve --continue
abort: corrupted shelved state file abort: corrupted shelved state file
(please run hg unshelve --abort to abort unshelve operation) (please run hg unshelve --abort to abort unshelve operation)
[255] [255]
Unshelve --abort works with a corrupted shelvedstate Unshelve --abort works with a corrupted shelvedstate
$ hg unshelve --abort $ hg abort
abort: could not read shelved state file, your working copy may be in an unexpected state abort: could not read shelved state file, your working copy may be in an unexpected state
please update to some commit please update to some commit
[255] [255]
Unshelve --abort fails with appropriate message if there's no unshelve in Unshelve --abort fails with appropriate message if there's no unshelve in
progress progress
$ hg unshelve --abort $ hg abort
abort: no unshelve in progress abort: no unshelve in progress (abortflag !)
abort: merge in progress but does not support 'hg abort' (no-abortflag !)
(use 'hg commit' or 'hg merge --abort') (no-abortflag !)
[255] [255]
$ cd .. $ cd ..
Unshelve respects --keep even if user intervention is needed Unshelve respects --keep even if user intervention is needed
mharbison72Unsubmitted
Not Done

I think that you can eliminate the duplication with this:

abort: no unshelve in progress (abortflag !)
abort: merge does not support 'hg abort' (no-abortflag !)
(use 'hg commit' or 'hg merge --abort') (no-abortflag !)
mharbison72: I think that you can eliminate the duplication with this: abort: no unshelve in progress…
$ hg init unshelvekeep && cd unshelvekeep $ hg init unshelvekeep && cd unshelvekeep
$ echo 1 > file && hg ci -Am 1 $ echo 1 > file && hg ci -Am 1
adding file adding file
$ echo 2 >> file $ echo 2 >> file
$ hg shelve $ hg shelve
shelved as default shelved as default
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ echo 3 >> file && hg ci -Am 13 $ echo 3 >> file && hg ci -Am 13
$ hg unshelve $ hg unshelve
unshelving change 'default' unshelving change 'default'
temporarily committing pending changes (restore with 'hg unshelve --abort') temporarily committing pending changes (restore with 'hg unshelve --abort')
rebasing shelved changes rebasing shelved changes
merging a merging a
warning: conflicts while merging a! (edit, then use 'hg resolve --mark') warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue') unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
[1] [1]
$ hg unshelve --abort $ hg abort
unshelve of 'default' aborted unshelve of 'default' aborted
Unshelve without .shelve metadata (can happen when upgrading a repository with old shelve) Unshelve without .shelve metadata (can happen when upgrading a repository with old shelve)
$ cat .hg/shelved/default.shelve $ cat .hg/shelved/default.shelve
node=82e0cb9893247d12667017593ce1e5655860f1ac node=82e0cb9893247d12667017593ce1e5655860f1ac
$ hg strip --hidden --rev 82e0cb989324 --no-backup $ hg strip --hidden --rev 82e0cb989324 --no-backup
$ rm .hg/shelved/default.shelve $ rm .hg/shelved/default.shelve
$ hg unshelve $ hg unshelve
unshelving change 'default' unshelving change 'default'
temporarily committing pending changes (restore with 'hg unshelve --abort') temporarily committing pending changes (restore with 'hg unshelve --abort')
rebasing shelved changes rebasing shelved changes
merging a merging a
warning: conflicts while merging a! (edit, then use 'hg resolve --mark') warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue') unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
[1] [1]
$ cat .hg/shelved/default.shelve $ cat .hg/shelved/default.shelve
node=82e0cb9893247d12667017593ce1e5655860f1ac node=82e0cb9893247d12667017593ce1e5655860f1ac
$ hg unshelve --abort $ hg abort
unshelve of 'default' aborted unshelve of 'default' aborted
#endif #endif
$ cd .. $ cd ..
Block merge abort when unshelve in progress(issue6160) Block merge abort when unshelve in progress(issue6160)
------------------------------------------------------ ------------------------------------------------------