Re: [PATCH v2 13/15] dir-iterator: option to iterate dirs in pre-order

[Plato Kiorpelidis <kioplato@gmail.com>]

On 22/05/10 02:07PM, Phillip Wood wrote:
> Hi Plato
> 
> On 09/05/2022 18:51, Plato Kiorpelidis wrote:
> > Introduce a new option to dir-iterator, using dir_iterator_begin()
> > flags parameter, allowing to control whether or not directories will
> > be exposed before their contents. In essence, pre-order traversal over
> > file system entries that are directories.
> > 
> > This changes the default behavior of the dir-iterator API. Instead
> > of iterating directories before doing so over their contents, the new
> > default behavior does not expose directories at all. Iteration is still
> > performed, however, within directories, iterating over any other entry.
> > Only directory paths are ignored.
> > 
> > To iterate over directories in pre-order, reproducing the previous
> > default behavior, enable the new flag DIR_ITERATOR_DIRS_BEFORE in the
> > flags parameter of dir_iterator_begin():
> >    * ignore directories by not setting DIR_ITERATOR_DIRS_BEFORE
> >    * iterate directories pre-order by enabling DIR_ITERATOR_DIRS_BEFORE
> > 
> > Adjust existing callers, in refs/files-backend.c and builtin/clone.c
> > to enable DIR_ITERATOR_DIRS_BEFORE since these callers require iteration
> > over directories before doing so over their contents.
> > 
> > Update t/t0066-dir-iterator.sh and t/helper/test-dir-iterator.c to test
> > the new iteration scheme, which is the new default behavior, and the new
> > flag DIR_ITERATOR_DIRS_BEFORE which reproduces the old default behavior.
> 
> It's great that you've split this change out from the next patch. I had not
> realized when I looked at the last round that all the existing callers
> require pre-order traversal. Given that is the case I'm finding it hard to
> see how changing the default behavior to one that no caller is using is an
> improvement.

Changing the default behavior is required to simplify entry.c remove_subtree().
I would have kept dir-iterator's default iteration scheme as is, but how are we
going to deal with remove_subtree()? remove_subtree() requires iterating dirs
after their contents. We need to find a flag encoding that is a good design
choice and serves both existing and remove_subtree(), without limiting future
dir-iterator customers.

This encoding of flags was heavily discussed in the patch series that my work
is based on[1], most notably here[2].

[1]: https://lore.kernel.org/git/1493226219-33423-1-git-send-email-bnmvco@gmail.com/
[2]: https://public-inbox.org/git/1751d788-d1f1-1c97-b33b-f53dab78ef86@alum.mit.edu/

Thanks,
Plato