( )⚙ D6567 abort: added support for graft

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6567

abort: added support for graft
ClosedPublic

Authored by taapas1128 on Jun 23 2019, 1:43 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG5171937ad0f9: abort: added support for graft
Summary

This adds support of graft to hg abort plan.

The patch creates a seperate function cmdutil.hgabortgraft
so that abort logic for graft can be called independently.
This logic is registered to the statedetection API as abortfunc.

Results are shown as tests.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

taapas1128 created this revision.Jun 23 2019, 1:43 PM
pulkit added a subscriber: pulkit.Jun 30 2019, 3:13 PM
pulkit added inline comments.
tests/test-abort.t
3

I think we can use cases functionality in existing tests to test hg abort.

I mean, in one case alias abort to graft --abort and in other case it will be the normal abort command. That will help us to prevent duplicating tests and making sure in future things work for both the cases.

pulkit added inline comments.Jun 30 2019, 3:15 PM
mercurial/commands.py
2766

Directly calling _abortgraft is not safe. graft --abort takes wlock first and then calls it. We skipped taking the lock in case of hg abort.

taapas1128 updated this revision to Diff 15732.Jul 1 2019, 5:50 PM
taapas1128 added inline comments.Jul 1 2019, 5:56 PM
mercurial/commands.py
2766

@pulkit the wlock is activated later in graft --abort during cleanup. That is same in the case of hg abort

tests/test-abort.t
3

I will alias this later if you want once the code for this is finalized. this is only a portion of tests taken and tests for no-backup and dry-run are added.

pulkit added inline comments.Jul 2 2019, 5:32 PM
mercurial/commands.py
2766

_dograft() is called after taking the lock. _abortgraft before taking the lock does a lot of reading which should be covered with lock. Otherwise there can be a process which changes state of things between we read the data and we operate on it.

tests/test-abort.t
3

Let's start aliasing from now and have current set of patches good to go.

taapas1128 retitled this revision from abort: added functionality for graft to abort: added support for graft.Jul 3 2019, 7:01 PM
taapas1128 edited the summary of this revision. (Show Details)
taapas1128 updated this revision to Diff 15751.
taapas1128 marked 2 inline comments as done.Jul 3 2019, 7:05 PM
taapas1128 updated this revision to Diff 15766.Jul 4 2019, 5:06 PM
pulkit added a comment.Jul 5 2019, 5:54 PM

Tests have been shown as test-abort.t

There is no such test now. :)

mercurial/commands.py
2745

let's move this function to cmdutil.py. Should move _readgraftstate, _abortgraftstate to cmdutil.py too. Movement can be done as a separate patch before this patch.

2747

aborting graft ...

2748

We can get rid of this if if we prevent calling abortfunc() in abort command.

taapas1128 marked 3 inline comments as done.Jul 6 2019, 2:57 PM
taapas1128 edited the summary of this revision. (Show Details)Jul 8 2019, 2:17 PM
taapas1128 updated this revision to Diff 15804.
pulkit accepted this revision.Jul 9 2019, 6:26 AM
This revision is now accepted and ready to land.Jul 9 2019, 6:26 AM
pulkit added a comment.Jul 9 2019, 6:29 AM

This logic is registered to the statesetection API as abortfunc.

s/statesection/statedetection

graft currently supports --dry-run flag.

This sounds ambiguous. Does this means hg graft supports --dry-run flag? IIUC, hg abort supports --dry-run flag which is mentioned in last patch and we can drop the mention here. Applies to next patches too which has similar mention in commit messages.

taapas1128 edited the summary of this revision. (Show Details)Jul 9 2019, 8:24 AM
taapas1128 updated this revision to Diff 15831.
pulkit accepted this revision.Jul 10 2019, 8:40 AM
taapas1128 added a commit: rHG5171937ad0f9: abort: added support for graft.Jul 10 2019, 9:13 AM
Closed by commit rHG5171937ad0f9: abort: added support for graft (authored by taapas1128). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (11 lines)
Mmercurial/state.py (5 lines)
AMtests/test-abort.t (139 lines)

Diff 15732

mercurial/commands.py

if opts.get('continue'): if opts.get('continue'):
raise error.Abort(_("cannot use '--continue' and " raise error.Abort(_("cannot use '--continue' and "
"'--abort' together")) "'--abort' together"))
if any((opts.get('edit'), opts.get('log'), opts.get('user'), if any((opts.get('edit'), opts.get('log'), opts.get('user'),
opts.get('date'), opts.get('currentdate'), opts.get('date'), opts.get('currentdate'),
opts.get('currentuser'), opts.get('rev'))): opts.get('currentuser'), opts.get('rev'))):
raise error.Abort(_("cannot specify any other flag with '--abort'")) raise error.Abort(_("cannot specify any other flag with '--abort'"))
return _abortgraft(ui, repo, graftstate) return _abortgraft(ui, repo)
elif opts.get('continue'): elif opts.get('continue'):
cont = True cont = True
if revs: if revs:
raise error.Abort(_("can't specify --continue and revisions")) raise error.Abort(_("can't specify --continue and revisions"))
# read in unfinished revisions # read in unfinished revisions
if graftstate.exists(): if graftstate.exists():
statedata = _readgraftstate(repo, graftstate) statedata = _readgraftstate(repo, graftstate)
if statedata.get('date'): if statedata.get('date'):
statedata['newnodes'].append(node) statedata['newnodes'].append(node)
# remove state when we complete successfully # remove state when we complete successfully
if not opts.get('dry_run'): if not opts.get('dry_run'):
graftstate.delete() graftstate.delete()
return 0 return 0
def _abortgraft(ui, repo, graftstate): def _abortgraft(ui, repo):
"""abort the interrupted graft and rollbacks to the state before interrupted """abort the interrupted graft and rollbacks to the state before interrupted
graft""" graft"""
graftstate = statemod.cmdstate(repo, 'graftstate')
if not graftstate.exists(): if not graftstate.exists():
raise error.Abort(_("no interrupted graft to abort")) raise error.Abort(_("no interrupted graft to abort"))
statedata = _readgraftstate(repo, graftstate) statedata = _readgraftstate(repo, graftstate)
newnodes = statedata.get('newnodes') newnodes = statedata.get('newnodes')
if newnodes is None: if newnodes is None:
# and old graft state which does not have all the data required to abort # and old graft state which does not have all the data required to abort
# the graft # the graft
raise error.Abort(_("cannot abort using an old graftstate")) raise error.Abort(_("cannot abort using an old graftstate"))
startctx = repo['.'] startctx = repo['.']
hg.updaterepo(repo, startctx.node(), overwrite=True) hg.updaterepo(repo, startctx.node(), overwrite=True)
ui.status(_("graft aborted\n")) ui.status(_("graft aborted\n"))
ui.status(_("working directory is now at %s\n") % startctx.hex()[:12]) ui.status(_("working directory is now at %s\n") % startctx.hex()[:12])
graftstate.delete() graftstate.delete()
return 0 return 0
def _readgraftstate(repo, graftstate): def _readgraftstate(repo, graftstate):
pulkitUnsubmitted
Done

let's move this function to cmdutil.py. Should move _readgraftstate, _abortgraftstate to cmdutil.py too. Movement can be done as a separate patch before this patch.

pulkit: let's move this function to cmdutil.py. Should move _readgraftstate, _abortgraftstate to…
"""read the graft state file and return a dict of the data stored in it""" """read the graft state file and return a dict of the data stored in it"""
try: try:
pulkitUnsubmitted
Done

aborting graft ...

pulkit: `aborting graft ...`
return graftstate.read() return graftstate.read()
pulkitUnsubmitted
Done

We can get rid of this if if we prevent calling abortfunc() in abort command.

pulkit: We can get rid of this if if we prevent calling abortfunc() in abort command.
except error.CorruptedState: except error.CorruptedState:
nodes = repo.vfs.read('graftstate').splitlines() nodes = repo.vfs.read('graftstate').splitlines()
return {'nodes': nodes} return {'nodes': nodes}
def _stopgraft(ui, repo, graftstate): def _stopgraft(ui, repo, graftstate):
"""stop the interrupted graft""" """stop the interrupted graft"""
if not graftstate.exists(): if not graftstate.exists():
raise error.Abort(_("no interrupted graft found")) raise error.Abort(_("no interrupted graft found"))
pctx = repo['.'] pctx = repo['.']
hg.updaterepo(repo, pctx.node(), overwrite=True) hg.updaterepo(repo, pctx.node(), overwrite=True)
graftstate.delete() graftstate.delete()
ui.status(_("stopped the interrupted graft\n")) ui.status(_("stopped the interrupted graft\n"))
ui.status(_("working directory is now at %s\n") % pctx.hex()[:12]) ui.status(_("working directory is now at %s\n") % pctx.hex()[:12])
return 0 return 0
statemod.addunfinished(
'graft', fname='graftstate', clearable=True, stopflag=True,
continueflag=True, abortfunc=_abortgraft,
pulkitUnsubmitted
Done

Directly calling _abortgraft is not safe. graft --abort takes wlock first and then calls it. We skipped taking the lock in case of hg abort.

pulkit: Directly calling `_abortgraft` is not safe. `graft --abort` takes wlock first and then calls it.
taapas1128AuthorUnsubmitted
Done

@pulkit the wlock is activated later in graft --abort during cleanup. That is same in the case of hg abort

taapas1128: @pulkit the wlock is activated later in `graft --abort` during cleanup. That is same in the…
pulkitUnsubmitted
Done

_dograft() is called after taking the lock. _abortgraft before taking the lock does a lot of reading which should be covered with lock. Otherwise there can be a process which changes state of things between we read the data and we operate on it.

pulkit: `_dograft()` is called after taking the lock. `_abortgraft` before taking the lock does a lot…
cmdhint=_("use 'hg graft --continue' or 'hg graft --stop' to stop")
)
@command('grep', @command('grep',
[('0', 'print0', None, _('end fields with NUL')), [('0', 'print0', None, _('end fields with NUL')),
('', 'all', None, _('print all revisions that match (DEPRECATED) ')), ('', 'all', None, _('print all revisions that match (DEPRECATED) ')),
('', 'diff', None, _('print all revisions when the term was introduced ' ('', 'diff', None, _('print all revisions when the term was introduced '
'or removed')), 'or removed')),
('a', 'text', None, _('treat all files as text')), ('a', 'text', None, _('treat all files as text')),
('f', 'follow', None, ('f', 'follow', None,
_('follow changeset history,' _('follow changeset history,'

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(
'graft', fname='graftstate', clearable=True, stopflag=True,
continueflag=True,
cmdhint=_("use 'hg graft --continue' or 'hg graft --stop' to stop")
)
addunfinished(
'unshelve', fname='shelvedstate', continueflag=True, 'unshelve', fname='shelvedstate', continueflag=True,
cmdmsg=_('unshelve already in progress') cmdmsg=_('unshelve already in progress')
) )
addunfinished( 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")

tests/test-abort.t

  • This file was added.
####TEST `hg abort` operation graft
pulkitUnsubmitted
Not Done

I think we can use cases functionality in existing tests to test hg abort.

I mean, in one case alias abort to graft --abort and in other case it will be the normal abort command. That will help us to prevent duplicating tests and making sure in future things work for both the cases.

pulkit: I think we can use cases functionality in existing tests to test `hg abort`. I mean, in one…
taapas1128AuthorUnsubmitted
Done

I will alias this later if you want once the code for this is finalized. this is only a portion of tests taken and tests for no-backup and dry-run are added.

taapas1128: I will alias this later if you want once the code for this is finalized. this is only a portion…
pulkitUnsubmitted
Not Done

Let's start aliasing from now and have current set of patches good to go.

pulkit: Let's start aliasing from now and have current set of patches good to go.
Testing the hg abort for `hg graft` which aborts and rollback to state
before the graft
$ hg init abortgraft
$ cd abortgraft
$ for ch in a b c d; do echo $ch > $ch; hg add $ch; hg ci -Aqm "added "$ch; done;
$ hg up '.^^'
0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ echo x > x
$ hg ci -Aqm "added x"
$ hg up '.^'
0 files updated, 0 files merged, 1 files removed, 0 files unresolved
$ echo foo > c
$ hg ci -Aqm "added foo to c"
$ hg log -GT "{rev}:{node|short} {desc}"
@ 5:36b793615f78 added foo to c
|
| o 4:863a25e1a9ea added x
|/
| o 3:9150fe93bec6 added d
| |
| o 2:155349b645be added c
|/
o 1:5f6d8a4bf34a added b
|
o 0:9092f1db7931 added a
$ hg up 9150fe93bec6
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg abort
abort: no operation in progress
[255]
when stripping is required
$ hg graft -r 4 -r 5
grafting 4:863a25e1a9ea "added x"
grafting 5:36b793615f78 "added foo to c" (tip)
merging c
warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
[255]
when in dry-run mode
$ hg abort --dry-run
abort: aborting graft
[255]
when in no-backup mode
$ hg abort --no-backup
abort: graft does not support no-backup
[255]
when dry-run mode is used with no backup
$ hg abort --dry-run --no-backup
abort: graft does not support no-backup
[255]
normal abort
$ hg abort
graft aborted
working directory is now at 9150fe93bec6
$ hg log -GT "{rev}:{node|short} {desc}"
o 5:36b793615f78 added foo to c
|
| o 4:863a25e1a9ea added x
|/
| @ 3:9150fe93bec6 added d
| |
| o 2:155349b645be added c
|/
o 1:5f6d8a4bf34a added b
|
o 0:9092f1db7931 added a
when stripping is not required
$ hg graft -r 5
grafting 5:36b793615f78 "added foo to c" (tip)
merging c
warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
[255]
$ hg abort
graft aborted
working directory is now at 9150fe93bec6
$ hg log -GT "{rev}:{node|short} {desc}"
o 5:36b793615f78 added foo to c
|
| o 4:863a25e1a9ea added x
|/
| @ 3:9150fe93bec6 added d
| |
| o 2:155349b645be added c
|/
o 1:5f6d8a4bf34a added b
|
o 0:9092f1db7931 added a
when some of the changesets became public
$ hg graft -r 4 -r 5
grafting 4:863a25e1a9ea "added x"
grafting 5:36b793615f78 "added foo to c" (tip)
merging c
warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
[255]
$ hg log -GT "{rev}:{node|short} {desc}"
@ 6:6ec71c037d94 added x
|
| o 5:36b793615f78 added foo to c
| |
| | o 4:863a25e1a9ea added x
| |/
o | 3:9150fe93bec6 added d
| |
o | 2:155349b645be added c
|/
o 1:5f6d8a4bf34a added b
|
o 0:9092f1db7931 added a
$ hg phase -r 6 --public
$ hg abort
cannot clean up public changesets 6ec71c037d94
graft aborted
working directory is now at 6ec71c037d94