git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH,RFC] Add git-mergetool to run an appropriate merge conflict resolution program
Date: Mon, 12 Mar 2007 09:16:05 -0400	[thread overview]
Message-ID: <20070312131605.GA4372@thunk.org> (raw)
In-Reply-To: <7vodmzt0ud.fsf@assigned-by-dhcp.cox.net>

On Sun, Mar 11, 2007 at 11:47:22PM -0700, Junio C Hamano wrote:
> Thanks.  By the way, is it fashionable to misspell "scenario" in
> the kernel circles? ;-)

Probably not; it's just that many of us are probably fast typists, and
there's a race condition between the 'n' and the 'e on the right and
left hands, respectively.  :-)

> 
> > +    git cat-file blob ":1:$path" > "$BASE" 2>/dev/null
> > +    git cat-file blob ":2:$path" > "$LOCAL" 2>/dev/null
> > +    git cat-file blob ":3:$path" > "$REMOTE" 2>/dev/null
> > +
> > +    if test -z "$local_mode" -o -z "$remote_mode"; then
> > +	echo "Deleted merge conflict for $path:"
> > +	describe_file "$local_mode" "local" "$LOCAL"
> > +	describe_file "$remote_mode" "remote" "$REMOTE"
> > +	resolve_deleted_merge
> > +	return
> > +    fi
> 
> Running cat-file even when you know it does not exist at that
> stage does not feel right here, although you are not checking
> the exit status and discarding 2>/dev/null...

OK, I'll fix that.  

> One situation that happens in the real life to cause "we deleted
> while they modified" is when in reality we moved then modified
> so much that the difference between our version and the common
> ancestor version is too large to be considered a rename anymore.
> 
> Such a misidentified rename would appear as one path that is "we
> deleted while they modified" (original path) and the other path
> that is "we created while they didn't do anything to the path".
> The latter does not conflict and is already resolved in the
> index when you would run git-mergetool.
> 
> So if you have "we deleted while they modified" conflict, it may
> make sense to give the paths the index adds (relative to HEAD),
> let the user pick one of them and allow 3-way merge to update
> the path we renamed to.  The original path which had conflicted
> would be removed as the result.

I agree that would be useful, but it I'm going to have to restructure
the code a bit to support that.  Hmm, let me think about that a bit.

It occurs to me that I might also want to think about the order in
which I present files to be merged to the user in the "git mergetool"
case.  If we force the user to resolve these cases first, probably in
this order:

	* 2/3-way modified symlink/symlink
	* The symlink/modified file case
	* 3-way modified file case
	* 2-way modified file case (no common ancestor)

It also occurs to me that listing all of the files and characterizing
them according to what needs to be done up-front might also be a good
idea.  

I can remember, in the pre-git, Bitkeeper days some *truly nasty* ACPI
merges where there were some 20-30 files getting renamed and merged at
the same time.  Blech.  Can't we just slap the developers hands when
they do something this evil?  (Oh right, we already gave the ACPI
developers octopus merges which are a bisect nightmare.
Sigh... sometimes I'm not sure why we encourage them like this; I
guess it makes the kernel have some really test cases for git, which
is the only reason why it exists, right?  :-)


Junio, when and how do you feel about merging this?  It's clear the
code is going to have to go through some significant changes still to
add the above functionality, which would be an argument against
merging it, but on the other hand, it's quite useful in its current
form and I'd love to get more testers.  Another argument in favor of
merging it is that we already have more functionality here than hg,
monotone, Bitkeeper (as of the last free version), et. al.  And hey, I
created a man page so that proves I'm going to keep
maintaining/improving it.  :-)

						- Ted

      reply	other threads:[~2007-03-12 13:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-12  6:00 [PATCH,RFC] Add git-mergetool to run an appropriate merge conflict resolution program Theodore Ts'o
2007-03-12  6:47 ` Junio C Hamano
2007-03-12 13:16   ` Theodore Tso [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070312131605.GA4372@thunk.org \
    --to=tytso@mit.edu \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).