Re: [test failure] Re: t4114 binary file becomes symlink

Jeff King [off-list ref] writes:

On Sat, Jul 18, 2009 at 04:16:58PM +0200, Nicolas Sebrecht wrote:

quoted

It fails on:
  - next
  - v1.6.3
  - b67b9612e1a90ae093445abeaeff930e9f4cf936
  - (other I don't remember, but does it really matter?)

Hmm. So it is clearly reproducible on your system, but not on mine. I
wonder what the difference could be.

Are you compiling with any special options? I usually compile with just
"-g -Wall -Werror", but I also tried with "-O2" and couldn't reproduce.

Me neither, but I found an unrelated anomaly in the output from the test
nearby.

    Switched to branch 'foo-baz-renamed-from-foo'
    *   ok 3: file renamed from foo/baz to foo

    * expecting success:
            git checkout -f foo-becomes-a-directory &&
            git diff-tree -p HEAD initial > patch &&
            git apply --index < patch

    error: Invalid path ''
    Switched to branch 'foo-becomes-a-directory'
    *   ok 4: directory becomes file

Notice the "error"?

This is coming from the oneway_merge() codepath in unpack-trees.c.

When we switch branches with "checkout -f", unpack_trees() feeds two 
cache_entries to oneway_merge() function in its src[] array argument.  The
zeroth entry comes from the current index, and the first entry represents
what the merge result should be, taken from the tree recorded in the
commit we are switching to.

When we have a blob (either regular file or a symlink) in the index and in
the work tree at path "foo", and the switched-to tree has "foo/bar",
i.e. "foo" becomes a directory, src[0] is obviously that blob currently
registered at "foo".  Even though we do not have anything at "foo" in the
switched-to tree, src[1] is _not_ NULL.

The unpack_trees() machinery places a special marker df_conflict_entry
to signal that no blob exists at "foo", but it will become a directory
that may have somthing underneath it, namely "foo/bar".

Passing that df_conflict_entry marker to merged_entry() happens to remove
the "foo" in the end because the df_conflict_entry does not have any name
(hence the "error" message) and its addition in add_index_entry() is
rejected, but it is wrong.

 unpack-trees.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 48d862d..720f7a1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c

@@ -999,7 +999,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)
 		return error("Cannot do a oneway merge of %d trees",
 			     o->merge_size);
 
-	if (!a)
+	if (!a || a == o->df_conflict_entry)
 		return deleted_entry(old, old, o);
 
 	if (old && same(old, a)) {

The 18/07/09, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

Are you compiling with any special options?

Normally, no (cf. my mail to Johannes Sixt).

quoted

                                            I usually compile with just
"-g -Wall -Werror", but I also tried with "-O2" and couldn't reproduce.

The Makefile have

  215:CFLAGS = -g -O2 -Wall
  217:ALL_CFLAGS = $(CFLAGS)

and it should be OK. I know that -Werror will fail on

  libgit.a(mkdtemp.o): In function `gitmkdtemp':
    /home/nicolas/dev/official_packages/git/compat/mkdtemp.c:5: warning: the
    use of `mktemp' is dangerous, better use `mkstemp'

but I think this is an unrelated problem.

$ gcc --version
gcc (GCC) 4.1.2 (Gentoo 4.1.2 p1.3)

Me neither, but I found an unrelated anomaly in the output from the test
nearby.

    Switched to branch 'foo-baz-renamed-from-foo'
    *   ok 3: file renamed from foo/baz to foo

    * expecting success:
            git checkout -f foo-becomes-a-directory &&
            git diff-tree -p HEAD initial > patch &&
            git apply --index < patch

    error: Invalid path ''
    Switched to branch 'foo-becomes-a-directory'
    *   ok 4: directory becomes file

Notice the "error"?

I couldn't reproduce this error here with

  sh t4114-apply-typechange.sh --verbose
and
  sh t4114-apply-typechange.sh --verbose --debug

on the current next.

With this patch, I still fail on the test.

-- 
Nicolas Sebrecht

On Sat, 18 Jul 2009, Junio C Hamano wrote:

The unpack_trees() machinery places a special marker df_conflict_entry
to signal that no blob exists at "foo", but it will become a directory
that may have somthing underneath it, namely "foo/bar".

Passing that df_conflict_entry marker to merged_entry() happens to remove
the "foo" in the end because the df_conflict_entry does not have any name
(hence the "error" message) and its addition in add_index_entry() is
rejected, but it is wrong.

Ack. This looks like a really old bug.

That whole 'df_conflict_entry' always confused me.

		Linus