Re: [PATCH] submodule deinit: clarify work tree removal message

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 17.04.2013 07:16, schrieb Junio C Hamano:
> Phil Hord <phil.hord@gmail.com> writes:
> 
>> On Mon, Apr 1, 2013 at 3:02 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>> Okay, so here is the patch for that. If someone could point out
>>> a portable and efficient way to check if a directory is already
>>> empty I would be happy to use that to silence the "Cleaned
>>> directory" message currently printed also when deinit is run on
>>> an already empty directory.
>>
>>    isemptydir() {
>>         test -d "$(find $1 -maxdepth 0 -empty)"
>>    }
> 
> Hrm, -maxdepth and -empty are not even in POSIX.  Folks on GNU
> platforms and BSDs (I checked NetBSD 6 and OpenBSD 5.2) should be
> fine, but it makes other platforms unhappy.

Ok, that is not acceptable.

> What is this check used for?  To avoid running "rmdir" on non-empty
> ones?  Saying "cleaning foo/" (or "cleaned foo/") when foo/ is
> already empty is not a crime; not omitting an empty one may actually
> be a better behaviour from the point of view of repeatability and
> uniformity.

It's no big issue, but 'init' issues the "Submodule ... registered
for path ..." message only once and is quiet on subsequent calls,
'deinit' does the same with "Submodule ... unregistered for path
...", only the "Cleared directory ..." message appears each time
'deinit' is called, which makes it stand out. I do not believe
this little inconsistency is important enough to write a helper in
C (to have a portable way to see if the directory has already been
cleared), but this simple additional "if" looked easy enough. That
should have made me suspicious ;-)