Re: [PATCH] ls-files: fix overeager pathspec optimization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> On Fri, 8 Jan 2010, Junio C Hamano wrote:
>>
>> Given pathspecs that share a common prefix, ls-files optimized its call
>> into recursive directory reader by starting at the common prefix
>> directory.
>> 
>> If you have a directory "t" with an untracked file "t/junk" in it, but the
>> top-level .gitignore file told us to ignore "t/", this resulted in an
>> unexpected behaviour:
>
> Ok, I'm not sure how "unexpected" this is, since arguably you are 
> overriding the ignore file by _being_ in that directory (the same way 
> index contents override ignore files), but I could go either way on that.
>
> Your patch looks fine, although I think you did this in a very odd way.
>
>> +	at = 0;
>> +	memcpy(path, path_, len);
>> +	while (1) {
>> +		char *cp;
>> +		path[at] = '\0';
>> +		/*
>> +		 * NOTE! NOTE! NOTE!: we might want to actually lstat(2)
>> +		 * path[] to make sure it is a directory.
>> +		 */
>> +		if (excluded(dir, path, &dtype))
>> +			return 1;
>
> The above starts by testing the empty string, and then after that test it 
> goes on to the next directory component. That is just _odd_.
>
> Wouldn't it be more natural to write the loop the other way around, ie 
> _first_ look up the next directory component, and _then_ do the exclude 
> processing for thoose components? 
>
> Or is there some subtle reason I'm missing for actually checking the empty 
> name?

No, just being paranoid in case somebody managed to .gitignore the
top-level of the working tree ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]