This is an archive of the discontinued Mercurial Phabricator instance.

graft: introduce --abort flag to abort interrupted graft
ClosedPublic

Authored by pulkit on Jun 15 2018, 4:09 PM.

Details

Summary

This patch introduces a new --abort flag to hg graft command which aborts an
interrupted graft and rollbacks to the state before graft.

The behavior when some of grafted changeset get's published while interrupted
graft or we have new descendants on grafted changesets is same as that of rebase
which is warn the user, don't strip and abort the abort the graft.

Tests are added for the new flag.

.. feature::

`hg graft` now has a `--abort` flag which aborts the interrupted graft and
rollbacks to state before the graft.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Jun 15 2018, 4:09 PM
yuja added a subscriber: yuja.Jun 16 2018, 10:04 AM

+def _abortgraft(ui, repo, graftstate):
+ """abort the interrupted graft and rollbacks to the state before interrupted
+ graft"""
+ if not graftstate.exists():
+ raise error.Abort(_("no interrupted graft to abort"))
+ statedata = _readgraftstate(repo, graftstate)
+ startnode = statedata.get('startnode')
+ if not startnode:
+ # and old graft state which does not have all the data required to abort
+ # the graft
+ raise error.Abort(_("cannot abort using an old graftstate"))
+ newnodes = statedata.get('newnodes')
+ hg.updaterepo(repo, startnode, True)
+ if newnodes:

It's probably better to not updating to the startnode if we can't strip
grafted revisions.

+ newnodes = [repo[r].rev() for r in newnodes]
+
+ # checking that none of the newnodes turned public or is public
+ immutable = [c for c in newnodes if not repo[c].mutable()]
+ if immutable:
+ repo.ui.warn(_("cannot clean up public changesets %s\n")
+ % ','.join(bytes(repo[r]) for r in immutable),
+ hint=_("see 'hg help phases' for details"))
+
+ # checking that no new nodes are created on top of grafted revs
+ desc = set(repo.changelog.descendants(newnodes))
+ if desc - set(newnodes):
+ repo.ui.warn(_("new changesets detected on destination "
+ "branch, can't strip\n"))
+
+ with repo.wlock(), repo.lock():
+ # stripping the new nodes created
+ strippoints = [c.node() for c in repo.set("roots(%ld)", newnodes)]
+ repair.strip(repo.ui, repo, strippoints, False)
+
+ ui.write(_("graft aborted\nworking directory is now at %s\n")
+ % repo[startnode].hex()[:12])
+ graftstate.delete()
+ return 0

This looks quite similar to rebase.abort(). Can we factor out a common part?
Maybe it's time to add a new module for rebase/graft thingy.

pulkit updated this revision to Diff 9275.Jun 25 2018, 6:05 AM
In D3754#58886, @yuja wrote:

This looks quite similar to rebase.abort(). Can we factor out a common part?

We need to cleanup some of the rebase code first to factor out the common part. I will do that.

Maybe it's time to add a new module for rebase/graft thingy.

I agree.

yuja added a comment.Jun 25 2018, 8:14 AM
> This looks quite similar to rebase.abort(). Can we factor out a common part?
We need to cleanup some of the rebase code first to factor out the common part. I will do that.

Awesome, thanks. Should I review the current patch? or are you planning to
revise it?

In D3754#59872, @yuja wrote:
> This looks quite similar to rebase.abort(). Can we factor out a common part?
We need to cleanup some of the rebase code first to factor out the common part. I will do that.

Awesome, thanks. Should I review the current patch? or are you planning to
revise it?

I incorporated other reviews except the cleanup one. I haven't yet started the cleanup work, so if you feel that's necessary, you can leave reviewing for now.

yuja added a comment.Jun 25 2018, 11:03 AM

Fixed minor string nits, and queued, thanks.

+ if newnodes:
+ newnodes = [repo[r].rev() for r in newnodes]
+ cleanup = True
+ # checking that none of the newnodes turned public or is public
+ immutable = [c for c in newnodes if not repo[c].mutable()]
+ if immutable:
+ repo.ui.warn(_("cannot clean up public changesets %s\n")
+ % ','.join(bytes(repo[r]) for r in immutable),
+ hint=_("see 'hg help phases' for details"))
+ cleanup = False

It's better to use r/rev, c/ctx, n/node in variable names consistently. Can
you send a followup?

+ ui.write(_("graft aborted\nworking directory is now at %s\n")
+ % startctx.hex()[:12])

s/ui.write/ui.status/ as we do that for graft --stop.

This revision was automatically updated to reflect the committed changes.
yuja added a comment.Jun 25 2018, 11:07 AM

+ # changeset from which graft operation was started
+ startctx = None
+ if len(newnodes) > 0:
+ startctx = repo[newnodes[0]].p1()

This can be moved under if newnodes:

+ else:
+ startctx = repo['.']
+ # whether to strip or not
+ cleanup = False
+ if newnodes:
+ newnodes = [repo[r].rev() for r in newnodes]

...

+ if not cleanup:
+ # we don't update to the startnode if we can't strip
+ startctx = repo['.']

since startctx is re-initialized here.

+ hg.updaterepo(repo, startctx.node(), True)