( )⚙ D6579 abort: added support for unshelve

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
Lint Skipped
Unit
Unit Tests Skipped

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
656

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
726

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
628

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.

648

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.

656

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

658

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.

896

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
649

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 (5 lines)
Mmercurial/shelve.py (60 lines)
Mmercurial/state.py (4 lines)
Mtests/test-shelve2.t (29 lines)

Diff 15833

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,
cmdmsg=_('unshelve already in progress'), abortfunc=shelvemod.unshelveabort
)
@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

shelve". shelve".
""" """
from __future__ import absolute_import from __future__ import absolute_import
import collections import collections
import errno import errno
import itertools import itertools
import stat import stat
import sys
from .i18n import _ from .i18n import _
from . import ( from . import (
bookmarks, bookmarks,
bundle2, bundle2,
bundlerepo, bundlerepo,
changegroup, changegroup,
cmdutil, cmdutil,
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, continuef=False, abortf=False, **opts):
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`…
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 continuef:
msg = _('corrupted shelved state file')
hint = _('please run hg unshelve --abort to abort unshelve '
'operation')
raise error.Abort(msg, hint=hint)
elif abortf:
msg = _('could not read shelved state file, your working copy '
'may be in an unexpected state\nplease update to some '
'commit\n')
ui.warn(msg)
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()`.
shelvedstate.clear(repo)
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…
sys.exit()
return (state, opts)
def unshelveabort(ui, repo, state=None):
"""subcommand that abort an in-progress unshelve""" """subcommand that abort an in-progress unshelve"""
with repo.lock(): if not state:
statetuple = _loadshelvedstate(ui, repo, abortf=True)
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})`
state, opts = statetuple
with repo.wlock(), repo.lock():
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…
try: try:
checkparents(repo, state) checkparents(repo, state)
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 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: statetuple = _loadshelvedstate(ui, repo, continuef=continuef,
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) abortf=abortf, **opts)
if opts.get('keep') is None: state, opts = statetuple
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:
msg = _('could not read shelved state file, your working copy '
'may be in an unexpected state\nplease update to some '
'commit\n')
ui.warn(msg)
shelvedstate.clear(repo)
return
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
could not read shelved state file, your working copy may be in an unexpected state 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
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)
------------------------------------------------------ ------------------------------------------------------