Re: [GSoC][PATCH 2/2] clone: use dir-iterator to avoid explicit dir traversal

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Feb 21, 2019, 04:46 UTC
Message-ID: <CAHd-oW6VPrHMo1QjAXn6G8P_gmJOQ91LVO+UDK1a8T7uhuM9uQ@mail.gmail.com>
In-Reply-To: <20190219234506.GL6085@hank.intra.tgummerer.com>

Ok, I think I'm almost there and I should be able to send a v2 on the weekend. But again, a few questions arose while I'm coding v2. Please, see inline.

On Tue, Feb 19, 2019 at 8:45 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 8 quoted lines

>
> On 02/19, Matheus Tavares Bernardino wrote:
> > Ok, I agree. I noticed copy_or_link_directory only skips hidden
> > directories, not hidden files. And I couldn't think why we would want
> > to skip one but not the other... Could that be unintentional? I mean,
> > maybe the idea was to skip just "." and ".."? If that is the case, I
> > don't even need to make an if check at copy_or_link_directory, since
> > dir-iterator already skips "." and "..". What do you think about that?

[...]

Show 9 quoted lines

> I feel like you are probably right in that it could be unintentional,
> and I don't think anyone should be messing with the objects
> directory.  However that is no guarantee that changing this wouldn't
> break *something*.
>
> For the purpose of this change I would try to keep the same behaviour
> as we currently have where possible, to not increase the scope too
> much.
>

I understand your point, but to make copy_or_link_directory() skip just hidden directories using dir-iterator seems, now, a little tricker than I though before. The "if (iter->basename[0] != '.')" statement in the patch I sent for v1 only skips the hidden directories creation, but it does not avoid the attempt to copy the hidden directory contents (which would obviously fail). If we were to do that, we would need to check every directory in the relative path string of each iteration entry looking for a hidden directory. That seems a little too much, IMHO. Because of that and the intuition that skipping over hidden dirs was an unintentional operation, I think we could modify copy_or_link_directory() to skip '.' and '..', only. Also, hidden dirs/files are not expected to be at .git/objects anyway, are they?

And to have copy_or_link_directory()'s behaviour changed as little as possible, I could add the option to not follow hidden paths (dirs and regular files) at dir-iterator.c and use it at copy_or_link_directory() (now that I'm adding the 'pedantic' option, this won't be so difficult, anyway). Would these suggestions be a good plan?

Thanks, Matheus Tavares