Expected Behavior?

Expected Behavior?

[Jon Loeliger]

I was working through some examples and found some
rather curious behavior.  I'm wondering if it is
expected or not.  This isn't quite minimal, but
it is still small and shows the weirdness:

    git-init-db
    echo "Stuff for file1" > file1
    echo "Stuff for file2" > file2

    git add file1 file2
    git commit -m "Initial file1 and file2"

    git checkout -b dev

    echo "More for file1" >> file1
    rm -f file2
    echo "Another file!" > file3

    git update-index file1
    git update-index --force-remove file2
    git add file3

    git commit -m "Updated some stuff."

    git checkout master
    echo "Stuff for a conflict." >> file3
    git add file3
    git commit -m "Master update of file3"

    git merge "Grab dev stuff" master dev

Then, the part that I think is odd is demonstrated by "git status":

    $ git status
    #
    # Updated but not checked in:
    #   (will commit)
    #
    #       modified: file1
    #       deleted:  file2
    #       unmerged: file3
    #
    #
    # Changed but not updated:
    #   (use git-update-index to mark for commit)
    #
    #       unmerged: file3
    #
    #
    # Untracked files:
    #   (use "git add" to add to commit)
    #
    #       file3

Why is file3 considered untracked and needing to be added?
It was present in both "dev" and "master" branches before
the merge.  It doesn't end up with "<<< one === other >>>"
style diffs either.

My guess is that the file is small, one line, in each branch.
When the diff happens, it sees the file as empty in the other
branch and considers that "new" directly, rather than asking
the index if it knows about it to determine "newness" status.

Or perhaps it is that the file  became new in each branch
independently and never really had a true common ancestor.

Thanks,
jdl

Re: Expected Behavior?

[Junio C Hamano]

Jon Loeliger <jdl@freescale.com> writes:

>     # Untracked files:
>     #   (use "git add" to add to commit)
>     #
>     #       file3
>
> Why is file3 considered untracked and needing to be added?
> It was present in both "dev" and "master" branches before
> the merge.  It doesn't end up with "<<< one === other >>>"
> style diffs either.

It is because Untracked uses different program than git-diff-*
and, it mishandles unmerged paths.  Let me see...

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.

-- >8 -- cut here -- >8 --
Subject: ls-files: --others should not say unmerged paths are unknown.

Jon Loeliger noticed that an unmerged path appears as
"Untracked" in git-status output, even though we show the same
path as updated/changed.  Since --others means "we have not told
git about that path", we should not show unmerged paths --
obviously, git knows about them; it just does not know what we
want to do about them yet.

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

---

 ls-files.c |   26 ++++++++++++++++++++++++--
 1 files changed, 24 insertions(+), 2 deletions(-)

applies-to: f571975209b0bc38ca099684f9731efe4163b396
fcbc3083e37f3c025d85d7b2c8a1c53d07a81fac
diff --git a/ls-files.c b/ls-files.c
index d9c8b21..f7653e7 100644
--- a/ls-files.c
+++ b/ls-files.c
@@ -348,6 +348,29 @@ static void show_dir_entry(const char *t
 	putchar(line_terminator);
 }
 
+static void show_other_files(void)
+{
+	int i;
+	for (i = 0; i < nr_dir; i++) {
+		/* We should not have a matching entry, but we
+		 * may have an unmerged entry for this path.
+		 */
+		struct nond_on_fs *ent = dir[i];
+		int pos = cache_name_pos(ent->name, ent->len);
+		struct cache_entry *ce;
+		if (0 <= pos)
+			die("bug in show-other-files");
+		pos = -pos - 1;
+		if (pos < active_nr) { 
+			ce = active_cache[pos];
+			if (ce_namelen(ce) == ent->len &&
+			    !memcmp(ce->name, ent->name, ent->len))
+				continue; /* Yup, this one exists unmerged */
+		}
+		show_dir_entry(tag_other, ent);
+	}
+}
+
 static void show_killed_files(void)
 {
 	int i;
@@ -438,8 +461,7 @@ static void show_files(void)
 		read_directory(path, base, baselen);
 		qsort(dir, nr_dir, sizeof(struct nond_on_fs *), cmp_name);
 		if (show_others)
-			for (i = 0; i < nr_dir; i++)
-				show_dir_entry(tag_other, dir[i]);
+			show_other_files();
 		if (show_killed)
 			show_killed_files();
 	}
---
0.99.9.GIT

Re: Expected Behavior?

[Junio C Hamano]

Jon Loeliger <jdl@freescale.com> writes:

>     git checkout -b dev
>
>     echo "More for file1" >> file1
>     rm -f file2
>     echo "Another file!" > file3
>
>     git update-index file1
>     git update-index --force-remove file2
>     git add file3

You do not have file2 in the working tree, so regular --remove
would do.

	git-update-index --add --remove file1 file2 file3

>     git merge "Grab dev stuff" master dev

This is good.  We used to use (and the tutorial only talks
about) git-resolve to do this step, like this:

	git-resolve master dev 'Merge dev branch'

And I've kept using git-resolve myself; not that I do not trust
git-merge but purely from inertia, although I was the one who
did 'git merge' ;-).  Maybe I should first update the tutorial
to use git-merge instead of git-resolve.

It appears that as the everyday workhorse, using Daniel's
git-merge-resolve is stable through git-merge have proven stable
enough.  So here is a question.  Do people mind if 'git-resolve'
and 'git-octopus' are dropped before 1.0?  This means 2 less
programs in your /usr/bin ;-).

Re: Expected Behavior?

[Jon Loeliger]

> It appears that as the everyday workhorse, using Daniel's
> git-merge-resolve is stable through git-merge have proven stable
> enough.  So here is a question.  Do people mind if 'git-resolve'
> and 'git-octopus' are dropped before 1.0?  This means 2 less
> programs in your /usr/bin ;-).

It's OK with me. :-)

jdl

Re: Expected Behavior?

[Jon Loeliger]

> 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

Re: Expected Behavior?

[Junio C Hamano]

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:

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

Re: Expected Behavior?

[Petr Baudis]

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

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

Re: Expected Behavior?

[Fredrik Kuivinen]

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.


- Fredrik

Re: Expected Behavior?

[Junio C Hamano]

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?

Re: Expected Behavior?

[Fredrik Kuivinen]

On Tue, Nov 08, 2005 at 01:41:20PM -0800, Junio C Hamano wrote:
> 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

Re: Expected Behavior?

[Jon Loeliger]

<Young Frankenstein> Froe-derick wrote: </Young Frankenstein>

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

Hmmm. That didn't go too well. Concisely:

    % git merge -s recursive "Grab dev stuff" master dev
    Trying really trivial in-index merge...
    fatal: Merge requires file-level merging
    Nope.
    Traceback (most recent call last):
      File "/usr/bin/git-merge-recursive", line 8, in ?
	from gitMergeCommon import *
    ImportError: No module named gitMergeCommon
    Automatic merge failed/prevented; fix up by hand

Full details below.

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

That'd be cool, because the first thing I tend to want to do
after a failed file merge is look at clear versions of both files.
Gives me a global sense of where the file needs to go...  You know.

jdl





[ tail end of the make install output ]

install -d -m755 '/usr/share/git-core/templates/'
(cd blt && tar cf - .) | \
(cd '/usr/share/git-core/templates/' && tar xf -)
make[1]: Leaving directory `/usr/src/git-core/templates'
install -d -m755 '/usr/share/git-core/python'
install gitMergeCommon.py '/usr/share/git-core/python'



% git merge -s recursive "Grab dev stuff" master dev
Trying really trivial in-index merge...
fatal: Merge requires file-level merging
Nope.
Traceback (most recent call last):
  File "/usr/bin/git-merge-recursive", line 8, in ?
    from gitMergeCommon import *
ImportError: No module named gitMergeCommon
Automatic merge failed/prevented; fix up by hand

% git -v
git version 0.99.9.GIT
% cat /usr/src/git-core/.git/HEAD
f8d294f0a44c4305a9f3a1c70beb6a1c7583f287


% ll /usr/share/git-core/python/
total 16
4 drwxr-xr-x  2 root root 4096 Nov  8 20:17 .
4 drwxr-xr-x  4 root root 4096 Sep 14 19:47 ..
8 -rwxr-xr-x  1 root root 6879 Nov  8 20:17 gitMergeCommon.py

% python
Python 2.3.5 (#2, May  4 2005, 08:51:39)
[GCC 3.3.5 (Debian 1:3.3.5-12)] on linux2

Re: Expected Behavior?

[Junio C Hamano]

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.

Re: Expected Behavior?

[Junio C Hamano]

Jon Loeliger <jdl@freescale.com> writes:

> Hmmm. That didn't go too well. Concisely:
>
>     % git merge -s recursive "Grab dev stuff" master dev
>     Trying really trivial in-index merge...
>     fatal: Merge requires file-level merging
>     Nope.
>     Traceback (most recent call last):
>       File "/usr/bin/git-merge-recursive", line 8, in ?
> 	from gitMergeCommon import *
>     ImportError: No module named gitMergeCommon
>     Automatic merge failed/prevented; fix up by hand

Could you check diff between /usr/bin/git-merge-recursive and
git-merge-recursive.py in your source tree and see if the
appending to sys.path points at the right place (i.e. where the
make installs gitMergeCommon.py)?  Mine looks like this:

  --- git-merge-recursive.py      2005-11-08 02:27:08.000000000 +0000
  +++ /usr/bin/git-merge-recursive        2005-11-09 06:17:58.000000000 +0000
  @@ -1,10 +1,10 @@
  -#!/usr/bin/python
  +#!/usr/bin/python2.4

   import sys, math, random, os, re, signal, tempfile, stat, errno, traceback
   from heapq import heappush, heappop
   from sets import Set

  -sys.path.append('''@@GIT_PYTHON_PATH@@''')
  +sys.path.append('''/usr/share/git-core/python''')
   from gitMergeCommon import *

   originalIndexFile = os.environ.get('GIT_INDEX_FILE',

By default we set prefix to $HOME so ~/bin/git-merge-recursive
would import ~/share/git-core/python/gitMergeCommon.py.  Binary
packaging typically set prefix to /usr, so the executable is in
/usr/bin, and should import from /usr/share/git-core/python/,
which is as your make log shows:

> [ tail end of the make install output ]
>
> install -d -m755 '/usr/share/git-core/templates/'
> (cd blt && tar cf - .) | \
> (cd '/usr/share/git-core/templates/' && tar xf -)
> make[1]: Leaving directory `/usr/src/git-core/templates'
> install -d -m755 '/usr/share/git-core/python'
> install gitMergeCommon.py '/usr/share/git-core/python'

Re: Expected Behavior?

[Fredrik Kuivinen]

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

- Fredrik

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Jon Loeliger]

> Could you check diff between /usr/bin/git-merge-recursive and
> git-merge-recursive.py in your source tree and see if the
> appending to sys.path points at the right place (i.e. where the
> make installs gitMergeCommon.py)?

    jdl.com 211 % diff /usr/src/git-core/git-merge-recursive /usr/bin/git-merge-recursive
    jdl.com 212 %

    jdl.com 213 % ll  /usr/src/git-core/git-merge-recursive /usr/bin/git-merge-recursive
    32 -rwxr-xr-x  1 root root 29181 Nov  8 20:17 /usr/bin/git-merge-recursive
    32 -rwxr-xr-x  1 jdl  src  29181 Nov  8 08:36 /usr/src/git-core/git-merge-recursive

This would be a problem, though:

    jdl.com 216 % head -15 /usr/bin/git-merge-recursive
    #!/usr/bin/python

    import sys, math, random, os, re, signal, tempfile, stat, errno, traceback
    from heapq import heappush, heappop
    from sets import Set

    sys.path.append('''/home/jdl/share/git-core/python''')
    from gitMergeCommon import *

    originalIndexFile = os.environ.get('GIT_INDEX_FILE',
				       os.environ.get('GIT_DIR', '.git') + '/index')temporaryIndexFile = os.environ.get('GIT_DIR', '.git') + \
			 '/merge-recursive-tmp-index'
    def setupIndex(temporary):
	try:


Looks like /home/jdl/share got left instead of /usr/share.

So.  The root of the problem is that I followed the INSTALL
instructions:

		    Git installation

    Normally you can just do "make" followed by "make install", and that
    will install the git programs in your own ~/bin/ directory.  If you want
    to do a global install, you can do

	    make prefix=/usr install

I ran "make" as myself, and then later
I ran "make prefix=/usr install" as root.


However, the path substitution happened as part of the
"all" target and not as part of the "install" target:

    SCRIPT_PYTHON = \
	    git-merge-recursive.py

    SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
	      $(patsubst %.perl,%,$(SCRIPT_PERL)) \
	      $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
	      gitk git-cherry-pick

    all: $(PROGRAMS) $(SCRIPTS)

    $(patsubst %.py,%,$(SCRIPT_PYTHON)) : % : %.py
	    rm -f $@
	    sed -e '1s|#!.*python|#!$(call shq,$(PYTHON_PATH))|' \
		-e 's|@@GIT_PYTHON_PATH@@|$(call shq,$(GIT_PYTHON_DIR))|g' \
		-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
		$@.py >$@
	    chmod +x $@


So, It looks like either:

    1) Change the INSTALL documentation and process
       to require "make prefix=/...." also during the build,
or
    2) Change the Makefile to do install-related path
       substitutions at "make install" time.

My preference would be the latter (ie, take $(SCRIPTS) off
the "all: " line), but you may have better notions here.
Like, do people run stuff out of the build directory between
a "make" and a "make install"?  Dunno.

jdl

Re: Expected Behavior?

[Junio C Hamano]

Jon Loeliger <jdl@freescale.com> writes:

>     Normally you can just do "make" followed by "make install", and that
>     will install the git programs in your own ~/bin/ directory.  If you want
>     to do a global install, you can do
>
> 	    make prefix=/usr install
>
> I ran "make" as myself, and then later
> I ran "make prefix=/usr install" as root.

Would this be explicit enough?

-- >8 -- cut here -- >8 --

diff --git a/INSTALL b/INSTALL
index bbb13f3..b2cdb31 100644
--- a/INSTALL
+++ b/INSTALL
@@ -7,8 +7,10 @@ to do a global install, you can do
 
 	make prefix=/usr install
 
-(or prefix=/usr/local, of course).  Some day somebody may send me a RPM
-spec file or something, and you can do "make rpm" or whatever.
+(or prefix=/usr/local, of course).  Just like any program suite
+that uses $prefix, the built results have some paths encoded,
+which are derived from $prefix, so "make all; make prefix=/usr
+install" would not work.
 
 Issues of note:
 

Re: Expected Behavior?

[Martin Langhoff]

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

Re: Expected Behavior?

[Petr Baudis]

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

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

Re: Expected Behavior?

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

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

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Martin Langhoff]

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

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Junio C Hamano]

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.

Re: Expected Behavior?

[Martin Langhoff]

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.

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

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Martin Langhoff]

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

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Petr Baudis]

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

Re: Expected Behavior?

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

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

Re: Expected Behavior?

[Petr Baudis]

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.