[v8,0/9] add ref content check for files backend

Message

shejialuo Nov. 14, 2024, 4:51 p.m. UTC

Hi all:

This new version solves the following problem:

1. when reading the content of the ref file, we do not use
"fsck_report_ref" function. It's not suitable.
2. Add a new test for symlink worktree test in the last patch. After
writing the tets, find a bug. Fix the bug described below.

Because we have introduced the check for worktrees, we should not use
"ref_store->gitdir", instead we need to use "ref_store->repo->gitdir" to
get the main worktree "gitdir". After fixing this, the test is passed.

Thank Patrick to remind me about this. I forgot to add test thus making
mistakes.

Thanks,
Jialuo

shejialuo (9):
  ref: initialize "fsck_ref_report" with zero
  ref: check the full refname instead of basename
  ref: initialize ref name outside of check functions
  ref: support multiple worktrees check for refs
  ref: port git-fsck(1) regular refs check for files backend
  ref: add more strict checks for regular refs
  ref: add basic symref content check for files backend
  ref: check whether the target of the symref is a ref
  ref: add symlink ref content check for files backend

 Documentation/fsck-msgids.txt |  35 +++
 builtin/refs.c                |  10 +-
 fsck.h                        |   6 +
 refs.c                        |   7 +-
 refs.h                        |   3 +-
 refs/debug.c                  |   5 +-
 refs/files-backend.c          | 195 +++++++++++-
 refs/packed-backend.c         |   8 +-
 refs/refs-internal.h          |   5 +-
 refs/reftable-backend.c       |   3 +-
 t/t0602-reffiles-fsck.sh      | 576 ++++++++++++++++++++++++++++++++--
 11 files changed, 791 insertions(+), 62 deletions(-)

Range-diff against v7:
 1:  bfb2a21af4 =  1:  bfb2a21af4 ref: initialize "fsck_ref_report" with zero
 2:  9efc83f7ea =  2:  9efc83f7ea ref: check the full refname instead of basename
 3:  5ea7d18203 =  3:  5ea7d18203 ref: initialize ref name outside of check functions
 4:  cb4669b64d =  4:  cb4669b64d ref: support multiple worktrees check for refs
 5:  4e1add6465 !  5:  c6c128c922 ref: port git-fsck(1) regular refs check for files backend
    @@ refs/files-backend.c: typedef int (*files_fsck_refs_fn)(struct ref_store *ref_st
     +	if (S_ISLNK(iter->st.st_mode))
     +		goto cleanup;
     +
    -+	if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) {
    -+		ret = fsck_report_ref(o, &report,
    -+				      FSCK_MSG_BAD_REF_CONTENT,
    -+				      "cannot read ref file '%s': %s",
    -+				      iter->path.buf, strerror(errno));
    ++	if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0 ) {
    ++		/*
    ++		 * Ref file could be removed by another concurrent process. We should
    ++		 * ignore this error and continue to the next ref.
    ++		 */
    ++		if (errno == ENOENT)
    ++			goto cleanup;
    ++
    ++		ret = error_errno(_("cannot read ref file '%s': %s"),
    ++				  iter->path.buf, strerror(errno));
     +		goto cleanup;
     +	}
     +
 6:  945322fab7 =  6:  911fa42717 ref: add more strict checks for regular refs
 7:  3006eb9431 =  7:  7aa6a99206 ref: add basic symref content check for files backend
 8:  c59d003d78 =  8:  dbb0787ad1 ref: check whether the target of the symref is a ref
 9:  bb6d7f3323 !  9:  a6d85b4864 ref: add symlink ref content check for files backend
    @@ refs/files-backend.c: static int files_fsck_refs_content(struct ref_store *ref_s
      
     -	if (S_ISLNK(iter->st.st_mode))
     +	if (S_ISLNK(iter->st.st_mode)) {
    -+		const char* relative_referent_path = NULL;
    ++		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->gitdir);
    ++		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, '/');
    @@ refs/files-backend.c: static int files_fsck_refs_content(struct ref_store *ref_s
      		goto cleanup;
     +	}
      
    - 	if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) {
    - 		ret = fsck_report_ref(o, &report,
    + 	if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0 ) {
    + 		/*
     @@ refs/files-backend.c: static int files_fsck_refs_content(struct ref_store *ref_store,
      			goto cleanup;
      		}
    @@ t/t0602-reffiles-fsck.sh: test_expect_success 'the target of the textual symref
     +	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" &&