From patchwork Thu Feb 6 05:58:27 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962240 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 92CAE1FE45E for ; Thu, 6 Feb 2025 05:56:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821411; cv=none; b=a5FV2Dk31d9DoEVZdpmhTpHt/jMn381CcocSinXFHtyx9rPUAOeoJdVezN8wOQpqHNuNoJ8yNm8ZgBNV/7rgtx00rVON94AIYTL/x6LgKgWOZv2DBoS9m6gJAI5tQ5vakKbjhlXx1B5+vloVaRhmSsEzN6ZaJAKJZahwJBS2Ges= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821411; c=relaxed/simple; bh=NiSz0kFXgj2oW62wBNpIa05qAzJdZQp1gZun8gTT0Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=g/UBsWrSpjplsLZHFYgIud87XoYkcvpmsaL2zDt2OeG8AF070CXtVtQoahkSVCsOgKWvlOFliQwcY8o/RUs7YkdyO9Ql887cRbJ6DxFmfjpLr+mLZ6kTsSDzIWsXrIMqnpa7LOWEYT6whOmH6y0/hVCHZmh9lyQanrBphR4RrQY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VFJ6kzHs; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VFJ6kzHs" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-21ddab8800bso8050505ad.3 for ; Wed, 05 Feb 2025 21:56:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821407; x=1739426207; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=tZWI8TgIs25Erf+1G0IhxEBg80nHgq+9lTGFbSIIC1w=; b=VFJ6kzHsFn9w9ZhzKr5Y8HnuYndn9B/K1rYk+8kqtWgOleh0GZLOIvRBD0101AcLoM Vq01eHtcO5MfB7HdzZE/Ey5rbUAPONdB92EDgygVHz/eLHjD9Nu7gYJAGVoUi6jLfD1W XJkaiR203Uq4jL2cQZxGWQH7W+UqNOJp8NE9B1JbZLF8he1Bkc2R0foZo2fa0CUU/P/v 2gABp++FDJxjXlnpDuY+WYuyxY5gORDShlLIhetKwkefv03XM4QK6GM4FHu4QrrG+8iT GgJxkAY+2eOuYxDKKLrQwVhafbrnAg+MKY0Hsv3W4HYJwTjV3+nNiU8aJIvQSnWEAoZ/ oc0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821407; x=1739426207; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=tZWI8TgIs25Erf+1G0IhxEBg80nHgq+9lTGFbSIIC1w=; b=Cuvx/JmqrUfjIOQAgpFUokCfocSVbiX8ZrDIdjJR116HL6bOuq5UJw/bNcenuVJeS1 9KBgUBSq2zHV2QLp0soyEJOZsy+uEAiThCNQQD+GaT0POdbZJG3tAbn7ucuBXotvCbAU AEjSimuQC9IYijxcFxFCF0EHBNFJEs4msvKpEqfww9rNs6X6tyPje8WncwhrAHUVdlPE 7x0dNtSidNnFK5rRRSHQAhxc50oLLH0/TB1zR58QsiNPEQ9JMvDJQrp/T2aeudMkWDA+ ohUSymoNtlASrPinq8qSrb5dZnv9ZQsUdEPV8u/QrO1kCrCaGYHVvdRO990E+MmlZ484 n5oA== X-Gm-Message-State: AOJu0YwsSqV25/P+qjBo/FGLv4NHVkOkYM5JiWC2gFfbMW5aNGHA7uID ZajZx5bBbdvEK61Ki81lWbEz+y07Bm6kbjIq0wzPYZQnqowZ2RHGid+Q+g== X-Gm-Gg: ASbGncuZiOIg3cQlguftGmYQOqBSzJbVaE5D3ZKCIif2oqs0PEAScdvkU1JmQKgBRys Uk1yma8ixGyYIMP+lk+q6hmGfCx8Q6iceVlQabYLrtGD+7anW9qNaC9mtHFj7hiZRoXDy2rB71q 413sQueYYv2RVn55FB884KBSl+46jyIebQwITiEPdIdc7XqT9NalM/fa0jihcFHQbWhRb+vUKRF Ykv0nOjVumppbTEUgRokLRIfGoaYEheAuuLeqv5FFHVSmGmVwSxvwae9KpLzMU66jxm3A== X-Google-Smtp-Source: AGHT+IE6Bu+T1HCTGq9RULcqtAmZ4RU60D1yR96xC0EafZzXD6xGnLnq8ix+GR1T7qp0Rbsw1VpKvw== X-Received: by 2002:a17:902:e74a:b0:216:4e8d:4803 with SMTP id d9443c01a7336-21f17edcab8mr80582385ad.42.1738821406896; Wed, 05 Feb 2025 21:56:46 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f368797c8sm3921135ad.159.2025.02.05.21.56.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:56:46 -0800 (PST) Date: Thu, 6 Feb 2025 13:58:27 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 1/8] t0602: use subshell to ensure working directory unchanged Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: For every test, we would execute the command "cd repo" in the first but we never execute the command "cd .." to restore the working directory. However, it's either not a good idea use above way. Because if any test fails between "cd repo" and "cd ..", the "cd .." will never be reached. And we cannot correctly restore the working directory. Let's use subshell to ensure that the current working directory could be restored to the correct path. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- t/t0602-reffiles-fsck.sh | 967 ++++++++++++++++++++------------------- 1 file changed, 494 insertions(+), 473 deletions(-) diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index d4a08b823b..cf7a202d0d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -14,222 +14,229 @@ test_expect_success 'ref name should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - - git commit --allow-empty -m initial && - git checkout -b default-branch && - git tag default-tag && - git tag multi_hierarchy/default-tag && - - cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && - git refs verify 2>err && - test_must_be_empty err && - rm $branch_dir_prefix/@ && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && - git refs verify 2>err && - rm $tag_dir_prefix/tag-1.lock && - test_must_be_empty err && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/.lock: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/.lock && - test_cmp expect err && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/$refname: badRefName: invalid refname format - EOF - rm "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + ( + cd repo && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + git commit --allow-empty -m initial && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/multi_hierarchy/$refname" && - test_cmp expect err || return 1 - done && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - mkdir "$branch_dir_prefix/$refname" && - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && + rm $branch_dir_prefix/@ && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && + git refs verify 2>err && + rm $tag_dir_prefix/tag-1.lock && + test_must_be_empty err && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/$refname/default-branch: badRefName: invalid refname format + error: refs/tags/.lock: badRefName: invalid refname format EOF - rm -r "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done + rm $tag_dir_prefix/.lock && + test_cmp expect err && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname: badRefName: invalid refname format + EOF + rm "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/multi_hierarchy/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + mkdir "$branch_dir_prefix/$refname" && + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname/default-branch: badRefName: invalid refname format + EOF + rm -r "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref name check should be adapted into fsck messages' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - git commit --allow-empty -m initial && - git checkout -b branch-1 && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=warn refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/.branch-1: badRefName: invalid refname format - EOF - rm $branch_dir_prefix/.branch-1 && - test_cmp expect err && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=ignore refs verify 2>err && - test_must_be_empty err + ( + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=warn refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/.branch-1: badRefName: invalid refname format + EOF + rm $branch_dir_prefix/.branch-1 && + test_cmp expect err && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=ignore refs verify 2>err && + test_must_be_empty err + ) ' test_expect_success 'ref name check should work for multiple worktrees' ' test_when_finished "rm -rf repo" && git init repo && - - cd repo && - test_commit initial && - git checkout -b branch-1 && - test_commit second && - git checkout -b branch-2 && - test_commit third && - git checkout -b branch-3 && - git worktree add ./worktree-1 branch-1 && - git worktree add ./worktree-2 branch-2 && - 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-3 - ) && ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && - - cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && - cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err && - - for worktree in "worktree-1" "worktree-2" - do + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + ( - cd $worktree && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err || return 1 - ) - done + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + for worktree in "worktree-1" "worktree-2" + do + ( + cd $worktree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err || return 1 + ) + done + ) ' test_expect_success 'regular ref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && - git refs verify 2>err && - test_must_be_empty err && + git refs verify 2>err && + test_must_be_empty err && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/a/b/branch-bad && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - for trailing_content in " garbage" " more garbage" - do - printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && cat >expect <<-EOF && - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $branch_dir_prefix/branch-garbage && - test_cmp expect err || return 1 - done && + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - '\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err && - printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + '\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - garbage'\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err + ) ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -237,99 +244,103 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' 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" && - - bad_content_1=$(git rev-parse main)x && - bad_content_2=xfsazqfxcadas && - bad_content_3=Xfsazqfxcadas && - printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && - printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && - printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 - error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 - error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + bad_content_1=$(git rev-parse main)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && + printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && + printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 + error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 + error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'textual symref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" + do + printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for good_referent in "refs/heads/branch" "HEAD" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" - do - printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && - - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-1 && - test_cmp expect err && - - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-2 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-3 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-complicated && - test_cmp expect err + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err + ) ' test_expect_success 'textual symref content should be checked (aggregate)' ' @@ -337,32 +348,34 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' 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" && - - printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && - printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'the target of the textual symref should be checked' ' @@ -370,28 +383,30 @@ test_expect_success 'the target of the textual symref should be checked' ' 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" && - - for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && - git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && - - for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" - do - printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' - EOF - rm $branch_dir_prefix/branch-bad-1 && - test_cmp expect err || return 1 - done + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" + do + printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err || return 1 + done + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked' ' @@ -399,201 +414,207 @@ test_expect_success SYMLINKS 'symlink symref content should be checked' ' 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 + ( + 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 && + 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/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'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree1_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>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-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'\'' + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree2_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && + 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'\'' + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree2_refdir_prefix/bad-symbolic && + rm $main_worktree_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + 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'\'' + 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 $worktree2_refdir_prefix/bad-symbolic && - test_cmp expect err || return 1 - done + 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 && - 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 && - 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 - ) && + 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 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && - test_must_fail git refs verify 2>err && + ( + 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 + ) && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $worktree1_refdir_prefix/bad-branch-1 && - test_cmp expect err || return 1 - done && + rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && - test_must_fail git refs verify 2>err && + printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' EOF - rm $worktree2_refdir_prefix/bad-branch-2 && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $worktree1_refdir_prefix/branch-no-newline && - test_cmp expect err && - - printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - EOF - rm $worktree1_refdir_prefix/branch-garbage && - test_cmp expect err + rm $worktree1_refdir_prefix/branch-garbage && + test_cmp expect err + ) ' test_done From patchwork Thu Feb 6 05:58:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962241 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 69D8F1FE45E for ; Thu, 6 Feb 2025 05:57:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821422; cv=none; b=lhxbSxhEjYwdYmmkgg3fu7QUGtVNR2kqQNn2ZxhdBW++gkvAiJLTsZL1cvdo6VLhcYXqwnpddxn66LRws5hj62gBp7+72854fmBBlHZt4o/gLXFkN1i/J3ovOo/A7Oq6RaPZ4yNdGbLM2dC+vb4RoNDcCKasAevtEm6MX9Pb4tA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821422; c=relaxed/simple; bh=Pw3z9o+kUxJpZxKRgSvXpacK5lCJj5GH/n7+SaPqQLY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rYRPwHnRpsVwUCvnPay27RUVX81cNw121t2rl4Jbcpu56rKjtFukTLwePzhhQXRWZukICAEYki+3mgNgqiqAZy31DxiPo9rPlAS3Dqjl2GS9xDMQchnWgS+fdYMGHvCWymwo54aQmpJMWxKsKkySwZqI2LznzkMM+7aNUu6ko1c= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=QkvqcNyj; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QkvqcNyj" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-21f2cfd821eso9462775ad.3 for ; Wed, 05 Feb 2025 21:57:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821420; x=1739426220; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=h1Qe1T9rOx9zjzY4gdVYk5gvUXaPpIeOaRjiZtE7aaY=; b=QkvqcNyjge13T+en5ah0BWdJ8W0KN/lhwLh4hKwQd+RKYpVgJr2HIWBbu5sWH6nxiG rOgttBtOGVzBZdSpK8YCzc9knJ++1e571oT37LBHK+8FLRysh9SLmQtFujePdnKtcU86 lquMeg3ceUAo0lX5izUBLSIuMcMnUECy85UJOEPudH0FMkWOZhd4Ek0KK0j/I9B5rWvQ yB2sckeIrQx5R/tFRF4v7Y7CGb8d21b+YGRL8wOf57hDRJY6K2BhoHyGooO11ohZ6iS9 d9RKUZIIzZixk+xiNM7aZ2tMffU4EkGO7jCtBjvWFSxrg0crCscZ8+DKg6xf2uqJzlAm TsDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821420; x=1739426220; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=h1Qe1T9rOx9zjzY4gdVYk5gvUXaPpIeOaRjiZtE7aaY=; b=QyzD/tmw+RmUeuf4Ffv2pVRSR1k0I81hnOMDt4moWDebtMpFdyL/j8BvJ3syF4rjdz gHKGD8pLoO9pDNNbLto1DAs+vpqqZJal4aC2a+WxINGsfSrsQ8jg3YZl5jmj14xmVdu9 0lt7RN3Eze27ZFON8rEICTF5xFr0vmbr1lBKtyUIpzKgu8vV/5feYiy0Kwp3xz3uOoe9 KsFIQSwdJUZw5hcLfEBbSwsr+3rlTz1IiPJC4nC1yuQoeW04gX8FkgWo6KwSDXMliBXg g7Je++bBv0ROoTfBQHBFUwbOM4Hf4Vneu+tERAOS2LselSRmzkdo/P1m31kea+ZW2J3j m3fw== X-Gm-Message-State: AOJu0Yw0cxSe0sSlF969sMz2bmPNDpiX3u1gQuXSh4PWzJmCgFje87Pp G4OP1x6s61YjWF2ItWtGRFSec0CguZJTwDcUJAvQ/FiWy426Uhr1ujd0Bw== X-Gm-Gg: ASbGncurmomIbk3kC+/Nf6kuxZIQxIHQfn7lskGUW/JtkGXu68ejGcSDamV1StoJIc3 4gZRrPyN1SpVo5fw3jYqPTKHuiCESU+ZwSHBtrLlJN1rOy4uY+qQKuLE2jRKqgC1fMa07jZVWjd 6QrlEO/vi+2VhuOb0neQi/kkzSlIqC2Cn7iA3D7H6VzxMYoTst6tIjdvdUZU49OYSHGOpzyhVHN 5YScngPROX6lNR0peqL+Y0DHqM5lesXZIQLecDs+b92PjD+rqzM2TRlezh1SEOtdsOYkA== X-Google-Smtp-Source: AGHT+IEaFU+Jmz36bu8ry0xKsTEzI7sY3x6K3wx2TZqNT+etA+j1u0HIk6VUeAdlx+nuZIcUzTX0nA== X-Received: by 2002:a17:902:cf12:b0:216:386e:dbf with SMTP id d9443c01a7336-21f17e4617bmr95815395ad.20.1738821419754; Wed, 05 Feb 2025 21:56:59 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f3683d7c2sm3868705ad.158.2025.02.05.21.56.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:56:59 -0800 (PST) Date: Thu, 6 Feb 2025 13:58:41 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 2/8] builtin/refs: get worktrees without reading head information Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c", there are some functions such as "create_snapshot" and "next_record" which would check the correctness of the content of the "packed-ref" file. When anything is bad, the program will die. It may seem that we have nothing relevant to above feature, because we are going to read and parse the raw "packed-ref" file without creating the snapshot and using the ref iterator to check the consistency. However, when using "get_worktrees" in "builtin/refs", we would parse the "HEAD" information. If the referent of the "HEAD" is inside the "packed-ref", we will call "create_snapshot" function to parse the "packed-ref" to get the information. No matter whether the entry of "HEAD" in "packed-ref" is correct, "create_snapshot" would call "verify_buffer_safe" to check whether there is a newline in the last line of the file. If not, the program will die. Although this behavior has no harm for the program, it will short-circuit the program. When the users execute "git refs verify" or "git fsck", we should avoid reading the head information, which may execute the read operation in packed backend with stricter checks to die the program. Instead, we should continue to check other parts of the "packed-refs" file completely. Fortunately, in 465a22b338 (worktree: skip reading HEAD when repairing worktrees, 2023-12-29), we have introduced a function "get_worktrees_internal" which allows us to get worktrees without reading head information. Create a new exposed function "get_worktrees_without_reading_head", then replace the "get_worktrees" in "builtin/refs" with the new created function. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 2 +- worktree.c | 5 +++++ worktree.h | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/builtin/refs.c b/builtin/refs.c index a29f195834..55ff5dae11 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -88,7 +88,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix, git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - worktrees = get_worktrees(); + worktrees = get_worktrees_without_reading_head(); for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); diff --git a/worktree.c b/worktree.c index 248bbb39d4..89b7d86cef 100644 --- a/worktree.c +++ b/worktree.c @@ -175,6 +175,11 @@ struct worktree **get_worktrees(void) return get_worktrees_internal(0); } +struct worktree **get_worktrees_without_reading_head(void) +{ + return get_worktrees_internal(1); +} + const char *get_worktree_git_dir(const struct worktree *wt) { if (!wt) diff --git a/worktree.h b/worktree.h index 38145df80f..1ba4a161a0 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,12 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. This is useful when checking + * the consistency, as reading HEAD may not be necessary. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Thu Feb 6 05:58:53 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962242 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F191F1F60A for ; Thu, 6 Feb 2025 05:57:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821434; cv=none; b=i89yA9AfFU28JpyrHFz/yYodD9LpV17jAadhUGIRZFrZFq1qyDTMciGbuoMWRVRfhEuNiCJEWCkMkump6nfLVHjecUNrQF3N/HRibjAS4XP5GB3NVfDS3ZPcNL1XyyLUxDgMPLXgBtag/4JYxCMNxdp6l7bqWNtbnpIH6R3ra1I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821434; c=relaxed/simple; bh=hvB8RSAeuHpGr5D4Vm3uLHj1yMDuoW9Kh8KYzt6v5wo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=nmB2/9pwH0M7tEcCUerk20t895vsooiG0pSLANER+aTv9rFAO+bA0vTjpEGCDoptn5GwVirJX0J+J2zMpcinLp+r5GAKKDbi0vtjHWV2OFMajv+TeNt6paPNAr5vO6rd2eQe2dkoJUB1YZEdyu0S7/D765ykTgKgX+KnjicFHfM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BqqjVNwT; arc=none smtp.client-ip=209.85.214.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BqqjVNwT" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-2163dc5155fso10373685ad.0 for ; Wed, 05 Feb 2025 21:57:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821432; x=1739426232; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=QScxn0YiELptPi/YqbglTfJ8vukbjirrSQIaMxw/KoY=; b=BqqjVNwT4IAscpGSh5w73vyKs2Tn3bRfjkPcsVQ7sqFzz3EbOstGgeg0+qexgeGfwc v1U0GY3r/0EA6+CA36fIZonyx57lGC+7YbbzL8KkGJq3yl9dtfPvbGsp8kR22o8t88um MaMEm+HnMx8JPafMCXkBgi4OlvQWA7DtWM1Vga5Xp9ja0w1dGv68FqYzhlElaliS/orp pWXuINQ08U0zZ6o0wtpEJ4cYZPromdAjFOLs02fsbYbBbfKWqLTKtBriybtFud5eCR5Y /lwivnSrNshVANL7ztf/1PCeR0N22iasZv4RN0DN2O7Ja+l9fP829fmxZTtis9z4D4TU Z69w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821432; x=1739426232; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=QScxn0YiELptPi/YqbglTfJ8vukbjirrSQIaMxw/KoY=; b=kIHej2eBkKONXVyYWtTMq06B8q30i++mhldSG5UXXAbWz2KClSAEjeooqPc+9Bguwp BELvlkIGvof/G+qqT+mgRE+axHGmu5BSl8cz9K5623OlIwbQ84q0wuuPQ2WhxqfAgiAw jWCFtVqxn8ysnHoLGepXfi6/MB9TbcP2jeoboRmbvbSLryHEY+9mHVv8h1wWiRnx26Ia QsIxFFdkAx4wa1TEbcwYGK/Zk0WTRiO/KUVeKUZVtzGp3PiA92MfkwP8NdwXVo1+c7/h EnsX4vu2kJMBVBUWK5uLtyObF9tSl1avEIPH4wHrMzUiQ0ugLMNjRjhJoxUH51Bj85fe QkBg== X-Gm-Message-State: AOJu0Yy9y+OuKvcV+tHrKlyBX2YVCu7dIAgmm0v2xnFLYlG03HhoYsZt sm5yDh76hwEgvz5ufHb4WhrtVkX63excX3Xr8twHvtXX9i2EU4HV85iPig== X-Gm-Gg: ASbGncskKjwbs7JClktAt/XAm5UVA9fi/ccl2O5qU043lkhtQgiqtuv7KGLNalp4R4g kvvtTYELRNwxiXss6e+K1cpA2QihVolZ4ns24RXQUI2hFLRTRm0m722KEYTd+H1qsFqCIhu0l6A okZc6ygsN6B6G3hcIOkJ3cE3QN7NAeK/YoaIkxJvux//jPSt+VvXlzfxLrk+fUpKeshbFvumuI5 EMf0mAMFp0vrGTc58MQzlYAGh6vC8FJcDp4tOtZUvInJLPNUItbWkSmp14D9w0YXgfcKA== X-Google-Smtp-Source: AGHT+IHfmGN0hjAXgOoLNgYnZT3asQn/anfU+LU6fdPl+EMBeOS4f7ZZpLTEddbKJabr1CIITfYvkw== X-Received: by 2002:a17:902:db04:b0:21f:3e2d:7d33 with SMTP id d9443c01a7336-21f3e2d7f3bmr2196985ad.11.1738821431674; Wed, 05 Feb 2025 21:57:11 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f3687c555sm3929785ad.182.2025.02.05.21.57.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:57:11 -0800 (PST) Date: Thu, 6 Feb 2025 13:58:53 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 3/8] packed-backend: check whether the "packed-refs" is regular file Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Although "git-fsck(1)" and "packed-backend.c" will check some consistency and correctness of "packed-refs" file, they never check the filetype of the "packed-refs". The user should always use "git pack-refs" command to create the raw regular "packed-refs" file, so we need to explicitly check this in "git refs verify". We could use "open_nofollow" wrapper to open the raw "packed-refs" file. If the returned "fd" value is less than 0, we could check whether the "errno" is "ELOOP" to report an error to the user. Reuse "FSCK_MSG_BAD_REF_FILETYPE" fsck message id to report the error to the user if "packed-refs" is not a regular file. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 39 +++++++++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 22 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e..6401cecd5f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -4,6 +4,7 @@ #include "../git-compat-util.h" #include "../config.h" #include "../dir.h" +#include "../fsck.h" #include "../gettext.h" #include "../hash.h" #include "../hex.h" @@ -1748,15 +1749,45 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } -static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED, +static int packed_fsck(struct ref_store *ref_store, + struct fsck_options *o, struct worktree *wt) { + struct packed_ref_store *refs = packed_downcast(ref_store, + REF_STORE_READ, "fsck"); + int ret = 0; + int fd; if (!is_main_worktree(wt)) - return 0; + goto cleanup; - return 0; + if (o->verbose) + fprintf_ln(stderr, "Checking packed-refs file %s", refs->path); + + fd = open_nofollow(refs->path, O_RDONLY); + if (fd < 0) { + /* + * If the packed-refs file doesn't exist, there's nothing + * to check. + */ + if (errno == ENOENT) + goto cleanup; + + if (errno == ELOOP) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_FILETYPE, + "not a regular file"); + goto cleanup; + } + + ret = error_errno(_("unable to open %s"), refs->path); + goto cleanup; + } + +cleanup: + return ret; } struct ref_storage_be refs_be_packed = { diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index cf7a202d0d..42c8d4ca1e 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -617,4 +617,26 @@ test_expect_success 'ref content checks should work with worktrees' ' ) ' +test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' + 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 pack-refs --all && + + mv .git/packed-refs .git/packed-refs-back && + ln -sf packed-refs-bak .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + test_done From patchwork Thu Feb 6 05:59:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962243 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 940231F60A for ; Thu, 6 Feb 2025 05:57:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821447; cv=none; b=D9DB2x++j+0/xM0Gx6oX3G8gBgT5/mtDYdIYWaA6M7z2GiwtJ2VLo2cND3CgDR1q4QYE0GOuDLhsJUt7jG6Tgx9M1xS4JVzflKkmrrI5OfXwsFPF7RDj3GW+f/1QxzP/iGIjXUP+LzFY2oR1VuQhE4cDcUgckwm5jrxW5qFVHq8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821447; c=relaxed/simple; bh=gPBfsbwXkeoIiqnOuTz9Bbjo9k2anmZmkafcdQgkwKw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cLjobrZvsxeg/4+5ImjxBJPu9kEkDIr5SRYJDhXpH0gh6GjApI6XKn/FjrDeG51R0u8nfzrewYNu/OvltJ0LWY1uBs1H0VDnV2vobg+3GO37/p0NVJW0OD7xk3mV+jg3QWgiOBk6rsgSOcc8804/A4VkaAUZb3hZ588jjU0iaqM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ia5cY5x/; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ia5cY5x/" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-21f05693a27so7928545ad.2 for ; Wed, 05 Feb 2025 21:57:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821444; x=1739426244; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=mc2Eab1ktUMhP2/CIDAlLxjTUidXxsJnONji6g8e9c4=; b=ia5cY5x/Om41SkplXldWHW9JT4KitKY+VVW2v4tY+Udz4qxhn/3qppA/MwXFlR4oz9 7nuem6nrQ7xXlcrtkba1e0Z6Z0v+ksLmJ58IfrLTI6aPkFt3avalr8S6EadfmlPtSZAa jME9t1diJ1lSN/uaKh5pkgyMr7OaoaGx2prn/gavlzHtKjOW1cuzYaGALl3S5sKtqNIk NASSNJQn9q8/MRSf65SchsaZc9RnPvEO+pkFNl6p6AcIETY/TQwYR4bB67mSSOyPEIUM 9l/rb5Nx8P8Alumy6fbqxloD3VHj7Yn9cN60g4/NCQTSB13Cf1+QlwqbFUHMqWTbE/Di AddQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821444; x=1739426244; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=mc2Eab1ktUMhP2/CIDAlLxjTUidXxsJnONji6g8e9c4=; b=LhSVVMOj4XQIsi71I1c+jE1JqMCsENBy9CvCvq0ppAGzqmY4B9PcSkWo1a9DjvtmBy uCk8fd7zLbRWzPOPTs66QO9QqTst9G1494W72xHkEpW5Vnn3P0Y4CXTkwZHw4JXbK/sK HELSPy4I1HFcuNPIkiKD/9j2KNaUFAsLLAgxJ0EGF3NE7nUDc+cGh4F8U5CSStD4qfnj 2kK18S74XSrr5BdRhPgEklp6nAf1COgIontXOnLqcUkWSGLHy+rhxZ2GkfyB/1MnVGmn FVU1dDlKsWZnrxK2Ehq/sCAHd0K4Vdu0IhdUJtzWR6r3wc8WMpOaETBvdBeJiMLuqjBB 9wZg== X-Gm-Message-State: AOJu0Yx5zE2Slh9sPB7IhN221yQfpSEz+clfjZ+z1SsxduTvMCZDMvhv dORbxGW8W0jUjhQcu6VX5boTtetd72PICoblzqg67ml9XGIDNn1xjx4+6g== X-Gm-Gg: ASbGncvLiOwYfoi5hd9ZegI0iJBFiuhVSB2eqBynnYJIrJDXRWu8lAZvJGxFHOmVfiK 8A9cAa1VnFICwD86DYRWFh0Ycsybb5aK0VXpG9hTl2R1/tZ+y5hRr9e4sAd7ycoiWKSOWuc8Qyg 6adi37RD1FIHfF179zrtCAoFUs5z1vzyAxdnF0Ci72con3CI/+aZyT6mM+6d2SpEAvFuE7zTpjo 6GgadN5OLHN9kIru+T8bt/Qa+cbxtfZUIAgQ0CQtbhViZAQ3iLP3NRuObAp3hrKwnXcbw== X-Google-Smtp-Source: AGHT+IH4u2S4hjHVyGwwr7m48ONJKVAuXrw18VDVDX4t8mDGzR5/zWgOBSgzgPgnfIJnUnxefaFJ8A== X-Received: by 2002:a17:902:f550:b0:21f:6dd:bee4 with SMTP id d9443c01a7336-21f17ebab4amr92313825ad.33.1738821443757; Wed, 05 Feb 2025 21:57:23 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f3683eb02sm3895725ad.153.2025.02.05.21.57.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:57:23 -0800 (PST) Date: Thu, 6 Feb 2025 13:59:04 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 4/8] packed-backend: add "packed-refs" header consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with:". As we are going to implement the header consistency check, we should port this check into "packed_fsck". However, we need to consider other situations and discuss whether we need to add checks. 1. If the header does not exist, we should not report an error to the user. This is because in older Git version, we never write header in the "packed-refs" file. Also, we do allow no header in "packed-refs" in runtime. 2. If the header content does not start with "# packed-ref with:", we should report an error just like what "create_snapshot" does. So, create a new fsck message "badPackedRefHeader(ERROR)" for this. 3. If the header content is not the same as the constant string "PACKED_REFS_HEADER". This is expected because we make it extensible intentionally. So, there is no need to report. As we have analyzed, we only need to check the case 2 in the above. In order to do this, read the "packed-refs" file via "strbuf_read". Like what "create_snapshot" and other functions do, we could split the line by finding the next newline in the buffer. When we cannot find a newline, we could report an error. So, create a function "packed_fsck_ref_next_line" to find the next newline and if there is no such newline, use "packedRefEntryNotTerminated(ERROR)" to report an error to the user. Then, parse the first line to apply the checks. Update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 8 ++++ fsck.h | 2 + refs/packed-backend.c | 73 +++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 25 ++++++++++++ 4 files changed, 108 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index b14bc44ca4..11906f90fd 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,10 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefHeader`:: + (ERROR) The "packed-refs" file contains an invalid + header. + `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. @@ -176,6 +180,10 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`packedRefEntryNotTerminated`:: + (ERROR) The "packed-refs" file contains an entry that is + not terminated by a newline. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index a44c231a5f..67e3c97bc0 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ @@ -53,6 +54,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE, ERROR) \ FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ + FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6401cecd5f..683cfe78dc 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1749,12 +1749,76 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +static int packed_fsck_ref_next_line(struct fsck_options *o, + struct strbuf *packed_entry, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct fsck_ref_report report = { 0 }; + + report.path = packed_entry->buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED, + "'%.*s' is not terminated with a newline", + (int)(eof - start), start); + + /* + * There is no newline but we still want to parse it to the end of + * the buffer. + */ + *eol = eof; + } + + return ret; +} + +static int packed_fsck_ref_header(struct fsck_options *o, + const char *start, const char *eol) +{ + if (!starts_with(start, "# pack-refs with:")) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs.header"; + + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_HEADER, + "'%.*s' does not start with '# pack-refs with:'", + (int)(eol - start), start); + } + + return 0; +} + +static int packed_fsck_ref_content(struct fsck_options *o, + const char *start, const char *eof) +{ + struct strbuf packed_entry = STRBUF_INIT; + unsigned long line_number = 1; + const char *eol; + int ret = 0; + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } + + strbuf_release(&packed_entry); + return ret; +} + static int packed_fsck(struct ref_store *ref_store, struct fsck_options *o, struct worktree *wt) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); + struct strbuf packed_ref_content = STRBUF_INIT; int ret = 0; int fd; @@ -1786,7 +1850,16 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } + if (strbuf_read(&packed_ref_content, fd, 0) < 0) { + ret = error_errno(_("unable to read %s"), refs->path); + goto cleanup; + } + + ret = packed_fsck_ref_content(o, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); + cleanup: + strbuf_release(&packed_ref_content); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 42c8d4ca1e..da321f16c6 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -639,4 +639,29 @@ test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' ) ' +test_expect_success 'packed-refs header should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + git refs verify 2>err && + test_must_be_empty err && + + for bad_header in "# pack-refs wit: peeled fully-peeled sorted " \ + "# pack-refs with traits: peeled fully-peeled sorted " \ + "# pack-refs with a: peeled fully-peeled" + do + printf "%s\n" "$bad_header" >.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs.header: badPackedRefHeader: '\''$bad_header'\'' does not start with '\''# pack-refs with:'\'' + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done + ) +' + test_done From patchwork Thu Feb 6 05:59:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962244 Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 74DC81F60A for ; Thu, 6 Feb 2025 05:57:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821458; cv=none; b=KfUC6Yz+vVkfGIME0N3FIRoSTR+EkGrF85wt087KjhoURTEbersBfTFlY9VtA3nsFSa1YMWydvX+jV2dBoyLEg313yf8IWmjt8gigSez0f/3hRBbNuApau7fchR4PzglonFWqNABdnGgnQOQnIqGoNfDTK4hNtg3xzml/3/YhqI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821458; c=relaxed/simple; bh=7+T55XJ8OYWOOEzXj7D1Ghh3F5Qp2lX+rsOa0qUNJZU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AuOLArWIkDXfTu8Y4mYON2IZBte+4K/+fLgvZci43wSHRUfdEFXQSAHU3/Y2qutPHwGWO33wYX3BpiOl/heW12NbYi4+bnganNMo0RiUYwO0aEaTWmoWNmg8xawYGRQFulD1lU2i9ZmQgtG8ZZJ+ppQ97Sc1Mob+Vh4ob20Wnig= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=TlQYSbcE; arc=none smtp.client-ip=209.85.216.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TlQYSbcE" Received: by mail-pj1-f52.google.com with SMTP id 98e67ed59e1d1-2f9e415fa42so865525a91.1 for ; Wed, 05 Feb 2025 21:57:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821455; x=1739426255; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=JIaRyQxKZ5Dr/nA1G+kAaUr/gDDbTIE36CuAYz0P/cY=; b=TlQYSbcEeoFidUgwkc7PTRz0pR3ZN2TAkNBncv/xU7JKPcABK5T6h/FuyM16TAUH9o cE/kKTydvyritV4JOq8gl9LP3H0SBPRtXDhqRlwVMmjSuQ19gM+91HTeaK+MdlTVHVmj M/oksjuIykfoyTAAvGvD6becFhPo9bK9/ofirE2AcVUS0NCZlRrWebXyYrpoRkt4d3On NAizEYusUmgqYyhZYfoIaafkO7zrdqU6/XF7V2np2Bnc2OWrmIWbrYF3JATKgTvO7uCs jJIS/YFds7Fl3BAvjSYrqscnvrEVQ6i7kiw4W+HIMfgl4xMVwcu1I36WwP/zGGvZm11U 369Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821455; x=1739426255; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=JIaRyQxKZ5Dr/nA1G+kAaUr/gDDbTIE36CuAYz0P/cY=; b=IEF63tMEzTSBGdKwu073s5ObjIq9F/PZWPQL/klUQPb9cfwkOlz/pWvgrbfA0XZ342 Isb4/SkAfXX1Pwszjcm88nZnd/8KWnP7ECfoNmJRqdVmuryDab8tFU8ZpHtcX4d7OD5R zlJ4m3fpUMR0tv726VjsiJIS2r4LgAafIxiYEsEYnbBcRBRyN3qJxtGY09UyqmFZwv3f 39/NKOxregNIpOL/iS4DcODKDSsInhmXWHKOHp857L9N/BQ/yl8axMKugZAH4MDjWpNM ORuV/Ghy8jrSIH1JKLNaht1EtBV58EdBbjHKmV8Lagdb9aDKb8JnLKa6Hl8i8KIpkfpU hSeg== X-Gm-Message-State: AOJu0YyZ+gJP4bj8d+E/F5BEZmtqPArw2z4+pRCsizb9fi+WNAlrppDy kNPErkM40AuKlLCvLRcqZloM+LDr6vnLZV/Pl2RcYnBi11LIx4EhXBfK0g== X-Gm-Gg: ASbGncvxvXFhlr6c2Avas9EHknPgFPDrroYOBHpryLIidwWmT1wwfvhV5v05T9Y1d2B 94yHOMfKHIll5EprSt2dJRaeE4Ur5B4/c0267n003UbI+JT7ED5a0rXWY+ZLAgXU53E0os1lwRw 8/Jolx83VXJu/aY6x9Ttx+CPDO/0nCpWjKpN3EAHTJcQvgm09jHVi9MNJyuH3tOYpx4ujK5hZwY WG+oz8FbYnAAh1lD/JB9fR8OFCM6MkwYfZIB6pGmg/pJh4ek83dNVEpJPBpVSfqu70aKQ== X-Google-Smtp-Source: AGHT+IHJV/3GlsONFT2vQ84TXkr2Ll1bfD+cRKLNXvCmZ7TN7pmHdI4j5Vzt7sTbQmjNHmCx05R93A== X-Received: by 2002:a17:90a:d605:b0:2f2:a664:df20 with SMTP id 98e67ed59e1d1-2f9e0753d07mr9152514a91.7.1738821455188; Wed, 05 Feb 2025 21:57:35 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 98e67ed59e1d1-2f9e1e2352asm2688771a91.34.2025.02.05.21.57.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:57:34 -0800 (PST) Date: Thu, 6 Feb 2025 13:59:16 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 5/8] packed-backend: check whether the refname contains NUL characters Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will use "check_refname_format" to check the consistency of the refname. If it is not OK, the program will die. However, it is reported in [1], we cannot catch some corruption. But we already have the code path and we must miss out something. We use the following code to get the refname: strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf In the above code, `p` is the start pointer of the refname and `eol` is the next newline pointer. We calculate the length of the refname by subtracting the two pointers. Then we add the memory range between `p` and `eol` to get the refname. However, if there are some NUL characters in the memory range between `p` and `eol`, we will see the refname as a valid ref name as long as the memory range between `p` and first occurred NUL character is valid. In order to catch above corruption, create a new function "refname_contains_nul" by searching the first NUL character. If it is not at the end of the string, there must be some NUL characters in the refname. Use this function in "next_record" function to die the program if "refname_contains_nul" returns true. [1] https://lore.kernel.org/git/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/ Reported-by: R. Diez Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 683cfe78dc..c8bb93bb18 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -494,6 +494,21 @@ static void verify_buffer_safe(struct snapshot *snapshot) last_line, eof - last_line); } +/* + * When parsing the "packed-refs" file, we will parse it line by line. + * Because we know the start pointer of the refname and the next + * newline pointer, we could calculate the length of the refname by + * subtracting the two pointers. However, there is a corner case where + * the refname contains corrupted embedded NUL characters. And + * `check_refname_format()` will not catch this when the truncated + * refname is still a valid refname. To prevent this, we need to check + * whether the refname contains the NUL characters. + */ +static int refname_contains_nul(struct strbuf *refname) +{ + return !!memchr(refname->buf, '\0', refname->len); +} + #define SMALL_FILE_SIZE (32*1024) /* @@ -895,6 +910,9 @@ static int next_record(struct packed_ref_iterator *iter) strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf; + if (refname_contains_nul(&iter->refname_buf)) + die("packed refname contains embedded NULL: %s", iter->base.refname); + if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { if (!refname_is_safe(iter->base.refname)) die("packed refname is dangerous: %s", From patchwork Thu Feb 6 05:59:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962245 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64A951F60A for ; Thu, 6 Feb 2025 05:58:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821483; cv=none; b=pEu/b+FwWoF+s6X+T04OWDpwEhonGDVvB/BrNku0x6DKJLoMWMy9TObYghLcp9Y3YCc+sda/Y0kiOiLrriTK+wnzrow2cse62Qkw9bIZoU/4Fm2cj01teAyOOUTeozE723WPeh/XiSyhiC8U3/HAOcd5vdaFFUUyAa7ferX47yE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821483; c=relaxed/simple; bh=zY+FQ25a97zTdGt9/RkWu2Nt2ZD1SMg5KaTyMqO0wDI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hvN8yaLrZk9n9R63tAHBAKaVBbGjlLJiPLa4INWBwodVFNIolg+pTUZ8GwerEpALnnAEzlxuzgqZldE/fkXWJcq7m8UYIo5X+jh7R6bmOlnbRmzYh2bGuPmI70QBsml2kh4ManOJDk0YRKff9adnpqOMZXDEQ+RtF/SJ0gKQ3Ao= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=imbrvRRW; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="imbrvRRW" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2f42992f608so718056a91.0 for ; Wed, 05 Feb 2025 21:58:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821480; x=1739426280; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=0tMypgULqhjYOPFr4x7VBSGXLenm5Qf+EYmP6BmvI/Y=; b=imbrvRRWjXDGwNtVdqPsMgmhCCTtFHzjxT0jbuSsAVz4zZaYm6BtNH/bEwL0QTdq1S woawbgZG1HfRiAcjteu0nrp3ToJcVlfj84+w2FilwaYxWazxoOV8wo+/ZxB9RWykXsQv ycRigrRABK//0+Sa+CZaG3rpmCt9KOJksZk27mVFAXUnGSAZm3PoKp76dLCBTg1sdcC/ PlnekUNxi7ZdRlTnpRSrkycvCafOhiicsLfqR3HR7RuoWdcbb/3SIWSnz0B0qNvmP0z3 +DIDXxt/VhbDErFcq9kfHXOR2UocOnAtlIbrbnks4qSU47KXK96i/8Tt5kxcIZgxPDPu rJ9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821480; x=1739426280; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0tMypgULqhjYOPFr4x7VBSGXLenm5Qf+EYmP6BmvI/Y=; b=tDmX02w2Z5SnbUQM66fpQka+BWkcKysgQWjd02tG8Br2xu6BTmbI2GInYT18LV/SVZ Ecgbldeq25dmjG46E102FZySsc0LPdLOaZp5aWVF4fEJBweeXAGvyKMdSelQyntZPbMw Nl4UrnpJowoh/Jx6/qs1494Jl8qETGSU+kwYr7YqsJtYkBq853oc2MF1wifgShmY9oRb vVNm6l3/YgKF35pJxSgkJuyUHnhvKNIxry58WPsw2LJYCkDVxEqXAVm+D0+27VdFbjCQ iTa9TahSTJtXaQo+ra/Xmn3MnXaMpZUdywEyQEjWXtBe/5B2y2/VL7ByNvMt8d9XozYb PBgw== X-Gm-Message-State: AOJu0Yx4Y7a0/Po3OKbgAJH6Waro2YobYHn7ZrUhoDq1btyCZSDg+Rt0 v8Dmi/CQX/h2Z1o4ouIVXviISJAXPj2S4H9PvkMCx54ISfhmGuevomQ6Fg== X-Gm-Gg: ASbGncsLEZGtHmeIfbmXG323rZq1mPTHW847UXI3TXflyPYR4gS2C8mNS2J9lDY96gf 0LhwNqMU+iexb3lmO/uAfUniXmpTD6ULFgEFgy/NjAq+35ymyOrqWzkz15Y0s0hyS9FmRpvGXln lh1vz3EAZkngPSNNiLrp9HJY1cJ091/jp7XeckXxvNY81SazVHwsus3teZo+JGEAc0cWhXzguwP qlJnjnCorvmwYWsNM5/TI0QrqT1/s7QnB2ja7UXB+iLWlOzstvI9QvGOIm6cbgkbu1/OQ== X-Google-Smtp-Source: AGHT+IGICMU9GdRklbZP5Gcqxr3fx50VN1PWiZpx3cdlYfOZeIydejz7Y9TdzQLLNpGQnmRthKXsfw== X-Received: by 2002:a05:6a00:400b:b0:72f:590f:2853 with SMTP id d2e1a72fcca58-730351057d0mr9333399b3a.9.1738821479686; Wed, 05 Feb 2025 21:57:59 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-73048bf143dsm445192b3a.118.2025.02.05.21.57.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:57:59 -0800 (PST) Date: Thu, 6 Feb 2025 13:59:40 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 6/8] packed-backend: add "packed-refs" entry consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will parse the ref entry to check the consistency. This function has already checked the following things: 1. Parse the main line of the ref entry to inspect whether the oid is not correct. Then, check whether the next character is oid. Then check the refname. 2. If the next line starts with '^', it would continue to parse the peeled oid and check whether the last character is '\n'. As we decide to implement the ref consistency check for "packed-refs", let's port these two checks and update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/packed-backend.c | 95 ++++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 42 ++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 11906f90fd..02a7bf0503 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,9 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefEntry`:: + (ERROR) The "packed-refs" file contains an invalid entry. + `badPackedRefHeader`:: (ERROR) The "packed-refs" file contains an invalid header. diff --git a/fsck.h b/fsck.h index 67e3c97bc0..14d70f6653 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_ENTRY, ERROR) \ FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index c8bb93bb18..658f6bc7da 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1809,10 +1809,83 @@ static int packed_fsck_ref_header(struct fsck_options *o, return 0; } +static int packed_fsck_ref_peeled_line(struct fsck_options *o, + struct ref_store *ref_store, + struct strbuf *packed_entry, + const char *start, const char *eol) +{ + struct fsck_ref_report report = { 0 }; + struct object_id peeled; + const char *p; + + report.path = packed_entry->buf; + + /* + * Skip the '^' and parse the peeled oid. + */ + start++; + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid peeled oid", + (int)(eol - start), start); + + if (p != eol) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has trailing garbage after peeled oid '%.*s'", + (int)(eol - p), p); + + return 0; +} + +static int packed_fsck_ref_main_line(struct fsck_options *o, + struct ref_store *ref_store, + struct strbuf *packed_entry, + struct strbuf *refname, + const char *start, const char *eol) +{ + struct fsck_ref_report report = { 0 }; + struct object_id oid; + const char *p; + + report.path = packed_entry->buf; + + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid oid", + (int)(eol - start), start); + + if (p == eol || !isspace(*p)) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has no space after oid '%s' but with '%.*s'", + oid_to_hex(&oid), (int)(eol - p), p); + + p++; + strbuf_reset(refname); + strbuf_add(refname, p, eol - p); + if (refname_contains_nul(refname)) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "refname '%s' contains NULL binaries", + refname->buf); + + if (check_refname_format(refname->buf, 0)) + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_NAME, + "has bad refname '%s'", refname->buf); + + return 0; +} + static int packed_fsck_ref_content(struct fsck_options *o, + struct ref_store *ref_store, const char *start, const char *eof) { struct strbuf packed_entry = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; unsigned long line_number = 1; const char *eol; int ret = 0; @@ -1826,6 +1899,26 @@ static int packed_fsck_ref_content(struct fsck_options *o, line_number++; } + while (start < eof) { + strbuf_reset(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + ret |= packed_fsck_ref_main_line(o, ref_store, &packed_entry, &refname, start, eol); + start = eol + 1; + line_number++; + if (start < eof && *start == '^') { + strbuf_reset(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + ret |= packed_fsck_ref_peeled_line(o, ref_store, &packed_entry, + start, eol); + start = eol + 1; + line_number++; + } + } + + strbuf_release(&packed_entry); + strbuf_release(&refname); strbuf_release(&packed_entry); return ret; } @@ -1873,7 +1966,7 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } - ret = packed_fsck_ref_content(o, packed_ref_content.buf, + ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf, packed_ref_content.buf + packed_ref_content.len); cleanup: diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index da321f16c6..3ab6b5bba5 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -664,4 +664,46 @@ test_expect_success 'packed-refs header should be checked' ' ) ' +test_expect_success 'packed-refs content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + git tag -a annotated-tag-2 -m tag-2 && + + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_2_oid=$(git rev-parse annotated-tag-2) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) && + short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s\n" "$short_oid refs/heads/branch-1" >>.git/packed-refs && + printf "%sx\n" "$branch_1_oid" >>.git/packed-refs && + printf "%s refs/heads/bad-branch\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/heads/branch.\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-3\n" "$tag_1_oid" >>.git/packed-refs && + printf "^%s\n" "$short_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-4.\n" "$tag_2_oid" >>.git/packed-refs && + printf "^%s garbage\n" "$tag_2_peeled_oid" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid + error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\'' + error: packed-refs line 4: badRefName: has bad refname '\'' refs/heads/bad-branch'\'' + error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\'' + error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid + error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\'' + error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\'' + EOF + test_cmp expect err + ) +' + test_done From patchwork Thu Feb 6 05:59:55 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962246 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E680A13C80E for ; Thu, 6 Feb 2025 05:58:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821496; cv=none; b=ieTqHDGwxTK5JoRYWjrd0B76lF0L0InptTSyhoyjTlWwVDVoLboWDYSMuXWNjsFSvQrUqGLDSMfE/AjxyMMhm7MArMpfesKianGd8v+xVIfjHZ/P6Byr1SXP/ml5DXMfTSp9GzG4u+88ChwD/mutT6D6B4eOzZ/anMLOb/hjzcQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821496; c=relaxed/simple; bh=VFERlilAa6J7pmVgmfD72zYCNVFXnmyRuQHMKGutrpI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ilYCKVaVkIj8d/arxpSuC8DK6byVaJdQjkw6DKtgqzS/SQ/wwwsGhnyFcmFgzGrtz+WzkDPM+h3qWN0KAnW6Ll8waD7q/2tySg0dcD5qZ8WP9s8/TyZGQ5dOi7n5HSSxC2OHchIBh0sHBeI80JA2pYn8L+c9SioMl78+cjSZw9o= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=THXXTHsI; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="THXXTHsI" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-2163dc5155fso10383505ad.0 for ; Wed, 05 Feb 2025 21:58:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821494; x=1739426294; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=0H/TpLiBfAlzOPKc+ELuD6P7WRl6QS/owcRlTdjQ5qI=; b=THXXTHsImytyjMXyNlBxHwKElYFjjUGnmhTzdnMKFH4IIzc7CzOOyY8ShPWVKXmEjM U5u0HpaGhbrND1xxCbw8ytmbdJM6OfgiNvGVhaLOZfQeqvhthh3MYEcCTjCC3Q3hsCub EiGYSnaVbx3ygcaiRGwiIhhs/4XvkH4g83rJnVZyQMk2TtZry1PjZt2reTi0umeh720n KjDzjkLqLwlMdvbmYzD0oZc8o8wAbhCbWwmCo5+FT9G/E0izm4W4yKeasNVUQWHh9c1t E6yhc62/ZIl6ns3w1PtGIR3l+C7Hz+yIcD8+8ASQy/FnrfVxu9SnS/FhXzeEwZ7F2qff OJIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821494; x=1739426294; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0H/TpLiBfAlzOPKc+ELuD6P7WRl6QS/owcRlTdjQ5qI=; b=bXdB3KRNVu6c3VJ5Jqxr3gGL2MZkG49sDEkavybuJRlZzFtHwQztXPbC8TiWeDNRF1 RqReinObE5+B4F1zu20zgMR87Kf0Mf/QfnzL9Hl1FmjSWTv3OdmIWNjO5yzb8o2ick0i RNGQtJjtB8UnhhLSBdHKYS579rOAJ9ikVz8sxazLPXzr8pTb7LVSSomZFGvAe3FP7nBT z6rCxGuIq5s13QYPT08ySqBVNQwOpZ4JjxbPImOiW3xeLOy6FbcYP5oA+cn+5NWsXKAu tmHMrvZsFg/lB5kNs5mYg11Ht0FzfSBiNskDfwpd2w5IhJmXPIfUZobGbYxlElrGRl9W +NmA== X-Gm-Message-State: AOJu0YxlQdMXCj2nJm0ME7RPDdwmnMMCvlaothIJHb7Rzr5WP38E4bm4 r0x26unrzo68zJPz5g40FXT0YPGe65k/kMcgtlPiPeW06fbviOMZbYH8qA== X-Gm-Gg: ASbGnctTSky2ysbgjLDBZIycVIfx72z0eIa44ypCIt1qfLIBs99S87lY+BoeQiKuOee irwfC7JvXaIQ8RRwQNyhGVZCdrNJmwODsudrsHcS/t1xcrwOZvk71i0T9UrdRmeASv9PGqlCdmw gVZyqCRxKCvUngmDwUup3yrXGmEvWYZ3Z14f0guUk/Z7YDapkeMauBqOGFMHA3zD2sXJmivisRR G6zQ9oPOIJLMppBMIsjxGgHAwtZdX6qTgtMda7WUAChfb3sr6KCDfx9ona8FFGdRZZQBQ== X-Google-Smtp-Source: AGHT+IEaFFwKvI8dV4/MHTQ6s1TDcNHzuJOY3fpODXbS9+LEibas61M2I7Aq6cuiscR8ctoo1M3snw== X-Received: by 2002:a17:902:e803:b0:216:725c:a137 with SMTP id d9443c01a7336-21f17e26d6amr108438575ad.28.1738821493909; Wed, 05 Feb 2025 21:58:13 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f368a25casm3901125ad.218.2025.02.05.21.58.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:58:13 -0800 (PST) Date: Thu, 6 Feb 2025 13:59:55 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 7/8] packed-backend: check whether the "packed-refs" is sorted Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When there is a "sorted" trait in the header of the "packed-refs" file, it means that each entry is sorted increasingly by comparing the refname. We should add checks to verify whether the "packed-refs" is sorted in this case. Update the "packed_fsck_ref_header" to know whether there is a "sorted" trail in the header. Then, create a new structure "fsck_packed_ref_entry" to store the state during the parsing process for every entry. It may seem that we could just add a new "struct strbuf refname" into the "struct fsck_packed_ref_entry" and during the parsing process, we could store the refname into this structure and thus we could compare later. However, this is not a good design due to the following reasons: 1. Because we need to store the state across the whole checking lifetime, we would consume a lot of memory if there are many entries in the "packed-refs" file. 2. The most important thing is that we cannot reuse the existing compare functions which cause repetition. So, instead of storing the "struct strbuf", let's use the existing structure "struct snaphost_record". And thus we could use the existing function "cmp_packed_ref_records". However, this function need an extra parameter for "struct snaphost". Extract the common part into a new function "cmp_packed_ref_records" to reuse this function to compare. Then, create a new function "packed_fsck_ref_sorted" to use the new fsck message "packedRefUnsorted(ERROR)" to report to the user. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/packed-backend.c | 131 ++++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 63 ++++++++++++++++ 4 files changed, 183 insertions(+), 15 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 02a7bf0503..9601fff228 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -187,6 +187,9 @@ (ERROR) The "packed-refs" file contains an entry that is not terminated by a newline. +`packedRefUnsorted`:: + (ERROR) The "packed-refs" file is not sorted. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index 14d70f6653..19f3cb2773 100644 --- a/fsck.h +++ b/fsck.h @@ -56,6 +56,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ + FUNC(PACKED_REF_UNSORTED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 658f6bc7da..0fbdc5c3fa 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -300,14 +300,9 @@ struct snapshot_record { size_t len; }; -static int cmp_packed_ref_records(const void *v1, const void *v2, - void *cb_data) -{ - const struct snapshot *snapshot = cb_data; - const struct snapshot_record *e1 = v1, *e2 = v2; - const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; - const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; +static int cmp_packed_refname(const char *r1, const char *r2) +{ while (1) { if (*r1 == '\n') return *r2 == '\n' ? 0 : -1; @@ -322,6 +317,17 @@ static int cmp_packed_ref_records(const void *v1, const void *v2, } } +static int cmp_packed_ref_records(const void *v1, const void *v2, + void *cb_data) +{ + const struct snapshot *snapshot = cb_data; + const struct snapshot_record *e1 = v1, *e2 = v2; + const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; + const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; + + return cmp_packed_refname(r1, r2); +} + /* * Compare a snapshot record at `rec` to the specified NUL-terminated * refname. @@ -1767,6 +1773,28 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +struct fsck_packed_ref_entry { + unsigned long line_number; + + struct snapshot_record record; +}; + +static struct fsck_packed_ref_entry *create_fsck_packed_ref_entry(unsigned long line_number, + const char *start) +{ + struct fsck_packed_ref_entry *entry = xcalloc(1, sizeof(*entry)); + entry->line_number = line_number; + entry->record.start = start; + return entry; +} + +static void free_fsck_packed_ref_entries(struct fsck_packed_ref_entry **entries, size_t nr) +{ + for (size_t i = 0; i < nr; i++) + free(entries[i]); + free(entries); +} + static int packed_fsck_ref_next_line(struct fsck_options *o, struct strbuf *packed_entry, const char *start, const char *eof, const char **eol) @@ -1794,19 +1822,33 @@ static int packed_fsck_ref_next_line(struct fsck_options *o, } static int packed_fsck_ref_header(struct fsck_options *o, - const char *start, const char *eol) + const char *start, const char *eol, + unsigned int *sorted) { - if (!starts_with(start, "# pack-refs with:")) { + struct string_list traits = STRING_LIST_INIT_NODUP; + char *tmp_line; + int ret = 0; + char *p; + + tmp_line = xmemdupz(start, eol - start); + if (!skip_prefix(tmp_line, "# pack-refs with:", (const char **)&p)) { struct fsck_ref_report report = { 0 }; report.path = "packed-refs.header"; - return fsck_report_ref(o, &report, - FSCK_MSG_BAD_PACKED_REF_HEADER, - "'%.*s' does not start with '# pack-refs with:'", - (int)(eol - start), start); + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_HEADER, + "'%.*s' does not start with '# pack-refs with:'", + (int)(eol - start), start); + goto cleanup; } - return 0; + string_list_split_in_place(&traits, p, " ", -1); + *sorted = unsorted_string_list_has_string(&traits, "sorted"); + +cleanup: + free(tmp_line); + string_list_clear(&traits, 0); + return ret; } static int packed_fsck_ref_peeled_line(struct fsck_options *o, @@ -1880,26 +1922,80 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, return 0; } +static int packed_fsck_ref_sorted(struct fsck_options *o, + struct ref_store *ref_store, + struct fsck_packed_ref_entry **entries, + size_t nr) +{ + size_t hexsz = ref_store->repo->hash_algo->hexsz; + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct strbuf refname1 = STRBUF_INIT; + struct strbuf refname2 = STRBUF_INIT; + int ret = 0; + + for (size_t i = 1; i < nr; i++) { + const char *r1 = entries[i - 1]->record.start + hexsz + 1; + const char *r2 = entries[i]->record.start + hexsz + 1; + + if (cmp_packed_refname(r1, r2) >= 0) { + const char *err_fmt = + "refname '%s' is not less than next refname '%s'"; + const char *eol; + eol = memchr(entries[i - 1]->record.start, '\n', + entries[i - 1]->record.len); + strbuf_add(&refname1, r1, eol - r1); + eol = memchr(entries[i]->record.start, '\n', + entries[i]->record.len); + strbuf_add(&refname2, r2, eol - r2); + + strbuf_addf(&packed_entry, "packed-refs line %lu", + entries[i - 1]->line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_UNSORTED, + err_fmt, refname1.buf, refname2.buf); + goto cleanup; + } + } + +cleanup: + strbuf_release(&packed_entry); + strbuf_release(&refname1); + strbuf_release(&refname2); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, const char *start, const char *eof) { struct strbuf packed_entry = STRBUF_INIT; + struct fsck_packed_ref_entry **entries; struct strbuf refname = STRBUF_INIT; unsigned long line_number = 1; + unsigned int sorted = 0; + size_t entry_alloc = 20; + size_t entry_nr = 0; const char *eol; int ret = 0; strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); if (*start == '#') { - ret |= packed_fsck_ref_header(o, start, eol); + ret |= packed_fsck_ref_header(o, start, eol, &sorted); start = eol + 1; line_number++; } + ALLOC_ARRAY(entries, entry_alloc); while (start < eof) { + struct fsck_packed_ref_entry *entry + = create_fsck_packed_ref_entry(line_number, start); + + ALLOC_GROW(entries, entry_nr + 1, entry_alloc); + entries[entry_nr++] = entry; strbuf_reset(&packed_entry); strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); @@ -1915,11 +2011,16 @@ static int packed_fsck_ref_content(struct fsck_options *o, start = eol + 1; line_number++; } + entry->record.len = start - entry->record.start; } + if (!ret && sorted) + ret |= packed_fsck_ref_sorted(o, ref_store, entries, entry_nr); + strbuf_release(&packed_entry); strbuf_release(&refname); strbuf_release(&packed_entry); + free_fsck_packed_ref_entries(entries, entry_nr); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 3ab6b5bba5..adcb5c1bda 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -706,4 +706,67 @@ test_expect_success 'packed-refs content should be checked' ' ) ' +test_expect_success 'packed-ref with sorted trait should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname1" >>.git/packed-refs && + printf "%s %s\n" "$branch_1_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname1'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$tag_1_oid" "$refname3" >>.git/packed-refs && + printf "^%s\n" "$tag_1_peeled_oid" >>.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname3'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + +test_expect_success 'packed-ref without sorted trait should not be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + printf "# pack-refs with: peeled fully-peeled \n" >.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname1" >>.git/packed-refs && + printf "%s %s\n" "$branch_1_oid" "$refname2" >>.git/packed-refs && + git refs verify 2>err && + test_must_be_empty err + ) +' + test_done From patchwork Thu Feb 6 06:00:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13962247 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE8B913C80E for ; Thu, 6 Feb 2025 05:58:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821509; cv=none; b=en9j9B9NrG/PjjCzcvgV45rqxTXeBRqPf8Qp7aQ0iCQkmRSsMHFe16tDZcP2xnW3iXl7izvyL+m9KAdK6q8S3OO1WNt5rupVzhT/w1k0Z0J866irLLRS0MO9Wvok2yIBld1P/6UlHc4Dal79kRXxUcrOgcsy6b/J8ouULLnrqeE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738821509; c=relaxed/simple; bh=sCHTY907ZcUJdidd3z1mE+0s0riA1TYdk6sIyEuL+OI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LuE3QXIwrZ419RlFt4+kjpS0N7jTD4dQp+frEUPjduML7yxnkaomsaHGK6ygN6oZWQK0gKLHXmB9ZgxlOszhYySiLo8l6onRGXfbc0Cq93ZopA9C04iDkBGK/aVbpQG21JuI7IHIQRreWPqsVeNUbS5PZQWJ+C28+o4iimGJnKk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=dok0tPLu; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dok0tPLu" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-21628b3fe7dso9918305ad.3 for ; Wed, 05 Feb 2025 21:58:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738821507; x=1739426307; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=RKzLIbC2fKixEovH6wh792J66Eu8I7gLyeAj7XJqT1Y=; b=dok0tPLumFLYyCVGMmswbzwakGhKnkdMD+Twee87qP/mRv8TEiAOUx/Pa44YWEjnXM cMltoLSWDVyKmManrnN3Banlhop752k0lCUbRdFNByAMaYeioAk2ub+Bilyhd//Mpi7Q wEI02HsogIgIf3Rj0JwaObTLbLuQCPmfP/68phq+HFrV8CJMQ/va0Kva+3WMGGoqlJ93 m1PdeppdXOqyx1Ak5hnPT4OrAQbS3r4zce8U0C0cJy7+udaMFDC2fKNrggBzj956S3f1 EdU8cvgpoTHewUNLj+JeU7cqoya4oyBIjK/roRB/8WrLFuK06p6KuDxlVsr6fercQ10T Mnhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738821507; x=1739426307; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=RKzLIbC2fKixEovH6wh792J66Eu8I7gLyeAj7XJqT1Y=; b=OrXLw2BtaBJtf9K7xFmKHfTAT3bLOmFhtzyIVsTkjauXIFiVhLH3P7GrCewPEHxq39 9xiXF1E7x4KpoxhkFt1HL8Bnx902gLK7U7NvIljksXVdIOaLHSgtgKz1kPfEpv97wXCE AmuPl+hLpvXCspIfIXV2Rmo+pjAhyB0Oyy1u6HrRhAXEJcC0DbZBz3jzQJNGVzFFRqrP 4NK3kRHYXuwro1InHxEk/z2vHMPIxprjM4wdoSYeIH7Ab6M2/ZVQ+EiFy+qZJngiah5s jlloiUiHk4AdoZuFbimWaU/5ao7X4qsqJVnOzXumJhlcy5hSuiqsZAXbKzKKVtahR4As Onrw== X-Gm-Message-State: AOJu0Yyx/LsVPv4UfIXuWvrpWFxFUHvPTrH98i14KTfBGnMjGtmkrIeI ZeDU4yt+vXdqJ9ZfL082Q9jd6tt4JPZFhJxBwV5VSvcog4rglmVlW5d6Sg== X-Gm-Gg: ASbGnctnWrET7TgC/EWLpZNj6iPuV7AV/ndEpN3mjqN1HzBbCI5t+RL245yZyIyu/ke LztmTC6GNWyjGtNYxZkqnIR++1JjWsdOe8i9YLLPNs44tgk5TnHo7hZCZ4KQO7J72XoTdSvLfRG PL94+pPnAArAvfDtvKlwPQcpCjxP6ayN+jfVXIz3MCtoWO48pqdx+midJihHFqZN5dMcbaJITsE Z5PgXxwAGn+SblxFICrbxBN2m10er2ONLcJ8+TJ3zzkDeCjvZY7RE2OALTapn1RUWQrHA== X-Google-Smtp-Source: AGHT+IGh6zRH/rPGEFLUFt/7shixQXeooq8PTxHqTSp8nkBb4JW+EG9dViIq8TcYXguaAXw2MFVxWQ== X-Received: by 2002:a17:902:d508:b0:215:9379:4650 with SMTP id d9443c01a7336-21f17edf7d4mr86479155ad.42.1738821506710; Wed, 05 Feb 2025 21:58:26 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21f3650ce93sm4107345ad.51.2025.02.05.21.58.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 21:58:26 -0800 (PST) Date: Thu, 6 Feb 2025 14:00:07 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v3 8/8] builtin/fsck: add `git refs verify` child process Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: At now, we have already implemented the ref consistency checks for both "files-backend" and "packed-backend". Although we would check some redundant things, it won't cause trouble. So, let's integrate it into the "git-fsck(1)" command to get feedback from the users. And also by calling "git refs verify" in "git-fsck(1)", we make sure that the new added checks don't break. Introduce a new function "fsck_refs" that initializes and runs a child process to execute the "git refs verify" command. In order to provide the user interface create a progress which makes the total task be 1. It's hard to know how many loose refs we will check now. We might improve this later. Then, introduce the option to allow the user to disable checking ref database consistency. Put this function in the very first execution sequence of "git-fsck(1)" due to that we don't want the existing code of "git-fsck(1)" which would implicitly check the consistency of refs to die the program. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/git-fsck.txt | 6 +++++- builtin/fsck.c | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt index 5b82e4605c..9bd433028f 100644 --- a/Documentation/git-fsck.txt +++ b/Documentation/git-fsck.txt @@ -12,7 +12,7 @@ SYNOPSIS 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] [--[no-]full] [--strict] [--verbose] [--lost-found] [--[no-]dangling] [--[no-]progress] [--connectivity-only] - [--[no-]name-objects] [...] + [--[no-]name-objects] [--[no-]references] [...] DESCRIPTION ----------- @@ -104,6 +104,10 @@ care about this output and want to speed it up further. progress status even if the standard error stream is not directed to a terminal. +--[no-]references:: + Control whether to check the references database consistency + via 'git refs verify'. See linkgit:git-refs[1] for details. + CONFIGURATION ------------- diff --git a/builtin/fsck.c b/builtin/fsck.c index 7a4dcb0716..f4f395cfbd 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -50,6 +50,7 @@ static int verbose; static int show_progress = -1; static int show_dangling = 1; static int name_objects; +static int check_references = 1; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 #define ERROR_PACK 04 @@ -905,11 +906,37 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress) return res; } +static void fsck_refs(struct repository *r) +{ + struct child_process refs_verify = CHILD_PROCESS_INIT; + struct progress *progress = NULL; + + if (show_progress) + progress = start_progress(r, _("Checking ref database"), 1); + + if (verbose) + fprintf_ln(stderr, _("Checking ref database")); + + child_process_init(&refs_verify); + refs_verify.git_cmd = 1; + strvec_pushl(&refs_verify.args, "refs", "verify", NULL); + if (verbose) + strvec_push(&refs_verify.args, "--verbose"); + if (check_strict) + strvec_push(&refs_verify.args, "--strict"); + + if (run_command(&refs_verify)) + errors_found |= ERROR_REFS; + + display_progress(progress, 1); + stop_progress(&progress); +} + static char const * const fsck_usage[] = { N_("git fsck [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n" " [--[no-]full] [--strict] [--verbose] [--lost-found]\n" " [--[no-]dangling] [--[no-]progress] [--connectivity-only]\n" - " [--[no-]name-objects] [...]"), + " [--[no-]name-objects] [--[no-]references] [...]"), NULL }; @@ -928,6 +955,7 @@ static struct option fsck_opts[] = { N_("write dangling objects in .git/lost-found")), OPT_BOOL(0, "progress", &show_progress, N_("show progress")), OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), + OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), OPT_END(), }; @@ -970,6 +998,9 @@ int cmd_fsck(int argc, git_config(git_fsck_config, &fsck_obj_options); prepare_repo_settings(the_repository); + if (check_references) + fsck_refs(the_repository); + if (connectivity_only) { for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(the_repository,