merge-recursive: do not rudely die on binary merge

merge-recursive: do not rudely die on binary merge

[Junio C Hamano]

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(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index f7d1b84..5326d7c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -677,6 +677,26 @@ struct ll_merge_driver {
 /*
  * Built-in low-levels
  */
+static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
+			   const char *path_unused,
+			   mmfile_t *orig,
+			   mmfile_t *src1, const char *name1,
+			   mmfile_t *src2, const char *name2,
+			   mmbuffer_t *result)
+{
+	/*
+	 * The tentative merge result is "ours" for the final round,
+	 * or common ancestor for an internal merge.  Still return
+	 * "conflicted merge" status.
+	 */
+	mmfile_t *stolen = index_only ? orig : src1;
+
+	result->ptr = stolen->ptr;
+	result->size = stolen->size;
+	stolen->ptr = NULL;
+	return 1;
+}
+
 static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			const char *path_unused,
 			mmfile_t *orig,
@@ -687,10 +707,15 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 	xpparam_t xpp;
 
 	if (buffer_is_binary(orig->ptr, orig->size) ||
-			buffer_is_binary(src1->ptr, src1->size) ||
-			buffer_is_binary(src2->ptr, src2->size))
-		return error("Cannot merge binary files: %s vs. %s\n",
+	    buffer_is_binary(src1->ptr, src1->size) ||
+	    buffer_is_binary(src2->ptr, src2->size)) {
+		warning("Cannot merge binary files: %s vs. %s\n",
 			name1, name2);
+		return ll_binary_merge(drv_unused, path_unused,
+				       orig, src1, name1,
+				       src2, name2,
+				       result);
+	}
 
 	memset(&xpp, 0, sizeof(xpp));
 	return xdl_merge(orig,
@@ -743,26 +768,6 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 	return 0;
 }
 
-static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
-			   const char *path_unused,
-			   mmfile_t *orig,
-			   mmfile_t *src1, const char *name1,
-			   mmfile_t *src2, const char *name2,
-			   mmbuffer_t *result)
-{
-	/*
-	 * The tentative merge result is "ours" for the final round,
-	 * or common ancestor for an internal merge.  Still return
-	 * "conflicted merge" status.
-	 */
-	mmfile_t *stolen = index_only ? orig : src1;
-
-	result->ptr = stolen->ptr;
-	result->size = stolen->size;
-	stolen->ptr = NULL;
-	return 1;
-}
-
 #define LL_BINARY_MERGE 0
 #define LL_TEXT_MERGE 1
 #define LL_UNION_MERGE 2
diff --git a/t/t6027-merge-binary.sh b/t/t6027-merge-binary.sh
new file mode 100755
index 0000000..a7358f7
--- /dev/null
+++ b/t/t6027-merge-binary.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='ask merge-recursive to merge binary files'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	cat ../test4012.png >m &&
+	git add m &&
+	git ls-files -s | sed -e "s/ 0	/ 1	/" >E1 &&
+	test_tick &&
+	git commit -m "initial" &&
+
+	git branch side &&
+	echo frotz >a &&
+	git add a &&
+	echo nitfol >>m &&
+	git add a m &&
+	git ls-files -s a >E0 &&
+	git ls-files -s m | sed -e "s/ 0	/ 3	/" >E3 &&
+	test_tick &&
+	git commit -m "master adds some" &&
+
+	git checkout side &&
+	echo rezrov >>m &&
+	git add m &&
+	git ls-files -s m | sed -e "s/ 0	/ 2	/" >E2 &&
+	test_tick &&
+	git commit -m "side modifies" &&
+
+	git tag anchor &&
+
+	cat E0 E1 E2 E3 >expect
+'
+
+test_expect_success resolve '
+
+	rm -f a* m* &&
+	git reset --hard anchor &&
+
+	if git merge -s resolve master
+	then
+		echo Oops, should not have succeeded
+		false
+	else
+		git ls-files -s >current
+		diff -u current expect
+	fi
+'
+
+test_expect_success recursive '
+
+	rm -f a* m* &&
+	git reset --hard anchor &&
+
+	if git merge -s recursive master
+	then
+		echo Oops, should not have succeeded
+		false
+	else
+		git ls-files -s >current
+		diff -u current expect
+	fi
+'
+
+test_done

Re: merge-recursive: do not rudely die on binary merge

[Chris Shoemaker]

On Tue, Aug 14, 2007 at 03:33:07PM -0700, Junio C Hamano wrote:
> 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.

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

Re: merge-recursive: do not rudely die on binary merge

[Junio C Hamano]

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

> Very nice. Thanks, Junio.  As an additional convenience, it would be
> nice to make the "theirs" version easily accessible.

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.

Re: merge-recursive: do not rudely die on binary merge

[Nikodemus Siivola]

On 8/15/07, Junio C Hamano <gitster@pobox.com> 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.

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

Re: merge-recursive: do not rudely die on binary merge

[Junio C Hamano]

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

> On 8/15/07, Junio C Hamano <gitster@pobox.com> 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.
>
> 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?

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.

[PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Steven Grimm]

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(-)

diff --git a/Documentation/git-rev-parse.txt 
b/Documentation/git-rev-parse.txt
index 4b4d229..4758c33 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -215,7 +215,10 @@ blobs contained in a commit.
 * A colon, optionally followed by a stage number (0 to 3) and a
   colon, followed by a path; this names a blob object in the
   index at the given path.  Missing stage number (and the colon
-  that follows it) names an stage 0 entry.
+  that follows it) names an stage 0 entry. During a merge, stage
+  1 is the common ancestor, stage 2 is the target branch's version
+  (typically the current branch), and stage 3 is the version from
+  the branch being merged.
 
 Here is an illustration, by Jon Loeliger.  Both node B and C are
 a commit parents of commit node A.  Parent commits are ordered
-- 
1.5.3.rc2.4.g726f9

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jeff King]

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

> 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.

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(-)

diff --git a/sha1_name.c b/sha1_name.c
index 2d727d5..eaa6bd7 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -664,6 +664,7 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	/* sha1:path --> object name of path in ent sha1
 	 * :path -> object name of path in index
 	 * :[0-3]:path -> object name of path in index at stage
+	 * :base|ours|theirs:path -> same as :[1-3]:path
 	 */
 	if (name[0] == ':') {
 		int stage = 0;
@@ -671,14 +672,36 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 		int pos;
 		if (namelen > 2 && name[1] == '/')
 			return get_sha1_oneline(name + 2, sha1);
-		if (namelen < 3 ||
-		    name[2] != ':' ||
-		    name[1] < '0' || '3' < name[1])
-			cp = name + 1;
-		else {
-			stage = name[1] - '0';
+		if (!strncmp(name+1, "0:", 2)) {
+			stage = 0;
+			cp = name + 3;
+		}
+		else if (!strncmp(name+1, "1:", 2)) {
+			stage = 1;
+			cp = name + 3;
+		}
+		else if (!strncmp(name+1, "base:", 5)) {
+			stage = 1;
+			cp = name + 6;
+		}
+		else if (!strncmp(name+1, "2:", 2)) {
+			stage = 2;
 			cp = name + 3;
 		}
+		else if (!strncmp(name+1, "ours:", 5)) {
+			stage = 2;
+			cp = name + 6;
+		}
+		else if (!strncmp(name+1, "3:", 2)) {
+			stage = 3;
+			cp = name + 3;
+		}
+		else if (!strncmp(name+1, "theirs:", 7)) {
+			stage = 3;
+			cp = name + 8;
+		}
+		else
+			cp = name + 1;
 		namelen = namelen - (cp - name);
 		if (!active_cache)
 			read_cache();

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Shawn O. Pearce]

Jeff King <peff@peff.net> wrote:
> On Mon, Aug 20, 2007 at 11:36:38AM +0800, Steven Grimm wrote:
> 
> > 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.
> 
> 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.

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.
 
>  	/* sha1:path --> object name of path in ent sha1
>  	 * :path -> object name of path in index
>  	 * :[0-3]:path -> object name of path in index at stage
> +	 * :base|ours|theirs:path -> same as :[1-3]:path
>  	 */

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

-- 
Shawn.

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Shawn O. Pearce]

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Jeff King <peff@peff.net> wrote:
> > On Mon, Aug 20, 2007 at 11:36:38AM +0800, Steven Grimm wrote:
> > 
> > > 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.
> > 
> > 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.
> 
> 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".

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.

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Florian Weimer]

* Shawn O. Pearce:

>> 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".
>
> Actually, what's wrong with the following:
>
> 	git show HEAD:foo.c
> 	git show MERGE_HEAD:foo.c
>
> ?

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.

-- 
Florian Weimer                <fweimer@bfk.de>
BFK edv-consulting GmbH       http://www.bfk.de/
Kriegsstraße 100              tel: +49-721-96201-1
D-76133 Karlsruhe             fax: +49-721-96201-99

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jeff King]

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

> > Actually, what's wrong with the following:
> >
> > 	git show HEAD:foo.c
> > 	git show MERGE_HEAD:foo.c
> 
> 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.

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`"

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Junio C Hamano]

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

>> Am I the only one who messes this up? If not, patch is below.
>
> 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".

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?

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jeff King]

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

> 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.

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

> 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.

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

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jakub Narebski]

Junio C Hamano wrote:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
>>> Am I the only one who messes this up? If not, patch is below.
>>
>> 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".
> 
> 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?

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

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jeff King]

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

> 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.

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.

> 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

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.

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

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

-Peff

Re: [PATCH] Document what the stage numbers in the :$n:path syntaxmean.

[Johannes Sixt]

Jeff King wrote:
> +               else if (!strncmp(name+1, "1:", 2)) {
> [etc.]

prefixcmp()?

-- Hannes

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> 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.

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.

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

On Sun, Aug 19, 2007 at 23:20:53 -0700, Junio C Hamano wrote:
> Steven Grimm <koreth@midwinter.com> writes:
> 
> > 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.
> 
> 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.

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 'Bulb' Hudec <bulb@ucw.cz>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Document what the stage numbers in the :$n:path syntax mean.

[Junio C Hamano]

Jan Hudec <bulb@ucw.cz> writes:

>> 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.
>
> Pardon me? In what are they not equal?

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.

Re: merge-recursive: do not rudely die on binary merge

[Junio C Hamano]

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

>> 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.
>
> Very nice.

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.

Re: merge-recursive: do not rudely die on binary merge

[Chris Larson]

On 8/14/07, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Shoemaker <c.shoemaker@cox.net> writes:
>
> >> 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.
> >
> > Very nice.
>
> 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.

I got bit by this one earlier, and the patch fixed it here.
-- 
Chris Larson - clarson at kergoth dot com
Dedicated Engineer - MontaVista - clarson at mvista dot com
Core Developer/Architect - TSLib, BitBake, OpenEmbedded, OpenZaurus

Re: merge-recursive: do not rudely die on binary merge

[Chris Shoemaker]

On Tue, Aug 14, 2007 at 05:09:05PM -0700, Junio C Hamano wrote:
> Chris Shoemaker <c.shoemaker@cox.net> writes:
> 
> >> 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.
> >
> > Very nice.
> 
> 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.

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

-chris