( )⚙ 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
2 ↗(On Diff #15647)

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
2682

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
2682

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

tests/test-abort.t
2 ↗(On Diff #15647)

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
2682

_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
2 ↗(On Diff #15647)

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
2669

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.

2671

aborting graft ...

2672

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/cmdutil.py (6 lines)
Mmercurial/commands.py (6 lines)
Mmercurial/state.py (5 lines)
Mtests/test-graft.t (28 lines)

Diff 15804

mercurial/cmdutil.py

def readgraftstate(repo, graftstate): def readgraftstate(repo, graftstate):
"""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:
return graftstate.read() return graftstate.read()
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 hgabortgraft(ui, repo, **opts):
""" abort logic for aborting graft using 'hg abort'"""
with repo.wlock():
graftstate = statemod.cmdstate(repo, 'graftstate')
return abortgraft(ui, repo, graftstate)

mercurial/commands.py

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 _stopgraft(ui, repo, graftstate): def _stopgraft(ui, 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…
"""stop the interrupted graft""" """stop the interrupted graft"""
if not graftstate.exists(): if not graftstate.exists():
pulkitUnsubmitted
Done

aborting graft ...

pulkit: `aborting graft ...`
raise error.Abort(_("no interrupted graft found")) raise error.Abort(_("no interrupted graft found"))
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.
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=cmdutil.hgabortgraft,
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-graft.t

#testcases abortcommand abortflag
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [extdiff] > [extdiff]
> # for portability: > # for portability:
> pdiff = sh "$RUNTESTDIR/pdiff" > pdiff = sh "$RUNTESTDIR/pdiff"
> EOF > EOF
#if abortflag
$ cat >> $HGRCPATH <<EOF
> [alias]
> abort = graft --abort
> EOF
#endif
Create a repo with some stuff in it: Create a repo with some stuff in it:
$ hg init a $ hg init a
$ cd a $ cd a
$ echo a > a $ echo a > a
$ echo a > d $ echo a > d
$ echo a > e $ echo a > e
$ hg ci -qAm0 $ hg ci -qAm0
|/ |/
o 1:5f6d8a4bf34a added b o 1:5f6d8a4bf34a added b
| |
o 0:9092f1db7931 added a o 0:9092f1db7931 added a
$ hg up 9150fe93bec6 $ hg up 9150fe93bec6
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg graft --abort $ hg abort
abort: no interrupted graft to abort abort: no interrupted graft to abort (abortflag !)
abort: no operation in progress (abortcommand !)
[255] [255]
when stripping is required when stripping is required
$ hg graft -r 4 -r 5 $ hg graft -r 4 -r 5
grafting 4:863a25e1a9ea "added x" grafting 4:863a25e1a9ea "added x"
grafting 5:36b793615f78 "added foo to c" (tip) grafting 5:36b793615f78 "added foo to c" (tip)
merging c merging c
warning: conflicts while merging c! (edit, then use 'hg resolve --mark') warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
$ hg graft --abort --currentuser $ hg graft --abort --currentuser
abort: cannot specify any other flag with '--abort' abort: cannot specify any other flag with '--abort'
[255] [255]
$ hg graft --abort --edit $ hg graft --abort --edit
abort: cannot specify any other flag with '--abort' abort: cannot specify any other flag with '--abort'
[255] [255]
$ hg graft --abort #if abortcommand
when in dry-run mode
$ hg abort --dry-run
graft in progress, will be aborted
#endif
$ hg abort
graft aborted graft aborted
working directory is now at 9150fe93bec6 working directory is now at 9150fe93bec6
$ hg log -GT "{rev}:{node|short} {desc}" $ hg log -GT "{rev}:{node|short} {desc}"
o 5:36b793615f78 added foo to c o 5:36b793615f78 added foo to c
| |
| o 4:863a25e1a9ea added x | o 4:863a25e1a9ea added x
|/ |/
| @ 3:9150fe93bec6 added d | @ 3:9150fe93bec6 added d
| | | |
| o 2:155349b645be added c | o 2:155349b645be added c
|/ |/
o 1:5f6d8a4bf34a added b o 1:5f6d8a4bf34a added b
| |
o 0:9092f1db7931 added a o 0:9092f1db7931 added a
when stripping is not required when stripping is not required
$ hg graft -r 5 $ hg graft -r 5
grafting 5:36b793615f78 "added foo to c" (tip) grafting 5:36b793615f78 "added foo to c" (tip)
merging c merging c
warning: conflicts while merging c! (edit, then use 'hg resolve --mark') warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue') (use 'hg resolve' and 'hg graft --continue')
[255] [255]
$ hg graft --abort $ hg abort
graft aborted graft aborted
working directory is now at 9150fe93bec6 working directory is now at 9150fe93bec6
$ hg log -GT "{rev}:{node|short} {desc}" $ hg log -GT "{rev}:{node|short} {desc}"
o 5:36b793615f78 added foo to c o 5:36b793615f78 added foo to c
| |
| o 4:863a25e1a9ea added x | o 4:863a25e1a9ea added x
|/ |/
| @ 3:9150fe93bec6 added d | @ 3:9150fe93bec6 added d
o | 2:155349b645be added c o | 2:155349b645be added c
|/ |/
o 1:5f6d8a4bf34a added b o 1:5f6d8a4bf34a added b
| |
o 0:9092f1db7931 added a o 0:9092f1db7931 added a
$ hg phase -r 6 --public $ hg phase -r 6 --public
$ hg graft --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
when we created new changesets on top of existing one when we created new changesets on top of existing one
$ hg up '.^^' $ hg up '.^^'
0 files updated, 0 files merged, 2 files removed, 0 files unresolved 0 files updated, 0 files merged, 2 files removed, 0 files unresolved
searching for changes searching for changes
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
added 1 changesets with 1 changes to 1 files (+1 heads) added 1 changesets with 1 changes to 1 files (+1 heads)
new changesets 311dfc6cf3bf (1 drafts) new changesets 311dfc6cf3bf (1 drafts)
(run 'hg heads .' to see heads, 'hg merge' to merge) (run 'hg heads .' to see heads, 'hg merge' to merge)
$ hg graft --abort $ hg abort
new changesets detected on destination branch, can't strip new changesets detected on destination branch, can't strip
graft aborted graft aborted
working directory is now at 6b98ff0062dd working directory is now at 6b98ff0062dd
$ cd .. $ cd ..
============================ ============================
Testing --no-commit option:| Testing --no-commit option:|