[PATCH RFC] commit: add --amend option to amend the parent changeset

Thu Feb 2 18:35:26 UTC 2012

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?

Does this do the right thing when amending merges? In particular,
preserving the very tricky to reproduce file-level history for merged
files?

> 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.

> 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.