Patchwork amend: support amending merge changesets (issue3778)

login
register
mail settings
Submitter Brodie Rao
Date Feb. 8, 2013, 9:14 p.m.
Message ID <ac4b064dde742fbdeaea.1360358085@s0-0.paconsult7.bbnplanet.net>
Download mbox | patch
Permalink /patch/839/
State Superseded
Commit 26627c30735a610f59979a36885b327b25d8dbff
Headers show

Comments

Brodie Rao - Feb. 8, 2013, 9:14 p.m.
# HG changeset patch
# User Brodie Rao <brodie@sf.io>
# Date 1360357714 0
# Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
# Parent  2fefd1170bf269e26bb304553009f38e0117c342
amend: support amending merge changesets (issue3778)
Idan Kamara - Feb. 8, 2013, 9:31 p.m.
On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie@sf.io> wrote:
>
> # HG changeset patch
> # User Brodie Rao <brodie@sf.io>
> # Date 1360357714 0
> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
> # Parent  2fefd1170bf269e26bb304553009f38e0117c342
> amend: support amending merge changesets (issue3778)

I haven't looked in depth at the tests you added (seems
like you're not testing conflicts?) but I'm surprised it works
with so little and trivial code added.

I recall Matt saying this is going to be quite tricky to get
right (I also believe I tried to do something close
to what you did here and saw it misbehave sometimes,
but I can't remember where exactly right now).

>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1644,7 +1644,13 @@ def amend(ui, repo, commitfunc, old, ext
>              # Also update it from the intermediate commit or from the
> wctx
>              extra.update(ctx.extra())
>
> -            files = set(old.files())
> +            if len(old.parents()) > 1:
> +                # ctx.files() isn't reliable for merges, so fall back to
> the
> +                # slower repo.status() method
> +                files = set([fn for st in repo.status(base, old)[:3]
> +                             for fn in st])
> +            else:
> +                files = set(old.files())
>
>              # Second, we use either the commit we just did, or if there
> were no
>              # changes the parent of the working directory as the version
> of the
> @@ -1709,7 +1715,7 @@ def amend(ui, repo, commitfunc, old, ext
>              extra['amend_source'] = old.hex()
>
>              new = context.memctx(repo,
> -                                 parents=[base.node(), nullid],
> +                                 parents=[base.node(), old.p2().node()],
>                                   text=message,
>                                   files=files,
>                                   filectxfn=filectxfn,
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1301,8 +1301,6 @@ def commit(ui, repo, *pats, **opts):
>          old = repo['.']
>          if old.phase() == phases.public:
>              raise util.Abort(_('cannot amend public changesets'))
> -        if len(old.parents()) > 1:
> -            raise util.Abort(_('cannot amend merge changesets'))
>          if len(repo[None].parents()) > 1:
>              raise util.Abort(_('cannot amend while merging'))
>          if (not obsolete._enabled) and old.children():
> diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
> --- a/tests/test-commit-amend.t
> +++ b/tests/test-commit-amend.t
> @@ -304,7 +304,7 @@ Same thing, different code path:
>    $ hg branches
>    default                        2:ce12b0b57d46
>
> -Refuse to amend merges:
> +Refuse to amend during a merge:
>
>    $ hg up -q default
>    $ hg merge foo
> @@ -314,9 +314,6 @@ Refuse to amend merges:
>    abort: cannot amend while merging
>    [255]
>    $ hg ci -m 'merge'
> -  $ hg ci --amend
> -  abort: cannot amend merge changesets
> -  [255]
>
>  Follow copies/renames:
>
> @@ -518,3 +515,145 @@ Test that rewriting leaving instability
>    date:        Thu Jan 01 00:00:00 1970 +0000
>    summary:     babar
>
> +
> +Amend a merge changeset (with renames from the second parent):
> +
> +  $ hg up default
> +  2 files updated, 0 files merged, 4 files removed, 0 files unresolved
> +  $ hg branch bar
> +  marked working directory as branch bar
> +  (branches are permanent and global, did you want a bookmark?)
> +  $ hg cp a aa
> +  $ hg mv z zz
> +  $ echo cc > cc
> +  $ hg add cc
> +  $ hg ci -m aazzcc
> +  $ hg up default
> +  1 files updated, 0 files merged, 3 files removed, 0 files unresolved
> +  $ echo a >> a
> +  $ hg ci -m aa
> +  $ hg merge bar
> +  merging a and aa to aa
> +  2 files updated, 1 files merged, 1 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +  $ hg ci -m 'merge bar'
> +  $ hg log --debug -r .
> +  changeset:   23:328607007cf9443df8344c585b87af07e0625c8b
> +  tag:         tip
> +  phase:       draft
> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
> +  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  files+:      aa cc zz
> +  files-:      z
> +  extra:       branch=default
> +  description:
> +  merge bar
> +
> +
> +  $ hg debugrename aa
> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
> +  $ hg debugrename zz
> +  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
> +  $ hg debugrename cc
> +  cc not renamed
> +  $ hg ci --amend -m 'merge bar (amend message)'
> +  $ hg log --debug -r .
> +  changeset:   24:499c685c2f5e0bc60baff5804b3971fd8716e412
> +  tag:         tip
> +  phase:       draft
> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
> +  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  files+:      aa cc zz
> +  files-:      z
> +  extra:       amend_source=328607007cf9443df8344c585b87af07e0625c8b
> +  extra:       branch=default
> +  description:
> +  merge bar (amend message)
> +
> +
> +  $ hg debugrename aa
> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
> +  $ hg debugrename zz
> +  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
> +  $ hg debugrename cc
> +  cc not renamed
> +  $ hg mv zz z
> +  $ hg ci --amend -m 'merge bar (undo rename)'
> +  $ hg log --debug -r .
> +  changeset:   26:dc515e583c85f92ca06c2e284ca18bff869df5a8
> +  tag:         tip
> +  phase:       draft
> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
> +  manifest:    22:e84b4fcd7a8b9d3f0d9bacd3b7aedbd18b0dd56e
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  files+:      aa cc
> +  extra:       amend_source=499c685c2f5e0bc60baff5804b3971fd8716e412
> +  extra:       branch=default
> +  description:
> +  merge bar (undo rename)
> +
> +
> +  $ hg debugrename z
> +  z not renamed
> +
> +Amend a merge changeset (with renames during the merge):
> +
> +  $ hg up bar
> +  3 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ echo x > x
> +  $ hg add x
> +  $ hg ci -m x
> +  $ hg up default
> +  3 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +  $ hg merge bar
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +  $ hg mv aa aaa
> +  $ hg ci -m 'merge bar again'
> +  $ hg log --debug -r .
> +  changeset:   28:805630024adb77fbc52d0d49352e3ac0cf61e999
> +  tag:         tip
> +  phase:       draft
> +  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
> +  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
> +  manifest:    24:c39fb616a5250433f226d0079a413d345158c644
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  files+:      aaa x
> +  files-:      aa
> +  extra:       branch=default
> +  description:
> +  merge bar again
> +
> +
> +  $ hg debugrename aaa
> +  aaa renamed from aa:37d9b5d994eab34eda9c16b195ace52c7b129980
> +  $ hg ci --amend -m 'merge bar again (amend message)'
> +  $ hg mv aaa aa
> +  $ hg ci --amend -m 'merge bar again (undo rename)'
> +  $ hg log --debug -r .
> +  changeset:   31:bdb0e875df5a657ff24e3fadcacd6727d3f8005f
> +  tag:         tip
> +  phase:       draft
> +  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
> +  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
> +  manifest:    26:099ae0f86f993a0c00e3663366bf3f5dfb8cfe86
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  files+:      x
> +  extra:       amend_source=4314c852235cea166cae0c6bc93f9cb0e3d7744d
> +  extra:       branch=default
> +  description:
> +  merge bar again (undo rename)
> +
> +
> +  $ hg debugrename aa
> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Brodie Rao - Feb. 8, 2013, 9:45 p.m.
On Fri, Feb 8, 2013 at 9:31 PM, Idan Kamara <idankk86@gmail.com> wrote:
> On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie@sf.io> wrote:
>>
>> # HG changeset patch
>> # User Brodie Rao <brodie@sf.io>
>> # Date 1360357714 0
>> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
>> # Parent  2fefd1170bf269e26bb304553009f38e0117c342
>> amend: support amending merge changesets (issue3778)
>
> I haven't looked in depth at the tests you added (seems
> like you're not testing conflicts?) but I'm surprised it works
> with so little and trivial code added.

I don't think a merge with content-level conflicts would be that
different than what's tested now. A merge with manifest-level
conflicts might be interesting though.

> I recall Matt saying this is going to be quite tricky to get
> right (I also believe I tried to do something close
> to what you did here and saw it misbehave sometimes,
> but I can't remember where exactly right now).

If you don't handle the fact that ctx.files() can be totally wrong in
a lot of merges (files that were changed aren't in the list, files
that were merged but didn't have content-level changes are in the
list, etc), I can see why you'd run into weird issues.

But, accounting for that, I can't think of why this should be
difficult to implement, but perhaps I'm overlooking something.

>
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -1644,7 +1644,13 @@ def amend(ui, repo, commitfunc, old, ext
>>              # Also update it from the intermediate commit or from the
>> wctx
>>              extra.update(ctx.extra())
>>
>> -            files = set(old.files())
>> +            if len(old.parents()) > 1:
>> +                # ctx.files() isn't reliable for merges, so fall back to
>> the
>> +                # slower repo.status() method
>> +                files = set([fn for st in repo.status(base, old)[:3]
>> +                             for fn in st])
>> +            else:
>> +                files = set(old.files())
>>
>>              # Second, we use either the commit we just did, or if there
>> were no
>>              # changes the parent of the working directory as the version
>> of the
>> @@ -1709,7 +1715,7 @@ def amend(ui, repo, commitfunc, old, ext
>>              extra['amend_source'] = old.hex()
>>
>>              new = context.memctx(repo,
>> -                                 parents=[base.node(), nullid],
>> +                                 parents=[base.node(), old.p2().node()],
>>                                   text=message,
>>                                   files=files,
>>                                   filectxfn=filectxfn,
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -1301,8 +1301,6 @@ def commit(ui, repo, *pats, **opts):
>>          old = repo['.']
>>          if old.phase() == phases.public:
>>              raise util.Abort(_('cannot amend public changesets'))
>> -        if len(old.parents()) > 1:
>> -            raise util.Abort(_('cannot amend merge changesets'))
>>          if len(repo[None].parents()) > 1:
>>              raise util.Abort(_('cannot amend while merging'))
>>          if (not obsolete._enabled) and old.children():
>> diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
>> --- a/tests/test-commit-amend.t
>> +++ b/tests/test-commit-amend.t
>> @@ -304,7 +304,7 @@ Same thing, different code path:
>>    $ hg branches
>>    default                        2:ce12b0b57d46
>>
>> -Refuse to amend merges:
>> +Refuse to amend during a merge:
>>
>>    $ hg up -q default
>>    $ hg merge foo
>> @@ -314,9 +314,6 @@ Refuse to amend merges:
>>    abort: cannot amend while merging
>>    [255]
>>    $ hg ci -m 'merge'
>> -  $ hg ci --amend
>> -  abort: cannot amend merge changesets
>> -  [255]
>>
>>  Follow copies/renames:
>>
>> @@ -518,3 +515,145 @@ Test that rewriting leaving instability
>>    date:        Thu Jan 01 00:00:00 1970 +0000
>>    summary:     babar
>>
>> +
>> +Amend a merge changeset (with renames from the second parent):
>> +
>> +  $ hg up default
>> +  2 files updated, 0 files merged, 4 files removed, 0 files unresolved
>> +  $ hg branch bar
>> +  marked working directory as branch bar
>> +  (branches are permanent and global, did you want a bookmark?)
>> +  $ hg cp a aa
>> +  $ hg mv z zz
>> +  $ echo cc > cc
>> +  $ hg add cc
>> +  $ hg ci -m aazzcc
>> +  $ hg up default
>> +  1 files updated, 0 files merged, 3 files removed, 0 files unresolved
>> +  $ echo a >> a
>> +  $ hg ci -m aa
>> +  $ hg merge bar
>> +  merging a and aa to aa
>> +  2 files updated, 1 files merged, 1 files removed, 0 files unresolved
>> +  (branch merge, don't forget to commit)
>> +  $ hg ci -m 'merge bar'
>> +  $ hg log --debug -r .
>> +  changeset:   23:328607007cf9443df8344c585b87af07e0625c8b
>> +  tag:         tip
>> +  phase:       draft
>> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
>> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
>> +  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  files+:      aa cc zz
>> +  files-:      z
>> +  extra:       branch=default
>> +  description:
>> +  merge bar
>> +
>> +
>> +  $ hg debugrename aa
>> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
>> +  $ hg debugrename zz
>> +  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
>> +  $ hg debugrename cc
>> +  cc not renamed
>> +  $ hg ci --amend -m 'merge bar (amend message)'
>> +  $ hg log --debug -r .
>> +  changeset:   24:499c685c2f5e0bc60baff5804b3971fd8716e412
>> +  tag:         tip
>> +  phase:       draft
>> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
>> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
>> +  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  files+:      aa cc zz
>> +  files-:      z
>> +  extra:       amend_source=328607007cf9443df8344c585b87af07e0625c8b
>> +  extra:       branch=default
>> +  description:
>> +  merge bar (amend message)
>> +
>> +
>> +  $ hg debugrename aa
>> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
>> +  $ hg debugrename zz
>> +  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
>> +  $ hg debugrename cc
>> +  cc not renamed
>> +  $ hg mv zz z
>> +  $ hg ci --amend -m 'merge bar (undo rename)'
>> +  $ hg log --debug -r .
>> +  changeset:   26:dc515e583c85f92ca06c2e284ca18bff869df5a8
>> +  tag:         tip
>> +  phase:       draft
>> +  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
>> +  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
>> +  manifest:    22:e84b4fcd7a8b9d3f0d9bacd3b7aedbd18b0dd56e
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  files+:      aa cc
>> +  extra:       amend_source=499c685c2f5e0bc60baff5804b3971fd8716e412
>> +  extra:       branch=default
>> +  description:
>> +  merge bar (undo rename)
>> +
>> +
>> +  $ hg debugrename z
>> +  z not renamed
>> +
>> +Amend a merge changeset (with renames during the merge):
>> +
>> +  $ hg up bar
>> +  3 files updated, 0 files merged, 1 files removed, 0 files unresolved
>> +  $ echo x > x
>> +  $ hg add x
>> +  $ hg ci -m x
>> +  $ hg up default
>> +  3 files updated, 0 files merged, 2 files removed, 0 files unresolved
>> +  $ hg merge bar
>> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> +  (branch merge, don't forget to commit)
>> +  $ hg mv aa aaa
>> +  $ hg ci -m 'merge bar again'
>> +  $ hg log --debug -r .
>> +  changeset:   28:805630024adb77fbc52d0d49352e3ac0cf61e999
>> +  tag:         tip
>> +  phase:       draft
>> +  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
>> +  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
>> +  manifest:    24:c39fb616a5250433f226d0079a413d345158c644
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  files+:      aaa x
>> +  files-:      aa
>> +  extra:       branch=default
>> +  description:
>> +  merge bar again
>> +
>> +
>> +  $ hg debugrename aaa
>> +  aaa renamed from aa:37d9b5d994eab34eda9c16b195ace52c7b129980
>> +  $ hg ci --amend -m 'merge bar again (amend message)'
>> +  $ hg mv aaa aa
>> +  $ hg ci --amend -m 'merge bar again (undo rename)'
>> +  $ hg log --debug -r .
>> +  changeset:   31:bdb0e875df5a657ff24e3fadcacd6727d3f8005f
>> +  tag:         tip
>> +  phase:       draft
>> +  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
>> +  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
>> +  manifest:    26:099ae0f86f993a0c00e3663366bf3f5dfb8cfe86
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  files+:      x
>> +  extra:       amend_source=4314c852235cea166cae0c6bc93f9cb0e3d7744d
>> +  extra:       branch=default
>> +  description:
>> +  merge bar again (undo rename)
>> +
>> +
>> +  $ hg debugrename aa
>> +  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>
Matt Mackall - Feb. 8, 2013, 10:23 p.m.
On Fri, 2013-02-08 at 21:45 +0000, Brodie Rao wrote:
> On Fri, Feb 8, 2013 at 9:31 PM, Idan Kamara <idankk86@gmail.com> wrote:
> > On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie@sf.io> wrote:
> >>
> >> # HG changeset patch
> >> # User Brodie Rao <brodie@sf.io>
> >> # Date 1360357714 0
> >> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
> >> # Parent  2fefd1170bf269e26bb304553009f38e0117c342
> >> amend: support amending merge changesets (issue3778)
> >
> > I haven't looked in depth at the tests you added (seems
> > like you're not testing conflicts?) but I'm surprised it works
> > with so little and trivial code added.
> 
> I don't think a merge with content-level conflicts would be that
> different than what's tested now. A merge with manifest-level
> conflicts might be interesting though.
> 
> > I recall Matt saying this is going to be quite tricky to get
> > right (I also believe I tried to do something close
> > to what you did here and saw it misbehave sometimes,
> > but I can't remember where exactly right now).
> 
> If you don't handle the fact that ctx.files() can be totally wrong in
> a lot of merges (files that were changed aren't in the list, files
> that were merged but didn't have content-level changes are in the
> list, etc), I can see why you'd run into weird issues.
> 
> But, accounting for that, I can't think of why this should be
> difficult to implement, but perhaps I'm overlooking something.

I think the particular issues I had in mind were to do with the nasty
business in localrepo._filecommit. We have to figure out which
file-level parent revisions to use for merged files and we can get
confused with merges involving copies especially.
Brodie Rao - Feb. 8, 2013, 10:42 p.m.
On Fri, Feb 8, 2013 at 10:23 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Fri, 2013-02-08 at 21:45 +0000, Brodie Rao wrote:
>> On Fri, Feb 8, 2013 at 9:31 PM, Idan Kamara <idankk86@gmail.com> wrote:
>> > On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie@sf.io> wrote:
>> >>
>> >> # HG changeset patch
>> >> # User Brodie Rao <brodie@sf.io>
>> >> # Date 1360357714 0
>> >> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
>> >> # Parent  2fefd1170bf269e26bb304553009f38e0117c342
>> >> amend: support amending merge changesets (issue3778)
>> >
>> > I haven't looked in depth at the tests you added (seems
>> > like you're not testing conflicts?) but I'm surprised it works
>> > with so little and trivial code added.
>>
>> I don't think a merge with content-level conflicts would be that
>> different than what's tested now. A merge with manifest-level
>> conflicts might be interesting though.
>>
>> > I recall Matt saying this is going to be quite tricky to get
>> > right (I also believe I tried to do something close
>> > to what you did here and saw it misbehave sometimes,
>> > but I can't remember where exactly right now).
>>
>> If you don't handle the fact that ctx.files() can be totally wrong in
>> a lot of merges (files that were changed aren't in the list, files
>> that were merged but didn't have content-level changes are in the
>> list, etc), I can see why you'd run into weird issues.
>>
>> But, accounting for that, I can't think of why this should be
>> difficult to implement, but perhaps I'm overlooking something.
>
> I think the particular issues I had in mind were to do with the nasty
> business in localrepo._filecommit. We have to figure out which
> file-level parent revisions to use for merged files and we can get
> confused with merges involving copies especially.

As mentioned in person, I don't think this is an issue. Amend properly
preserves copy information, and it just reuses the filelogs from the
temporary commit, which are carried over from the merge commit we're
replacing, so all the filelogs should be consistent.

In other words, because amend properly preserves everything, it
doesn't need to be aware of the things localrepo._filecommit does.
Idan Kamara - Feb. 9, 2013, 9:38 a.m.
On Sat, Feb 9, 2013 at 12:42 AM, Brodie Rao <brodie@sf.io> wrote:
>
> On Fri, Feb 8, 2013 at 10:23 PM, Matt Mackall <mpm@selenic.com> wrote:
> > On Fri, 2013-02-08 at 21:45 +0000, Brodie Rao wrote:
> >> On Fri, Feb 8, 2013 at 9:31 PM, Idan Kamara <idankk86@gmail.com> wrote:
> >> > On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie@sf.io> wrote:
> >> >>
> >> >> # HG changeset patch
> >> >> # User Brodie Rao <brodie@sf.io>
> >> >> # Date 1360357714 0
> >> >> # Node ID ac4b064dde742fbdeaea4818569ca3d134ed92bf
> >> >> # Parent  2fefd1170bf269e26bb304553009f38e0117c342
> >> >> amend: support amending merge changesets (issue3778)
> >> >
> >> > I haven't looked in depth at the tests you added (seems
> >> > like you're not testing conflicts?) but I'm surprised it works
> >> > with so little and trivial code added.
> >>
> >> I don't think a merge with content-level conflicts would be that
> >> different than what's tested now. A merge with manifest-level
> >> conflicts might be interesting though.
> >>
> >> > I recall Matt saying this is going to be quite tricky to get
> >> > right (I also believe I tried to do something close
> >> > to what you did here and saw it misbehave sometimes,
> >> > but I can't remember where exactly right now).
> >>
> >> If you don't handle the fact that ctx.files() can be totally wrong in
> >> a lot of merges (files that were changed aren't in the list, files
> >> that were merged but didn't have content-level changes are in the
> >> list, etc), I can see why you'd run into weird issues.
> >>
> >> But, accounting for that, I can't think of why this should be
> >> difficult to implement, but perhaps I'm overlooking something.
> >
> > I think the particular issues I had in mind were to do with the nasty
> > business in localrepo._filecommit. We have to figure out which
> > file-level parent revisions to use for merged files and we can get
> > confused with merges involving copies especially.
>
> As mentioned in person, I don't think this is an issue. Amend properly
> preserves copy information, and it just reuses the filelogs from the
> temporary commit, which are carried over from the merge commit we're
> replacing, so all the filelogs should be consistent.
>
> In other words, because amend properly preserves everything, it
> doesn't need to be aware of the things localrepo._filecommit does.

Ok, wouldn't hurt to add tests for conflict resolution then.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1644,7 +1644,13 @@  def amend(ui, repo, commitfunc, old, ext
             # Also update it from the intermediate commit or from the wctx
             extra.update(ctx.extra())
 
-            files = set(old.files())
+            if len(old.parents()) > 1:
+                # ctx.files() isn't reliable for merges, so fall back to the
+                # slower repo.status() method
+                files = set([fn for st in repo.status(base, old)[:3]
+                             for fn in st])
+            else:
+                files = set(old.files())
 
             # Second, we use either the commit we just did, or if there were no
             # changes the parent of the working directory as the version of the
@@ -1709,7 +1715,7 @@  def amend(ui, repo, commitfunc, old, ext
             extra['amend_source'] = old.hex()
 
             new = context.memctx(repo,
-                                 parents=[base.node(), nullid],
+                                 parents=[base.node(), old.p2().node()],
                                  text=message,
                                  files=files,
                                  filectxfn=filectxfn,
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1301,8 +1301,6 @@  def commit(ui, repo, *pats, **opts):
         old = repo['.']
         if old.phase() == phases.public:
             raise util.Abort(_('cannot amend public changesets'))
-        if len(old.parents()) > 1:
-            raise util.Abort(_('cannot amend merge changesets'))
         if len(repo[None].parents()) > 1:
             raise util.Abort(_('cannot amend while merging'))
         if (not obsolete._enabled) and old.children():
diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
--- a/tests/test-commit-amend.t
+++ b/tests/test-commit-amend.t
@@ -304,7 +304,7 @@  Same thing, different code path:
   $ hg branches
   default                        2:ce12b0b57d46
 
-Refuse to amend merges:
+Refuse to amend during a merge:
 
   $ hg up -q default
   $ hg merge foo
@@ -314,9 +314,6 @@  Refuse to amend merges:
   abort: cannot amend while merging
   [255]
   $ hg ci -m 'merge'
-  $ hg ci --amend
-  abort: cannot amend merge changesets
-  [255]
 
 Follow copies/renames:
 
@@ -518,3 +515,145 @@  Test that rewriting leaving instability 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     babar
   
+
+Amend a merge changeset (with renames from the second parent):
+
+  $ hg up default
+  2 files updated, 0 files merged, 4 files removed, 0 files unresolved
+  $ hg branch bar
+  marked working directory as branch bar
+  (branches are permanent and global, did you want a bookmark?)
+  $ hg cp a aa
+  $ hg mv z zz
+  $ echo cc > cc
+  $ hg add cc
+  $ hg ci -m aazzcc
+  $ hg up default
+  1 files updated, 0 files merged, 3 files removed, 0 files unresolved
+  $ echo a >> a
+  $ hg ci -m aa
+  $ hg merge bar
+  merging a and aa to aa
+  2 files updated, 1 files merged, 1 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+  $ hg ci -m 'merge bar'
+  $ hg log --debug -r .
+  changeset:   23:328607007cf9443df8344c585b87af07e0625c8b
+  tag:         tip
+  phase:       draft
+  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
+  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
+  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  files+:      aa cc zz
+  files-:      z
+  extra:       branch=default
+  description:
+  merge bar
+  
+  
+  $ hg debugrename aa
+  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
+  $ hg debugrename zz
+  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
+  $ hg debugrename cc
+  cc not renamed
+  $ hg ci --amend -m 'merge bar (amend message)'
+  $ hg log --debug -r .
+  changeset:   24:499c685c2f5e0bc60baff5804b3971fd8716e412
+  tag:         tip
+  phase:       draft
+  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
+  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
+  manifest:    20:24810812512c7ae79bd8f6cd3be3d4b762400314
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  files+:      aa cc zz
+  files-:      z
+  extra:       amend_source=328607007cf9443df8344c585b87af07e0625c8b
+  extra:       branch=default
+  description:
+  merge bar (amend message)
+  
+  
+  $ hg debugrename aa
+  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e
+  $ hg debugrename zz
+  zz renamed from z:69a1b67522704ec122181c0890bd16e9d3e7516a
+  $ hg debugrename cc
+  cc not renamed
+  $ hg mv zz z
+  $ hg ci --amend -m 'merge bar (undo rename)'
+  $ hg log --debug -r .
+  changeset:   26:dc515e583c85f92ca06c2e284ca18bff869df5a8
+  tag:         tip
+  phase:       draft
+  parent:      22:ab15c66e8f33a801daae0714088a7b7af1467c34
+  parent:      21:1aa437659d19aecddc045900b54c8a525b2c11cd
+  manifest:    22:e84b4fcd7a8b9d3f0d9bacd3b7aedbd18b0dd56e
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  files+:      aa cc
+  extra:       amend_source=499c685c2f5e0bc60baff5804b3971fd8716e412
+  extra:       branch=default
+  description:
+  merge bar (undo rename)
+  
+  
+  $ hg debugrename z
+  z not renamed
+
+Amend a merge changeset (with renames during the merge):
+
+  $ hg up bar
+  3 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo x > x
+  $ hg add x
+  $ hg ci -m x
+  $ hg up default
+  3 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  $ hg merge bar
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+  $ hg mv aa aaa
+  $ hg ci -m 'merge bar again'
+  $ hg log --debug -r .
+  changeset:   28:805630024adb77fbc52d0d49352e3ac0cf61e999
+  tag:         tip
+  phase:       draft
+  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
+  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
+  manifest:    24:c39fb616a5250433f226d0079a413d345158c644
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  files+:      aaa x
+  files-:      aa
+  extra:       branch=default
+  description:
+  merge bar again
+  
+  
+  $ hg debugrename aaa
+  aaa renamed from aa:37d9b5d994eab34eda9c16b195ace52c7b129980
+  $ hg ci --amend -m 'merge bar again (amend message)'
+  $ hg mv aaa aa
+  $ hg ci --amend -m 'merge bar again (undo rename)'
+  $ hg log --debug -r .
+  changeset:   31:bdb0e875df5a657ff24e3fadcacd6727d3f8005f
+  tag:         tip
+  phase:       draft
+  parent:      26:dc515e583c85f92ca06c2e284ca18bff869df5a8
+  parent:      27:4c94d5bc65f5ec1700417b11a74e5bce2737ac38
+  manifest:    26:099ae0f86f993a0c00e3663366bf3f5dfb8cfe86
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  files+:      x
+  extra:       amend_source=4314c852235cea166cae0c6bc93f9cb0e3d7744d
+  extra:       branch=default
+  description:
+  merge bar again (undo rename)
+  
+  
+  $ hg debugrename aa
+  aa renamed from a:a80d06849b333b8a3d5c445f8ba3142010dcdc9e