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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 24, 2021, 05:46 UTC
Message-ID: <xmqqa6hu3y3g.fsf@gitster.g>
In-Reply-To: <CABPp-BHUj=3b8xZhD3weBPYuWaVR5MrrM8+KCwhtcB8diZcJyg@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 14 quoted lines

> On Tue, Nov 23, 2021 at 1:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Glen Choo <chooglen@google.com> 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.

Show 15 quoted lines

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