Hi,
William Chargin wrote:
> Subject: t/test-lib: make `test_dir_is_empty` more robust
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. How about something like
test_dir_is_empty: avoid being confused by $'.\n.' file
or
test_dir_is_empty: simplify by using "ls --almost-all"
?
[...]
> + test_expect_success 'should fail with dot-newline-dot filename' '
> + mkdir pathological_dir &&
> + printf \"pathological_dir/.\\\\n.\\\\0\" | xargs -0 touch &&
I don't think "xargs -0" is present on all supported platforms. I'd
be tempted to use
>pathological_dir/$'.\n.'
but $'' is too recent of a shell feature to count on (e.g., dash doesn't
support it). See t/t3600-rm.sh for an example of a portable way to do
this.
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.
[...]
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -568,7 +568,7 @@ test_path_is_dir () {
> # Check if the directory exists and is empty as expected, barf otherwise.
> test_dir_is_empty () {
> test_path_is_dir "$1" &&
> - if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
> + if test "$(ls -A1 "$1" | wc -c)" != 0
Another portability gotcha: wc output includes a space on Mac so this
test would always return true there. How about
if test -n "$(ls -A1 "$1")"
?
"ls -A" was added in POSIX.1-2017. Its rationale section explains
Earlier versions of this standard did not describe the BSD -A
option (like -a, but dot and dot-dot are not written out). It
has been added due to widespread implementation.
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.
Can you say a little more about where you ran into this? Did you
discover it by code inspection?
I do think the resulting implementation using -A is simpler. Thanks
for writing it.
Thanks and hope that helps,
Jonathan
