[PATCH RFC] commit: add --amend option to amend the parent changeset
Idan Kamara
idankk86 at gmail.com
Thu Feb 2 13:04:50 CST 2012
On Thu, Feb 2, 2012 at 8:35 PM, Matt Mackall <mpm at selenic.com> wrote:
> On Thu, 2012-02-02 at 14:35 +0200, Idan Kamara wrote:
> > # HG changeset patch
> > # User Idan Kamara <idankk86 at gmail.com>
> > # Date 1328185747 -7200
> > # Branch stable
> > # Node ID cc7dec00d5016f03acf789c35ce6ef50b204f0cb
> > # Parent 0620421044a2bcaafd054a6ee454614888699de8
> > commit: add --amend option to amend the parent changeset
> >
> > Commits a new changeset incorporating both the changes to the given files
> > and all the changes from the current parent changeset into the
> repository.
>
> Not sure what you mean by "incorporating" here. Is the result not a
> snapshot of the working directory?
>
It is. But I think the emphasis should be that the new commit contains
all the changes from the parent commit and the working directory in
a single amending commit.
>
> Does this do the right thing when amending merges? In particular,
> preserving the very tricky to reproduce file-level history for merged
> files?
>
No. Currently aborts on merge commits.
>
> > You cannot amend public changesets. Amending a changeset with children
> > results in a warning (we might want to forbid this).
>
> I assume this just creates a new head rather than implicitly rebasing
> all the children. Seems like the former is a surprise and the latter is
> overly-complex. So I think we should just forbid it.
>
I agree that rebasing automatically is too much magic happening.
But limiting this to commits without children makes this
less useful for advanced users (who can manually rebase).
Kind of like how rollback is only applicable to the last transaction.
I think a warning with a "see hg help rebase" suggestion
might be better.
>
> > Behind the scenes, first commit the update as a regular child of the
> current
> > parent. Then create a new commit on the parent's parents with the updated
> > contents. Then change the working copy parent to this new combined
> changeset.
> > Finally, strip the intermediate commit created in the beginning (might
> > want to also strip the amended commit if it has no children).
> >
> > This is an RFC, which last discussed one year ago:
> http://markmail.org/message/skalggb4typm27um
> >
> > I think this attempt is less intrusive and more contained than the
> previous one
> > (it was done quickly to see if there's interest, so still unfinished),
> with a
> > much cleaner implementation (largely due to parren's work on evolution,
> thanks ;).
> >
> > It doesn't try to be too smart, and now that we have phases it should be
> safe.
> >
> > This is arguably one of the most common history editing operations. I
> don't
> > think referring people to mq (or rollback, which is worse) for this is
> particularly good.
> > It requires activating a 'heavy' extension and use new commands that the
> user
> > isn't familiar with. This way the user is in familiar territory, knowing
> all the flags etc.
> >
> > diff --git a/mercurial/commands.py b/mercurial/commands.py
> > --- a/mercurial/commands.py
> > +++ b/mercurial/commands.py
> > @@ -9,7 +9,7 @@
> > from lock import release
> > from i18n import _, gettext
> > import os, re, difflib, time, tempfile, errno
> > -import hg, scmutil, util, revlog, extensions, copies, error, bookmarks
> > +import hg, scmutil, util, revlog, extensions, copies, error, bookmarks,
> repair
> > import patch, help, url, encoding, templatekw, discovery
> > import archival, changegroup, cmdutil, hbisect
> > import sshserver, hgweb, hgweb.server, commandserver
> > @@ -1163,6 +1163,7 @@
> > _('mark new/missing files as added/removed before committing')),
> > ('', 'close-branch', None,
> > _('mark a branch as closed, hiding it from the branch list')),
> > + ('', 'amend', None, _('amend a commit')),
> > ] + walkopts + commitopts + commitopts2 + subrepoopts,
> > _('[OPTION]... [FILE]...'))
> > def commit(ui, repo, *pats, **opts):
> > @@ -1209,6 +1210,15 @@
> > branch = repo[None].branch()
> > bheads = repo.branchheads(branch)
> >
> > + if opts.get('amend'):
> > + curr = repo['.']
> > + if len(curr.parents()) > 1:
> > + raise util.Abort(_('cannot amend merge changesets'))
> > + if curr.phase() == phases.public:
> > + raise util.Abort(_('cannot amend public changesets'))
> > + if len(curr.children()):
> > + ui.warn(_('warning: amending changeset with children'))
> > +
> > node = cmdutil.commit(ui, repo, commitfunc, pats, opts)
> > if not node:
> > stat = repo.status(match=scmutil.match(repo[None], pats, opts))
> > @@ -1222,6 +1232,68 @@
> > ctx = repo[node]
> > parents = ctx.parents()
> >
> > + if opts.get('amend'):
> > + old = repo['.'].p1()
> > + base = old.p1()
> > + bm = bookmarks.readcurrent(repo)
> > +
> > + # commit a new version of the old changeset, including the
> update
> > + # collect all files which might be affected
> > + files = set(old.files())
> > + files.update(ctx.files())
> > + # prune files which were reverted by the updates
> > + def samefile(f):
> > + if f in ctx.manifest():
> > + a = ctx.filectx(f)
> > + if f in base.manifest():
> > + b = base.filectx(f)
> > + return (a.data() == b.data()
> > + and a.flags() == b.flags()
> > + and a.renamed() == b.renamed())
> > + else:
> > + return False
> > + else:
> > + return f not in base.manifest()
> > + files = [f for f in files if not samefile(f)]
> > + # commit version of these files as defined by head
> > + headmf = ctx.manifest()
> > + def filectxfn(repo, ctx_, path):
> > + if path in headmf:
> > + return ctx.filectx(path)
> > + raise IOError()
> > + new = context.memctx(repo,
> > + parents=[old.p1().node(), old.p2().node()],
> > + text=opts.get('message') or
> old.description(),
> > + files=files,
> > + filectxfn=filectxfn,
> > + user=opts.get('user'),
> > + date=opts.get('date'),
> > + extra=extra)
> > + newid = repo.commitctx(new)
> > +
> > + wlock = repo.wlock()
> > + try:
> > + # reroute the working copy parent to the new changeset
> > + repo.dirstate.setparents(newid, nullid)
> > +
> > + # update the bookmark
> > + if bm:
> > + repo._bookmarks[bm] = newid
> > + bookmarks.write(repo)
> > + finally:
> > + wlock.release()
> > +
> > + ui.note('stripping intermediate commit %s\n' % ctx)
> > + lock = repo.lock()
> > + try:
> > + # no backup
> > + repair.strip(ui, repo, node, '')
> > + finally:
> > + lock.release()
> > +
> > + ctx = repo[newid]
> > + parents = ctx.parents()
> > +
> > if (bheads and node not in bheads and not
> > [x for x in parents if x.node() in bheads and x.branch() ==
> branch]):
> > ui.status(_('created new head\n'))
> > diff --git a/tests/test-commit.t b/tests/test-commit.t
> > --- a/tests/test-commit.t
> > +++ b/tests/test-commit.t
> > @@ -88,6 +88,36 @@
> > dir/file
> > committed changeset 4:49176991390e
> >
> > +commit --amend
> > +
> > + $ echo foo >> foo
> > + $ hg commit -m 'base'
> > + $ hg diff --nodates -c . foo
> > + diff -r 49176991390e -r 1846759e3d5f foo
> > + --- a/foo
> > + +++ b/foo
> > + @@ -1,2 +1,3 @@
> > + foo
> > + foo
> > + +foo
> > + $ echo bar >> foo
> > + $ hg phase -r . -p
> > + $ hg commit --amend
> > + abort: cannot amend public changesets
> > + [255]
> > + $ hg phase -f -r . -d
> > + $ hg commit --amend -m 'base amend'
> > + created new head
> > + $ hg diff --nodates -c . foo
> > + diff -r 49176991390e -r 013d43f99c45 foo
> > + --- a/foo
> > + +++ b/foo
> > + @@ -1,2 +1,4 @@
> > + foo
> > + foo
> > + +foo
> > + +bar
> > +
> > An empty date was interpreted as epoch origin
> >
> > $ echo foo >> foo
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
>
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120202/4d3a6164/attachment.html>
More information about the Mercurial-devel
mailing list