Re: [PATCH v4] test_dir_is_empty: fix edge cases with newlines and hyphens

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

William Chargin <wchargin@gmail.com> writes:

> While the `test_dir_is_empty` function appears correct in most normal
> use cases, it can improperly pass if a directory contains a filename
> with a newline, and can improperly fail if an empty directory looks like
> an argument to `ls`. This patch changes the implementation to check that
> the output of `ls -a` has at most two lines (for `.` and `..`), which
> should be better behaved, and adds the `--` delimiter before the
> directory name when invoking `ls`.

AFIAK dot and dot-dot are allowed not to exist; "at most two" is not
a good test.

Quite honestly, our tests are still run inside a sort-of controlled
environment, so if it _requires_ use of things we have avoided
depending on, like "ls -A" and "xargs -0", or the fact that most
filesystems always have "." and ".." even in an empty directory, in
order to be resistant to funnily-named files like dot-LF-dot, I
would say it is not worth worrying about these funny names--instead
we can simply refrain from using such a pathological name, can't we?

In other words, is there a real-world need in the context of our
test suite for this change?

Also, I find that its support for directories whose names begin with
a dash red-herring.  All the test scripts in our test suite knows that
they can prefix "./" to avoid problems, i.e.

	test_dir_is_empty ./--wat

So it appears that the only problematic case is when we create a
directory, create a file or a directory whose name is dot-LF-dot and
nothing else, and then do something that ought to cause that file to
disappear, and make sure that the directory is empty, e.g.

	mkdir empty &&
	echo foo >"empty/$dotLFdot" &&
	git add "empty/$dotLFdot" &&
	git reset --hard &&
	test_dir_is_empty empty

We do want to make sure funny names can be added with "git add" and
"git reset --hard" to HEAD that lacked those paths with funny names
to remove them correctly.  But the funny names used in such a test
do not have to be $dotLFdot; you can use "${dotLFdot}X" instead in
the above and can ensure whatever the original test wanted to
ensure.

So...