Re: [PATCH] t/Makefile: make sure that file names are truly platform-independent

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

 



Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:

>> > > By the way, doesn't ls-files take pathspec glob, saving one extra
>> > > process to run grep?
>> 
>> I specifically did not do that, sorry for omitting the rationale from the
>> commit message. The reason why I have that grep is so that the backslash
>> can also catch non-ASCII characters, such as "Hellö-Jüniö".

That's clever and I did miss that.  It deserves an in-code comment
next to the use of separate grep to prevent others from making the
same mistake in the future.  Having an explanation in the log message
would help but not sufficient for a subtle construct like this.

The trick to catch non-ascii depends on core.quotepath set to true
(which is the default).  You would need to run ls-files with an
explicit "-c core.quotepath=false" to be safe.

It also means that the addition of HT to the grep pattern I
suggested is not necessary, because ls-files will always use "a\tb"
form for a pathname with HT, and you will catch it by looking for
the backslash.  You can lose '"' from the grep pattern for the same
reason.

Two other tricky things are

 (1) the test may be run inside a tarball extract and "git ls-files"
     may not report the pathnames in t/.

 (2) the user may not have a working Git yet in her path.

The invocation of "git -c core.quotepath=false ls-files" needs to at
least have 2>/dev/null to squelch an unnecessary error message.  In
such a scenario, we may miss a new offending pathname, but we will
not have false positive and I think that is the best we can do.

Thanks.
--
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]