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

From: Jens Lehmann <jens.lehmann@web.de>
Date: Apr 17, 2013, 20:30 UTC
Message-ID: <516F0666.5080308@web.de>
In-Reply-To: <7vppxthh44.fsf@alter.siamese.dyndns.org>

Am 17.04.2013 07:16, schrieb Junio C Hamano:

Show 16 quoted lines

> 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.

Show 5 quoted lines

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