Re: {bug} warning: unable to access 'RelNotes/.gitattributes'

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Thu, Sep 13, 2012 at 12:40:39PM -0700, Junio C Hamano wrote:
>
>> > Interesting. I don't get any such warning on repack. And RelNotes points
>> > to a file, so I'm not sure why stat() would make us think it was a dir.
>> 
>> Interesting.  The command in question is
>> 
>>  git-pack-objects --keep-true-parents --honor-pack-keep --non-empty \
>>     --all --reflog --delta-base-offset </dev/null .junk-pack
>
> Weird. I don't see any problems with that command, either (I tried it
> with the current 'next'). Thinking that maybe delta reuse was getting in
> the way, I also tried it with --no-reuse-delta.
>
>>  - "rev-list --object --all" does not produce "Relnotes/1.7.4.txt"
>>    (it does have "Documentation/RelNotes/1.7.4.txt", of course).
>>    Somebody in this callchain is screwing the name up.
>
> Yeah, that sounds like a pretty huge bug. But since I can't reproduce,
> you're on your own for tracking it down.

I have a remote tracking branch refs/remotes/repo/html that has the
path RelNotes/1.7.4.txt at the top ;-)  Depending on how traversal
goes, if the tree that represents that RelNotes directory in the html
tree is found before the tree that represents Documentation/RelNotes 
directory in the main history at the corresponding commit, it is
perfectly normal that we discover the blob as RelNotes/1.7.4.txt, so
there is no bug.

So among the three points I raised, the first one was a false issue,
the second one is real (we do look for attributes in the working
tree for historical commit, or for a commit that does not belong to
the same lineage as the one that is currently checked out, hence we
must ignore ENOTDIR), and the third one is unrelated.

> I think that this:
>
>> diff --git i/attr.c w/attr.c
>> index f12c83f..056d702 100644
>> --- i/attr.c
>> +++ w/attr.c
>> @@ -353,7 +353,7 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
>>  	int lineno = 0;
>>  
>>  	if (!fp) {
>> -		if (errno != ENOENT)
>> +		if (errno != ENOENT && errno != ENOTDIR)
>>  			warn_on_inaccessible(path);
>>  		return NULL;
>>  	}
>
> is the right thing to do. It's cool that it uncovered a bug in this
> case, but it is easy to construct a non-bug case that would exhibit the
> same bogus warning (just convert a directory into a file).

Yes.