From patchwork Wed Feb 26 13:49:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992460 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (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 D4D4F21D3FB for ; Wed, 26 Feb 2025 13:49:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577771; cv=none; b=LfKjtSRp+icD/P36oW/KhbIMnQJsk0rOwA5Z/d6+AqgXUnz7MHm26uuumrGB6dGI9QNMev7Oq1BsLwiP1mR3tyJ1sc1X6o7EzxyugXqkH/4QMMIrHbD3a9XDy0+MR99K7SrCfuOTvBf5WoiXl45Fjw8N0XItB0bGjRqWuWxIy78= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577771; c=relaxed/simple; bh=NiSz0kFXgj2oW62wBNpIa05qAzJdZQp1gZun8gTT0Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lowW4Hy+HYkSgXRHylLzNYhJPpLdLaH2teeV9cp5yNp+itp/SvnhXI3GcEFF4b5qoOYH/o9c2n9NiWMI0x0FYVTVgTci9qg2Z9y3u8RKaCM8nManvHI8cybjFmxBBQOV4oUkVTZxi+qOdLNJAoU4p29OBey5oVjcu5+Cq7uVlIM= 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=JfDxu8JA; arc=none smtp.client-ip=209.85.214.177 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="JfDxu8JA" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-219f8263ae0so144953705ad.0 for ; Wed, 26 Feb 2025 05:49:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577767; x=1741182567; 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=JfDxu8JArCcbxUB0UdAyVughRCyiB55xiSVoJKnqgrP19KPhCbT8nWcfyg+fsr3NxW YBjFNq8GyqxFx8EF6JkKeZf29HuQ7W4pc0UTBSajnwL6wkYkHGdPcYT9oteArYvAZwOq bitNVrZuvH8tQWK3ScvR2wBK9uU9wqsFThc5LyViCmJEo0D/kiS+Beu/4Dj57dtAA4bD MGXOKWNd+nLPyP7qKjlF56WzAWnwP1N06gj5dQ2X7ifBVglAssTZr+Na5nnNCVRkDBTa 90tGrtWIhINlc0ApWwikvjIxgMkNOo6WCd11r7j567x+I85A4Rcpck7mVyku3r+NqMxl 97ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577767; x=1741182567; 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=h5qCDiUtZ2X9mk1/cEgqzqTrUuqjuns0ZrpPSn0yi6rTx4SqClNZKt06AwAHFo6aQC z2l8DxR7rCbjwdTGHwFQBUDflttOLT4zzq9MYmBYVLD+kTt7l4rT5kOiMQnGwEjYSbB2 UhqYuDUqur3OtoP2ivOdUVVbKCSo/u1/dUTzPCAEuW+QmPuvLPGEvlehj5DItnCNNEh3 KITKRtoIDbaUWnpAalrPat6VoAA6qgjWHORa7LkKsx7RjTa4JgyuWET6cHNDjxpgjluq 6JbKIXkYIn7LQgyB6Qik3d94+Fwcm3wXk8gt8WBiX3XprqJvrScTYplEJxLDS4ijfkBV m7Ng== X-Gm-Message-State: AOJu0Yww5IjhC6MloT6rzR9fZZ/9pvvtaASTpkoBymSvDyr3Un05osYn YHvK17F7I3M38UecXyQokxAnvIfPUAeD13Y19EeTZuSVl7UrbUxVOtol5Q== X-Gm-Gg: ASbGncv6Ia6LG+x4WMS3PPjh0chT1EWySbZ4o3ffSaoOOwCdG2kYtajAUK36vxnREhi ij0BUGJepRtogLxuqpRKEgcoekhsCIHw4UAwF5sgevtZXxPGg0VgjPxxwTizFOGMAuN3N/umIG2 FLbs98La42TjVvuUmTRuIjt0DsYQoE8wDavBzUsgpFuW8j8ywj0KydUIKrEzCnkByxuWrdmbOyF CtYSUW366nSMzl82tL1b/KNkNiRxi7nZZikopCfy3V0jz8StxwQ2Dod7vrUcJHweo+3gE/rmi1a O/WUzCdlt47bfVr5NCCWbA== X-Google-Smtp-Source: AGHT+IFxB9uApI9xeznO+mH9kgXLBJOSvLekil8k8+2wf3PIHnMBmrntpHzWoNqdd5yUeL8pLGJ1WQ== X-Received: by 2002:a05:6a00:c81:b0:725:9f02:489a with SMTP id d2e1a72fcca58-73426d72880mr31270746b3a.17.1740577767302; Wed, 26 Feb 2025 05:49:27 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-7347a81ef23sm3496646b3a.139.2025.02.26.05.49.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:49:26 -0800 (PST) Date: Wed, 26 Feb 2025 21:49:34 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 1/9] 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 Wed Feb 26 13:49:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992461 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) (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 794632163AA for ; Wed, 26 Feb 2025 13:49:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577778; cv=none; b=WFHKx9bBYbibrRatvSXqvlBrrGp11NwYcsq/xD5sjrrgzLZ04FuMffhTmYtPEbEkIIIIQO1wI57HB1GXKhkPhuLslXRSMNjtio3zXfBBQ+9chsuQ284TsqgSSd16mII7BdvyVAQB7Arkr9IEnTNpLgO8zeTFqHcCgDWe4lumMTk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577778; c=relaxed/simple; bh=91zfuoLoQwgrw0TlcEswgv67DDwoA1hs2npm5rnt8rU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MymVu+7yx4/Qz/Vm5R55f7ekJ0/oTlXom6cL7KsZZ1lrjO9RinEliGJqPk2fNaGYAqrqwIJsPp4hfoMGK1IqkJ0bpdn8dSVe7wn4r0zWnfv73lVbym/ONzMpNDN3JDlJrSQw+ucpvGSSVlHSwyzCuwcl5gwe2/3RaOPMknPCPz8= 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=Na6xTuTt; arc=none smtp.client-ip=209.85.214.180 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="Na6xTuTt" Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-221206dbd7eso141884405ad.2 for ; Wed, 26 Feb 2025 05:49:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577776; x=1741182576; 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=kn5rEPmZULamx/qgElC3YO1aE9Gj/Fu7ltZCF1dh5ds=; b=Na6xTuTtNLyeDpgiGko1pv2sBsc0gjpnD24qGrvYUeV+tnHJ8wcDbgOfgo/6yAf9co /rJUGLJJaGqqwgF2+qv8OOyUrr2AT++U95RlGyoy6ZUBpDjsZTScvrNYh6KG9nx6Z3wk yiUKeATAgCPlahkqOZfg29aZ6Ki2y3/VSIdUfO1Kui1PJOfndh3oFU6R+y9dkejoVKNd SA0e0J/myKhAg6ZiWQDAPjtXXrTWBiJzoufqMcSejmvaRGMw1JYTWiik47VVyFL8Gekg /y9XP9PFBS0mD4KpchwY5MIgHUBqaDwGqMdj8e8ElLOS1ikSlRXigrTPrPMZ3v0ziqyl 8uWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577776; x=1741182576; 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=kn5rEPmZULamx/qgElC3YO1aE9Gj/Fu7ltZCF1dh5ds=; b=t637L+68zhxipSpQ6cg+7s7HSUzK4k89u8GUWe8fdbZTIZ13QTTiHWpAORRDO0vvzy UGRWe+QMYpB2sbuv+MhgvWICyItnHFnSm3GKAElO602vhz2FRUN/bEyzc4/kVMEjOTeV ikD7DXIFXYxoT6hdoL0UEr1jJ1j9XlEbqJ/Opjp0/vQ4YqOYucmvbfUoNQM1cCkZ1jlV qW4FV7DgEq9ekNC/vvbYRz/7s8ZvsnKWRDam0H0mQNCmMBvIAjbSsspm+K4WHPG9J+JH VKMyfMC6dwpyjZyKWccBIYdQVAbktzAQxXDoHZDLj1dW5UceEbXnjhdJzHFXX4Duq6sl BxOQ== X-Gm-Message-State: AOJu0Yx+dBP/dBzExAtV4ZqSZjoyWcqU5R8kMSfSalrZsAxjFSzIho3S YcdCzoHJoILnnGpJg3oqUPQUv+Tm02bGo/PEZKXWLaSr5FCOI0Z+SkBePA== X-Gm-Gg: ASbGncvfMLHuTbHH5wm88Sm7GjVM0FD74ULodNv1XYh1KweYtDdx5tqCsaL7hi3XKAE IlWIb2TZdRspRj4ZHCKbZ2cUaBx7nHLWftlYhuO3KJFFm81/8Nk5W1f0HQUQFi8bAM5PiuNkzTn AIxJ7rx5e+Jm2gUSxc7bTTFHZoAQeYCoToGs1+PR+fJfMyUtQLsnEmLU0q4nQ34LetZv3IkiGX4 e9oQqwboMN+/iDIUJgiUfm/qEN9L5hkNK8zfzQs0kX4thNq0Rn9rkjs5ajYGl9d6oZO/TcgM3tk c/B6ak2NFb9IUFqzFKwjUg== X-Google-Smtp-Source: AGHT+IFo+c4NtH1VRUYqZKUnND4CKvYC2/v0d56OcJ/CS+WNGta86T4vCN/woRoU/Y6s5ovmwMVX1w== X-Received: by 2002:a17:903:94c:b0:220:d81a:bebf with SMTP id d9443c01a7336-2231fe0f976mr62910395ad.0.1740577776158; Wed, 26 Feb 2025 05:49:36 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a0a3befsm32352005ad.165.2025.02.26.05.49.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:49:35 -0800 (PST) Date: Wed, 26 Feb 2025 21:49:45 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 2/9] 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 | 8 ++++++++ 3 files changed, 14 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 d4a68c9c23..d23482a746 100644 --- a/worktree.c +++ b/worktree.c @@ -198,6 +198,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..a305c7e2c7 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,14 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. Skip reading HEAD allows to + * get the worktree without worrying about failures pertaining to parsing + * the HEAD ref. This is useful in contexts where it is assumed that the + * refdb may not be in a consistent state. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Wed Feb 26 13:49:53 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992462 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 58ED221A457 for ; Wed, 26 Feb 2025 13:49:46 +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=1740577787; cv=none; b=QmgmvdDiTOagy4F3kx7O85Aj0KkmIJ/VWQN+c4AjSgIgiNCtdjACAAztn+Q1V+Sn9nCJqbx1EbyPiB+5uvctzL7lWYUVBhdaSRgsoue+UEfhvFANs0a1sCshbG5ZY/VNERDz4Lsxy2OmUkJPoydVWXuSXKNYEHQTY6gmIXY/1EY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577787; c=relaxed/simple; bh=IZV0J37PkKofX6QMa/x2sR6IZ1lLNiXZzPzAI2+OL3I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=A4vV4G8jIXU0Rmk9CDa46LH7FnbSpFZeVi/8tZh8T2XAfzxbV2v8KBksf1IlC3tanpnHoH0iT69uCb6QSBllnP/GeJBtSh+xtRbPjeC5nQhH/wcbrDNsI2GCdrjMISXqrPSw2FyR0dhtYRa16yJRRU4hI0h7Fli12F+OKc9FqJE= 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=Gpo4dQ5/; 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="Gpo4dQ5/" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2fc4418c0e1so1474296a91.1 for ; Wed, 26 Feb 2025 05:49:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577785; x=1741182585; 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=+mgNGpP6ABinu1LeaSa1E+jezbzNNGy/bMIQGIBRb2Y=; b=Gpo4dQ5/I+b7xwfkw5j5zNFOBMQfzycbIT03pyYnROxvhjgLQHvSRyDF87yh3xPyye ZGR6QYEDhWEifLfgoZaN5tOrNVjEk+xQypoGqEoYr0iPSqvXPQDmXvN0HiXBgU5VT8it u95pm+WMRKcjZR+gKmZZy/wUS5mYX27uIyic2uwTVl+U5Vs6OSxzQ8EGWBcryOP7GluA 3o1y8+6TpiqwXwymf6EChchmrdWwusJTT7w/9Tcr0vY068LOI2DBLEa2pDD6ko/VhaTt d3b04a7CHNrkxHEmdiZDHef8dybziXwMN/+iP+L1TU2XVE3s61rX1Ao/Sny7ks2KC8Qv Sy/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577785; x=1741182585; 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=+mgNGpP6ABinu1LeaSa1E+jezbzNNGy/bMIQGIBRb2Y=; b=dx+06KIAAys9aJDM8lYPvrhM+QUPoKp6GnilZg/79uuM5XfD7cNOHg+drRJk9+3Agf g+oYzgATWb3pTE1idYwCFBWJ0OoHdnLYN8tWCBAf+GdJKThPsyQyIkuIQiYQ/o2IeDS0 0dJVGgog5HSAcMIgFeJlpv/Fv5PMV/vEoDtA10FeKLXoH6QnSUB0b0eZqEPcLobDeKrI fRbXULr0nHrkJbUq52L/CnJB4G4V1TC2XvOAxLf+7ulUCmiVH6keciFZxi7GiF/mzNFN /MKRkox7C5ymV21QphEDoh+mTVxU8lAOzsFN5bna05YuhGHKbd3+xZ1IKJDoD32ImdNe ibIw== X-Gm-Message-State: AOJu0Yx1SGjAHd4H/Pbc+K9RPGQTQFifKzNAolxLtga44qhQ9kdJQlOn ZWSYv3agKtlgW8EqOG9SkVqi+4pa1kKWQAnkt4JWprkhxpaJFNoN3YZU+Q== X-Gm-Gg: ASbGnctsYOEFNxCU8mDJlt4Wbe1DvsEtLHV9sRWCtZxY8/f/TlC11ewIWX71GqV9bU1 nQ1FIJuB9fWm2l/uLIOJmRRxTREAGN3PvDPffxFim0JlJ1uHXKCyzYX4PLmeiJQDh8lHwaNH8+f wvNoJfKMFiGHmYq2vlmN6bYtZf2WLAIKhXpWEYIRQ+quphyiHMcWHcNDsrP+XlFmZzfRT2xBaJP XXm8h7q/U2o5xn4BIytQTh8QFDTfGcCzS8QztJk7oYeboB7NSrPlY1geJ9ZIfZ5/co82ZQeNy/l xyiX8UkYL+uaXoZaBxM46g== X-Google-Smtp-Source: AGHT+IGezl4s/cgUtO/IjS4W+m5bNbDJLSjRFb9+M5eFyK+NnHalPZL+5pZI/EARKA5HBJOkeZzq9A== X-Received: by 2002:a05:6a00:991:b0:732:6276:b46c with SMTP id d2e1a72fcca58-73425a1fab8mr36113516b3a.0.1740577785106; Wed, 26 Feb 2025 05:49:45 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-7347a72f4c7sm3415876b3a.79.2025.02.26.05.49.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:49:44 -0800 (PST) Date: Wed, 26 Feb 2025 21:49:53 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 3/9] 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". Let's verify that the "packed-refs" has the expected filetype, confirming it is created by "git pack-refs" command. 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. And then we use "fstat" to check whether the "packed-refs" file is a regular file. 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 | 50 +++++++++++++++++++++++++++++++++++++--- t/t0602-reffiles-fsck.sh | 30 ++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e..f69a0598c7 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,58 @@ 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"); + struct stat st; + int ret = 0; + int fd; if (!is_main_worktree(wt)) return 0; - 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 but a symlink"); + goto cleanup; + } + + ret = error_errno(_("unable to open '%s'"), refs->path); + goto cleanup; + } else if (fstat(fd, &st) < 0) { + ret = error_errno(_("unable to stat '%s'"), refs->path); + goto cleanup; + } else if (!S_ISREG(st.st_mode)) { + 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; + } + +cleanup: + if (fd >= 0) + close(fd); + 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..68b7d4999e 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -617,4 +617,34 @@ 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-back .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file but a symlink + EOF + rm .git/packed-refs && + test_cmp expect err && + + mkdir .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file + EOF + rm -r .git/packed-refs && + test_cmp expect err + ) +' + test_done From patchwork Wed Feb 26 13:50:03 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992463 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (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 8EC5F84A2B for ; Wed, 26 Feb 2025 13:49:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577797; cv=none; b=DFYpWQ2n8qi50ZW4/LwazMDVjUtOSTnPYVObPdVTIszlBDbuB+fUiQ5/pKv5FJyg5U0i5vmLGYG4T8Vurv/tCC3fg/0QprOt1psfYi5R8/Lypd1Fn1AhOOuWWXLSLJuVrCdMbemh9yMpajPqFAHKIuGrtPeUzdlgO2ZSnoUAPyg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577797; c=relaxed/simple; bh=9hCCkJ+oO+v030s9XZReTLgv95w22u/vH/zWWdvw5Cw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=NyxbmLaeImUNxh8YKLrhyPD6icx5gwD8Gw9/DG0UOj7BmJw5ZId9Z3jJY4G9spAebB2iTq4ls1WkY61DKTvaqcOC++pYFu6fyv0Q7P3E99BCj0+W5cMuJ0oPAUWQN+0L+kPaofsnjd9FxGGbsi7F+YSFjMuiljO8B3C/M8x3ZL8= 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=AX0pGLZ2; arc=none smtp.client-ip=209.85.214.177 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="AX0pGLZ2" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-22113560c57so52904565ad.2 for ; Wed, 26 Feb 2025 05:49:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577794; x=1741182594; 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=ELSZTUjJoWvdKhri4SRQIotqUgncELf7FCh1d37h7C4=; b=AX0pGLZ2//YQONFkPJWNg91Wgi6Ie15T7T7W16z0rUZ9Ik2CMpadmhqD3nuSoJkOHZ m4qThXkXp4JRiTgR4975wNI97YNhe0L5JpWo1uoIIphqRwJXKB9S2wk8nIpNloaNF61R gN60kLyFCcnOUMsGF7wwV5rLaGP2uKljuCoOq5EaAZ4QuCq5anvA9bbJTqDDIkwrP2VN Q0i8BteOLPgfIPdxAx4GXj80LMCywSK9XO+vsJX8iFaQbYqR/siJE5UETSySOLPa+4iW n2XPTIIFx5jbQaM/3EZojG3gVJI38kABo0rcWfElmbFYFUrkTVhhLVsw68lJpiqFv1Pe Df0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577794; x=1741182594; 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=ELSZTUjJoWvdKhri4SRQIotqUgncELf7FCh1d37h7C4=; b=PmEF+w2Xb5+zIeBaw4AlKZzKOcgKqvDPUGLLYtlexd4JEa3Mzx4udWxJLQ8Xw55lMt LeXxPose5GP+9Cp5Y13LejxpEyf8caXJy1iIZdtgv4gHed7Dkauo8jr3qq4o54Y5U6JL lDkR/3IbL3Qkxy46FTzdE/EL/CQW9fo9ZpS/doCX1ndKbnSuM5ORU/BM3ZMVb1T3Tozs 9iEEEpTotMQNxHr3KA9l6pc2FqqHwzn8XGwr5GAagd+mrVX9TrLmHG9SIqo35VBfgwaU khZea5nf777TfRtBaRPq44kLGwdkJGZMsKGRiTL+7kwMrGkgjq8EGT0w+VWtfFuuTs2O LLOg== X-Gm-Message-State: AOJu0Yy4N2IGy4rob8ltj1jrRtctds98CD7p4wmqUSOB4GzQYOKY3lmK dHaa2EOvvDrKT8LW3eskJlThXHIZ0fYguuZ/OBadVAVzKFSznwtwMsiWdA== X-Gm-Gg: ASbGncsNe7RxJXl30MRYSE5hOWKSaPCSJc8FCvhwBF0oecwBdd/4WX8oaLtoTbT/Ho+ Yeip84rdnAz6WCQj9890qtJ2JXzrGB7tDajvvQjcsWiVv+u4iO+fUFzZ1jCen/crhymh4eq6kTQ tujnUonuwJE5KBRNBhrv1rJERuJs/kdCK3nSKN+ndZ7rB5Q2fw/+/5eJGoH+NOThPlXxPAaCUyX YXHH5keeLHLWbAGZYVfO9NwC3Lea1e/zQQa0uavEkTxPGuFkh0jSrOmk2O/n3N+hPJGjONFEOuQ eSZ6RtYKcctSs76P1gdSFQ== X-Google-Smtp-Source: AGHT+IH1v5FygJ2osnNl+p8B8IiZpvWfvgcrwX0T4X+BVulorz5PChwW0eLwEmtRT6O2Z2IrqR6NSA== X-Received: by 2002:a17:902:f64c:b0:21f:49f2:e33f with SMTP id d9443c01a7336-221a0ed7885mr342037405ad.21.1740577794350; Wed, 26 Feb 2025 05:49:54 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-22334002d9esm9904985ad.229.2025.02.26.05.49.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:49:53 -0800 (PST) Date: Wed, 26 Feb 2025 21:50:03 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 4/9] packed-backend: check if header starts with "# pack-refs with: " 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: We always write a space after "# pack-refs with:" but we don't align with this rule in the "create_snapshot" method where we would check whether header starts with "# pack-refs with:". It might seem that we should undoubtedly tighten this rule, however, we don't have any technical documentation about this and there is a possibility that we would break the compatibility for other third-party libraries. By investigating influential third-party libraries, we could conclude how these libraries handle the header of "packed-refs" file: 1. libgit2 is fine and always writes the space. It also expects the whitespace to exist. 2. JGit does not expect th header to have a trailing space, but expects the "peeled" capability to have a leading space, which is mostly equivalent because that capability is typically the first one we write. It always writes the space. 3. gitoxide expects the space t exist and writes it. 4. go-git doesn't create the header by default. As many third-party libraries expect a single space after "# pack-refs with:", if we forget to write the space after the colon, "create_snapshot" won't catch this. And we would break other re-implementations. So, we'd better tighten the rule by checking whether the header starts with "# pack-refs with: ". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index f69a0598c7..3dd3fec459 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -694,7 +694,7 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs) tmp = xmemdupz(snapshot->buf, eol - snapshot->buf); - if (!skip_prefix(tmp, "# pack-refs with:", (const char **)&p)) + if (!skip_prefix(tmp, "# pack-refs with: ", (const char **)&p)) die_invalid_line(refs->path, snapshot->buf, snapshot->eof - snapshot->buf); From patchwork Wed Feb 26 13:50:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992464 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) (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 4BB5C84A2B for ; Wed, 26 Feb 2025 13:50:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577808; cv=none; b=ZH31zbWs9kEOIFOkIOgXb4lUeW+MCboz5kl44ZnIvejj/0x7YcvG8+bIeDFGKW66bEYaTOZn9Tjmz+q52hA330a2nsSazflpBl7OsIYuHYf/cOCCLU/NlTUBHOgEWDi+66dn8yR41sDqZLmzowBvg6Uw5HS4NJkB2Y5qwUwiw1Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577808; c=relaxed/simple; bh=BgMWZaz0waGjfQ+acAtJPqVUH8HRL8HUlY772yXUf+s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Gnkd0nmzqDzTy2E1xNqspSFYLZ50eYgrcziZxi6K43W3uIvCJyNH5Ke5Fa0nGEz3RD3wzyWFA12k7hsQjeUgP4L9g/zRVkBnnGZ/KGCoMkAx+ct2KAmD0smEJDXh8jSI9YoZ2O2qfBC/x/o+UZZWkJh5dABLJ3KID/5GlNH8Gto= 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=K6NOmXIQ; arc=none smtp.client-ip=209.85.216.53 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="K6NOmXIQ" Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2fbf77b2b64so13625749a91.2 for ; Wed, 26 Feb 2025 05:50:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577804; x=1741182604; 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=cCRnXhlIBy9kxJdFEvVm7Zei2EMNOIDE+Q6hVs9UerM=; b=K6NOmXIQCI10pHO4FT65w3wmHy1iIsw+ulNMoYk8DA7Lp5XPj4NqKoobJ0GleKwUvc QoPus/AOfqDWnKAz+xzsoCtKxcyJidkcIPqQFwT9WwjSrVKzvWfjQ5Exy861bJoSMloN Gqj716RmgpXREnREgRN9t3EDKj8ZkS2lkVhRvxFE68LhaPSQ460S6FCCU/xZ3YjSAR4b uD/hhB2k992uvm2+bDX1DUiVSZE9X2hErstP/VayOCogj57Q/HlM4YNvJk/4p1rpljwO zTw/ROAmYtdG7cmuwA4zP4YSVBbX2PxZRTnro/hoU/TQNZibWL3XTLUPjRJSlyP6yH7/ IV1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577804; x=1741182604; 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=cCRnXhlIBy9kxJdFEvVm7Zei2EMNOIDE+Q6hVs9UerM=; b=O3zmZKJFBE9LHqoy0qJsFz/7vybPDzitHYjFijtRP8tReZwQwZCqYgNyIioG/elIK9 Q2IG67A0jxAJAZkym3tIehpp6f3029T2PKAVDqx+l6Mg0bLj9lOXXIvJIE1GzkUGnSKM fzFzwT2pSsRLzFAIsjFpWAQp0LCT/gwjKpeod8yjDgccTFdPTHDkj0sDqdQG0ORExrA/ b87Fy8a7xaV1oC0ksolE8eMksNR21+znsFa1Wz5axlWSy2OIW2JW21XjpQ/fqIp6PY51 DZ3GsyJokgVUIzE4OCJDvvthA5HO+mozLvEM4CEEk05WSB8EHQd15DuvRGkl4JO26DvF Syhg== X-Gm-Message-State: AOJu0Yw757VM12R+NTd+GvjA3N/xMMc7WtaKXhce9d04NjtrI4GEfQSI 9h1kf0tPHMjhgheW2YVGO4EavO/nHgkJpN1uKpG3BVRDjKaaiyQlOe7Oyg== X-Gm-Gg: ASbGncs/vtdfCjKrOszjoMLomeA9bHh2q6dADyqiP80V5eZ4ghCXBXKSgtQutHSCXYZ AZuO6xhxnsYKSz7MerfDPWTja8ozJXE9vMy+Bute/deu2K8v9pWw8MenM3RG9QZ1lhrEdk06vrY z9Ei5F3MGIaWY/dS/UfxDSABGAcFfspYojNWNvOtOxb24MHx6Seq3tcPKZnQfqw678clkc1Tz3+ G7oBSnenoYPBAyZsLS5kWP3xyjvu8yHsBf6vnHfHBcnxac4K2DVfzOMdGSfwoJB6PtT2o7OOis3 O/udOYRi819v8uPuQKfY9g== X-Google-Smtp-Source: AGHT+IFgT+r6MhD1ywHlcRztU6gaPxwDGducHJJNYooFaIkY9ZufXZ+Ukk8cWfgTOCtiFvrEQ0wSCw== X-Received: by 2002:a17:90b:5547:b0:2f9:9ddd:68b9 with SMTP id 98e67ed59e1d1-2fe7e39f0c1mr4573543a91.26.1740577803908; Wed, 26 Feb 2025 05:50:03 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id 41be03b00d2f7-aee6f8c06b3sm725868a12.30.2025.02.26.05.50.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:50:03 -0800 (PST) Date: Wed, 26 Feb 2025 21:50:12 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 5/9] 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: ". 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 and runtime "create_snapshot" won't complain about unknown traits. In order to align with the runtime behavior. 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, use "open_nofollow" function to get the file descriptor and then 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.adoc | 8 ++++ fsck.h | 2 + refs/packed-backend.c | 73 ++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 52 ++++++++++++++++++++++++ 4 files changed, 135 insertions(+) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index b14bc44ca4..11906f90fd 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -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 3dd3fec459..b00fca6501 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, + unsigned long line_number, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + 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; + strbuf_release(&packed_entry); + } + + 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) +{ + unsigned long line_number = 1; + const char *eol; + int ret = 0; + + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } + + 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; struct stat st; int ret = 0; int fd; @@ -1797,9 +1861,18 @@ 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: if (fd >= 0) close(fd); + strbuf_release(&packed_ref_content); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 68b7d4999e..74d876984d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -647,4 +647,56 @@ 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" \ + "# pack-refs with:peeled fully-peeled sorted" + 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_expect_success 'packed-refs missing header should not be reported' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + printf "$(git rev-parse HEAD) refs/heads/main\n" >.git/packed-refs && + git refs verify 2>err && + test_must_be_empty err + ) +' + +test_expect_success 'packed-refs unknown traits should not be reported' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + printf "# pack-refs with: peeled fully-peeled sorted foo\n" >.git/packed-refs && + git refs verify 2>err && + test_must_be_empty err + ) +' + test_done From patchwork Wed Feb 26 13:50:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992465 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) (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 3947584A2B for ; Wed, 26 Feb 2025 13:50:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577816; cv=none; b=ICGGCb73XVx0HceVE5o1GhOgKhqiWw17eyZ+4A0guJ7sD0y3kJVFZQ1kW1IB1OaVYk+4Ra+npq1aaLrnloZFd4KtHE/LMvqgUMF1eqVfChGtUmtsBikcen13yIpCNjPJU+kjcgpjgv4oXBA2qh3g2s9rRJLs7nLp0BPOD2bxlwg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577816; c=relaxed/simple; bh=PNW0cEBGXMMy6yEq1XteCd6b2wGVd/X0vf95GE+Lwaw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YI2l4NnSrgG9gMX9gDVmzwKcR4Ce6FJvUpE2FBappl7yHZ/F2Bkwsu6FiL5Az4h55DWGFHDnt0zPRPNiiPxCzglj9WoWlQYDFTmPHIFW9Z3aJYUOB8nsIw7rWNbXbr1nzhl27afgWWJQciZq9j5nJrH7F/njrWiQLs7ANC7OXro= 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=kcJ3R+J4; arc=none smtp.client-ip=209.85.216.53 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="kcJ3R+J4" Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-2fc33aef343so13649901a91.1 for ; Wed, 26 Feb 2025 05:50:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577814; x=1741182614; 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=QnSizut7KyAt1a1NZL3LWl/aUIax0VTYua/Q8bKzBfw=; b=kcJ3R+J47VLP/qXmHoZzlkGrYsx+wJk2GMSyL6Ma3NMusMmBXOb9Ernp4YZTWp/8gp FKaGTEd0sFY8rzLpoHOrlD9I1zjHmz46eeeAfNwGKbSGOGzV6jl7txzESpIhG6jY6oXd d9vtZtECl4C4P5IG9dgyLC9bS7bv6qvVGkGSbX1KJOgAOd3iFWbiOPFX+oCcQr1F7dDz NRDqDNaRmXm8nl4BJZ4wiWjU0pMPJcdHGCLqX1CRV7qXNqI7d5bSow4rvyk3/devgXZF /RzqX04Tjksmj/jcQlrio+gKp8qsshi3iX3RdtwhYY31C1mnS8VHlP4jKIkAGG0tKabC tfLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577814; x=1741182614; 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=QnSizut7KyAt1a1NZL3LWl/aUIax0VTYua/Q8bKzBfw=; b=ODDtamEf67YfpBE4NzWc5DqTVsTZodI/9n0XOVfTtVMS6t7zXzQYZ1vZcaZVX95SUm ue7DkX0IPwbbaNIHGEFQ2vCG127WvWKHuiGwsfkfu4fqKt6QwqXYyNZ+gwKoti3fIsCM A8yVnhOPqsstk07buN9+DPX7QeAfE8jnA8Gqgjjntfd2l92vLOICNtWaMgEBmYF5EACG W1oxxcUTxhqbDE0B1+zAUIL3sXVNVlUrNA7GQo1Pt2ankYxrIQDgk3qDGVVMMvOzwuNP 4Zf1foCmtfCB5vae+eHQf30e1Tjc0PjS+YmoCamvwHKFNr6fsnbsLczpklnbjFa3QvR7 pQlA== X-Gm-Message-State: AOJu0YymYrcPAi/6ToTbGY1OrW2gX8T08BNIQI/2Z5dsb37q//2GTA0P mnZWExxSQBJWztb3ZFMXeLckTIhuizQhrJhOCv7Hq4lxpATV3OfZ2+VVEA== X-Gm-Gg: ASbGnctSka1rgzYAaIoDnT6lkg2iyzafxdtEjnDMzVr+ArRebAwLGxDjDHkwqU78YB1 eKqI77RFDeTvk4yW42or5lD/e0MPu495pJZXbB45M+PopZa3g+8UTjwPbYL4GOod4fuzN68pjn7 SYrMXobBOF2erVLyNb4/TsJaQR8ztXvnS9SO915me9sIY1lKYqfOwvm5EgLyFI6eMvSelGkFmZi WUM7vsdI0BK2JabofZf4d75f3eTYwIM/wMirh/smjUtW0WdDbiNwUAAK3QSvKM20CIiGBjenYGZ lp3YJ9m6XifLnOieHpxPqA== X-Google-Smtp-Source: AGHT+IG1yDd0yoY3nOHdAl0MF4a72eXQvbB4CVJm20+kW44w6ZLVQhJWx8jl7RM/KVKmqLXf06qHjw== X-Received: by 2002:a05:6a21:103:b0:1ee:68e3:ff45 with SMTP id adf61e73a8af0-1f10aecf2c2mr6652581637.35.1740577813890; Wed, 26 Feb 2025 05:50:13 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id 41be03b00d2f7-aeda818a652sm3133399a12.37.2025.02.26.05.50.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:50:13 -0800 (PST) Date: Wed, 26 Feb 2025 21:50:22 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 6/9] 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 b00fca6501..6e7d08c565 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 Wed Feb 26 13:50:32 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992466 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (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 AB8E384A2B for ; Wed, 26 Feb 2025 13:50:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577827; cv=none; b=GE/qPrNMheZbvpGppGuzJcWF/u9/K6XHhThJOdET2rcRA05IVNAZqGGdo9lhRoLxpIeni+rXNPWSwUo7+/gDZConF2t05nDPbekhLcvykpngUh6elq6YI6Tj98TbMbo++EjJ5Sfq1v6wYEosPxWf5w+XcSJUMgfwCwk3zXQUHsY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577827; c=relaxed/simple; bh=oFTBlsy9IjXJjInJlm+ScXwZ/9yKwBDQlYoGLLEiTdI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=M3Ec1zMxL32HYPV14NJ0HQTwMV7MEEgj70f5rxuB23tqUwwRLczrUaM1wluftiqJDGGHFQPRUKawM/fD7HOAmS2/Tc0E/lyY6Eujxtp3vNbd+WcCvRDieMrXmhZZQpQI6cX5qefyCckP8qKosNirAoEnb8ca41ogRhtfgi4vlMY= 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=LgJonDfI; arc=none smtp.client-ip=209.85.214.178 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="LgJonDfI" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-22185cddbffso15320285ad.1 for ; Wed, 26 Feb 2025 05:50:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577824; x=1741182624; 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=LfOeXxaMjNG0QOGpu6hUW3RzudpGpSVGY8SDb+OpIVA=; b=LgJonDfI/dH9p3M/wBeu1bqhmytZYYKJvsxQoCWeo9V2gZfZZ2U66c2xNljyj83gm1 LetOWU0dHJhTBai4eSpYWfVODmkLXQEYS7QlHqcX0oNIyBrnvORx0MGNMPk/VdgITkgB UQDe9G10Eav116FYcVG0JnQM5ZXKFrkBusEMmXGlG+rR2ehgXQW7FHdzH4QZfTUKO+r8 KE9jzrofadX5WiwSZIYGX1rq1aRcgJua1tjDYf9UFIy8UgGmzZbj3x1nlRFkE5S/70ju 1cEZFEGO6LNyeWgUGuVuYka6YPNVtdlksNrmAc6KuoeNi8qbl0fV/7SHQxAahnTFi9LX 2mWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577824; x=1741182624; 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=LfOeXxaMjNG0QOGpu6hUW3RzudpGpSVGY8SDb+OpIVA=; b=f2t5rigHjJyo5qUu08XuRQ5gwtvxQB/vmFf4JkQq7WNJhSInPit98MnBwOR5yWmRLT e5HKntA7VbpGVdQbBmHT/KDbvPm4egAFabLTVNrBmJ5Lsr5FUKJxRtucEFaIUqvoyS6X Rowui2bTsjP2ZP6v8tQ7QXBMDqifh3e0ctd3wdkntIdHXvUzVEN/kSumiOgjVIcB+O1Z Mj9ynGRsyCzYEOsKZzIOWxkELwNCdM+xNb9hMC3iN4Ps3P7/a0vTFIeq9UD31Zv6vrwv K8J0ErL31H1RC7oT0g++R+Ks26JDLX5SRzYDvZb9sk3VTE+vFJ+qM3SfOziltsUn6Aw/ LwhA== X-Gm-Message-State: AOJu0YyVtrVZNDNS6YAs5x8REs00THauWvl7aXSbtlx06lpUrGcFBka4 EC4/aH4Rc7FlNaWJpSBG2/hIuw1JbV9BjVEZUkptqfE53O4CQYdUatfCzw== X-Gm-Gg: ASbGncvZ5tuoRtFh6CsXCaDDalKtDmdXCkKSlar1+tBea8FdJtQAdmjzUISYrzg0ybK /h4zdZBuFQHlu372CYjIYeeXJOcP/Bl5DOWtpCW5vRyup6Q6fhRmgi1SodCI3v9npbHQUS7prFS iF7dNwEZgMrB/E7jh/y+h1l3dM5yJnFc3q2MRYVLH2Vqp4V1EZ1TUv3pEtvm67DI888s2cv/HYk wG7f4Fj110rbPr3knsUK6Imc5mqzDfPzpukftuRfznmoAjxMjjnzwtzkeQTvnoJc24YG7gOC7s/ c3tA2PzPcwVFcBZrrxkeDg== X-Google-Smtp-Source: AGHT+IHFp3sfrZRUj7LL3WWt7hCmSEQZZS2QaErMBXlknm/DNQNNrRTnoxcXa+gFVxaUt+J1ACALRg== X-Received: by 2002:a05:6a00:8219:b0:72a:83ec:b1cb with SMTP id d2e1a72fcca58-73413f844bamr36715419b3a.0.1740577824435; Wed, 26 Feb 2025 05:50:24 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-7347a81ed1bsm3525371b3a.142.2025.02.26.05.50.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:50:23 -0800 (PST) Date: Wed, 26 Feb 2025 21:50:32 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 7/9] 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.adoc | 3 + fsck.h | 1 + refs/packed-backend.c | 122 ++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 44 ++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index 11906f90fd..02a7bf0503 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -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 6e7d08c565..8c410fca77 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1812,9 +1812,114 @@ 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, + unsigned long line_number, + const char *start, const char *eol) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object_id peeled; + const char *p; + int ret = 0; + + /* + * Skip the '^' and parse the peeled oid. + */ + start++; + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid peeled oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p != eol) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has trailing garbage after peeled oid '%.*s'", + (int)(eol - p), p); + goto cleanup; + } + +cleanup: + strbuf_release(&packed_entry); + return ret; +} + +static int packed_fsck_ref_main_line(struct fsck_options *o, + struct ref_store *ref_store, + unsigned long line_number, + struct strbuf *refname, + const char *start, const char *eol) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object_id oid; + const char *p; + int ret = 0; + + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p == eol || !isspace(*p)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = 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); + goto cleanup; + } + + p++; + strbuf_reset(refname); + strbuf_add(refname, p, eol - p); + if (refname_contains_nul(refname)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = 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)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_NAME, + "has bad refname '%s'", refname->buf); + } + +cleanup: + strbuf_release(&packed_entry); + 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 refname = STRBUF_INIT; unsigned long line_number = 1; const char *eol; int ret = 0; @@ -1827,6 +1932,21 @@ static int packed_fsck_ref_content(struct fsck_options *o, line_number++; } + while (start < eof) { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_main_line(o, ref_store, line_number, &refname, start, eol); + start = eol + 1; + line_number++; + if (start < eof && *start == '^') { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number, + start, eol); + start = eol + 1; + line_number++; + } + } + + strbuf_release(&refname); return ret; } @@ -1884,7 +2004,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 74d876984d..a88c792ce1 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -699,4 +699,48 @@ test_expect_success 'packed-refs unknown traits should not be reported' ' ) ' +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) && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $short_oid refs/heads/branch-1 + ${branch_1_oid}x + $branch_2_oid refs/heads/bad-branch + $branch_2_oid refs/heads/branch. + $tag_1_oid refs/tags/annotated-tag-3 + ^$short_oid + $tag_2_oid refs/tags/annotated-tag-4. + ^$tag_2_peeled_oid garbage + EOF + 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 Wed Feb 26 13:50:56 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992467 Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) (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 D3BFA84A2B for ; Wed, 26 Feb 2025 13:50:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577850; cv=none; b=E4cKvhYJIYyx+FD14vfUtS6JENU7CVquoo9apM91rqHJl5Ih00vVWrDuW7yxZmHTi6Jj40MTpnyWtDBVPuyKVCkwSqgRfrcQ1OxudeBMgEbmWCvJWtQpDjS79nDBPMG0YYBhtRxWdI+flnCU5pE28jYqNvM7KO8upnHZggx6bgg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577850; c=relaxed/simple; bh=2M1tOKXfHyc6p05BE2V4dhMhNPEp/XSabKzBqMC5KKQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tD587qpKwy2wk4qJyn48btEqyJjxA5NPOKUWAWxUhm13Sju2noAwTR3STiEzZcGbEbli6cDA7vtuRgIdOckbzlzMyAWPwpOFq3Nk7NTLngBf2GeV7tQEffRFcevIeLizbJn01zc1v66HRkiagMSTOxm8t+fj1M+Pcbk63Hgibio= 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=DWuIwURp; arc=none smtp.client-ip=209.85.216.54 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="DWuIwURp" Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-2fe848040b1so1257169a91.3 for ; Wed, 26 Feb 2025 05:50:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577848; x=1741182648; 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=+BFKf6n8p7GWH69K7+nNjovE64N3rwTNW29Xdnf3gtg=; b=DWuIwURpcKhixCZa/qHEpGRms8Cc/01oA0/hFjnfm8VABXa9IHxwzyKpaGu4BJzKvU QZw+wwvet7vlJpPgTH4DnGn4kPEmhZcc2iVjOWkzvr6sFY/baIFmIIoJdnaXqiucqSo+ bdLRVJ6UJg8Zu1qrC1Q57scXdF0X4d1AWChU3was3gGMYYWjx1T5Z1feKJtSB6zet3Zi nAZaNb9wq4hPye3yf1lsInMS/xC03FZnAll7M4KopDZp8PPsbWmi2HGIk/uA6MqUHu6w S/PYIWkKnHBmJXnVG9cD53v4rUAaekzlZad+4efaEx/U/W1v+hZWWgOmli0EeLQHFn34 E/3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577848; x=1741182648; 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=+BFKf6n8p7GWH69K7+nNjovE64N3rwTNW29Xdnf3gtg=; b=iZljeN+3Lpjz9MbJu1QSkk404LOP5mG1tyU9TJ7b3F3+3l4ea4S7sLPFrJDBQKHqJb biF801n6ZHHC3jkAsQbV6+XSQdxzhmfhWdyPi5B8uNXZm2nWrD07YEjQvqUeWhd1e+c/ CaQsH1oJ0tMINiXYlyGiWgGPfyGFG9ebkMcAeeck4npT61tpOL1RzkBpZAjfmWEul/mp XdnHC0oa3NIuHDZL0FWzkw/ZiNjHSnb4UYyk89Sjd1cf027orwDimhhc6HBWoEVicjJ0 Ax7RxFSB5stNYbZSOIO9I2tkemiA4RHd91vkekVcIWotIJT3EQ9f0XOuPujRiQSQFGDY mzaw== X-Gm-Message-State: AOJu0Yy/vol0T+7zpky73C+Os0xkrVX8l9IqaDSgmJjrubqfAnc/4zd0 cUEfCSYE+PQjk1de0GGwGWSgKUrRfiFp1cotymZg7hiPe4cHIXNocOouQw== X-Gm-Gg: ASbGncu9QUnWbbqGZY3Ur24D1sX15bkPzNaK6waki8IaUWZ8inNwUZ4bAwFm+rk+Q/+ vqMafiG6Jwwa+LAp0D/vgcI987+IVRxMObU6RcXKkq/M0uGi+ZmMV8Yhbl3+84LRt0TwgPobKLq hpvKJTMPzKeF3jcpUZ4YiVqTB9YQC9tMZIosjew/WlLfpX/1HuuR//1v4qL56TkzIG/I+WqMZP8 Ku1XjGV5+ez4Uy3VIbp/xNfy+GKn6eXOTSXiKtJ7XyqqkbNyuWAMEGxer3Drtr5TH5Gs/w2v4vX iFFKI8vAoZ+zIIBUvwU2wQ== X-Google-Smtp-Source: AGHT+IGiYtwvvHdL62mL8aBnwHY867kwB9GTa3W4FxySk//Lb9oZJHTNDGFEzfqmeKjEuaq4AJZXvw== X-Received: by 2002:a17:90b:2d87:b0:2ee:d024:e4fc with SMTP id 98e67ed59e1d1-2fce8740f48mr37701523a91.33.1740577847577; Wed, 26 Feb 2025 05:50:47 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id 98e67ed59e1d1-2fe825d7d7csm1517262a91.30.2025.02.26.05.50.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:50:46 -0800 (PST) Date: Wed, 26 Feb 2025 21:50:56 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 8/9] 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. It may seem that we could record all refnames during the parsing process and then 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. We cannot reuse the existing compare function "cmp_packed_ref_records" which cause repetition. Because "cmp_packed_ref_records" needs an extra parameter "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 parse the file again and user the new fsck message "packedRefUnsorted(ERROR)" to report to the user if the file is not sorted. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.adoc | 3 + fsck.h | 1 + refs/packed-backend.c | 116 ++++++++++++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 87 +++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 16 deletions(-) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index 02a7bf0503..9601fff228 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -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 8c410fca77..a1710d7c2a 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. @@ -1797,19 +1803,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, @@ -1915,8 +1935,68 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, return ret; } +static int packed_fsck_ref_sorted(struct fsck_options *o, + struct ref_store *ref_store, + const char *start, const char *eof) +{ + 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; + unsigned long line_number = 1; + const char *former = NULL; + const char *current; + const char *eol; + int ret = 0; + + if (*start == '#') { + eol = memchr(start, '\n', eof - start); + start = eol + 1; + line_number++; + } + + for (; start < eof; line_number++, start = eol + 1) { + eol = memchr(start, '\n', eof - start); + + if (*start == '^') + continue; + + if (!former) { + former = start + hexsz + 1; + continue; + } + + current = start + hexsz + 1; + if (cmp_packed_refname(former, current) >= 0) { + const char *err_fmt = + "refname '%s' is less than previous refname '%s'"; + + eol = memchr(former, '\n', eof - former); + strbuf_add(&refname1, former, eol - former); + eol = memchr(current, '\n', eof - current); + strbuf_add(&refname2, current, eol - current); + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_UNSORTED, + err_fmt, refname2.buf, refname1.buf); + goto cleanup; + } + former = current; + } + +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, + unsigned int *sorted, const char *start, const char *eof) { struct strbuf refname = STRBUF_INIT; @@ -1926,7 +2006,7 @@ static int packed_fsck_ref_content(struct fsck_options *o, ret |= packed_fsck_ref_next_line(o, line_number, 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++; @@ -1957,6 +2037,7 @@ static int packed_fsck(struct ref_store *ref_store, struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); struct strbuf packed_ref_content = STRBUF_INIT; + unsigned int sorted = 0; struct stat st; int ret = 0; int fd; @@ -2004,8 +2085,11 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } - ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf, + ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf, packed_ref_content.buf + packed_ref_content.len); + if (!ret && sorted) + ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); cleanup: if (fd >= 0) diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index a88c792ce1..767e2bd4a0 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -743,4 +743,91 @@ 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" && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + EOF + git refs verify 2>err && + rm .git/packed-refs && + test_must_be_empty err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $branch_2_oid $refname1 + EOF + git refs verify 2>err && + rm .git/packed-refs && + test_must_be_empty err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $branch_2_oid $refname1 + $branch_1_oid $refname2 + $tag_1_oid $refname3 + EOF + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 3: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname1'\'' + EOF + rm .git/packed-refs && + test_cmp expect err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $tag_1_oid $refname3 + ^$tag_1_peeled_oid + $branch_2_oid $refname2 + EOF + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 4: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname3'\'' + 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" && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled + $branch_2_oid $refname1 + $branch_1_oid $refname2 + EOF + git refs verify 2>err && + test_must_be_empty err + ) +' + test_done From patchwork Wed Feb 26 13:51:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13992468 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 19A6784A2B for ; Wed, 26 Feb 2025 13:50:56 +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=1740577858; cv=none; b=TtlgwSYF/GO+768g7j1Qa8S8i3kmL1RLvMiAUfBDWsSJvoC3+Oor/T0FLdhL0KkehcX53UTz31afjpqYjbXwTE11YBveKsfiUnR5HRO/Ej7zX/pw6nzb5Rotp9YkYBCguRdmP2mNxDuBa+v+xN9HiVx68aDYIAAYm6mtDcjYcvI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740577858; c=relaxed/simple; bh=+aHq/0mhg3mqmhS9d2qjcyWfrd49Ck2gOcmj77QlhX4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IFBkDMtT5Ganxe+LOZq+yIyDSQEM1g0+GNE4dUEPLTrBoaHf7BLQs9q4TlazNAvL/E4gfgDOG3lNywN+c1uCk6m9nCkY56ox/2krP1HUEhhQHBdqIhJp17LECCBTsmp3DK0BvFOsl6S1ED0fw2//W02kJSBr5WRRTw7VD7Xz2WQ= 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=inpA9tXs; 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="inpA9tXs" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-22100006bc8so120017935ad.0 for ; Wed, 26 Feb 2025 05:50:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740577856; x=1741182656; 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=P97hX1r72VErTTUpW28u0qQ0ZvGnJob0r/Tm2EjItaM=; b=inpA9tXsu9T2iwbqtM9iTiUiuis5AzySKnN5rGhebY45s7wGAhFZUyzCCpKd+oDKPq +HfaQBfUeFYoU4ub1qtSdEsf/fk5PaYhpJny/EPMH4aA8ruTwJFYTUbTdTUoK+uwoMRB gC6NUdUaIIYCL1de6De2N3jBWNIokti8P4iNhHlOMLMA8DhIxPdlJJIfapT7KyKlFERT eaCq6Wd5jdo64S44qKJ6lBqefhGwf1hlISz9XPaGHVAjSgXJDBl8pmHfQPJyrYwJC/u8 D7aYuTqtmYIHlfFzvhWpWb5ez/QWXOYx7IQFaAZWmfgIWbfUNqy6yayEZowjyLo2hZT5 9PYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740577856; x=1741182656; 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=P97hX1r72VErTTUpW28u0qQ0ZvGnJob0r/Tm2EjItaM=; b=LsCIETrs2vZW4/NVwbdKGx3aTyvs8A+jHaRHyoku45iAGr7nrzGpGLlsQWS3ayWqoN RDPBT6Ekft20NhdTcEoh+53DA+HQnar9Gka8v6xEVjXe/ysa63ehlLGp/K3xYxQCTB0j Ckd/GBZuO025EgWmzj3IK2IjvzoGCG+dWwK1Hutrdg7GDwAqE9ZbS8BIcBx/jbYzULxd /H5Jn5LYuUgPKSADkfxvwEuxxONrNVkJIUjmoLON4KtupbOb6geC9Jjk6TIApAXgSXPV 6R5Jk44Aq0JmqQa2jBOfoC1HQlEa4MadLQPho8BYwoOY4GvDC9OROR9BBalT4h8Wu4zC m9JQ== X-Gm-Message-State: AOJu0YwK+BRdcyVaTpNpikzDtsUBoMmvYolrrgIoPtWGEiPXTA2+SG5Q pxQ9Wt4wnZE9Rnc7qtLlvRDNn7cSiXnzzHsSX5w4QtygX3h351MUM+/CZA== X-Gm-Gg: ASbGncs5O067JM4z/XDmSh52EwYQNIMGfK5liFGIzzxRXP6j7j1XL415LGJlOh4P0Nq LAmEGvnZ+Mc5ffsgjyYjiBh9CWKW6E6TrTRDhr0nbbeRLN4ytRrXRc+WHDjVQsrGBQzOcsCQKMX lB3DFQuszTcAcyudGyWBcVey4CIi0NJgK53xtGwUC6wKULFYfqQas1ay+tNe2pDB9y3AAy8aJSb Onh4balV+WfyMKrRmXez8blZB2bVa2A294q2S78CpX3wsvCvIennklQjFxqNohq+x2jhUzAC7f9 H5j3u/vD3e818D7e4jroXw== X-Google-Smtp-Source: AGHT+IEMa6omrjFcmrkBSZj8yIOt0zYRIssC7Skv6Xt9FxQN0bRmMTqFGKTeDxAdv1E+dNGR/Uq8ZQ== X-Received: by 2002:a05:6a00:1ad4:b0:732:5c88:990 with SMTP id d2e1a72fcca58-7347918c4e9mr9514893b3a.17.1740577855882; Wed, 26 Feb 2025 05:50:55 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id 41be03b00d2f7-aeda7f8453csm3111061a12.28.2025.02.26.05.50.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Feb 2025 05:50:55 -0800 (PST) Date: Wed, 26 Feb 2025 21:51:04 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v7 9/9] 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. Last, update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/git-fsck.adoc | 7 ++++++- builtin/fsck.c | 33 ++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 39 +++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/Documentation/git-fsck.adoc b/Documentation/git-fsck.adoc index 8f32800a83..11203ba925 100644 --- a/Documentation/git-fsck.adoc +++ b/Documentation/git-fsck.adoc @@ -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,11 @@ 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. + The default is to check the references database. + 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, diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 767e2bd4a0..9d1dc2144c 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -830,4 +830,43 @@ test_expect_success 'packed-ref without sorted trait should not be checked' ' ) ' +test_expect_success '--[no-]references option should apply to fsck' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + ( + cd repo && + test_commit default && + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck 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 && + + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck --references 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 && + + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck --no-references 2>err && + rm $branch_dir_prefix/branch-garbage && + test_must_be_empty err || return 1 + done + ) +' + test_done