( )⚙ D6655 continue: added support for graft

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6655

continue: added support for graft
ClosedPublic

Authored by taapas1128 on Jul 18 2019, 1:45 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0fff3cc9098c: continue: added support for graft
Summary

This adds support of graft to hg continue plan.

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

Results are shown as tests.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

taapas1128 created this revision.Jul 18 2019, 1:45 PM
durin42 added a subscriber: durin42.Jul 18 2019, 1:58 PM

I'm landing this, but in the future I suspect we should make sure we just have a mix of hg graft --continue and hg graft rather than run the whole test twice, as it'll severely bloat the already-slow testsuite to do that for many tests.

taapas1128 added a commit: rHG0fff3cc9098c: continue: added support for graft.Jul 18 2019, 1:59 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0fff3cc9098c: continue: added support for graft (authored by taapas1128). · Explain Why
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.Jul 18 2019, 2:20 PM

There is a lot of duplication here. Can you try to extract common parts?

taapas1128 added a comment.Jul 18 2019, 3:12 PM

@martinvonz I will send a patch doing that.

mharbison72 added a subscriber: mharbison72.Jul 19 2019, 12:19 PM
mharbison72 added inline comments.
tests/test-graft.t
1

This has the same problem D6579 had where there are 4 separate test cases, instead of 2 with sub cases.

martinvonz added a comment.Jul 19 2019, 6:03 PM
In D6655#97447, @taapas1128 wrote:

@martinvonz I will send a patch doing that.

tests/test-graft.t
1

I tried to fix that but the test fails if I do that.

martinvonz added inline comments.Jul 19 2019, 6:21 PM
mercurial/cmdutil.py
3482–3498

It looks like pretty much this entire function is copies commands.graft() and then all this code is left dead here (since cont = True above). I'd prefer to de-queue this commit and let you clean this up and then submit a new patch, but maybe other reviewers are less picky or stubborn than I am, so I'll leave it queued until I hear from someone else.

taapas1128 added inline comments.Jul 19 2019, 11:48 PM
mercurial/cmdutil.py
3482–3498

@martinvonz That code is not dead but it takes care not to merge the first commit when continuing graft. cont=True is just the first iteration of the for loop at line 3455 after that it becomes False. Although it may seem like all the code has been copied from commands.graft but only that part is copied but only that part is imported that works when --continue flag is called. Regarding cleaning I do agree spliting code from commands.graft() would be nice. So have a look at D6659.

yuja added a subscriber: yuja.Jul 20 2019, 3:36 AM

It looks like pretty much this entire function is copies commands.graft()
and then all this code is left dead here (since cont = True above).
I'd prefer to de-queue this commit and let you clean this up and then
submit a new patch

That's my feeling. Perhaps the core for loop can be extracted to a helper
function. Or alternatively, continuegraft() can be implemented on top of
commands.graft()?

taapas1128 added a comment.Jul 20 2019, 9:46 AM

@yuja have a look at D6659 and suggest changes there. Extracting the core for loop as a helper function sounds like a nice idea. I will update the patch for the same.

tests/test-graft.t
1

I will have a look.

taapas1128 added inline comments.Jul 20 2019, 10:06 AM
tests/test-graft.t
1

@mharbison72 I have generated a patch for that in D6659. I minimized it to 3 cases as @durin42 stated about bloating of the test sets.

martinvonz added a comment.Jul 21 2019, 10:10 AM

Sorry, but since both Yuya and I are uncomfortable with this patch, I'll de-queue it so it doesn't block other patches from getting queued. I think it would be good if you can fold D6659 into this one and then split it up so have only the refactoring in a first patch and the new support for continue in a second patch. Thanks.

taapas1128 added a comment.Jul 21 2019, 10:12 AM

@martinvonz No problem, I will update patch D6659.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (87 lines)
Mmercurial/commands.py (1 line)
Mtests/test-graft.t (37 lines)

Diff 15966

mercurial/cmdutil.py

nodes = repo.vfs.read('graftstate').splitlines() nodes = repo.vfs.read('graftstate').splitlines()
return {'nodes': nodes} return {'nodes': nodes}
def hgabortgraft(ui, repo): def hgabortgraft(ui, repo):
""" abort logic for aborting graft using 'hg abort'""" """ abort logic for aborting graft using 'hg abort'"""
with repo.wlock(): with repo.wlock():
graftstate = statemod.cmdstate(repo, 'graftstate') graftstate = statemod.cmdstate(repo, 'graftstate')
return abortgraft(ui, repo, graftstate) return abortgraft(ui, repo, graftstate)
def continuegraft(ui, repo):
"""logic to resume interrupted graft using 'hg continue'"""
with repo.wlock():
graftstate = statemod.cmdstate(repo, 'graftstate')
statedata = readgraftstate(repo, graftstate)
opts = {}
cont = True
if statedata.get('date'):
opts['date'] = statedata['date']
if statedata.get('user'):
opts['user'] = statedata['user']
if statedata.get('log'):
opts['log'] = True
if statedata.get('no_commit'):
opts['no_commit'] = statedata.get('no_commit')
nodes = statedata['nodes']
revs = [repo[node].rev() for node in nodes]
skipped = set()
for rev in repo.revs('%ld and merge()', revs):
ui.warn(_('skipping ungraftable merge revision %d\n') % rev)
skipped.add(rev)
revs = [r for r in revs if r not in skipped]
if not revs:
return -1
for pos, ctx in enumerate(repo.set("%ld", revs)):
desc = '%d:%s "%s"' % (ctx.rev(), ctx,
ctx.description().split('\n', 1)[0])
names = repo.nodetags(ctx.node()) + repo.nodebookmarks(ctx.node())
if names:
desc += ' (%s)' % ' '.join(names)
ui.status(_('grafting %s\n') % desc)
source = ctx.extra().get('source')
extra = {}
if source:
extra['source'] = source
extra['intermediate-source'] = ctx.hex()
else:
extra['source'] = ctx.hex()
user = ctx.user()
if opts.get('user'):
user = opts['user']
statedata['user'] = user
date = ctx.date()
if opts.get('date'):
date = opts['date']
statedata['date'] = date
message = ctx.description()
if opts.get('log'):
message += '\n(grafted from %s)' % ctx.hex()
statedata['log'] = True
if not cont:
# perform the graft merge with p1(rev) as 'ancestor'
overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
base = ctx.p1()
with ui.configoverride(overrides, 'graft'):
stats = mergemod.graft(repo, ctx, base, ['local', 'graft'])
# report any conflicts
if stats.unresolvedcount > 0:
# write out state for --continue
nodes = [repo[rev].hex() for rev in revs[pos:]]
statedata['nodes'] = nodes
stateversion = 1
graftstate.save(stateversion, statedata)
hint = _("use 'hg resolve' and 'hg graft --continue'")
raise error.Abort(
_("unresolved conflicts, can't continue"),
hint=hint)
martinvonzUnsubmitted
Not Done

It looks like pretty much this entire function is copies commands.graft() and then all this code is left dead here (since cont = True above). I'd prefer to de-queue this commit and let you clean this up and then submit a new patch, but maybe other reviewers are less picky or stubborn than I am, so I'll leave it queued until I hear from someone else.

martinvonz: It looks like pretty much this entire function is copies `commands.graft()` and then all this…
taapas1128AuthorUnsubmitted
Done

@martinvonz That code is not dead but it takes care not to merge the first commit when continuing graft. cont=True is just the first iteration of the for loop at line 3455 after that it becomes False. Although it may seem like all the code has been copied from commands.graft but only that part is copied but only that part is imported that works when --continue flag is called. Regarding cleaning I do agree spliting code from commands.graft() would be nice. So have a look at D6659.

taapas1128: @martinvonz That code is not dead but it takes care not to merge the first commit when…
else:
cont = False
editor = getcommiteditor(editform='graft',
**pycompat.strkwargs(opts))
if not opts.get('no_commit'):
node = repo.commit(text=message, user=user, date=date,
extra=extra, editor=editor)
if node is None:
ui.warn(
_('note: graft of %d:%s created no changes to commit\n') %
(ctx.rev(), ctx))
# checking that newnodes exist because old state files won't have it
if statedata.get('newnodes') is not None:
statedata['newnodes'].append(node)
graftstate.delete()
return 0

mercurial/commands.py

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( statemod.addunfinished(
'graft', fname='graftstate', clearable=True, stopflag=True, 'graft', fname='graftstate', clearable=True, stopflag=True,
continueflag=True, abortfunc=cmdutil.hgabortgraft, continueflag=True, abortfunc=cmdutil.hgabortgraft,
continuefunc=cmdutil.continuegraft,
cmdhint=_("use 'hg graft --continue' or 'hg graft --stop' to stop") 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')),

tests/test-graft.t

#testcases abortcommand abortflag #testcases abortcommand abortflag continueflag continuecommand
mharbison72Unsubmitted
Not Done

This has the same problem D6579 had where there are 4 separate test cases, instead of 2 with sub cases.

mharbison72: This has the same problem D6579 had where there are 4 separate test cases, instead of 2 with…
martinvonzUnsubmitted
Not Done

I tried to fix that but the test fails if I do that.

martinvonz: I tried to fix that but the test fails if I do that.
taapas1128AuthorUnsubmitted
Done

I will have a look.

taapas1128: I will have a look.
taapas1128AuthorUnsubmitted
Done

@mharbison72 I have generated a patch for that in D6659. I minimized it to 3 cases as @durin42 stated about bloating of the test sets.

taapas1128: @mharbison72 I have generated a patch for that in D6659. I minimized it to 3 cases as @durin42…
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [extdiff] > [extdiff]
> # for portability: > # for portability:
> pdiff = sh "$RUNTESTDIR/pdiff" > pdiff = sh "$RUNTESTDIR/pdiff"
> EOF > EOF
#if abortflag #if abortflag
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [alias] > [alias]
> abort = graft --abort > abort = graft --abort
> EOF > EOF
#endif #endif
#if continueflag
$ cat >> $HGRCPATH <<EOF
> [alias]
> continue = graft --continue
> 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
note: possible conflict - c was deleted and renamed to: note: possible conflict - c was deleted and renamed to:
a a
note: graft of 2:5c095ad7e90f created no changes to commit note: graft of 2:5c095ad7e90f created no changes to commit
Can't continue without starting: Can't continue without starting:
$ hg -q up -cr tip $ hg -q up -cr tip
$ hg rm -q e $ hg rm -q e
$ hg graft --continue $ hg continue
abort: no graft in progress abort: no graft in progress (continueflag !)
abort: no operation in progress (no-continueflag !)
[255] [255]
$ hg revert -r . -q e $ hg revert -r . -q e
Need to specify a rev: Need to specify a rev:
$ hg graft $ hg graft
abort: no revisions specified abort: no revisions specified
[255] [255]
Writing the nodes in old format to graftstate Writing the nodes in old format to graftstate
$ hg log -r 1 -r 2 -T '{node}\n' > .hg/graftstate $ hg log -r 1 -r 2 -T '{node}\n' > .hg/graftstate
$ echo foo > b $ echo foo > b
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue
#if continuecommand
$ hg continue --dry-run
graft in progress, will be resumed
#endif
$ hg continue
grafting 1:80e6d2c47cfe "added b" grafting 1:80e6d2c47cfe "added b"
grafting 2:8be98ac1a569 "added c" grafting 2:8be98ac1a569 "added c"
Testing that --user is preserved during conflicts and value is reused while Testing that --user is preserved during conflicts and value is reused while
running `hg graft --continue` running `hg graft --continue`
$ hg log -G $ hg log -G
@ changeset: 5:711e9fa999f1 @ changeset: 5:711e9fa999f1
(use 'hg resolve' and 'hg graft --continue') (use 'hg resolve' and 'hg graft --continue')
[255] [255]
$ echo wat > b $ echo wat > b
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue $ hg continue
grafting 1:80e6d2c47cfe "added b" grafting 1:80e6d2c47cfe "added b"
grafting 2:8be98ac1a569 "added c" grafting 2:8be98ac1a569 "added c"
$ hg log -Gr 3:: $ hg log -Gr 3::
@ changeset: 7:11a36ffaacf2 @ changeset: 7:11a36ffaacf2
| tag: tip | tag: tip
| user: batman | user: batman
| date: Thu Jan 01 00:00:00 1970 +0000 | date: Thu Jan 01 00:00:00 1970 +0000
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]
$ echo foobar > b $ echo foobar > b
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue $ hg continue
grafting 1:80e6d2c47cfe "added b" grafting 1:80e6d2c47cfe "added b"
grafting 2:8be98ac1a569 "added c" grafting 2:8be98ac1a569 "added c"
$ hg log -Gr '.^^::.' $ hg log -Gr '.^^::.'
@ changeset: 9:1896b76e007a @ changeset: 9:1896b76e007a
| tag: tip | tag: tip
| user: test | user: test
| date: Fri Feb 13 21:18:00 2009 -0002 | date: Fri Feb 13 21:18:00 2009 -0002
(use 'hg resolve' and 'hg graft --continue') (use 'hg resolve' and 'hg graft --continue')
[255] [255]
$ echo foobar > b $ echo foobar > b
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue $ hg continue
grafting 1:80e6d2c47cfe "added b" grafting 1:80e6d2c47cfe "added b"
grafting 2:8be98ac1a569 "added c" grafting 2:8be98ac1a569 "added c"
$ hg log -GT "{rev}:{node|short} {desc}" -r '.^^::.' $ hg log -GT "{rev}:{node|short} {desc}" -r '.^^::.'
@ 11:30c1050a58b2 added c @ 11:30c1050a58b2 added c
| (grafted from 8be98ac1a56990c2d9ca6861041b8390af7bd6f3) | (grafted from 8be98ac1a56990c2d9ca6861041b8390af7bd6f3)
o 10:ec7eda2313e2 added b o 10:ec7eda2313e2 added b
| (grafted from 80e6d2c47cfe5b3185519568327a17a061c7efb6) | (grafted from 80e6d2c47cfe5b3185519568327a17a061c7efb6)
| |
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 abort $ hg abort
abort: no interrupted graft to abort (abortflag !) abort: no interrupted graft to abort (abortflag !)
abort: no operation in progress (abortcommand !) abort: no operation in progress (no-abortflag !)
[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')
[255] [255]
Resolve conflict: Resolve conflict:
$ echo A>a $ echo A>a
$ hg resolve --mark $ hg resolve --mark
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue $ hg continue
grafting 3:09e253b87e17 "A in file a" grafting 3:09e253b87e17 "A in file a"
$ hg log -GT "{rev}:{node|short} {desc}\n" $ hg log -GT "{rev}:{node|short} {desc}\n"
@ 4:2aa9ad1006ff B in file a @ 4:2aa9ad1006ff B in file a
| |
| o 3:09e253b87e17 A in file a | o 3:09e253b87e17 A in file a
| | | |
| o 2:d36c0562f908 c | o 2:d36c0562f908 c
| | | |
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]
$ echo A>a $ echo A>a
$ hg resolve --mark $ hg resolve --mark
(no more unresolved files) (no more unresolved files)
continue: hg graft --continue continue: hg graft --continue
$ hg graft --continue $ hg continue
grafting 3:09e253b87e17 "A in file a" grafting 3:09e253b87e17 "A in file a"
$ hg diff $ hg diff
diff -r 2aa9ad1006ff a diff -r 2aa9ad1006ff a
--- a/a Thu Jan 01 00:00:00 1970 +0000 --- a/a Thu Jan 01 00:00:00 1970 +0000
+++ b/a Thu Jan 01 00:00:00 1970 +0000 +++ b/a Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +1,1 @@ @@ -1,1 +1,1 @@
-B -B
+A +A