Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

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

 



William Chargin wrote:
> Jonathan Nieder wrote:

>> This subject line will appear out of context in "git shortlog" output,
>> so it's useful to pack in enough information to briefly summarize what
>> the change does.
>
> I'm happy to do so. I think that "simplify" is misleading, because this
> is a bug fix, not a refactoring. I like your first suggestion, though it
> exceeds the 50-character soft limit. What do you think of:
>
>         test_dir_is_empty: find files with newline in name

Thanks.  I think we can ignore the 50-character soft limit.  It's
often too low and expressivity is more important.  So if you like my
first suggestion, then I'd say start with that and tweak to your taste
from there.

[...]
>> I don't think "xargs -0" is present on all supported platforms
>
> Wow---I'm shocked that it's not POSIX, but you're right. (That makes
> `xargs` so much less useful!)

To be clear, as Junio mentioned, POSIX is useful as a guide to what
*might* be portable, but what we care about is what is portable to the
platforms used in practice.

[...]
>> Not all filesystems support newlines in filenames.  I think we'd want
>> to factor out the FUNNYNAMES prereq from there into a test_lazy_prereq
>> so this test can be skipped on such filenames.
>
> This makes sense. Would you like me to send in a separate patch to add
> this `test_lazy_prereq` to `t/test-lib.sh`, fixing up existing uses (of
> which there are several), and then rebase this patch on top of it?

Thanks for the offer!  Yes, that would be nice: such a patch would be
nice as a cleanup in its own right, too.

Such a patch would be helpful at any time.  For rebasing this patch,
my advice would be to hold off until the discussion has settled down a
bit.  If we're lucky, people in other time zones might have an idea
beyond the ones we've come up with.

[...]
>> "ls -A" was added in POSIX.1-2017. [...]
>> That's very recent, but the widespread implementation it mentions is
>> less so.  This would be our first use of "ls -A", so I'd be interested
>> to hear from people on more obscure platforms.  It does seem to be
>> widespread.
>
> ...if you prefer, a variant of `test "$(ls -a1 | wc -l)" -eq 2` should
> satisfy all the crtieria that you mention above (POSIX since forever,
> should work on Mac). The assumption is that a file with a newline
> character may take up more than one line, but every file must take up
> _at least_ one line, and we get two lines from `.` and `..`. If this
> assumption is false, then I will have learned yet another new thing!

As a piece of trivia, '.' and '..' are allowed not to exist.  So this test
can have false negatives and false positives.

Filesystems omitting them are quite rare so this might work reasonably
well in practice, though.

Thanks,
Jonathan



[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]

  Powered by Linux