threads / discuss / 2394

Re: Expected Behavior?

Subject: Re: Expected Behavior?

## tl;dr

28 messages between Nov 8, 2005 and Nov 10, 2005.

replies: 27people: 5as markdown or json

Jon Loeliger· Nov 8, 2005, 03:43 UTC · lore
Show 9 quoted lines
> Yup, it uses "git-ls-files --others", which considers that
> unmerged paths do not "exist" in the index.  This is wrong.
> 
> The attached is a patch to fix ls-files.
> 
> I also think that file3 should not appear in "Updated but not
> checked in (will commit)" list -- we are _not_ going to commit
> unmerged paths until you tell git what you want to do with
> them.  The patch in the next message fixes it.
Patch looks good here!  Thanks!
    % git status
    #
    # Updated but not checked in:
    #   (will commit)
    #
    #       modified: file1
    #       deleted:  file2
    #
    #
    # Changed but not updated:
    #   (use git-update-index to mark for commit)
    #
    #       unmerged: file3
    #
But I have a lingering question.  Same script as before.
    % git diff file3
    * Unmerged path file3
    % cat file3
    Stuff for a conflict.
Why didn't file3 show something like:
    % cat file3
    <<<<<
    Stuff for a conflict.
    =====
    Another file!
    >>>>>

That is, after the merge, file3 appears to have simply kept the contents of the current, master branch. Why wasn't the dev branch represented here?

I _almost_ think I get it, and then *poof*...

Thanks, jdl

Junio C Hamano· Nov 8, 2005, 06:00 UTC · re: Jon Loeliger · lore
Jon Loeliger <jdl@freescale.com> writes:
Show 5 quoted lines
> That is, after the merge, file3 appears to have simply kept
> the contents of the current, master branch.  Why wasn't the
> dev branch represented here?
>
> I _almost_ think I get it, and then *poof*...

Automerge completely punted for this path, and at this point, it is still unmerged:

------------ $ git ls-files --unmerged 100644 c4da0eb.... 2 file3 100644 fbc2aa4.... 3 file3 ------------

Three-way "git-read-tree -m -u O A B" (O is for old, A is ours and B is hers) puts O in stage1, A in stage2 and B in stage3. This path did not exist in O so we only have them in stage2 and stage3. You could compare the stages like this:

------------ $ git diff-stages -p 2 3 file3

diff --git a/file3 b/file3
index c4da0eb..fbc2aa4 100644
--- a/file3
+++ b/file3
@@ -1 +1 @@
-Stuff for a conflict.
+Another file!
------------

When we do a file-level merge (and possibly leave conflicts), we
register _our_ version in the index and leave the merge result
in the working tree.  However, when O is empty like this case,
we leave the conflicting path simply unmerged and do not touch
the working tree.  We say "ERROR: not handling case" when this
happens.

Nobody complained so far about this, probably because two side
adding different versions is rare enough.  And the reasoning
behind the current behaviour is probably because the tool cannot
automerge them sensibly anyway.  Leaving the SHA1 in index file
might probably be easier to clean up by hand (e.g. see ls-files
--unmerged, and cut&paste the desired SHA1 to 'git-cat-file
blob' command line, or something silly like that).  But if we
were to go that route, adding --stage=[123] flag so that the
user can say 'git-checkout-index --stage=3 file3'might have
helped a bit more.

We could instead use the attached patch to get the behaviour you
are expecting.  I have a feeling that the result from this might
be a little more intuitive and easier to resolve by hand than
the current one.  Although we may end up unresolvable mess in
file3 if either side is binary, in that case the user can still
sift through 'diff-tree A B file3' output to find out the
relevant SHA1 to recover the blobs from both sides by hand
anyway.  Does anybody have strong opinion on this?

--
[PATCH] merge with /dev/null as base, instead of not handling O=='' case

Instead of leaving the path unmerged in a case where each side
adds different version of the same path, attempt to merge it
with empty base and leave "our" version in the index file, just
like we do for the case in conflicting merge.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 5419e59..32e17cb 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -56,9 +56,18 @@ case "${1:-.}${2:-.}${3:-.}" in
 #
 # Modified in both, but differently.
 #
-"$1$2$3")
-	echo "Auto-merging $4."
-	orig=`git-unpack-file $1`
+"$1$2$3" | ".$2$3")
+	case "$1" in
+	'')
+		echo "Added $4 in both, but differently."
+		orig=`git-unpack-file $2`
+		: >$orig
+		;;
+	*)
+		echo "Auto-merging $4."
+		orig=`git-unpack-file $1`
+		;;
+	esac
 	src2=`git-unpack-file $3`
 
 	# We reset the index to the first branch, making
@@ -73,6 +82,9 @@ case "${1:-.}${2:-.}${3:-.}" in
 		echo "ERROR: Permissions conflict: $5->$6,$7."
 		ret=1
 	fi
+	if [ "$1" = '' ]; then
+		ret=1
+	fi
 
 	if [ $ret -ne 0 ]; then
 		echo "ERROR: Merge conflict in $4."
Petr Baudis· Nov 8, 2005, 09:56 UTC · re: Junio C Hamano · lore

Dear diary, on Tue, Nov 08, 2005 at 07:00:11AM CET, I got a letter where Junio C Hamano <junkio@cox.net> told me that...

Show 10 quoted lines
> Nobody complained so far about this, probably because two side
> adding different versions is rare enough.  And the reasoning
> behind the current behaviour is probably because the tool cannot
> automerge them sensibly anyway.  Leaving the SHA1 in index file
> might probably be easier to clean up by hand (e.g. see ls-files
> --unmerged, and cut&paste the desired SHA1 to 'git-cat-file
> blob' command line, or something silly like that).  But if we
> were to go that route, adding --stage=[123] flag so that the
> user can say 'git-checkout-index --stage=3 file3'might have
> helped a bit more.
#
# Added in both (different in each).
#
".$id1$id2")
        #echo "Adding $file"
        filev="$file"
        while [ -e "$filev~1" ] || [ -e "$filev~2" ]; do
                filev="$filev~"
        done
        error "File $file added in both branches, but different in each!"
        error "Conflicting versions saved as '$filev~1' and '$filev~2'."
        git-update-index --add --cacheinfo "$mode1" "$id1" "$file" &&
                git-checkout-index -u -f -- "$file" &&
                mv "$file" "$filev~1" ||
                error "Cannot create '$filev~1'"
        git-update-index --add --cacheinfo "$mode2" "$id2" "$file" &&
                git-checkout-index -u -f -- "$file" &&
                mv "$file" "$filev~2" ||
                error "Cannot create '$filev~2'"
        exit 1
        ;;

is what Cogito's automerger does. It ain't very pretty, but it works. :-) (At least it seems to.) I yet need to add some conflicts tracking to Cogito to prevent accidental checking in of conflicts.

Show 8 quoted lines
> We could instead use the attached patch to get the behaviour you
> are expecting.  I have a feeling that the result from this might
> be a little more intuitive and easier to resolve by hand than
> the current one.  Although we may end up unresolvable mess in
> file3 if either side is binary, in that case the user can still
> sift through 'diff-tree A B file3' output to find out the
> relevant SHA1 to recover the blobs from both sides by hand
> anyway.  Does anybody have strong opinion on this?
I think having
	<<<<<
	file1
	=====
	file2
	>>>>>

is an awful PITA to resolve, especially when the files actually are similar. Running some vimdiff (or just diff and possibly applying either way) on two separate files is much more convenient.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Nov 10, 2005, 04:41 UTC · re: Petr Baudis · lore

merge-one-file: use common as base, instead of emptiness.

Petr Baudis <pasky@suse.cz> writes:
Show 11 quoted lines
> I think having
>
> 	<<<<<
> 	file1
> 	=====
> 	file2
> 	>>>>>
>
> is an awful PITA to resolve, especially when the files actually are
> similar. Running some vimdiff (or just diff and possibly applying either
> way) on two separate files is much more convenient.
You are right.

How about something like this? This adds a specialized hackery flag, --no-add, to git-apply, and uses it to compute common base to be used for 2-file merge, instead of using /dev/null.

On top of the previous round.
 -- >8 -- cut here -- >8 --
Unlike the previous round that merged the path added differently
in each branches using emptiness as the base, compute a common
version and use it as input to 'merge' program.

This would show the resulting (still conflicting) file left in the working tree as:

	common file contents...
	<<<<<< FILENAME
	version from our branch...
	======
	version from their branch...
	>>>>>> .merge_file_XXXXXX
	more common file contents...
when both sides added similar contents.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 apply.c               |   11 +++++++++--
 git-merge-one-file.sh |    6 ++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

applies-to: 75c7cdf0addfcb4df5ed093f9b57bb98432489e1 f158ab58fd3846a784805849bd9228bf060e4b2d

diff --git a/apply.c b/apply.c
index 3e53b34..7584888 100644
--- a/apply.c
+++ b/apply.c
@@ -23,6 +23,7 @@ static int numstat = 0;
 static int summary = 0;
 static int check = 0;
 static int apply = 1;
+static int no_add = 0;
 static int show_index_info = 0;
 static int line_termination = '\n';
 static const char apply_usage[] =
@@ -1099,8 +1100,10 @@ static int apply_one_fragment(struct buf
 				break;
 		/* Fall-through for ' ' */
 		case '+':
-			memcpy(new + newsize, patch + 1, plen);
-			newsize += plen;
+			if (*patch != '+' || !no_add) {
+				memcpy(new + newsize, patch + 1, plen);
+				newsize += plen;
+			}
 			break;
 		case '@': case '\\':
 			/* Ignore it, we already handled it */
@@ -1697,6 +1700,10 @@ int main(int argc, char **argv)
 			excludes = x;
 			continue;
 		}
+		if (!strcmp(arg, "--no-add")) {
+			no_add = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--stat")) {
 			apply = 0;
 			diffstat = 1;
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 32e17cb..d9ee458 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -57,18 +57,20 @@ case "${1:-.}${2:-.}${3:-.}" in
 # Modified in both, but differently.
 #
 "$1$2$3" | ".$2$3")
+	src2=`git-unpack-file $3`
 	case "$1" in
 	'')
 		echo "Added $4 in both, but differently."
+		# This extracts OUR file in $orig, and uses git-apply to
+		# remove lines that are unique to ours.
 		orig=`git-unpack-file $2`
-		: >$orig
+		diff -u -La/$orig -Lb/$orig $orig $src2 | git-apply --no-add 
 		;;
 	*)
 		echo "Auto-merging $4."
 		orig=`git-unpack-file $1`
 		;;
 	esac
-	src2=`git-unpack-file $3`
 
 	# We reset the index to the first branch, making
 	# git-diff-file useful
---
0.99.9.GIT
Petr Baudis· Nov 10, 2005, 19:43 UTC · re: Junio C Hamano · lore

Re: merge-one-file: use common as base, instead of emptiness.

Dear diary, on Thu, Nov 10, 2005 at 05:41:10AM CET, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 19 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> 
> > I think having
> >
> > 	<<<<<
> > 	file1
> > 	=====
> > 	file2
> > 	>>>>>
> >
> > is an awful PITA to resolve, especially when the files actually are
> > similar. Running some vimdiff (or just diff and possibly applying either
> > way) on two separate files is much more convenient.
> 
> You are right.
> 
> How about something like this?  This adds a specialized hackery
> flag, --no-add, to git-apply, and uses it to compute common base
> to be used for 2-file merge, instead of using /dev/null.
Wow, astonishingly simple.
Show 17 quoted lines
>  -- >8 -- cut here -- >8 --
> Unlike the previous round that merged the path added differently
> in each branches using emptiness as the base, compute a common
> version and use it as input to 'merge' program.
> 
> This would show the resulting (still conflicting) file left in
> the working tree as:
> 
> 	common file contents...
> 	<<<<<< FILENAME
> 	version from our branch...
> 	======
> 	version from their branch...
> 	>>>>>> .merge_file_XXXXXX
> 	more common file contents...
> 
> when both sides added similar contents.

But obviously now the trouble is opposite, when the files are completely unrelated, since now you likely get large conflicting areas interleaved with some scarce common lines... And this might get to be a big PITA to resolve as well.

That said, I still really like --no-add and it would be heart-wrenching to just coldly dismiss it. It is a great tool, but I would let the user to use it manually. Possibly something like

	git-twofile-merge foo~1 foo~2
(the name is stupid, obviously) or a button in some GUI tool.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Nov 10, 2005, 20:30 UTC · re: Petr Baudis · lore

Re: merge-one-file: use common as base, instead of emptiness.

Petr Baudis <pasky@suse.cz> writes:
> But obviously now the trouble is opposite, when the files are completely
> unrelated, since now you likely get large conflicting areas interleaved
> with some scarce common lines... And this might get to be a big PITA to
> resolve as well.

Yeah, you can always count lines in $orig and $src2 after two-file merge runs, and if it appears we do not have enough common section just empty the $orig before running merge.

Petr Baudis· Nov 10, 2005, 20:35 UTC · re: Junio C Hamano · lore

Re: merge-one-file: use common as base, instead of emptiness.

Dear diary, on Thu, Nov 10, 2005 at 09:30:55PM CET, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 10 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> 
> > But obviously now the trouble is opposite, when the files are completely
> > unrelated, since now you likely get large conflicting areas interleaved
> > with some scarce common lines... And this might get to be a big PITA to
> > resolve as well.
> 
> Yeah, you can always count lines in $orig and $src2 after
> two-file merge runs, and if it appears we do not have enough
> common section just empty the $orig before running merge.

Reasonable solution, I like that. Let's say 50%, like for the renames detection?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Fredrik Kuivinen· Nov 8, 2005, 21:03 UTC · re: Junio C Hamano · lore
On Mon, Nov 07, 2005 at 10:00:11PM -0800, Junio C Hamano wrote:
Show 22 quoted lines
> Jon Loeliger <jdl@freescale.com> writes:
> 
> > That is, after the merge, file3 appears to have simply kept
> > the contents of the current, master branch.  Why wasn't the
> > dev branch represented here?
> >
> > I _almost_ think I get it, and then *poof*...
> 
> Automerge completely punted for this path, and at this point, it
> is still unmerged:
> 
> ------------
> $ git ls-files --unmerged
> 100644 c4da0eb.... 2       file3
> 100644 fbc2aa4.... 3       file3
> ------------
> 
> Three-way "git-read-tree -m -u O A B" (O is for old, A is ours
> and B is hers) puts O in stage1, A in stage2 and B in stage3.
> This path did not exist in O so we only have them in stage2 and
> stage3.  You could compare the stages like this:
> 
Jon: You could try to this merge with the recursive merge strategy
(git merge -s recursive 'merge message' master dev) If you do, you
_should_ get something like:
    CONFLICT (add/add): File file3 added non-identically in both
    branches. Adding as file3_master and file3_dev instead.

You will then end up with file3_master and file3_dev in your working tree, which corresponds to file3 in the master branch and file3 in the dev branch, respectively.

- Fredrik
Junio C Hamano· Nov 8, 2005, 21:41 UTC · re: Fredrik Kuivinen · lore
Fredrik Kuivinen <freku045@student.liu.se> writes:
Show 10 quoted lines
> Jon: You could try to this merge with the recursive merge strategy
> (git merge -s recursive 'merge message' master dev) If you do, you
> _should_ get something like:
>
>     CONFLICT (add/add): File file3 added non-identically in both
>     branches. Adding as file3_master and file3_dev instead.
>
> You will then end up with file3_master and file3_dev in your working
> tree, which corresponds to file3 in the master branch and file3 in the
> dev branch, respectively.

Oops, I missed that part. This is unsafe in theory, if you could overwrite existing file3_master or file3_dev. Does that matter in practice?

Fredrik Kuivinen· Nov 8, 2005, 22:53 UTC · re: Junio C Hamano · lore
On Tue, Nov 08, 2005 at 01:41:20PM -0800, Junio C Hamano wrote:
Show 17 quoted lines
> Fredrik Kuivinen <freku045@student.liu.se> writes:
> 
> > Jon: You could try to this merge with the recursive merge strategy
> > (git merge -s recursive 'merge message' master dev) If you do, you
> > _should_ get something like:
> >
> >     CONFLICT (add/add): File file3 added non-identically in both
> >     branches. Adding as file3_master and file3_dev instead.
> >
> > You will then end up with file3_master and file3_dev in your working
> > tree, which corresponds to file3 in the master branch and file3 in the
> > dev branch, respectively.
> 
> Oops, I missed that part.  This is unsafe in theory, if you
> could overwrite existing file3_master or file3_dev.  Does that
> matter in practice?
>

It wont overwrite any existing files. If there is a file named 'file3_master' then the new file will be named 'file3_master_1' and if that file also exists the new file will be named 'file3_master_2', and so on.

- Fredrik
Junio C Hamano· Nov 9, 2005, 05:50 UTC · re: Fredrik Kuivinen · lore
Fredrik Kuivinen <freku045@student.liu.se> writes:
Show 8 quoted lines
>> Oops, I missed that part.  This is unsafe in theory, if you
>> could overwrite existing file3_master or file3_dev.  Does that
>> matter in practice?
>
> It wont overwrite any existing files. If there is a file named
> 'file3_master' then the new file will be named 'file3_master_1' and if
> that file also exists the new file will be named 'file3_master_2', and
> so on.

Another thing to watch out is that a branch name could have a slash in it. It might make more sense to just name the heads file3~2 or file3~3 (with as many ~s repeated to avoid name clashes) like Pasky does.

Fredrik Kuivinen· Nov 9, 2005, 08:19 UTC · re: Junio C Hamano · lore
On Tue, Nov 08, 2005 at 09:50:54PM -0800, Junio C Hamano wrote:
Show 16 quoted lines
> Fredrik Kuivinen <freku045@student.liu.se> writes:
> 
> >> Oops, I missed that part.  This is unsafe in theory, if you
> >> could overwrite existing file3_master or file3_dev.  Does that
> >> matter in practice?
> >
> > It wont overwrite any existing files. If there is a file named
> > 'file3_master' then the new file will be named 'file3_master_1' and if
> > that file also exists the new file will be named 'file3_master_2', and
> > so on.
> 
> Another thing to watch out is that a branch name could have a
> slash in it.  It might make more sense to just name the heads file3~2
> or file3~3 (with as many ~s repeated to avoid name clashes) like
> Pasky does.
>

Oups, I haven't thought about that. I kind of like the idea that you can see the branch name in the file names though. How about replacing any slashes in the branch names with underscores? So the branch 'foo/bar' will give rise to files with suffixes like '_foo_bar' and '_foo_bar_<number>'.

- Fredrik
Fredrik Kuivinen· Nov 9, 2005, 10:42 UTC · re: Fredrik Kuivinen · lore

[PATCH] merge-recursive: Fix support for branch names containing slashes

On Wed, Nov 09, 2005 at 09:19:06AM +0100, Fredrik Kuivinen wrote:
Show 24 quoted lines
> On Tue, Nov 08, 2005 at 09:50:54PM -0800, Junio C Hamano wrote:
> > Fredrik Kuivinen <freku045@student.liu.se> writes:
> > 
> > >> Oops, I missed that part.  This is unsafe in theory, if you
> > >> could overwrite existing file3_master or file3_dev.  Does that
> > >> matter in practice?
> > >
> > > It wont overwrite any existing files. If there is a file named
> > > 'file3_master' then the new file will be named 'file3_master_1' and if
> > > that file also exists the new file will be named 'file3_master_2', and
> > > so on.
> > 
> > Another thing to watch out is that a branch name could have a
> > slash in it.  It might make more sense to just name the heads file3~2
> > or file3~3 (with as many ~s repeated to avoid name clashes) like
> > Pasky does.
> > 
> 
> Oups, I haven't thought about that. I kind of like the idea that you
> can see the branch name in the file names though. How about replacing
> any slashes in the branch names with underscores? So the branch
> 'foo/bar' will give rise to files with suffixes like '_foo_bar' and
> '_foo_bar_<number>'.
>

And here is a patch to implement this. Should apply cleanly on top of '[PATCH] merge-recursive: Fix limited output of rename messages'.

---
Signed-off-by: Fredrik Kuivinen <freku045@student.liu.se>
---
 git-merge-recursive.py |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

applies-to: 1dd8c1800d7dd0425bc93f4c9441acfb207b1488 795e2900e8804af53263821c177a270cd2d7c2c0

diff --git a/git-merge-recursive.py b/git-merge-recursive.py
index 3657875..90e889c 100755
--- a/git-merge-recursive.py
+++ b/git-merge-recursive.py
@@ -295,6 +295,7 @@ def uniquePath(path, branch):
             else:
                 raise
 
+    branch = branch.replace('/', '_')
     newPath = path + '_' + branch
     suffix = 0
     while newPath in currentFileSet or \
---
0.99.9.GIT
Petr Baudis· Nov 10, 2005, 20:34 UTC · re: Fredrik Kuivinen · lore

Dear diary, on Wed, Nov 09, 2005 at 09:19:06AM CET, I got a letter where Fredrik Kuivinen <freku045@student.liu.se> said that...

Show 23 quoted lines
> On Tue, Nov 08, 2005 at 09:50:54PM -0800, Junio C Hamano wrote:
> > Fredrik Kuivinen <freku045@student.liu.se> writes:
> > 
> > >> Oops, I missed that part.  This is unsafe in theory, if you
> > >> could overwrite existing file3_master or file3_dev.  Does that
> > >> matter in practice?
> > >
> > > It wont overwrite any existing files. If there is a file named
> > > 'file3_master' then the new file will be named 'file3_master_1' and if
> > > that file also exists the new file will be named 'file3_master_2', and
> > > so on.
> > 
> > Another thing to watch out is that a branch name could have a
> > slash in it.  It might make more sense to just name the heads file3~2
> > or file3~3 (with as many ~s repeated to avoid name clashes) like
> > Pasky does.
> > 
> 
> Oups, I haven't thought about that. I kind of like the idea that you
> can see the branch name in the file names though. How about replacing
> any slashes in the branch names with underscores? So the branch
> 'foo/bar' will give rise to files with suffixes like '_foo_bar' and
> '_foo_bar_<number>'.
I like it too. :-)

Now, this would look like file3~master in Cogito (or file3~~master in case of name conflict, etc.).

Thanks for the idea,
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Nov 10, 2005, 22:52 UTC · re: Petr Baudis · lore
Petr Baudis <pasky@suse.cz> writes:
Show 15 quoted lines
> Dear diary, on Wed, Nov 09, 2005 at 09:19:06AM CET, I got a letter
> where Fredrik Kuivinen <freku045@student.liu.se> said that...
>> On Tue, Nov 08, 2005 at 09:50:54PM -0800, Junio C Hamano wrote:
>> > Fredrik Kuivinen <freku045@student.liu.se> writes:
>> > 
>> Oups, I haven't thought about that. I kind of like the idea that you
>> can see the branch name in the file names though. How about replacing
>> any slashes in the branch names with underscores? So the branch
>> 'foo/bar' will give rise to files with suffixes like '_foo_bar' and
>> '_foo_bar_<number>'.
>
> I like it too. :-)
>
> Now, this would look like file3~master in Cogito (or file3~~master in
> case of name conflict, etc.).

One thing I like about Cogito's is the use of tilde instead of underscore -- much much less likely to clash with real filenames and easiliy recognizable as throwaway copies.

One thing I _don't_ like about both is there is no way for 'git reset --hard' to get rid of them, when the user decides to retry from scratch after seeing a failed merge that left too many of them.

Petr Baudis· Nov 10, 2005, 23:22 UTC · re: Junio C Hamano · lore

Dear diary, on Thu, Nov 10, 2005 at 11:52:46PM CET, I got a letter where Junio C Hamano <junkio@cox.net> said that...

> One thing I _don't_ like about both is there is no way for 'git
> reset --hard' to get rid of them, when the user decides to retry
> from scratch after seeing a failed merge that left too many of
> them.

Yes. You can cg-clean, but that'll wipe out other stuff as well. I plan to fix that by adding some conflicts tracking to Cogito - I'm not yet decided on the particular implementation (only that I certainly don't want to require the user to manually tell Cogito about resolved conflicts).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Petr Baudis· Nov 9, 2005, 11:24 UTC · re: Fredrik Kuivinen · lore

Dear diary, on Tue, Nov 08, 2005 at 10:03:32PM CET, I got a letter where Fredrik Kuivinen <freku045@student.liu.se> said that...

Show 34 quoted lines
> On Mon, Nov 07, 2005 at 10:00:11PM -0800, Junio C Hamano wrote:
> > Jon Loeliger <jdl@freescale.com> writes:
> > 
> > > That is, after the merge, file3 appears to have simply kept
> > > the contents of the current, master branch.  Why wasn't the
> > > dev branch represented here?
> > >
> > > I _almost_ think I get it, and then *poof*...
> > 
> > Automerge completely punted for this path, and at this point, it
> > is still unmerged:
> > 
> > ------------
> > $ git ls-files --unmerged
> > 100644 c4da0eb.... 2       file3
> > 100644 fbc2aa4.... 3       file3
> > ------------
> > 
> > Three-way "git-read-tree -m -u O A B" (O is for old, A is ours
> > and B is hers) puts O in stage1, A in stage2 and B in stage3.
> > This path did not exist in O so we only have them in stage2 and
> > stage3.  You could compare the stages like this:
> > 
> 
> Jon: You could try to this merge with the recursive merge strategy
> (git merge -s recursive 'merge message' master dev) If you do, you
> _should_ get something like:
> 
>     CONFLICT (add/add): File file3 added non-identically in both
>     branches. Adding as file3_master and file3_dev instead.
> 
> You will then end up with file3_master and file3_dev in your working
> tree, which corresponds to file3 in the master branch and file3 in the
> dev branch, respectively.

The world would be so much better if there would be just a _single_ per-file automerger instead of three right now...

I'm planning to feed back the automerger stuff from Cogito to GIT like I already did once, but it's not a priority right now so it may take some time (unless someone else does it, which is something I certainly wouldn't oppose).

But in the longer term even the recursive merge should use the same one, so either we should enhance the shell one to handle all the cases, or split out the python per-file automerger from recursive merge and make it possible to use this one separately. This is probably more viable option in the longer term, even though I personally don't grok python very well - whatever, at least I'll have to get better. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Martin Langhoff· Nov 9, 2005, 23:04 UTC · re: Petr Baudis · lore
On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> The world would be so much better if there would be just a _single_
> per-file automerger instead of three right now...

I don't quite agree with this. The multi-merger mechanism is something (relatively) unique and powerful in GIT. Having fast+stupid, with fallback to slow+smart is an excellent strategy, and having the mechanism in place means that if someone is crazy enough to write a smarter merge script for a language or a particular project (say, to ease the transition to a new directory layout) it is entirely possible.

In the end, merges are tricky and need to be controlled by the end user. We can help this giving the user good control and good visibility of what decisions the merger is taking.

> I'm planning to feed back the automerger stuff from Cogito to GIT like I
> already did once.

I use both mergers (cg and git) and I'd really _really_ like to turn cg-merge into a wrapper for git-merge. The cg merger does the basics: trivial merge, fast forwards and diff3 merges. Adding the rest is _hard_ and debugging/auditing it is even harder. IMHO, having strong, high quality mergers is a must, and it's a complex enough space that a porcelain should align itself with git so we get a strong set of mergers done in a way that all porcelains can hook into.

I made an initial half-attempt but it's trickier that I had expected because cg-merge relies on cg-commit to wrap up, marke the index as resolved, and commit, and I'm not entirely sure of how that works in detail. My changes were going in the direction of breaking the current usage patterns, so it was a no-go really.

cheers,
martin
Petr Baudis· Nov 9, 2005, 23:12 UTC · re: Martin Langhoff · lore

Dear diary, on Thu, Nov 10, 2005 at 12:04:11AM CET, I got a letter where Martin Langhoff <martin.langhoff@gmail.com> said that...

Show 6 quoted lines
> On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> > The world would be so much better if there would be just a _single_
> > per-file automerger instead of three right now...
> 
> I don't quite agree with this. The multi-merger mechanism is something
> (relatively) unique and powerful in GIT.

I'm not talking about mechanism but about the recommended policy (if you have something better, fine, use it - just keep _one_ default one instead of three, since this doesn't have anything or much to do with the actual merge strategy). Besides,

Show 5 quoted lines
> Having fast+stupid, with fallback to slow+smart is an excellent
> strategy, and having the mechanism in place means that if someone is
> crazy enough to write a smarter merge script for a language or a
> particular project (say, to ease the transition to a new directory
> layout) it is entirely possible.

what mechanism are you talking about? All the scripts have it hardcoded what to use, it seems - cg-Xmergefile, git-merge-one-file or some subroutine...

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Martin Langhoff· Nov 9, 2005, 23:43 UTC · re: Petr Baudis · lore
On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> what mechanism are you talking about?

I suspect you haven't seen the git-merge mechanism in action -- or perhaps I'm a bit lost and we're talking about different things. At least last time I merged with git-merge.sh, it tried several strategies in order in a temporary index. Actually, it tried in order: stupid, resolve, recursive... It shortcuts if one merges cleanly, but it'll definitely work through them. It's very nice to see this in action.

> All the scripts have it hardcoded

Well, no. git-merge -s 'foo' works pretty well and isn't hardcoded. Even if we agree on defaulting to only one relatively smart merger for simplicity's sake, the mechanism we have in git-merge.sh (thanks to Junio I think) is excellent, and should be kept. Linus will probably do his merges with "stupid resolve" or just "stupid" and the rest of us will use something else.

The mergers and things like git-merge-base are probably the areas of git that need the most focus and polish -- that's why I'm uncomfortable with cg-merge doing its own thing, as it gets a lot less review than git-merge-XX resolvers.

Right now I am seeing some slightly abnormal things (*) in how some our our merges are going, so I' ll have to roll up my sleeves at some point and try and figure out what's going on. And as I look into cg-merge and cg-Xmergefile, not many eyes have been through it...

* - (nothing serious, just an annoying tendency to show spurious
conflicts with the mergebase commit itself)
cheers,
martin
Petr Baudis· Nov 9, 2005, 23:49 UTC · re: Martin Langhoff · lore

Dear diary, on Thu, Nov 10, 2005 at 12:43:04AM CET, I got a letter where Martin Langhoff <martin.langhoff@gmail.com> said that...

Show 10 quoted lines
> On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> > what mechanism are you talking about?
> 
> I suspect you haven't seen the git-merge mechanism in action -- or
> perhaps I'm a bit lost and we're talking about different things. At
> least last time I merged with git-merge.sh, it tried several
> strategies in order in a temporary index. Actually, it tried in order:
> stupid, resolve, recursive... It shortcuts if one merges cleanly, but
> it'll definitely work through them. It's very nice to see this in
> action.
But that's something different that I'm talking about. :-)

As I said, I'm talking about per-file automergers. git-merge-one-file and that stuff. I know about merge strategies and they are cool.

> The mergers and things like git-merge-base are probably the areas of
> git that need the most focus and polish -- that's why I'm
> uncomfortable with cg-merge doing its own thing, as it gets a lot less
> review than git-merge-XX resolvers.

Yes. My longer plan is to use the strategy resolvers as well, but my TODO list is big... And I don't perceive this as a critical thing (the standard strategy seems to work well enough), although I would like to see this before 1.0.

> Right now I am seeing some slightly abnormal things (*) in how some
> our our merges are going, so I' ll have to roll up my sleeves at some
> point and try and figure out what's going on. And as I look into
> cg-merge and cg-Xmergefile, not many eyes have been through it...

At least this is the one area of Cogito that has at least a bit reasonable automated testing suite. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Martin Langhoff· Nov 10, 2005, 02:47 UTC · re: Petr Baudis · lore
On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> But that's something different that I'm talking about. :-)
Ack! Sorry about that!
> Yes. My longer plan is to use the strategy resolvers as well, but my
> TODO list is big... And I don't perceive this as a critical thing (the
> standard strategy seems to work well enough), although I would like to
> see this before 1.0.

Fair enough. If you can outline how the interaction between cg-merge and cg-commit are expected to work, I'll try and find some time for that.

Show 7 quoted lines
> > Right now I am seeing some slightly abnormal things (*) in how some
> > our our merges are going, so I' ll have to roll up my sleeves at some
> > point and try and figure out what's going on. And as I look into
> > cg-merge and cg-Xmergefile, not many eyes have been through it...
>
> At least this is the one area of Cogito that has at least a bit
> reasonable automated testing suite. ;-)
The good news is that git-merge is doing almost the same. Posted separately...
martin
Petr Baudis· Nov 10, 2005, 19:34 UTC · re: Martin Langhoff · lore

Dear diary, on Thu, Nov 10, 2005 at 03:47:38AM CET, I got a letter where Martin Langhoff <martin.langhoff@gmail.com> said that...

Show 9 quoted lines
> On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
> > Yes. My longer plan is to use the strategy resolvers as well, but my
> > TODO list is big... And I don't perceive this as a critical thing (the
> > standard strategy seems to work well enough), although I would like to
> > see this before 1.0.
> 
> Fair enough. If you can outline how the interaction between cg-merge
> and cg-commit are expected to work, I'll try and find some time for
> that.
Well, basically like right now ;-).

Merging is a two-stage process, where the two stages are isolated and the latter does not interfere with the former. The former one is doing the actual content merge, and that's what cg-merge does. The latter one is recording the merge in history, and that's what cg-commit does.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Martin Langhoff· Nov 10, 2005, 19:54 UTC · re: Petr Baudis · lore
On 11/11/05, Petr Baudis <pasky@suse.cz> wrote:
> Well, basically like right now ;-).

Exactly. Which isn't documented, and has several subtleties that I don't know of.

> Merging is a two-stage process, where the two stages are isolated and
> the latter does not interfere with the former. The former one is doing
> the actual content merge, and that's what cg-merge does. The latter one
> is recording the merge in history, and that's what cg-commit does.

There are some semantics to exchange info between cg-merge and cg-commit and for handling the index while the user is resolving conflicts or reviewing the merge before committing. I'm not familiar with them, and I'm unsure what the design is...

 - how do cg-merge and cg-commit pass around the parents, commit msg,
files being merged vs dirty files on tree, etc? (easy: those lowercase
temp files in .git)
 - how do you run cg-status/cg-diff without messing up the index? does
running cg-status or cg-diff stand any risk of accidentally marking
for inclusion dirty files that are not part of the merge? hopefully
not, and then what's the technique?
 - should cg-status show more about the status of the index ?
 - how does cg-commit know whether conflicts have been resolved at
all? (perhaps it doesn't even try!)
 - how do you reset the 'we are merging' status?

Anyway, that's off the top of my head, and probably shows you that I'm not 200% familiar with all the tricks available for handling the index.

cheers,
martin
Petr Baudis· Nov 10, 2005, 20:10 UTC · re: Martin Langhoff · lore

Dear diary, on Thu, Nov 10, 2005 at 08:54:43PM CET, I got a letter where Martin Langhoff <martin.langhoff@gmail.com> said that...

Show 5 quoted lines
> On 11/11/05, Petr Baudis <pasky@suse.cz> wrote:
> > Well, basically like right now ;-).
> 
> Exactly. Which isn't documented, and has several subtleties that I
> don't know of.

Yes - from the user standpoint it is supposed to be a foolproof (for a reasonable value of 'fool') and should always guide you around. From a developer's standpoint it might not be entirely crystal-clear, though. ;-)

> There are some semantics to exchange info between cg-merge and
> cg-commit and for handling the index while the user is resolving
> conflicts or reviewing the merge before committing. I'm not familiar
> with them, and I'm unsure what the design is...

The design is "keep arbitrary bunch of files in .git recording our state wrt. merging". I want to tidy that up and keep all that in .git/cg-merge-state/ or something.

>  - how do cg-merge and cg-commit pass around the parents, commit msg,
> files being merged vs dirty files on tree, etc? (easy: those lowercase
> temp files in .git)

Yes, lowercase temp files. Well, the commit msg itself is composed only in cg-commit.

>  - how do you run cg-status/cg-diff without messing up the index? does
> running cg-status or cg-diff stand any risk of accidentally marking
> for inclusion dirty files that are not part of the merge? hopefully
> not, and then what's the technique?

cg-status and cg-diff are safe anytime, being basically read-only commands wrt. the interesting index information.

Well, they _do_ touch your index (but only modifying the stat information), and that may get you into some trouble, but only in case of some seemingly quite rare permission problem, I think (and that will trash your index anytime, not just during a merge). There was a thread about that about a month or two ago, IIRC.

>  - should cg-status show more about the status of the index ?

In what sense? It shows if merge is in progress. It doesn't show conflicts, but that's because...

>  - how does cg-commit know whether conflicts have been resolved at
> all? (perhaps it doesn't even try!)

...we don't actually track them yet. That's considered a bug, I want to fix it, and it even has some non-zero priority.

>  - how do you reset the 'we are merging' status?

cg-reset (except when you were merging over a dirty state; hmm, we don't have a direct command for cancelling that, it seems).

cg-merge will now hint you about cg-reset.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Nov 9, 2005, 23:36 UTC · re: Martin Langhoff · lore
Martin Langhoff <martin.langhoff@gmail.com> writes:
Show 6 quoted lines
> On 11/10/05, Petr Baudis <pasky@suse.cz> wrote:
>> The world would be so much better if there would be just a _single_
>> per-file automerger instead of three right now...
>
> I don't quite agree with this. The multi-merger mechanism is something
> (relatively) unique and powerful in GIT.

If I am reading you two correctly, you are not disagreeing, but are talking about different issues.

Pasky is talking about what to do after merge strategy finds that a path (from three stages) needs to be merged. git-core uses git-merge-one-file or git-merge-recursive::mergeFile (which are the implementations of the same logic), and Pasky has his own in Cogito.

Fredrik's using his own mergeFile is defensible from both performance and implementation point of view. After the rename processing stage, he already extracted enough information from trees involved, and there is no reason for him to be preparing three stages in the index file with that information (which is cumbersome because there is no interface to populate the higher stage directly using git-update-index --- you have to first create temporary tree objects and ask git-read-tree -m to do so) and asking git-merge-index to call git-merge-one-file. If you read git-merge-one-file, it is not a rocket science. While we _may_ want to keep the logic that decides what to do in various cases across these three implementations, at least for me, it does not particularly feel disturbing, especially when there are good reasons.

Petr Baudis· Nov 9, 2005, 23:42 UTC · re: Junio C Hamano · lore

Dear diary, on Thu, Nov 10, 2005 at 12:36:44AM CET, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 10 quoted lines
> Fredrik's using his own mergeFile is defensible from both
> performance and implementation point of view.  After the rename
> processing stage, he already extracted enough information from
> trees involved, and there is no reason for him to be preparing
> three stages in the index file with that information (which is
> cumbersome because there is no interface to populate the higher
> stage directly using git-update-index --- you have to first
> create temporary tree objects and ask git-read-tree -m to do so)
> and asking git-merge-index to call git-merge-one-file.  If you
> read git-merge-one-file, it is not a rocket science.

I'm not arguing against mergeFile but rather git-merge-one-file (and by implication cg-Xmergefile), actually. My point is - couldn't be mergeFile generalized a bit or padded with suitable interface to be usable as git-merge-one-file's drop-in replacement?

> While we _may_ want to keep the logic that decides what to do in
> various cases across these three implementations, at least for me, it
> does not particularly feel disturbing, especially when there are good
> reasons.

I'm not sure I follow you here - what exactly does not feel disturbing and what good reasons are there for whatever they are? :-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.
Junio C Hamano· Nov 10, 2005, 00:03 UTC · re: Petr Baudis · lore
Petr Baudis <pasky@suse.cz> writes:
> I'm not sure I follow you here - what exactly does not feel disturbing
> and what good reasons are there for whatever they are? :-)

My misunderstanding, I guess. I somehow thought you were suggesting that recursive should implement mergeFile through merge-index. Going the other way, to make the current merge-index users to feed mergeFile with what is stored as three stages in an index file, would make a lot more sense.

← back to recent threads