On Sun, Mar 31, 2019 at 2:40 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
>
> On 03/30, Matheus Tavares wrote:
> > There is currently an odd behaviour when locally cloning a repository
> > with symlinks at .git/objects: using --no-hardlinks all symlinks are
> > dereferenced but without it, Git will try to hardlink the files with the
> > link() function, which has an OS-specific behaviour on symlinks. On OSX
> > and NetBSD, it creates a hardlink to the file pointed by the symlink
> > whilst on GNU/Linux, it creates a hardlink to the symlink itself.
> >
> > On Manjaro GNU/Linux:
> > $ touch a
> > $ ln -s a b
> > $ link b c
> > $ ls -li a b c
> > 155 [...] a
> > 156 [...] b -> a
> > 156 [...] c -> a
> >
> > But on NetBSD:
> > $ ls -li a b c
> > 2609160 [...] a
> > 2609164 [...] b -> a
> > 2609160 [...] c
> >
> > It's not good to have the result of a local clone to be OS-dependent and
> > besides that, the current behaviour on GNU/Linux may result in broken
> > symlinks. So let's standardize this by making the hardlinks always point
> > to dereferenced paths, instead of the symlinks themselves. Also, add
> > tests for symlinked files at .git/objects/.
> >
> > Note: Git won't create symlinks at .git/objects itself, but it's better
> > to handle this case and be friendly with users who manually create them.
> >
> > Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx>
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> > Co-authored-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> > ---
> > builtin/clone.c | 5 ++++-
> > t/t5604-clone-reference.sh | 27 ++++++++++++++++++++-------
> > 2 files changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 50bde99618..f975b509f1 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -443,7 +443,10 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
> > if (unlink(dest->buf) && errno != ENOENT)
> > die_errno(_("failed to unlink '%s'"), dest->buf);
> > if (!option_no_hardlinks) {
> > - if (!link(src->buf, dest->buf))
> > + char *resolved_path = real_pathdup(src->buf, 1);
> > + int status = link(resolved_path, dest->buf);
> > + free(resolved_path);
> > + if (!status)
>
> Is there any reason why we can't use 'real_path()' here? As I
> mentioned in [*1*], 'real_path()' doesn't require the callers to free
> any memory, so the above could become much simpler, and could just be
>
> + if (!link(real_path(src->buf), dest->buf))
>
Yes, you are right. I will change this! I sent this v5 before
carefully reading your previous email and studding strbuf functions
and real_path(), now that I did that, I see that real_path() is the
best option here. Thanks!
> *1*: <20190330192738.GQ32487@xxxxxxxxxxxxxxxxxxxxxxxx>
>
> > continue;
> > if (option_local > 0)
> > die_errno(_("failed to create link '%s'"), dest->buf);
