There is currently an odd behaviour when locally clonning 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
since the behaviour on GNU/Linux may result in broken symlinks, let's
re-implement it with linkat() instead of link() using a flag to always
follow symlinks and make the hardlink be to the pointed file. With this,
besides standardizing the behaviour, no broken symlinks will be
produced. 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.bernard...@usp.br>
Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
Co-authored-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
---
 builtin/clone.c            |  2 +-
 t/t5604-clone-reference.sh | 26 +++++++++++++++++++-------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 50bde99618..b76f33c635 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -443,7 +443,7 @@ 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))
+                       if (!linkat(AT_FDCWD, src->buf, AT_FDCWD, dest->buf, 
AT_SYMLINK_FOLLOW))
                                continue;
                        if (option_local > 0)
                                die_errno(_("failed to create link '%s'"), 
dest->buf);
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 708b1a2c66..76d45f1187 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -266,7 +266,7 @@ test_expect_success 'clone a repo with garbage in 
objects/*' '
        test_cmp expected actual
 '
 
-test_expect_success SYMLINKS 'setup repo with manually symlinked dirs and 
unknown files at objects/' '
+test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown 
files at objects/' '
        git init T &&
        (
                cd T &&
@@ -282,10 +282,18 @@ test_expect_success SYMLINKS 'setup repo with manually 
symlinked dirs and unknow
                        cd .git/objects &&
                        find ?? -type d >loose-dirs &&
                        last_loose=$(tail -n 1 loose-dirs) &&
-                       rm -f loose-dirs &&
                        mv $last_loose a-loose-dir &&
                        ln -s a-loose-dir $last_loose &&
+                       first_loose=$(head -n 1 loose-dirs) &&
+                       rm -f loose-dirs &&
+                       (
+                               cd $first_loose &&
+                               obj=$(ls *) &&
+                               mv $obj ../an-object &&
+                               ln -s ../an-object $obj
+                       ) &&
                        find . -type f | sort >../../../T.objects-files.raw &&
+                       find . -type l | sort >../../../T.objects-symlinks.raw 
&&
                        echo unknown_content> unknown_file
                )
        ) &&
@@ -294,7 +302,7 @@ test_expect_success SYMLINKS 'setup repo with manually 
symlinked dirs and unknow
 '
 
 
-test_expect_success SYMLINKS 'clone repo with symlinked dirs and unknown files 
at objects/' '
+test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at 
objects/' '
        for option in --local --no-hardlinks --shared --dissociate
        do
                git clone $option T T$option || return 1 &&
@@ -303,7 +311,8 @@ test_expect_success SYMLINKS 'clone repo with symlinked 
dirs and unknown files a
                test_cmp T.objects T$option.objects &&
                (
                        cd T$option/.git/objects &&
-                       find . -type f | sort 
>../../../T$option.objects-files.raw
+                       find . -type f | sort 
>../../../T$option.objects-files.raw &&
+                       find . -type l | sort 
>../../../T$option.objects-symlinks.raw
                )
        done &&
 
@@ -317,6 +326,7 @@ test_expect_success SYMLINKS 'clone repo with symlinked 
dirs and unknown files a
        ./Y/Z
        ./Y/Z
        ./a-loose-dir/Z
+       ./an-object
        ./Y/Z
        ./info/packs
        ./pack/pack-Z.idx
@@ -326,15 +336,17 @@ test_expect_success SYMLINKS 'clone repo with symlinked 
dirs and unknown files a
        ./unknown_file
        EOF
 
-       for option in --local --dissociate --no-hardlinks
+       for option in --local --no-hardlinks --dissociate
        do
-               test_cmp expected-files T$option.objects-files.raw.de-sha || 
return 1
+               test_cmp expected-files T$option.objects-files.raw.de-sha || 
return 1 &&
+               test_must_be_empty T$option.objects-symlinks.raw.de-sha || 
return 1
        done &&
 
        cat >expected-files <<-EOF &&
        ./info/alternates
        EOF
-       test_cmp expected-files T--shared.objects-files.raw
+       test_cmp expected-files T--shared.objects-files.raw &&
+       test_must_be_empty T--shared.objects-symlinks.raw
 '
 
 test_done
-- 
2.20.1

Reply via email to