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

[Christian Couder <christian.couder@gmail.com>]

On Mon, Feb 25, 2019 at 11:25 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Mon, Feb 25 2019, Matheus Tavares Bernardino wrote:
>
> > Hi, Christian and Ævar
> >
> > First of all, thanks for the fast and attentive reviews.
> >
> > I am a little confused about what I should do next. How should I
> > proceed with this series?
> >
> > By what was said, I understood that the series:
> > 1) Is indeed an improvement under --local, because it won't deference
> > symlinks in this case.
> > 2) Don't make --dissociate any better but as it is already buggy that
> > would be some work for another patchset.
> > 3) Makes git-clone copy hidden paths which is a good behaviour.
> > 4) Breaks --no-hardlinks when there are symlinks at the repo's objects
> > directory.
> >
> > I understood that even though git itself does not create symlinks in
> > .git/objects, we should take care of the case where the user manually
> > creates them, right? But what would be the appropriate behaviour: to
> > follow (i.e. deference) symlinks (the way it is done now) or just copy
> > the link file itself (the way my series currently do)? And shouldn't
> > we document this decision somewhere?
> >
> > About the failure with --no-hardlinks having symlinks at .dir/objects,
> > it's probably because copy_or_link_directory() is trying to copy a
> > file which is a symlink to a dir and the copy function used is trying
> > to copy the dir not the link itself. A possible fix is to change
> > copy.c to copy the link file, but I haven't studied yet how that could
> > be accomplished.
> >
> > Another possible fix is to make copy_or_link_directory() deference
> > symlink structures when --no-hardlinks is given. But because the
> > function falls back to no-hardlinks when failing to hardlink, I don't
> > think it would be easy to accomplish this without making the function
> > *always* deference symlinks. And that would make the series lose the
> > item 1), which I understand you liked.
>
> I don't really have formed opinions one way or the other about what
> these specific flags should do in combination with such a repository,
> e.g. should --dissociate copy data rather than point to the same
> symlinks?
>
> I'm inclined to think so, but I've only thought about it for a couple of
> minutes. Maybe if someone starts digging they'll rightly come to a
> different conclusion.

And maybe one day we will find a very good way to take advantage of
symlinks in .git/objects/ when Git is used normally, but that will go
against what we have decided now, though we have no real need at all
to decide now.

That's why I think it can actually be a good thing not to decide
anything now, and to let us free to decide later.

It's kind of the same as with short options versus long options. It's
a good idea not to use up all your short options too soon (in the name
of current ease of use), and instead wait until people really want a
short option for one option they use really often before attributing
it to this option.

> Rather, my comment is on the process. Clone behavior is too important to
> leave to prose in a commit message.

The same argument could be made to say that what happens in
.git/objects/ when cloning and there are symlinks is too important a
still free design option we have left to give it up right now for no
good reason.