( )⚙ 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
630

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
734 ↗(On Diff #15781)

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 ↗(On Diff #15781)

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

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.

630

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

632

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.

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.

904

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 (5 lines)
Mmercurial/shelve.py (22 lines)
Mmercurial/state.py (4 lines)
Mtests/test-abort.t (99 lines)

Diff 15734

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

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 unshelveabort(ui, repo, state=None):
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(): if not state:
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})`
state = shelvedstate.load(repo)
except IOError as err:
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…
if err.errno != errno.ENOENT:
raise
cmdutil.wrongtooltocontinue(repo, _('unshelve'))
except error.CorruptedState as err:
ui.debug(pycompat.bytestr(err) + '\n')
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
with repo.wlock(), repo.lock():
try: try:
checkparents(repo, state) checkparents(repo, state)
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()`.
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…
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')
'operation') 'operation')
raise error.Abort(msg, hint=hint) raise error.Abort(msg, hint=hint)
elif abortf: elif abortf:
msg = _('could not read shelved state file, your working copy ' msg = _('could not read shelved state file, your working copy '
'may be in an unexpected state\nplease update to some ' 'may be in an unexpected state\nplease update to some '
'commit\n') 'commit\n')
ui.warn(msg) ui.warn(msg)
shelvedstate.clear(repo) shelvedstate.clear(repo)
return return
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`
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-abort.t

####TEST `hg abort` operation rebase
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [extensions] > [extensions]
> rebase= > rebase=
> > shelve=
> mq =
> [phases] > [phases]
> publish=False > publish=False
> >
> [alias] > [alias]
> tglog = log -G --template "{rev}:{phase} '{desc}' {branches}\n" > tglog = log -G --template "{rev}:{phase} '{desc}' {branches}\n"
> EOF > EOF
####TEST `hg abort` operation rebase
$ hg init a $ hg init a
$ cd a $ cd a
$ touch .hg/rebasestate $ touch .hg/rebasestate
$ hg sum $ hg sum
parent: -1:000000000000 tip (empty repository) parent: -1:000000000000 tip (empty repository)
branch: default branch: default
commit: (clean) commit: (clean)
o 0:9092f1db7931 added a o 0:9092f1db7931 added a
$ hg phase -r 6 --public $ hg phase -r 6 --public
$ hg abort $ hg abort
cannot clean up public changesets 6ec71c037d94 cannot clean up public changesets 6ec71c037d94
graft aborted graft aborted
working directory is now at 6ec71c037d94 working directory is now at 6ec71c037d94
$ cd ..
###TEST `hg abort` operation unshelve
Prepare unshelve with a corrupted shelvedstate
$ hg init r1 && cd r1
$ echo text1 > file && hg add file
$ hg shelve
shelved as default
0 files updated, 0 files merged, 1 files removed, 0 files unresolved
$ echo text2 > file && hg ci -Am text1
adding file
$ hg unshelve
unshelving change 'default'
rebasing shelved changes
merging file
warning: conflicts while merging file! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
[1]
$ echo somethingsomething > .hg/shelvedstate
Unshelve abort works with a corrupted shelvedstate
$ hg abort
could not read shelved state file, your working copy may be in an unexpected state
please update to some commit
Unshelve abort fails with appropriate message if there's no unshelve in
progress
$ hg abort
abort: merge does not support 'hg abort'
(use 'hg commit' or 'hg merge --abort')
[255]
$ cd ..
Abort due to pending changes
$ hg init onlypendingchanges
$ cd onlypendingchanges
$ touch a
$ hg ci -Aqm a
$ echo f > f
$ hg add f
$ hg shelve
shelved as default
0 files updated, 0 files merged, 1 files removed, 0 files unresolved
$ echo g > f
$ echo 1 > a
$ hg unshelve
unshelving change 'default'
temporarily committing pending changes (restore with 'hg unshelve --abort')
rebasing shelved changes
merging f
warning: conflicts while merging f! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg unshelve --continue')
[1]
$ hg st
M f
? f.orig
$ cat f
<<<<<<< shelve: f0be9a229575 - shelve: pending changes temporary commit
g
=======
f
>>>>>>> working-copy: 7d4133053e12 - shelve: changes to: a
$ cat f.orig
g
$ hg ci a -m 'intermediate other change'
abort: unshelve already in progress
(use 'hg unshelve --continue' or 'hg unshelve --abort')
[255]
when in dry-run mode
$ hg abort --dry-run
abort: aborting unshelve
[255]
when in no-backup mode
$ hg abort --no-backup
abort: unshelve does not support no-backup
[255]
when dry-run mode is used with no backup
$ hg abort --dry-run --no-backup
abort: unshelve does not support no-backup
[255]
normal abort
$ hg abort
unshelve of 'default' aborted
$ cd ..