merge-recursive: do not rudely die on binary merge

When you try to merge a path that involves binary file-level merge, merge-recursive died rudely without cleaning up its own mess. A files added by the merge were left in the working tree, but the index was not written out (because it just punted and died), so it was cumbersome for the user to retry it by first running "git reset --hard".

This changes merge-recursive to still warn but do the "binary" merge for such a path; leave the "our" version in the working tree, but still keep the path unmerged so that the user can sort it out.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 merge-recursive.c       |   51 +++++++++++++++++++----------------
 t/t6027-merge-binary.sh |   67 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 23 deletions(-)

On Tue, Aug 14, 2007 at 03:33:07PM -0700, Junio C Hamano wrote:

Very nice. Thanks, Junio. As an additional convenience, it would be nice to make the "theirs" version easily accessible. Perhaps, by leaving an untracked file in the working tree, with the original filename, suffixed with a hash-prefix. Or alternatively, cut-n-pastable instuctions on stdout for replacing the file with the "theirs" version.

On the other hand, I tend to think that "theirs" would be a better default than "ours" anyway - still leaving the path unmerged, of course.

-chris

Chris Shoemaker <c.shoemaker@cox.net> writes:

People should learn this command.  Really.

	$ git cat-file -p :$n:path

where $n == 2 is ours, $n == 1 is common ancestor, and $n == 3 is theirs.

On 8/15/07, Junio C Hamano <gitster@pobox.com> wrote:

A question related to this: as a user, how can I tell if a command is something I'm expected to use, or if thinking I need it is a sign that I'm doing something wrong?

Git has many commands, and telling the business-as-usual apart from the deviant ones is not always easy. It may be that it's just a question of knowing what is plumbing and what is a porcelain, but I'm not sure.

Cheers,

 -- Nikodemus

"Nikodemus Siivola" <nikodemus@random-state.net> writes:

Good question.

I guess "git show" instead of "git cat-file -p" is probably the recommended way these days. Mostly sticking to what the user manual demonstrates would be a safe thing to do, as J. Bruce Fields has really did a great job carefully picking the order of the commands to be presented in the manual.

Junio C Hamano wrote:
 > People should learn this command.  Really.
 >
 >       $ git cat-file -p :$n:path
 >
 > where $n == 2 is ours, $n == 1 is common ancestor, and $n == 3
 > is theirs.

The git-rev-parse manpage talks about the :$n:path notation (buried deep in a list of other syntax) but it just says $n is a "stage number" -- someone who is not familiar with the internals of git's merge implementation is never going to be able to figure out that "1", "2", and "3" mean what Junio said. ---

    Not sure if this is correct for octopus merges -- corrections welcome.

 Documentation/git-rev-parse.txt |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

On Mon, Aug 20, 2007 at 11:36:38AM +0800, Steven Grimm wrote:

I often forget which number corresponds to which source. I seem to recall somebody proposing :ours:$path a while ago, but I couldn't find any reference in the archive, so perhaps I just dreamed it.

Am I the only one who messes this up? If not, patch is below.

-- >8 -- sha1_name: allow human-readable stage aliases

This adds the alias ":ours:$path" to mean the same thing as ":2:$path", as well as "base" (for 1) and "theirs" (for 2), for those of us who merge infrequently and forget which is which.

The parsing is as strict as possible in order to minimize impact on filenames with colons. However, for some (presumably unlikely) filenames, the behavior is changed. Previously, you could look at stage 0 of any file beginning with the string "ours:" as simply "git-show :ours:foo". Now, because of the parsing conflict, you must use "git-show :0:ours:foo".

---

 sha1_name.c |   35 +++++++++++++++++++++++++++++------
 1 files changed, 29 insertions(+), 6 deletions(-)

Jeff King <peff@peff.net> wrote:

Maybe.  ;-)

I've memorized it long long ago. But my coworkers haven't and always get it wrong, and look at me funny when I tell them "trust me, your data is in stage 2 and theirs is in stage 3... because that's the convention all of the tools you are using follows".

Keywords in that last part: "convention" and "tools you are using". Someone could redefine what the stages mean and load content into them using `update index --index-info`. You might even be able to load the stages in odd ways yourself from Porcelain.

Oh, like say git-rebase. During a rebase "theirs" (stage 3) is your file and "ours" (stage 2) is the upstream. Confusing now, ain't it? Mine is theirs and ours is theirs? Huh? Yeeaaaah.

This is why I've never liked most merge tools.  They get hung up on
what is theirs and what is mine and then at some point they wind up
confusing the stages and getting them inverted.  And this is exactly
why git-merge.sh/git-rebase.sh/git-am.sh try to setup GITHEAD_* for
git-merge-recursive, and why they set it up using branch names and
patch subject lines, because it makes the conflict markers easier
to understand.
 

At least document the new syntax in git-rev-parse documentation?

"Shawn O. Pearce" <spearce@spearce.org> wrote:

Actually, what's wrong with the following:

	git show HEAD:foo.c
	git show MERGE_HEAD:foo.c

?

That gives you yours (HEAD) and theirs (MERGE_HEAD). And it doesn't abuse sha1_file.c. Granted it only works during a true merge and doesn't work during a rebase, but remember I just pointed out life is backwards anyway during a rebase, so uh, yea...

* Shawn O. Pearce:

I think that in the staged versions, the non-conflicting parts of the merge are in fact merged. For the HEAD/MERGE_HEAD versions, this isn't the case, obviously.

On Mon, Aug 20, 2007 at 09:15:01AM +0200, Florian Weimer wrote:

No, the stage versions are not merged at all (but the working tree copy has all non-conflicting parts merged).

Here's a script that creates a conflicted file with one easily resolved part and one conflict. You can see the staged hashes at the end (and check the working tree copy to see that only the one conflict is marked).

-Peff

-- >8 -- mkdir repo && cd repo && git-init

head -n 100 </usr/share/dict/words >words git-add words git-commit -m words

sed '10d' <words >words.tmp mv words.tmp words git-commit -a -m 'remove 10'

git-checkout -b other HEAD^
sed '9d
     90d' <words >words.tmp
mv words.tmp words
git-commit -a -m 'remove 9 and 90'

git-merge master

echo "stage 2 `git-rev-parse :2:words`" echo "HEAD `git-rev-parse HEAD:words`" echo "stage 3 `git-rev-parse :3:words`" echo "MERGE_HEAD `git-rev-parse MERGE_HEAD:words`"

"Shawn O. Pearce" <spearce@spearce.org> writes:

I am not _opposed_ to :ours:$path syntax, but I suspect there is something else that is wrong if you need to use :$n:$path syntax that often.

I have never been in a situation I had to say :base:$path, unless I am debugging the merge driver. So it is between :ours:$path and :theirs:$path.

But aren't they by definition HEAD:$path and MERGE_HEAD:$path, which are far more descriptive?

On Sun, Aug 19, 2007 at 11:30:19PM -0700, Junio C Hamano wrote:

That's the problem: I don't use it that often, so when I do, the numbers seem nonsensical.

I used to need it to look at the 3-way merge, but git-mergetool now does a nice job of hiding these details from me.

Thinking about it more, I really _haven't_ used the stages at all since git-mergetool came around. This thread just reminded me of all the prior times when I had had trouble with it.

So Junio, please consider my patch retracted. What _would_ be useful is GITHEAD_* support for git-mergetool, which has been on my 'todo' list for some time. I'll see if I can work up a patch for that.

-Peff

Junio C Hamano wrote:

Nice idea, if only this was mentioned in the documentation...

On Mon, Aug 20, 2007 at 02:05:22AM -0400, Shawn O. Pearce wrote:

Ugh, I hadn't even thought of that. git-diff _does_ respect "--base", "--ours", and "--theirs" to mean the same thing, but I am now wondering if that is a bit of a mistake.

However, as the intent of my patch was to _increase_ usability, I think a gotcha like that is probably counterproductive. OTOH, users of git-rebase already have to make the switch mentally.

Yes, I agree that the GITHEAD markers are much more sensible. Unfortunately, I'm not sure of the best way to translate them into stage names. The MERGE_HEAD/HEAD suggestion you made is a nice way of avoiding the whole issue, though it doesn't easily provide the "base" version.

I was about to, but your message has convinced me that this is perhaps not a very good idea.

-Peff

Jeff King wrote:

prefixcmp()?

-- Hannes

Steven Grimm <koreth@midwinter.com> writes:

The patch makes sense.  Thanks.

Just to give historical background to new readers, this is primarily because the really core level of the plumbing started as not caring between stages 2 and 3 (iow, as far as the merge is concerned, both heads are equal), and the description in the manual was written back then.

These days, all the merge strategies and other non-merge programs such as "git am" that can record conflicts as multi-stage index entries consistently use stage #2 as our version, and stages #2 and #3 are not equals anymore.

On Sun, Aug 19, 2007 at 23:20:53 -0700, Junio C Hamano wrote:

Pardon me? In what are they not equal?

In merge, the parents *are* equall. They are just recorded in the resulting commit in particular order and the stages are in that order.

Besides if I prepare a merge locally to push out to a shared repo, I will probably switch to the mainline and merge my branch in, so it will actually be my changes in stage #3. That is to say 'ours' and 'theirs' don't really express what is going on IMHO.

Jan Hudec <bulb@ucw.cz> writes:

The version left in the worktree always corresponds to stage #2. There is no way to say "please use stage #2 for their version and use stage #3 for our version" (that is not necessary, so don't take this as if I am complaining about a lack of feature). "git-read-tree -m" knows that stage #2 corresopnds to the index and the worktree and performs up-to-date check on them to make sure you do not lose local changes.

Merge parent order does not matter --- they are more equal than stages between #2 and #3. But that's not what we are discussing here.

In some minor corners merge parents are not exactly equal -- history simplification and other places that wants to pick one parent iterates from the first to the last parent and pick the first one, so strictly speaking earlier parents have a slight edge over later ones.

Chris Shoemaker <c.shoemaker@cox.net> writes:

Forgot to ask. I did this because you had trouble on #git yesterday and then at around the same time today somebody else had the same issue. Did this patch solve your problem? I do not think this has big risk of regression, but if it does not help anything there is no reason to put it in 1.5.3, so I am asking for a success report.

On 8/14/07, Junio C Hamano <gitster@pobox.com> wrote:

I got bit by this one earlier, and the patch fixed it here.

On Tue, Aug 14, 2007 at 05:09:05PM -0700, Junio C Hamano wrote:

Yes, with this patch, the merge completes, and leaves the index and working tree in a sane state. Thanks again.

-chris