Re: two-way merge corner case bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 01, 2013 at 03:49:23PM -0800, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> >> I actually was wondering if we can remove that sole uses of two-way
> >> merge with --reset -u from "git am" and replace them with something
> >> else.  But we do want to keep local change that existed before "am"
> >> started, so we cannot avoid use of two-way merge, I guess...
> >
> > Yeah, I think that is a case we definitely want to keep, as it means any
> > intermediate work done by the user in applying the patch is not lost.
> 
> I am not sure what you mean by "intermediate" here.  When the user
> attempts to resolve conflict in a path manually and gives up, we do
> want to revert changes to such a path to that of HEAD.

I thought we left it unchanged intentionally, but I cannot seem to find
the conversation I am thinking of. I did find this:

  http://thread.gmane.org/gmane.comp.version-control.git/164002

But that is not about "save the intermediate work done on applying the
series", but rather to help with this case:

  1. You "git am" a series. It doesn't apply.

  2. You get distracted and work on something else, forgetting that the
     dotest directory is sitting there.

  3. Time passes.

  4. You try to "git am" a new series. An "am" is already in progress,
     so you are advised to "git am --abort".

At that point you do not want to reset the commit to the original head
(which was fixed by 7b3b7e3), but you do not necessarily want to reset
the working tree contents either. We leave them in the name of safety
and letting the user deal with it.

> Clarifying the semantics of "--reset" needs to be done carefully.
> 
> I think any difference "git diff --cached" shows are fair game to
> revert to HEAD.  In the earlier example, path "Z" that was created
> by recursive merge in an attempt to rename "A" should be removed,
> and path "A" should be recreated to that of HEAD, as we know at the
> point of "am --skip/--abort" that these two paths were involved in
> the recursive merge invoked by the patch application process (that
> is the only possible reason why these are different in the index
> from HEAD).  Also any conflicting entries can only come from
> three-way merge and they should be reverted to that of HEAD.
> 
> On the other hand, the paths that appear in "git diff" (except for
> those that appear in "git diff --cached", which we will revert to
> HEAD following the logic of the previous paragraph) must be kept.
> They are changes that were already present in the working tree
> before the user decided to accept a trivial patch via "am" that does
> not overlap with what the user was doing.  We allow a dirty working
> tree but disallow a dirty index when "git am" starts, exactly
> because we want to ensure this property.
> 
> By doing both of the above, we should be able to satisfy the user
> who uses "am --abort/--skip", in order to restore paths that were
> involved in the failed attempt to three-way merge to revert back to
> that of HEAD, while keeping unrelated changes that were present
> before "am" started.

I think that makes sense, but I don't think we have a read-tree mode to
do that in one pass, do we? Not that we can't add one if we need.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]