ref: add symlink ref content check for files backend

Besides the textual symref, we also allow symbolic links as the symref.
So, we should also provide the consistency check as what we have done
for textual symref. And also we consider deprecating writing the
symbolic links. We first need to access whether symbolic links still
be used. So, add a new fsck message "symlinkRef(INFO)" to tell the
user be aware of this information.

We have already introduced "files_fsck_symref_target". We should reuse
this function to handle the symrefs which use legacy symbolic links. We
should not check the trailing garbage for symbolic refs. Add a new
parameter "symbolic_link" to disable some checks which should only be
executed for textual symrefs.

And we need to also generate the "referent" parameter for reusing
"files_fsck_symref_target" by the following steps:

1. Use "strbuf_add_real_path" to resolve the symlink and get the
   absolute path "ref_content" which the symlink ref points to.
2. Generate the absolute path "abs_gitdir" of "gitdir" and combine
   "ref_content" and "abs_gitdir" to extract the relative path
   "relative_referent_path".
3. If "ref_content" is outside of "gitdir", we just set "referent" with
   "ref_content". Instead, we set "referent" with
   "relative_referent_path".

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt
index f82ebc58e84d0c8f11c3cc2c59a1d64055241c56..b14bc44ca4791fa0cfd42b6320ecac566a3586e6 100644 (file)
--- a/Documentation/fsck-msgids.txt
+++ b/Documentation/fsck-msgids.txt
@@ -183,6 +183,12 @@
        git@vger.kernel.org mailing list if you see this error, as
        we need to know what tools created such a file.
 
+`symlinkRef`::
+       (INFO) A symbolic link is used as a symref. Report to the
+       git@vger.kernel.org mailing list if you see this error, as we
+       are assessing the feasibility of dropping the support to drop
+       creating symbolic links as symrefs.
+
 `symrefTargetIsNotARef`::
        (INFO) The target of a symbolic reference points neither to
        a root reference nor to a reference starting with "refs/".

diff --git a/fsck.h b/fsck.h
index 53a47612e67c4d63384dd49551c6a7331c979fb9..a44c231a5f1391f07e4c02b0d0fecb45eda0a886 100644 (file)
--- a/fsck.h
+++ b/fsck.h
@@ -86,6 +86,7 @@ enum fsck_msg_type {
        FUNC(MAILMAP_SYMLINK, INFO) \
        FUNC(BAD_TAG_NAME, INFO) \
        FUNC(MISSING_TAGGER_ENTRY, INFO) \
+       FUNC(SYMLINK_REF, INFO) \
        FUNC(REF_MISSING_NEWLINE, INFO) \
        FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \
        FUNC(TRAILING_REF_CONTENT, INFO) \

diff --git a/refs/files-backend.c b/refs/files-backend.c
index c2b99fdf40861f6f75931ba67a420ffaf31d4bd3..ea5961e48cfe5072b8501bfecbab9a58fab72327 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "../git-compat-util.h"
+#include "../abspath.h"
 #include "../config.h"
 #include "../copy.h"
 #include "../environment.h"
@@ -3511,7 +3512,8 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
 
 static int files_fsck_symref_target(struct fsck_options *o,
                                    struct fsck_ref_report *report,
-                                   struct strbuf *referent)
+                                   struct strbuf *referent,
+                                   unsigned int symbolic_link)
 {
        int is_referent_root;
        char orig_last_byte;
@@ -3520,7 +3522,8 @@ static int files_fsck_symref_target(struct fsck_options *o,
 
        orig_len = referent->len;
        orig_last_byte = referent->buf[orig_len - 1];
-       strbuf_rtrim(referent);
+       if (!symbolic_link)
+               strbuf_rtrim(referent);
 
        is_referent_root = is_root_ref(referent->buf);
        if (!is_referent_root &&
@@ -3539,6 +3542,9 @@ static int files_fsck_symref_target(struct fsck_options *o,
                goto out;
        }
 
+       if (symbolic_link)
+               goto out;
+
        if (referent->len == orig_len ||
            (referent->len < orig_len && orig_last_byte != '\n')) {
                ret = fsck_report_ref(o, report,
@@ -3562,6 +3568,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
                                   struct dir_iterator *iter)
 {
        struct strbuf ref_content = STRBUF_INIT;
+       struct strbuf abs_gitdir = STRBUF_INIT;
        struct strbuf referent = STRBUF_INIT;
        struct fsck_ref_report report = { 0 };
        const char *trailing = NULL;
@@ -3572,8 +3579,30 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
 
        report.path = target_name;
 
-       if (S_ISLNK(iter->st.st_mode))
+       if (S_ISLNK(iter->st.st_mode)) {
+               const char *relative_referent_path = NULL;
+
+               ret = fsck_report_ref(o, &report,
+                                     FSCK_MSG_SYMLINK_REF,
+                                     "use deprecated symbolic link for symref");
+
+               strbuf_add_absolute_path(&abs_gitdir, ref_store->repo->gitdir);
+               strbuf_normalize_path(&abs_gitdir);
+               if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1]))
+                       strbuf_addch(&abs_gitdir, '/');
+
+               strbuf_add_real_path(&ref_content, iter->path.buf);
+               skip_prefix(ref_content.buf, abs_gitdir.buf,
+                           &relative_referent_path);
+
+               if (relative_referent_path)
+                       strbuf_addstr(&referent, relative_referent_path);
+               else
+                       strbuf_addbuf(&referent, &ref_content);
+
+               ret |= files_fsck_symref_target(o, &report, &referent, 1);
                goto cleanup;
+       }
 
        if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) {
                /*
@@ -3611,13 +3640,14 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
                        goto cleanup;
                }
        } else {
-               ret = files_fsck_symref_target(o, &report, &referent);
+               ret = files_fsck_symref_target(o, &report, &referent, 0);
                goto cleanup;
        }
 
 cleanup:
        strbuf_release(&ref_content);
        strbuf_release(&referent);
+       strbuf_release(&abs_gitdir);
        return ret;
 }
 

diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
index 692b30727a14deb790a5a270fcb979f6860b5047..f8f27cfc6cb512400cba2518203f788b1c18ce37 100755 (executable)
--- a/t/t0602-reffiles-fsck.sh
+++ b/t/t0602-reffiles-fsck.sh
@@ -395,6 +395,147 @@ test_expect_success 'the target of the textual symref should be checked' '
        done
 '
 
+test_expect_success SYMLINKS 'symlink symref content should be checked' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       branch_dir_prefix=.git/refs/heads &&
+       tag_dir_prefix=.git/refs/tags &&
+       cd repo &&
+       test_commit default &&
+       mkdir -p "$branch_dir_prefix/a/b" &&
+
+       ln -sf ./main $branch_dir_prefix/branch-symbolic-good &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref
+       EOF
+       rm $branch_dir_prefix/branch-symbolic-good &&
+       test_cmp expect err &&
+
+       ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref
+       warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\''
+       EOF
+       rm $branch_dir_prefix/branch-symbolic &&
+       test_cmp expect err &&
+
+       ln -sf ./"branch   " $branch_dir_prefix/branch-symbolic-bad &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref
+       error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch   '\''
+       EOF
+       rm $branch_dir_prefix/branch-symbolic-bad &&
+       test_cmp expect err &&
+
+       ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 &&
+       test_must_fail git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref
+       error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\''
+       EOF
+       rm $tag_dir_prefix/tag-symbolic-1 &&
+       test_cmp expect err
+'
+
+test_expect_success SYMLINKS 'symlink symref content should be checked (worktree)' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       cd repo &&
+       test_commit default &&
+       git branch branch-1 &&
+       git branch branch-2 &&
+       git branch branch-3 &&
+       git worktree add ./worktree-1 branch-2 &&
+       git worktree add ./worktree-2 branch-3 &&
+       main_worktree_refdir_prefix=.git/refs/heads &&
+       worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree &&
+       worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree &&
+
+       (
+               cd worktree-1 &&
+               git update-ref refs/worktree/branch-4 refs/heads/branch-1
+       ) &&
+       (
+               cd worktree-2 &&
+               git update-ref refs/worktree/branch-4 refs/heads/branch-1
+       ) &&
+
+       ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref
+       EOF
+       rm $worktree1_refdir_prefix/branch-symbolic-good &&
+       test_cmp expect err &&
+
+       ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref
+       EOF
+       rm $worktree2_refdir_prefix/branch-symbolic-good &&
+       test_cmp expect err &&
+
+       ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref
+       EOF
+       rm $main_worktree_refdir_prefix/branch-symbolic-good &&
+       test_cmp expect err &&
+
+       ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic &&
+       git refs verify 2>err &&
+       cat >expect <<-EOF &&
+       warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref
+       warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\''
+       EOF
+       rm $worktree1_refdir_prefix/branch-symbolic &&
+       test_cmp expect err &&
+
+       for bad_referent_name in ".tag" "branch   "
+       do
+               ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic &&
+               test_must_fail git refs verify 2>err &&
+               cat >expect <<-EOF &&
+               warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref
+               error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\''
+               EOF
+               rm $worktree1_refdir_prefix/bad-symbolic &&
+               test_cmp expect err &&
+
+               ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic &&
+               test_must_fail git refs verify 2>err &&
+               cat >expect <<-EOF &&
+               warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref
+               error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\''
+               EOF
+               rm $worktree1_refdir_prefix/bad-symbolic &&
+               test_cmp expect err &&
+
+               ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic &&
+               test_must_fail git refs verify 2>err &&
+               cat >expect <<-EOF &&
+               warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref
+               error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\''
+               EOF
+               rm $worktree2_refdir_prefix/bad-symbolic &&
+               test_cmp expect err &&
+
+               ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic &&
+               test_must_fail git refs verify 2>err &&
+               cat >expect <<-EOF &&
+               warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref
+               error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\''
+               EOF
+               rm $worktree2_refdir_prefix/bad-symbolic &&
+               test_cmp expect err || return 1
+       done
+'
+
 test_expect_success 'ref content checks should work with worktrees' '
        test_when_finished "rm -rf repo" &&
        git init repo &&