Re: [GSoC][PATCH v7 00/10] clone: dir-iterator refactoring with tests

[Matheus Tavares Bernardino <matheus.bernardino@usp.br>]

On Tue, Jun 18, 2019 at 8:28 PM Matheus Tavares
<matheus.bernardino@usp.br> wrote:
>
> This patchset contains:
> - tests to the dir-iterator API;
> - dir-iterator refactoring to make its state machine simpler
>   and feature adding with tests;
> - a replacement of explicit recursive dir iteration at
>   copy_or_link_directory for the dir-iterator API;
> - some refactoring and behavior changes at local clone, mainly to
>   take care of symlinks and hidden files at .git/objects, together
>   with tests for these types of files.
>
> Changes since v6:
> - Rebased with master;
> - Added to dir-iterator documentation that ENOENT errors and hence broken
>   symlinks are both ignored.
>
> With the changes brought by this patchset, dir_iterator_begin() may now
> return NULL (setting errno) when it finds an error. Also, it's possible
> to pass a pedantic flag to it so that dir_iterator_advance() return
> immediately on errors. But at refs/files-backend.c, the only user of
> the API so far, the flag wasn't used and an empty iterator is
> returned in case of errors at dir_iterator_begin(). These actions were
> taken in order to keep the files-backend's behavior as close as
> possible to the one it previously had. But since it already has guards
> for possible errors at dir_iterator_advance(), I'm wondering whether I
> should send a follow-up patch making it use the pedantic flag.
>
> Also, should I perhaps call die_errno() on dir_iterator_begin() errors
> at files-backend? I mean, we should continue returning an empty
> iterator on ENOENT errors since ".git/logs", the dir it iterates over,
> may not be present. But we could possibly abort on other errors, just
> to be sure...

I got ahead of myself in this last paragraph. ".git/logs" is one of the dirs
that files-backend.c is used to iterate over, but it doesn't mean it's the only
one. This dir, in particular, is iterated when we run 'git rev-list
--reflog', for
example. And upon ENOENTs, the iteration is expected to end
successfully but with no entries.

(also adding Michael and Daniel to CC, in case they have some input on
these ideas)