[PATCH] amend: support amending merge changesets (issue3778)

[Idan Kamara]

On Sat, Feb 9, 2013 at 12:42 AM, Brodie Rao <brodie at sf.io> wrote:
>
> On Fri, Feb 8, 2013 at 10:23 PM, Matt Mackall <mpm at 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 at gmail.com> wrote:
> >> > On Fri, Feb 8, 2013 at 11:14 PM, Brodie Rao <brodie at sf.io> wrote:
> >> >>
> >> >> # HG changeset patch
> >> >> # User Brodie Rao <brodie at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130209/526d05aa/attachment.html>