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

Am 17.04.2013 07:16, schrieb Junio C Hamano:

Phil Hord [off-list ref] writes:

quoted

On Mon, Apr 1, 2013 at 3:02 PM, Jens Lehmann [off-list ref] wrote:

quoted

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 ;-)