Subject: Re: [PATCH 8/8] dir: avoid removing the current working directory

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Nov 2021 21:46:11 -0800

Elijah Newren <[email-masked]> writes:

> On Tue, Nov 23, 2021 at 1:57 PM Junio C Hamano <[email-masked]> wrote:
>>
>> Glen Choo <[email-masked]> writes:
>>
>> > This doesn't sound like a typical definition of 'emptiness' to me, but I
>> > can accept it if others also find it compelling. IOW if your definition
>> > of 'emptiness' is compelling enough, then I'll be convinced that there
>> > is no mixing of concerns and there would be no objection.
>>
>> FWIW, I do not find it compelling.  I can grant that it might be
>> convenient, but I do not think it is a good idea to explain the
>> reason why the directory is protected is because it is "not empty".
>
> Is the objection to my hand-wavy explanation?  If so, point taken.

The objection is against the definition of 'emptiness' Glen
perceived in your explanation ;-)

A directory is empty when there is no filesystem entity hangs below
it.  A process can have any directory as its cwd, even an empty one,
but the presense of such a process does not make an empty directory
suddenly non-empty.  That is the objection.

> However, I'm curious if you're also objecting to my commit message
> and/or the patch as well.
>
> If your objection also includes my commit message, but not the patch,
> would the following suit your taste better? :
>
> """
> remove_path() was added in 4a92d1bfb784 (Add remove_path: a function to
> remove as much as possible of a path, 2008-09-27) to, as it says, remove
> as much of a path as possible.  Why remove as much as possible?  Well,
> at the time we probably would have said something like:
>
>   * removing leading directories makes things feel tidy
>   * removing leading directories doesn't hurt anything so long as they
>     had no files in them.

I think you meant trailing, but I do not think you need to say it
twice---they say pretty much the same thing.  We are removing as
many directories that contain no files (i.e. non-directories)
underneath to make things tidy, as such a directory serves no useful
purpose.

I am not saying that an empty directory should not be protected even
when the current process sits there.  I just do not think it is a
good idea to call the protection "we protect a non-empty directory".
It is something else.