Subject: Re: Bug report about symlinks

From: René Scharfe <[email-masked]>
Date: Sat, 02 Aug 2014 16:10:21 +0200

Am 01.08.2014 um 18:23 schrieb Junio C Hamano:
> René Scharfe <[email-masked]> writes:
> 
>> # Create test repo with two directories with two files each.
>> $ git init
>> Initialized empty Git repository in /tmp/.git/
>> $ mkdir a b
>> $ echo x >a/equal
>> $ echo x >b/equal
>> $ echo y >a/different
>> $ echo z >b/different
>> $ git add a b
>> $ git commit -minitial
>> [master (root-commit) 6d36895] initial
>>   4 files changed, 4 insertions(+)
>>   create mode 100644 a/different
>>   create mode 100644 a/equal
>>   create mode 100644 b/different
>>   create mode 100644 b/equal
>>
>> # Replace one directory with a symlink to the other.
>> $ rm -rf b
>> $ ln -s a b
>>
>> # First git status call.
>> $ git status
>> On branch master
>> Changes not staged for commit:
>>    (use "git add/rm <file>..." to update what will be committed)
>>    (use "git checkout -- <file>..." to discard changes in working directory)
>>
>>          deleted:    b/different
>>
>> Untracked files:
>>    (use "git add <file>..." to include in what will be committed)
>>
>>          b
>>
>> no changes added to commit (use "git add" and/or "git commit -a")
>> ...
>>
>> It could be debated if the first git status call should also report
>> b/equal as deleted.
> 
> Hmph.
> 
> I wonder if "could be" is an understatement.  The difference of
> reporting between b/equal and b/different may indicate that somebody
> is mistakenly comparing the index with these paths, without first
> checking each path with has_symlink_leading_path(), or something,
> no?

How about the patch below?  Before it checks if an index entry exists
in the work tree, it checks if its path includes a symlink.  After
the patch, git status reports b/equal as deleted, too.  The test
suite still passes.

And do we need to use the threaded_ variant of the function here?

diff --git a/read-cache.c b/read-cache.c
index 5d3c8bd..6f0057f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1064,6 +1064,14 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 		return ce;
 	}
 
+	if (has_symlink_leading_path(ce->name, ce_namelen(ce))) {
+		if (ignore_missing)
+			return ce;
+		if (err)
+			*err = ENOENT;
+		return NULL;
+	}
+
 	if (lstat(ce->name, &st) < 0) {
 		if (ignore_missing && errno == ENOENT)
 			return ce;