Subject: Re: [PATCH] [GSOC] prune_worktrees(): reimplement with dir_iterator

From: Junio C Hamano <[email-masked]>
Date: Sat, 01 Apr 2017 11:33:05 -0700

Robert Stanca <[email-masked]> writes:

> On Sat, Apr 1, 2017 at 5:29 AM, Junio C Hamano <[email-masked]> wrote:
>>
>> Using dir_iterator() to make it recursive is not just overkill but
>> is a positively wrong change, isn't it?
>
> Thanks for the review, and yes the commit message is misleading (my
> bad there). I understood that remove_temp_files has no recursion
> component to it and it removes all ".tmp-id-pack-" files from
> /objects/pack , but shouldn't dir-iterator work even if there's no
> recursion level?

The point is what should happen when somebody (perhaps a newer
version of Git, or a third-party add-on that works with Git) adds a
subdirectory in .git/objects/pack/ or .git/worktrees/.  The answer
is that files and directories in such a subdirectory must not be
touched, because prune_worktrees() or remove_temporary_files() do
not know what these files and directories are.

The dir-iterator API does not allow you to say "only scan the
directory without recursing into the subdirectories" so your code
ends up recursing into them, without knowing what the files and
directories inside are (or are not).  As Peff mentioned in his
review on the other one, if we add a knob to dir-iterator API to
allow you to tell it not to recurse, then it would make it possible
to do a faithful conversion from the original, but without such a
change, you are changing the behaviour.