git-add fails after file type change

git-add fails after file type change

[Steven Grimm]

In the course of experimenting with using git for my snapshot backups, I 
ran into what looks like a bug in git-add: it croaks when it tries to 
add a file whose type has changed, specifically when a directory gets 
moved and a symbolic link is put in the old location pointing to the new 
one. Here's a simple test case:

$ git init-db
defaulting to local storage area
$ mkdir dir
$ echo foo > dir/file
$ git add .
$ git commit -m "initial commit" -a
Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
 create mode 100644 dir/file
$ mv dir dir.real
$ ln -s dir.real dir
$ git add .
fatal: unable to add dir to index

Is "git add ." the wrong thing to do here? I have been using it as a 
generic "pick up all the files I haven't added yet" command. Or is this 
a bug?

For what it's worth, "git update-index dir" and "git update-index --add 
dir" both fail too.

Re: git-add fails after file type change

[Jakub Narebski]

Steven Grimm wrote:

> In the course of experimenting with using git for my snapshot backups, I 
> ran into what looks like a bug in git-add: it croaks when it tries to 
> add a file whose type has changed, specifically when a directory gets 
> moved and a symbolic link is put in the old location pointing to the new 
> one. Here's a simple test case:
> 
> $ git init-db
> defaulting to local storage area
> $ mkdir dir
> $ echo foo > dir/file
> $ git add .
> $ git commit -m "initial commit" -a
> Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
>  create mode 100644 dir/file
> $ mv dir dir.real
> $ ln -s dir.real dir
> $ git add .
> fatal: unable to add dir to index

Works if you use "git mv dir dir.real".

$ git init-db
defaulting to local storage area
$ mkdir dir
$ echo foo > dir/file
$ git add .
$ git commit -m "initial commit" -a
Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
$ git mv dir dir.real
$ ln -s dir.real dir
$ git add .
$ git commit -m "second commit" -a
$ git ls-tree HEAD
120000 blob e05f72eddb14362b836c3612c13d441b097a065a    dir
040000 tree 4a1c03029e7407c0afe9fc0320b3258e188b115e    dir.real
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-add fails after file type change

[A Large Angry SCM]

Steven Grimm wrote:
> In the course of experimenting with using git for my snapshot backups, I 
> ran into what looks like a bug in git-add: it croaks when it tries to 
> add a file whose type has changed, specifically when a directory gets 
> moved and a symbolic link is put in the old location pointing to the new 
> one. Here's a simple test case:
> 
> $ git init-db
> defaulting to local storage area
> $ mkdir dir
> $ echo foo > dir/file
> $ git add .
> $ git commit -m "initial commit" -a
> Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
> create mode 100644 dir/file
> $ mv dir dir.real
> $ ln -s dir.real dir
> $ git add .
> fatal: unable to add dir to index
> 
> Is "git add ." the wrong thing to do here? I have been using it as a 
> generic "pick up all the files I haven't added yet" command. Or is this 
> a bug?
> 
> For what it's worth, "git update-index dir" and "git update-index --add 
> dir" both fail too.

Re: git-add fails after file type change

[Steven Grimm]

Jakub Narebski wrote:
> Works if you use "git mv dir dir.real".
>   

This came up during my testing of snapshot backups. The *real* sequence 
is more like

$ git checkout -b new-snapshot
$ rsync /live/directory .
$ git add .

In other words, I don't know in advance that there's a rename or (short 
of turning on verbose rsync output and parsing it) which parts of the 
tree have changed at all. So I can't easily use git-mv here.

It still feels like a bug that "git add" can fail with no useful 
diagnostic. It actually took me a fair while to figure out what was 
going on here -- at first I thought it was having trouble with symlinks 
in general, then with absolute-path symlinks (which the actual symlink 
in question is), then I thought maybe it was a corrupt index. It wasn't 
until I went back and looked at the previous snapshot that I was 
overlaying this one on top of that I realized there used to be a 
directory where that symlink lives now.

Re: git-add fails after file type change

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> $ mkdir dir
> $ echo foo > dir/file
> $ git add .
> ...
> Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
> create mode 100644 dir/file
> $ mv dir dir.real
> $ ln -s dir.real dir
> $ git add .
> fatal: unable to add dir to index

You didn't tell the index to remove dir/file.  You cannot have
dir and dir/file at the same time.

Re: git-add fails after file type change

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Steven Grimm wrote:
>> In the course of experimenting with using git for my snapshot
>> backups, I ran into what looks like a bug in git-add: it croaks when
>> it tries to add a file whose type has changed, specifically when a
>> directory gets moved and a symbolic link is put in the old location
>> pointing to the new one. Here's a simple test case:
>>
>> $ git init-db
>> defaulting to local storage area
>> $ mkdir dir
>> $ echo foo > dir/file
>> $ git add .
>> $ git commit -m "initial commit" -a
>> Committing initial tree f4bc9c50d08b041f5e096fa68e243c34170f1cd8
>> create mode 100644 dir/file
>> $ mv dir dir.real
>> $ ln -s dir.real dir
>> $ git add .
>> fatal: unable to add dir to index
>>
>> Is "git add ." the wrong thing to do here? I have been using it as a
>> generic "pick up all the files I haven't added yet" command. Or is
>> this a bug?
>>
>> For what it's worth, "git update-index dir" and "git update-index
>> --add dir" both fail too.
>
> Did you try "git-update-index --replace dir"?

Good point.  I've forgotten about "--replace" codepath, although
that is all my code (May 7, 2005).

Maybe "git add" should internally use ADD_CACHE_OK_TO_REPLACE
(or error out and have an option to enable it)?

In any case, the error message could be made a bit more helpful,
like this.

---

diff --git a/read-cache.c b/read-cache.c
index eae4745..a602010 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -609,7 +609,7 @@ int add_cache_entry(struct cache_entry *ce, int option)
 	if (!skip_df_check &&
 	    check_file_directory_conflict(ce, pos, ok_to_replace)) {
 		if (!ok_to_replace)
-			return -1;
+			return error("'%s' appears as both a file and as a directory", ce->name);
 		pos = cache_name_pos(ce->name, ntohs(ce->ce_flags));
 		pos = -pos-1;
 	}

Re: git-add fails after file type change

[Steven Grimm]

Junio C Hamano wrote:
> Maybe "git add" should internally use ADD_CACHE_OK_TO_REPLACE
> (or error out and have an option to enable it)?
>   

That sounds right to me. Maybe there are cases where this isn't true, 
but -- especially given the recent move toward making "git add" the 
suggested porcelain command for updating the index -- I want "git add 
foo" to stick the current contents of "foo" into the index, regardless 
of what might have been there under that name previously. I'll grant 
that it's not a super-common use case to rename or delete a directory 
and put a file in its place, but it's IMO not an unreasonable thing to 
want git to track without undue hassle, given that it claims to support 
renames. I'm fine with having to supply an extra option to get that 
automatic behavior.

But barring that, the patch would at least have saved me a lot of 
head-scratching.

-Steve

Re: git-add fails after file type change

[Steven Grimm]

A Large Angry SCM wrote:
> Did you try "git-update-index --replace dir"?

Turns out that doesn't work. It gives me the same error I get without 
the --replace option:

error: dir: cannot add to the index - missing --add option?
fatal: Unable to process file dir

However, "git rm" followed by "git add" does seem to work. So for now it 
looks like that'll be the best bet when I run into this problem (should 
be possible to automate it, even.) The better error message from Junio's 
patch will at least make the failure less mysterious.

IMO it'd still be nice if the porcelain could provide a "make the index 
look like the working directory" operation that worked even in the face 
of changes like this, adding and/or removing files as needed. But it's 
possible that my crazy git-as-backup-device setup is the only place 
where this is even an issue; typical git users can use "git mv" and 
never run into the problem.

-Steve

Re: git-add fails after file type change

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> A Large Angry SCM wrote:
>> Did you try "git-update-index --replace dir"?
>
> Turns out that doesn't work. It gives me the same error I get without
> the --replace option:
>
> error: dir: cannot add to the index - missing --add option?
> fatal: Unable to process file dir

"update-index --replace --add" would be the way.

        $ git ls-files -s
        100644 fa457baf8abbf5dd3bb4cbfab0c5a4cf0523d7f8 0	1/2
        100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	3
        $ ls -F
        ./  ../  1/  3	.git/

There is file 1/2 in directory 1.

        $ mv 1 tmp ; mv 3 1 ; mv tmp 3
        $ ls -F
        ./  ../  1  3/	.git/

I just swapped them.

        $ git update-index --replace --add 1
        $ git ls-files -s
        100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	1
        100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	3

You are allowing update-index to 'add' things so you would need
to say --add regardless of --replace (--replace is only to allow
removal of conflicting entries while adding).  In the hindsight,
we could have implied --add with --replace, but that is the way
it is, and update-index is not a Porcelain so there is not much
point fixing it now.

But I think you helped me to spot a bug ;-).

        $ git update-index --replace --add 3/2
        $ git ls-files -s
        100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	1
	100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	3
        100644 fa457baf8abbf5dd3bb4cbfab0c5a4cf0523d7f8 0	3/2

The entry '3' should have been removed when we did --replace.
This index cannot be written out as a tree:

	$ git write-tree
        You have both 3 and 3/2
        fatal: git-write-tree: error building trees

Currently we need to remove '3' by hand X-<.

        $ git update-index --remove 3
        $ git ls-files -s
        100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0	1
        100644 fa457baf8abbf5dd3bb4cbfab0c5a4cf0523d7f8 0	3/2
        $ git write-tree
        77be0dd800d74913a90662e35215ee648815fc17

Re: git-add fails after file type change

[Junio C Hamano]

When replacing an existing file A with a directory B that has a
file B/C in it in the index, 'update-index --replace --add B/C'
did not properly remove the file to make room for the new
directory.

There was a trivial logic error, most likely a cut & paste one,
dating back to quite early days of git.

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

  Junio C Hamano <junkio@cox.net> writes:

  > But I think you helped me to spot a bug ;-).
  >
  >         $ git update-index --replace --add 3/2
  >         $ git ls-files -s
  >         100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0     1
  >         100644 00750edc07d6415dcc07ae0351e9397b0222b7ba 0     3
  >         100644 fa457baf8abbf5dd3bb4cbfab0c5a4cf0523d7f8 0     3/2
  >
  > The entry '3' should have been removed when we did --replace.
  > This index cannot be written out as a tree:
  >
  >       $ git write-tree
  >         You have both 3 and 3/2
  >         fatal: git-write-tree: error building trees

  And this fixes it.

 read-cache.c     |    2 +-
 t/t0000-basic.sh |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index a602010..e856a2e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -517,7 +517,7 @@ static int has_dir_name(const struct cache_entry *ce, int pos, int ok_to_replace
 		pos = cache_name_pos(name, ntohs(create_ce_flags(len, stage)));
 		if (pos >= 0) {
 			retval = -1;
-			if (ok_to_replace)
+			if (!ok_to_replace)
 				break;
 			remove_cache_entry_at(pos);
 			continue;
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 3260d1d..0cd1c41 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -272,4 +272,13 @@ test_expect_success \
 	 wc -l) &&
      test $numparent = 1'
 
+test_expect_success 'update-index D/F conflict' '
+	mv path0 tmp &&
+	mv path2 path0 &&
+	mv tmp path2 &&
+	git update-index --add --replace path2 path0/file2 &&
+	numpath0=$(git ls-files path0 | wc -l) &&
+	test $numpath0 = 1
+'
+
 test_done

[PATCH] git-add: remove conflicting entry when adding.

[Junio C Hamano]

When replacing an existing file A with a directory A that has a
file A/B in it in the index, 'git add' did not succeed because
it forgot to pass the allow-replace flag to add_cache_entry().

It might be safer to leave this as an error and require the user
to explicitly remove the existing A first before adding A/B
since it is an unusual case, but doing that automatically is
much easier to use.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index e856a2e..b8d83cc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -358,7 +358,7 @@ int add_file_to_index(const char *path, int verbose)
 
 	if (index_path(ce->sha1, path, &st, 1))
 		die("unable to index file %s", path);
-	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD))
+	if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))
 		die("unable to add %s to index",path);
 	if (verbose)
 		printf("add '%s'\n", path);
-- 
1.4.4.2.g83c5