From patchwork Thu Jan 30 04:06:58 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954275 Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) (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 EC5AED2FB for ; Thu, 30 Jan 2025 04:05:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209932; cv=none; b=mPILyTUE48qmW+XSDgfNg3RUpfR8/3ZpnnNOeXWZ43s3feXaBUTzsv96JnD5jtNqjcMcjqWVgvbSKZtRKEMhExBszEpCqdHT42ab4SS/4B3g+SY5/j/blpIYMrqe0+TyXpxYr5YJcEDvqeZOKvVm43xy6ZG01eH/FglQ8+VMrJo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209932; c=relaxed/simple; bh=NiSz0kFXgj2oW62wBNpIa05qAzJdZQp1gZun8gTT0Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BdDZpDZOVEEF/g1Q75oqscE/QtaOYifJqrshbjiSg9p6jsR9rIFMGmAQx8iuCfH8xLo5ASVZ4gRonUt99W8F4iMvwTJrCof7hhosp80pZ61gmgTGwWA7WOI5C+URTBJm5HVACKi6CNUHEHkn65xuP3GBXs/WlnEdJns5znJRL1E= 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=E/59ncz+; arc=none smtp.client-ip=209.85.214.179 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="E/59ncz+" Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-21631789fcdso19694365ad.1 for ; Wed, 29 Jan 2025 20:05:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209928; x=1738814728; 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=E/59ncz+ROLm1J2s+sGqedGg5OYqN69ePoBSeGxZhW6i5BQ2PImKNusrHrSaJgHyaG ILEuSkatO6KPIgKhMIwqcvr0l312rtxNs4YTUDSvxnMxGLL/NaC/Hk8EZppbnad9SIQK hgi3XFFMPqf1IfZ1q2DGZihbCjbrEP5Uz+XZFLJul+RLBST0SelWHK5G3qRtoPvneHex KvAI1Eokr/OSbJXhUaDamhSt4AwMGUyUjUDHOTtCJzE7Tor4d3h98b6XsziGc2aCHv34 PwPVviZsYx2P54RZQe6LSM43UYTgN0tZQWElMNHUiZOADOTCdyb8FvDLLsu8HHoR+85n MX8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209928; x=1738814728; 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=SFLlb6bVqPy0hnxEbgl6JcYmE5FpqaJ8RfEm8IxI5q5MLopabd9wtWKhHEPf+zfLGq GQvrxil3Yo68eqXpSBhkXppXV2uRl0vTcqnTlPdo1GgvhLjnbfj34WgAv06UFSIfhP8y PX5a7IgpQu3ip/JtNgAqw9h34/h7UQTGgsB2d1pbqokSNBrhmT7hyFO9inrk5kEkY16m TpGDAYBr6t/4vjiei3jWoWS3sYM00oLZc19gLGllCayj6+RxnJJmIoUprDm04kBQpZhf 4goB+xE0iR1DGU4YmDqUK3vpMa5pD3ZavyrFqav8hIXkA3PipSTrVtnJegvtWnJJegQf wmFA== X-Gm-Message-State: AOJu0YzhmovhS6b8yZC+Y6raW0gMSXQteht+D3tHnSA8x9usFq7SYYy3 B5+n6Y1l5pjuliEcW3rgymutkP4G89UoANPRtE+UlJGvjShz7UJ5qGBY9rGX X-Gm-Gg: ASbGncsI5v5XiUulWj4WvwmB0NELKJFaD8o7m4feaPm/eOpA1h8W2cefjd4CQ/4BnrW bun80YvEqr5zv+oKg+53mKArAAjTMGOrE9wdlnu4v4OHVp6fUWSkoKR/QeYl5ESrRSLnuj4YH2u 9XPhPkLPfaYm1Ti2V6/iNLjJWIWkjVvGMcBH6JrmdrviGNUTzZj/i9es5xoGtkQCbHKPNahpkQ7 rRaLsNaK6YBLbgkATcjshvKS6PrdgE/4x7PBWMwERSc2qHVNdDRSun371DUbp1WgWKNIg== X-Google-Smtp-Source: AGHT+IEMx5l2Hm+lBXayrYXyJ4j7R/irJBOiGnQ7NheCIk5m94G2iDR04oUNiHlv8QfO7OnTZz38Sg== X-Received: by 2002:a05:6a00:3286:b0:725:4301:ed5a with SMTP id d2e1a72fcca58-72fe2cfdcfemr3012882b3a.2.1738209927768; Wed, 29 Jan 2025 20:05:27 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-72fe6a1cb5asm331956b3a.174.2025.01.29.20.05.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:05:27 -0800 (PST) Date: Thu, 30 Jan 2025 12:06:58 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 1/8] t0602: use subshell to ensure working directory unchanged Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: For every test, we would execute the command "cd repo" in the first but we never execute the command "cd .." to restore the working directory. However, it's either not a good idea use above way. Because if any test fails between "cd repo" and "cd ..", the "cd .." will never be reached. And we cannot correctly restore the working directory. Let's use subshell to ensure that the current working directory could be restored to the correct path. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- t/t0602-reffiles-fsck.sh | 967 ++++++++++++++++++++------------------- 1 file changed, 494 insertions(+), 473 deletions(-) diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index d4a08b823b..cf7a202d0d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -14,222 +14,229 @@ test_expect_success 'ref name should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - - git commit --allow-empty -m initial && - git checkout -b default-branch && - git tag default-tag && - git tag multi_hierarchy/default-tag && - - cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && - git refs verify 2>err && - test_must_be_empty err && - rm $branch_dir_prefix/@ && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && - git refs verify 2>err && - rm $tag_dir_prefix/tag-1.lock && - test_must_be_empty err && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/.lock: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/.lock && - test_cmp expect err && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/$refname: badRefName: invalid refname format - EOF - rm "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + ( + cd repo && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + git commit --allow-empty -m initial && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/multi_hierarchy/$refname" && - test_cmp expect err || return 1 - done && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - mkdir "$branch_dir_prefix/$refname" && - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && + rm $branch_dir_prefix/@ && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && + git refs verify 2>err && + rm $tag_dir_prefix/tag-1.lock && + test_must_be_empty err && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/$refname/default-branch: badRefName: invalid refname format + error: refs/tags/.lock: badRefName: invalid refname format EOF - rm -r "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done + rm $tag_dir_prefix/.lock && + test_cmp expect err && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname: badRefName: invalid refname format + EOF + rm "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/multi_hierarchy/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + mkdir "$branch_dir_prefix/$refname" && + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname/default-branch: badRefName: invalid refname format + EOF + rm -r "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref name check should be adapted into fsck messages' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - git commit --allow-empty -m initial && - git checkout -b branch-1 && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=warn refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/.branch-1: badRefName: invalid refname format - EOF - rm $branch_dir_prefix/.branch-1 && - test_cmp expect err && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=ignore refs verify 2>err && - test_must_be_empty err + ( + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=warn refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/.branch-1: badRefName: invalid refname format + EOF + rm $branch_dir_prefix/.branch-1 && + test_cmp expect err && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=ignore refs verify 2>err && + test_must_be_empty err + ) ' test_expect_success 'ref name check should work for multiple worktrees' ' test_when_finished "rm -rf repo" && git init repo && - - cd repo && - test_commit initial && - git checkout -b branch-1 && - test_commit second && - git checkout -b branch-2 && - test_commit third && - git checkout -b branch-3 && - git worktree add ./worktree-1 branch-1 && - git worktree add ./worktree-2 branch-2 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && - - cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && - cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err && - - for worktree in "worktree-1" "worktree-2" - do + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + ( - cd $worktree && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err || return 1 - ) - done + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + for worktree in "worktree-1" "worktree-2" + do + ( + cd $worktree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err || return 1 + ) + done + ) ' test_expect_success 'regular ref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && - git refs verify 2>err && - test_must_be_empty err && + git refs verify 2>err && + test_must_be_empty err && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/a/b/branch-bad && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - for trailing_content in " garbage" " more garbage" - do - printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && cat >expect <<-EOF && - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $branch_dir_prefix/branch-garbage && - test_cmp expect err || return 1 - done && + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - '\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err && - printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + '\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - garbage'\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err + ) ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -237,99 +244,103 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - bad_content_1=$(git rev-parse main)x && - bad_content_2=xfsazqfxcadas && - bad_content_3=Xfsazqfxcadas && - printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && - printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && - printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 - error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 - error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + bad_content_1=$(git rev-parse main)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && + printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && + printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 + error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 + error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'textual symref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" + do + printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for good_referent in "refs/heads/branch" "HEAD" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" - do - printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && - - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-1 && - test_cmp expect err && - - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-2 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-3 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-complicated && - test_cmp expect err + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err + ) ' test_expect_success 'textual symref content should be checked (aggregate)' ' @@ -337,32 +348,34 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && - printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'the target of the textual symref should be checked' ' @@ -370,28 +383,30 @@ test_expect_success 'the target of the textual symref should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && - git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && - - for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" - do - printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' - EOF - rm $branch_dir_prefix/branch-bad-1 && - test_cmp expect err || return 1 - done + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" + do + printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err || return 1 + done + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked' ' @@ -399,201 +414,207 @@ test_expect_success SYMLINKS 'symlink symref content should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - ln -sf ./main $branch_dir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $branch_dir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $branch_dir_prefix/branch-symbolic && - test_cmp expect err && - - ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref - error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' - EOF - rm $branch_dir_prefix/branch-symbolic-bad && - test_cmp expect err && - - ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref - error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' - EOF - rm $tag_dir_prefix/tag-symbolic-1 && - test_cmp expect err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + ln -sf ./main $branch_dir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $branch_dir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' + EOF + rm $branch_dir_prefix/branch-symbolic-bad && + test_cmp expect err && + + ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref + error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' + EOF + rm $tag_dir_prefix/tag-symbolic-1 && + test_cmp expect err + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked (worktree)' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - main_worktree_refdir_prefix=.git/refs/heads && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - - ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree1_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree2_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $main_worktree_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $worktree1_refdir_prefix/branch-symbolic && - test_cmp expect err && - - for bad_referent_name in ".tag" "branch " - do - ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + main_worktree_refdir_prefix=.git/refs/heads && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree1_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree2_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree2_refdir_prefix/bad-symbolic && + rm $main_worktree_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' EOF - rm $worktree2_refdir_prefix/bad-symbolic && - test_cmp expect err || return 1 - done + rm $worktree1_refdir_prefix/branch-symbolic && + test_cmp expect err && + + for bad_referent_name in ".tag" "branch " + do + ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && - test_must_fail git refs verify 2>err && + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $worktree1_refdir_prefix/bad-branch-1 && - test_cmp expect err || return 1 - done && + rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && - test_must_fail git refs verify 2>err && + printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' EOF - rm $worktree2_refdir_prefix/bad-branch-2 && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $worktree1_refdir_prefix/branch-no-newline && - test_cmp expect err && - - printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - EOF - rm $worktree1_refdir_prefix/branch-garbage && - test_cmp expect err + rm $worktree1_refdir_prefix/branch-garbage && + test_cmp expect err + ) ' test_done From patchwork Thu Jan 30 04:07:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954276 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 5D29BD2FB for ; Thu, 30 Jan 2025 04:05:41 +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=1738209943; cv=none; b=KDFTGGH4CxgudSSb9cf9VM3VaKI8LYDMijFoZgTSIMpt/QFfvGZ8ceJJzWpbhoRoMZq1yiHZLgTaEcRpksNMfl2v7D6ygTp6Pu8r/KlMqskdKdLBnhUtL+42Fk8izPJzZPVfdiIOjvuwoTBMAawu2p1o5tvjaw05Ehn4NShCkkw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209943; c=relaxed/simple; bh=RJ+/KtB4oHuOOWTrleOLNBPb2UDAx7GKq2u9+tKiwdk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TOFrzvNoDtwMlZ+EfnF7HinHEQwbybRnuRzr8Is3qrD+tO6E5Pjnal6lyJmR6zLAA8DxddUcKEvWkbnAIN3FMnyQH5x5mfVpVjV27NqtpRA1fiONTRt1zTeYzxIZIcXCMNw8xvV9r0Sshl2fys2oxJB7a4Cu2jOZ/kZmwi8h0as= 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=Idp/CcMD; 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="Idp/CcMD" Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-216281bc30fso8216335ad.0 for ; Wed, 29 Jan 2025 20:05:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209940; x=1738814740; 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=3+rQIqQi9bILdJ/YeMf5sG6gz5qveqPPh1jQvi84pNA=; b=Idp/CcMD7/16NAb0+yArtc7eEdJ/a1JsB2TnEA/6v1Hr0q31odwjiN40hGdomJbVSj tHiANocfbDBGyOfaBLPW6hQ78zUXnInYLnK58fNQfiN1MX+CzkEverOm1qaXcHyg/0fo 1UQHwr5LrP7kFZt5pxHXBTsHrbW7wSQGm03HlagryCwkKG2Clu8ITqW5vtrqsbqytuij pQ7bU8hnQEmywSHJl2XyRVGKn9Ck2VZXYPIhi3zJFrEZt9UM3Ap2alSFHsmIuOF8WllC MEwBrnQyMcHBPN5nBY17w3O5YDVBQTaNTZi1k0R6mGL1zXoPAEyKA+u79TkHREeMd1I8 ROdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209940; x=1738814740; 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=3+rQIqQi9bILdJ/YeMf5sG6gz5qveqPPh1jQvi84pNA=; b=gaNuUYqY7i7TeiPwHBvk/vv4w1gvlysK64FNJYNFdIYtpnL3UItFKStiXxYTTbMheZ 0VqcJQZqDxCk0OKuY138xxXcdts0KI31WcG7MQb21puT6Bc5EX3/+nE2abH90VE9/tKY bTAGvCLAC4aZnF9lTjIowLzHo1kUqjiRWxXm9zE/TeyH+P4BwNiI9DIY5izmvW6xoJpS 6j8E426Xq0uGRzh9FYLhCQJjbGjJXKlgZ2n/GrAN/UjFnkMUST16PvJF7Uy10WDnR79j EA6qeLLg+ldsh+6tTq+qjEQX5eSnuQijA0g3gfTFYNRKuMJd708TMjh8CaC7ZVHchh3R sHgA== X-Gm-Message-State: AOJu0YyOmpG9Can9PVcNclebScCvSvNE6g0L494PCjHwr481cdpvhBTH ofjl2Y2JABORDvjtJks1gn7B4iVXjl4tbhCTDaUrXzCnTdDjrJyRJYmn537A X-Gm-Gg: ASbGncsbe1pg153gRv2ugN04sLeL50nJ//8ZAlJ4TdKmbRsHTxNsQRc4bu02W46jT1g Sr1hiXIqrI1FNvmf1uekKH2lWLSBxiD9CJrcSaEHOwuGi7wILyoWuwTEUBFOfoBxSMG2fMKVFz0 tvVWIK0wjY4C0Szyt0D5QMZdT2Dcdv4STFQMUECSzJaWK/xvtsRVEBKAgQMG6nZWk7aowTID+WK XmUsGqkE/UcemKegYR5lpP0W5r8yqdrjcuymzcKAF8syjM5rJEUhEvUAW0keNSK8HdgBw== X-Google-Smtp-Source: AGHT+IHzTC7MbP/qbAv7y1qrFsB0Pqf4MhuOFR+ZH+hvfuGrc0iGUu0VVoR5pVDCWDE91H/g1/Ixdg== X-Received: by 2002:a17:902:ccc2:b0:21a:8d8c:450d with SMTP id d9443c01a7336-21dd7df960cmr96261525ad.53.1738209939857; Wed, 29 Jan 2025 20:05:39 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21de33007f1sm4018245ad.174.2025.01.29.20.05.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:05:39 -0800 (PST) Date: Thu, 30 Jan 2025 12:07:11 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 2/8] builtin/refs: get worktrees without reading head info 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 don't want to simply die the program but rather show the warnings or errors as many as possible to info the users. So, we should avoid reading the head info. 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 info. Create a new exposed function "get_worktrees_without_reading_head", then replace the "get_worktrees" in "builtin/refs" with the new created function. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 2 +- worktree.c | 5 +++++ worktree.h | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/builtin/refs.c b/builtin/refs.c index a29f195834..55ff5dae11 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -88,7 +88,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix, git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - worktrees = get_worktrees(); + worktrees = get_worktrees_without_reading_head(); for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); diff --git a/worktree.c b/worktree.c index 248bbb39d4..89b7d86cef 100644 --- a/worktree.c +++ b/worktree.c @@ -175,6 +175,11 @@ struct worktree **get_worktrees(void) return get_worktrees_internal(0); } +struct worktree **get_worktrees_without_reading_head(void) +{ + return get_worktrees_internal(1); +} + const char *get_worktree_git_dir(const struct worktree *wt) { if (!wt) diff --git a/worktree.h b/worktree.h index 38145df80f..1ba4a161a0 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,12 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. This is useful when checking + * the consistency, as reading HEAD may not be necessary. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Thu Jan 30 04:07:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954277 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (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 D7FF72770C for ; Thu, 30 Jan 2025 04:05:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.43 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209954; cv=none; b=XfsZ5o/1O92TISXH9bN2ss3DqNFFk2zw114iCyCcAaOJDS4hgBozPmbttJPTawIswG9vq6wAMUqPIFwtmvdCCglzaO624lHFmXosbmK+SIaGPZSdE4ZvA91Hvg0Rn7HMqarEgOV3FXNtwVGQq4RT3p2f0sbXtYPjKE4Q/OTHRhw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209954; c=relaxed/simple; bh=zKM0Eoc9VHZz0AvI2yWD5QXIp7fi1kGHvgc4TJOkCd4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=arnwCNjuNGqZP23EDZjEvtaA+PCcOh5OPNrMwrQO0rJEUafJE/xpkQTeKcCHdTqHt3k8D1Ms4mxJVrdiS4Y8xhXc87r7N6KQeMcBDTYYITwuUdab6T2ryv6zmuy4sFt+8rIlnHOwANXIyg/M9BQa8wjXpeTlRLspUZynnfOx9Dk= 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=BZUyXXz6; arc=none smtp.client-ip=209.85.216.43 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="BZUyXXz6" Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-2ee50ffcf14so2342880a91.0 for ; Wed, 29 Jan 2025 20:05:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209952; x=1738814752; 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=cxedQoDButnnQ/p6lCvAmJMZOsm/Q1LU5IpGg2OrhM4=; b=BZUyXXz6fuCkHeB66pnilqScTdgewcBQ8kA5STPfxcxWLwacfOpJ0UfKbDO0ALJNj/ shJlJVOQXPd1CvBCyXX6iUaj0HclZyhINTlpqtey9GPxhD9x8F+gmChye6+hDQpLJhIV ejgW0TvLkVtdd/CV/s3Djh7Q4MDeJDmrSwR21ahCsT+1MhQOAGUHRwHe0PmUWZeCxOVF hBZoqUrUyUUm/P84TkPZsUL7PWBQRSBS2Fa0AIvFFhcNM7DKE+jl8k8SdVNpbiGmS/6A 7GoCZqDqu3AJ+WDlkAaHUHxSM/KFOkSxNZRfYBVLtziwaOaw+c4Wf8NoXjofykD9dOap 7Grg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209952; x=1738814752; 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=cxedQoDButnnQ/p6lCvAmJMZOsm/Q1LU5IpGg2OrhM4=; b=jOPky/kmg6ClZD4RN+g7U/NTPdn27KTX4PZ04MKW+Crqfi0xIFnSdNEdiiA6awM3tC 4UiWUeW5dEcZirY8RjeDcP59hK8/Jo33DeFcHHQujOdVJHpsm1IIav0DKXHz/T57oitP S12b+K0DUvYwlXoXNIVrvQ69b4UzDW47JvZyqeA8Uc59Bid2TutSrt/ST3CoDfVWbSq7 GJ2DZrOuVRp5qgL1lmMCg/mtpnB/BEBAxiWu5LIkL2/yhNeyJRAU2ILAdpfK5O0RjyA+ 6GYecUMr8SB2Riym6RIFmDFt9kNa81wqHWV5Lc78npBYNqvMTO/oI6oqppJ3XXquvkNF vVqA== X-Gm-Message-State: AOJu0YzELn1wOFqGJux/ba3ExUc8Fw5WmZ9Hq1c4iQvUP22PwcfNziOw qQVkPdmD6D9slFlTb8A7nFK94TZ8jhQEXA0nzzDs+4dP8WFmXmhNNhZ+xBU/ X-Gm-Gg: ASbGncs0ddi3wKr9dcMDo++LQJ/Xdx3HCnLu+qgYuSt7udrcH5cFRy4uMBAqvRE1ET4 3B7JLYLTnliZB807t2DBwMFD66AQPN9zmEJ35ainl62A0Mo2yKlxpt/Ew+poTKeoELKtMmIxRzI cCc3lHN3RN0wJnM6LzHUBGepqFELkem9QuThPlI+M7nJzKv63W+BMPVSuCTxFPYZEWjD7Tn8Am1 F2jSDjf2qfWCNDLP49sdQr892wJ39gsO3vSJfc6FVOmcBfodr7x9glpq71vYofRd/TMkw== X-Google-Smtp-Source: AGHT+IFvskboRP12e9tlfbQniYMslWKTjUgR4kvJGzGFEchh1o0+z3KlORnpn/ZkPvlhWxH1yAsDCQ== X-Received: by 2002:a05:6a00:6404:b0:725:4a1b:38ec with SMTP id d2e1a72fcca58-72fe2ccc1b9mr2629725b3a.3.1738209951588; Wed, 29 Jan 2025 20:05:51 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-72fe69ba3d9sm328099b3a.120.2025.01.29.20.05.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:05:51 -0800 (PST) Date: Thu, 30 Jan 2025 12:07:23 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 3/8] packed-backend: check whether the "packed-refs" is regular Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Although "git-fsck(1)" and "packed-backend.c" will check some consistency and correctness of "packed-refs" file, they never check the filetype of the "packed-refs". The user should always use "git packed-refs" command to create the raw regular "packed-refs" file, so we need to explicitly check this in "git refs verify". We could use the following two ways to check whether the "packed-refs" is regular: 1. We could use "lstat" system call to check the file mode. 2. 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. It might seems that the method one is much easier than method two. However, method one has a significant drawback. When we have checked the file mode using "lstat", we will need to read the file content, there is a possibility that when finishing reading the file content to the memory, the file could be changed into a symlink and we cannot notice. With method two, we could get the "fd" firstly. Even if the file is changed into a symlink, we could still operate the "fd" in the memory which is consistent across the checking which avoids race condition. Reuse "FSCK_MSG_BAD_REF_FILETYPE" fsck message id to report the error to the user if "packed-refs" is not a regular file. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 39 +++++++++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 22 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e..6401cecd5f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -4,6 +4,7 @@ #include "../git-compat-util.h" #include "../config.h" #include "../dir.h" +#include "../fsck.h" #include "../gettext.h" #include "../hash.h" #include "../hex.h" @@ -1748,15 +1749,45 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } -static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED, +static int packed_fsck(struct ref_store *ref_store, + struct fsck_options *o, struct worktree *wt) { + struct packed_ref_store *refs = packed_downcast(ref_store, + REF_STORE_READ, "fsck"); + int ret = 0; + int fd; if (!is_main_worktree(wt)) - return 0; + goto cleanup; - return 0; + if (o->verbose) + fprintf_ln(stderr, "Checking packed-refs file %s", refs->path); + + fd = open_nofollow(refs->path, O_RDONLY); + if (fd < 0) { + /* + * If the packed-refs file doesn't exist, there's nothing + * to check. + */ + if (errno == ENOENT) + goto cleanup; + + if (errno == ELOOP) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_FILETYPE, + "not a regular file"); + goto cleanup; + } + + ret = error_errno(_("unable to open %s"), refs->path); + goto cleanup; + } + +cleanup: + return ret; } struct ref_storage_be refs_be_packed = { diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index cf7a202d0d..42c8d4ca1e 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -617,4 +617,26 @@ test_expect_success 'ref content checks should work with worktrees' ' ) ' +test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git pack-refs --all && + + mv .git/packed-refs .git/packed-refs-back && + ln -sf packed-refs-bak .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + test_done From patchwork Thu Jan 30 04:07:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954278 Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) (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 E81782770C for ; Thu, 30 Jan 2025 04:06:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.51 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209965; cv=none; b=qLgFzur/9BuMjUBXXx+zKyrUuFwTnfP0ziULubvKtYAThNXZHs8FRHF0Om6RKa3Bj5+dKI8G61WlmEJYsT7ws/QV8dxIZxZQ8Mo/1vSqgctDRlPtUD0/68tWSU4BzWWppP0yOopTBeRtQJRjPjoW4HNwouNYlU11cXo9MvZK5KI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209965; c=relaxed/simple; bh=UkLzD1y8krICV0EmpOHrMqZqwry2yIarf7MtlO50xiM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OO0ul4lnNJwOSQ6tgdT+RNJcz4mGl/d/UMtEyl0P9ZOUCUMgosx80407kRYy+KDfx1reh3PvlnDTbFTfnu2Njh7HxhkEGgPdldJdABUOOXl/K8XkuepB7vx47lu1wuoaxk0dpUm2THsGunG8Ja2wva00kxiDM03GoKONZnP9jto= 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=bjjdP1xw; arc=none smtp.client-ip=209.85.216.51 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="bjjdP1xw" Received: by mail-pj1-f51.google.com with SMTP id 98e67ed59e1d1-2ef8c012913so401767a91.3 for ; Wed, 29 Jan 2025 20:06:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209963; x=1738814763; 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=ySgrcBhoJ9k2KuUU3ZPcumBnsMGEpq+xgLyUMjjwrJU=; b=bjjdP1xwIFN5EQPZtejFSvUr/4IFnIlbWVlOnP0gK9/FwysPtLTYZ4xk/PKHVW+YIf XwCbzex4EuVq9k77ZvkLQBJoOOSne5jcvIIVHOC1PttHu+mI1sugJ+Qq/cp1rtq3oAQn 2u6sFD/gqoWWQ9RAa7LSz0TlQtuw4orHx/B7BF4RBGRegQF82e2psDMUm9cePiDmOXKS hVf+qlK1lQz5Ho3eTytkjahAdk0VzeiYDxqLvdXI3Cpz6f/WZ9OZZvhGbF+DajSIAnpN u1XeU60HUhc800trfm+SdhNFKkfC5IQdJtVrl4N0PwFdnQo3ST/N47y6303qceAgH6FV LcCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209963; x=1738814763; 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=ySgrcBhoJ9k2KuUU3ZPcumBnsMGEpq+xgLyUMjjwrJU=; b=mpASpbiOL9UEBM5F1QXGRXjsXKUtdsisEv3dxHd4SExtUJxOrnb2rL8guQQ8n6QZW+ 3bEe3/6FGgYyUidHVvQ4cChjnL9VgGnbfhf+rIXdsSJp+NkvJnFgh5SVSsVRFC9mbdl7 NiR97KUwDzy7DvWie95JVZ27OvIpSvvvM3OLSjPKGUwgN8hARKo110EFm+n+c+rqkipP HKVhMsCG9kVAlBsH1ya5SHQRK7qMRgGFeA+x4ntATfHQMPz2apBDc5UvWZI5Jc3d2WWu aYk7G/PWzjxkMk1DEkFlFAuOCZaEbx00Q7nrg5s444SVThJZOTpZQ1xGPQYlcxq1vSh4 hiVw== X-Gm-Message-State: AOJu0YypfqdUvcTzMfw2UugkIzjPeKHUa2Qqasc873H0vTFCGjsHS6bt 9xYjGX4G/WzuR7ssyqYXrOXa9NxKkXVQkIpEWwKLGp1YM75SVv39R+Bk8wZn X-Gm-Gg: ASbGncvVnJgcALGpTLJM6DxAM7DIH37TL2ZfsdJ9hqttVtAwpyRcR1hWZg2AIraau1A uTAqkbrOiZtA6B7e/F3o6GxigDRVjHVkaWHsrXANBNwNl/A+ig4uT4QC1Kljiz9fFKyzbzezPb9 VS+4bviqj0D2wd+KUZvCG9jieNnDF6iCTXdx0qUBLTueRo1ZmtTmp3LvOi3MeV4Yxlinx2sAqvA B7ou3kpMl0VBwrGQHfjJcG9ZFK5dfCNdYndsUwFIXsrOKx1sQR+kjEJXQQIxq3yZ/yGTg== X-Google-Smtp-Source: AGHT+IFzLPrUD+L4tfqI+k12+U6d6u3tESe23+gv1uV1vDdYxCm9jF9g1eA+9tdXII/v0MvgFtKtaA== X-Received: by 2002:a17:90b:3a0e:b0:2ee:b2fe:eeeb with SMTP id 98e67ed59e1d1-2f83ac66135mr6911636a91.22.1738209962566; Wed, 29 Jan 2025 20:06:02 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21de331f0d6sm3918065ad.219.2025.01.29.20.06.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:06:02 -0800 (PST) Date: Thu, 30 Jan 2025 12:07:34 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 4/8] packed-backend: add "packed-refs" header consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with:". As we are going to implement the header consistency check, we should port this check into "packed_fsck". However, the above check is not enough, this is because "git pack-refs" will always write "PACKED_REFS_HEADER" which is a constant string to the "packed-refs" file. So, we should check the following things for the header. 1. If the header does not exist, we may report an error to the user because it should exist, but we do allow no header in "packed-refs" file. So, create a new fsck message "packedRefMissingHeader(INFO)" to warn the user and also keep compatibility. 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", ideally, we should report an error to the user. However, we allow other contents as long as the header content starts with "# packed-ref with:". To keep compatibility, create a new fsck message "unknownPackedRefHeader(INFO)" to warn about this. We may tighten this rule in the future. In order to achieve above checks, 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 above three checks. Update the test to excise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 16 +++++++ fsck.h | 4 ++ refs/packed-backend.c | 89 +++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 46 ++++++++++++++++++ 4 files changed, 155 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index b14bc44ca4..34375a3143 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,10 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefHeader`:: + (ERROR) The "packed-refs" file contains an invalid + header. + `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. @@ -176,6 +180,13 @@ `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. + +`packedRefMissingHeader`:: + (INFO) The "packed-refs" file does not contain the header. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref @@ -208,6 +219,11 @@ `treeNotSorted`:: (ERROR) A tree is not properly sorted. +`unknownPackedRefHeader`:: + (INFO) The "packed-refs" header starts with "# pack-refs with:" + but the remaining content is not the same as what `git pack-refs` + would write. + `unknownType`:: (ERROR) Found an unknown object type. diff --git a/fsck.h b/fsck.h index a44c231a5f..3107a0093d 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) \ @@ -90,6 +92,8 @@ enum fsck_msg_type { FUNC(REF_MISSING_NEWLINE, INFO) \ FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ + FUNC(UNKNOWN_PACKED_REF_HEADER, INFO) \ + FUNC(PACKED_REF_MISSING_HEADER, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6401cecd5f..883189f3a1 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1749,12 +1749,92 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +static int packed_fsck_ref_next_line(struct fsck_options *o, + struct strbuf *packed_entry, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct fsck_ref_report report = { 0 }; + + report.path = packed_entry->buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED, + "'%.*s' is not terminated with a newline", + (int)(eof - start), start); + + /* + * There is no newline but we still want to parse it to the end of + * the buffer. + */ + *eol = eof; + } + + return ret; +} + +static int packed_fsck_ref_header(struct fsck_options *o, const char *start, const char *eol) +{ + const char *err_fmt = NULL; + int fsck_msg_id = -1; + + if (!starts_with(start, "# pack-refs with:")) { + err_fmt = "'%.*s' does not start with '# pack-refs with:'"; + fsck_msg_id = FSCK_MSG_BAD_PACKED_REF_HEADER; + } else if (strncmp(start, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER))) { + err_fmt = "'%.*s' is an unknown packed-refs header"; + fsck_msg_id = FSCK_MSG_UNKNOWN_PACKED_REF_HEADER; + } + + if (err_fmt && fsck_msg_id >= 0) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs.header"; + + return fsck_report_ref(o, &report, fsck_msg_id, err_fmt, + (int)(eol - start), start); + + } + + return 0; +} + +static int packed_fsck_ref_content(struct fsck_options *o, + const char *start, const char *eof) +{ + struct strbuf packed_entry = STRBUF_INIT; + int line_number = 1; + const char *eol; + int ret = 0; + + strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } else { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + + ret |= fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_MISSING_HEADER, + "missing header line"); + } + + strbuf_release(&packed_entry); + return ret; +} + static int packed_fsck(struct ref_store *ref_store, struct fsck_options *o, struct worktree *wt) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); + struct strbuf packed_ref_content = STRBUF_INIT; int ret = 0; int fd; @@ -1786,7 +1866,16 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } + if (strbuf_read(&packed_ref_content, fd, 0) < 0) { + ret = error_errno(_("unable to read %s"), refs->path); + goto cleanup; + } + + ret = packed_fsck_ref_content(o, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); + cleanup: + strbuf_release(&packed_ref_content); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 42c8d4ca1e..a7b46b6cb9 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -639,4 +639,50 @@ 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 && + + printf "$(git rev-parse main) refs/heads/main\n" >.git/packed-refs && + git refs verify 2>err && + cat >expect <<-EOF && + warning: packed-refs: packedRefMissingHeader: missing header line + EOF + rm .git/packed-refs && + test_cmp expect err && + + for bad_header in "# pack-refs wit: peeled fully-peeled sorted " \ + "# pack-refs with traits: peeled fully-peeled sorted " \ + "# pack-refs with a: peeled fully-peeled" + do + printf "%s\n" "$bad_header" >.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs.header: badPackedRefHeader: '\''$bad_header'\'' does not start with '\''# pack-refs with:'\'' + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done && + + for unknown_header in "# pack-refs with: peeled fully-peeled sorted garbage" \ + "# pack-refs with: peeled" \ + "# pack-refs with: peeled peeled-fully sort" + do + printf "%s\n" "$unknown_header" >.git/packed-refs && + git refs verify 2>err && + cat >expect <<-EOF && + warning: packed-refs.header: unknownPackedRefHeader: '\''$unknown_header'\'' is an unknown packed-refs header + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done + ) +' + test_done From patchwork Thu Jan 30 04:07:46 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954279 Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) (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 8017FD2FB for ; Thu, 30 Jan 2025 04:06:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209979; cv=none; b=aD/GZ+m0O4EBkK7CiXrJ0vk01v0nPjyIWSCpwJk9JLRe2cIGahhD0V/lz2ztPDKeQeT2v9TEGMt0OV5g5BJYkmkjsBdaj0V0SRQejmr9PbAR7xngKATQePVjNpLBD/twe///+FweBoU0QeKyt4Y0cuQwrfXgZWWz2UtOhl6Z14g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209979; c=relaxed/simple; bh=KbxnRuHyJXge0KaPND7CZm3J4WUpkuSy5j4cuSoNVAU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WNf4HMPtpFPO0kg2iTP7Z3mE5uICA8NBbcbZVbUS/KH04jDfRLq2sq+2yEeSGFG9zL7TK07nZHXKtT6D8h9Hcr81+CrvFb3yn/IqtfmKudog5HFYdF+13soNlvlFiXmMUSIY/+hHEp3CUmtZgAXfJb9dzfboXEB5XWe3GGQw8es= 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=X/HroMNY; arc=none smtp.client-ip=209.85.216.42 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="X/HroMNY" Received: by mail-pj1-f42.google.com with SMTP id 98e67ed59e1d1-2ee46851b5eso404972a91.1 for ; Wed, 29 Jan 2025 20:06:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209975; x=1738814775; 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=oMqAIJocEcBVMffq7qJNZiRlafmS6n4xlxyv4gFV2BI=; b=X/HroMNYPLaxv59qBl8f0aP47Dd0vvxpVbbkY/DDTIr0SKQkhhL1Hernid+CDRk2Fg L6Zx13SYEUEmDD5iCh6/PobSDCneRP0k0UHE8bSptD6X4kEBUoyClNRsmkzVM4dcs2Xk 99zL+0gaPs8+8l3Ed+syCJWtRWO1JZEXk0qaZ3nXjSCrWAYEd95cb0KpK3hqyjeuQ4VR vEj34Ydy3oadHPZRoHvmjDCjwDyVd8lp3NKS3MSbQcqA0DBLEPFBYE5AgGoljend63Rl BlK0eZUM8BJ+Uy+4KqE+sSGGCJozvb6WTQeQVOTgnrnaShFJYDwolRCgzN7PxKFB4+FL XVRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209975; x=1738814775; 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=oMqAIJocEcBVMffq7qJNZiRlafmS6n4xlxyv4gFV2BI=; b=aMnUDLsVYa0piiZEqAQz4ZoaYx+VKmgFTgd/TtKb4yKylv/dY0XHBQ2waEHdQTJNcu 8b6aPJYLrQkLVUrVHTiDRFUcL+gcrEKHPbRZJj3dNLmKvG5CXzNvxjb5qLwzkciGPGtO XZ2S+pKUSzm56Crtk2q6ayl0Adn1dkJhL7rn280idMffcya/xVN1dryOarZveQ2NAtWI kwuJKczfpeUgvVKL0mehYNE+a/myNKidz35Xpl0alSOKHx++HaQWCqUQayHIBnq5sLa5 n4evMzlkuht00kdbLrolHrtd+ozTXitQLWGHk8gLq0qXHsEaW1osvwQvkZYAMooqEZwO 8ZCw== X-Gm-Message-State: AOJu0YzIOj3Y0AvcVdmevUH5MXrIsMtzqmJnkkydGoJW6mKqGnfmrk8v pkICokbwHXF2Sawrgf4QjqG9sQwrCj/ep5LuDbN0aWBLU95A4yXlx1Ly/WRf X-Gm-Gg: ASbGnctpCNEAiBKZN/Loelb+U874QSnssHqhKNO2xxlKdsR7a+0nNq/buNXuYhTYUfD 56bNBRreYniihSBduzpnsToNZuog48INYGBV8BF4tHMbFIbLzdL5y8SiSJhc1XSUZKj+Dosvf1v 78rYDKqqbOVJ5LTdQ6AhL9YkUX7TDyjiJ+819y3PO0rJh7fTGqUUBbxIWJqYBPtmIXADXiBFqYA GzXJxHBvMOjuGBy6pUk1IUZWIvDE01k/drt8QH2TsmeXULhOlLDEngd6KnqtX9ATaKOcw== X-Google-Smtp-Source: AGHT+IFethyzDAzcN35V2sCn/0U3J4zVPagPj4gRybGYsKt6ikenktakjuUkw5Yziw7gKNVOelwoPQ== X-Received: by 2002:a17:90b:2802:b0:2ee:a127:ba96 with SMTP id 98e67ed59e1d1-2f83ac5e619mr7195784a91.23.1738209975289; Wed, 29 Jan 2025 20:06:15 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 98e67ed59e1d1-2f848ade8c2sm448328a91.48.2025.01.29.20.06.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:06:14 -0800 (PST) Date: Thu, 30 Jan 2025 12:07:46 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 5/8] packed-backend: check whether the refname contains NUL characters Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already implemented the header consistency check for the raw "packed-refs" file. Before we implement the consistency check for each ref entry, let's analysis [1] which reports that "git fsck" cannot detect some NUL characters. "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. So, 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 | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 883189f3a1..870c8e7aaa 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -494,6 +494,22 @@ 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) +{ + const char *pos = memchr(refname->buf, '\0', refname->len + 1); + return pos < refname->buf + refname->len; +} + #define SMALL_FILE_SIZE (32*1024) /* @@ -895,6 +911,9 @@ static int next_record(struct packed_ref_iterator *iter) strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf; + if (refname_contains_nul(&iter->refname_buf)) + die("packed refname contains embedded NULL: %s", iter->base.refname); + if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { if (!refname_is_safe(iter->base.refname)) die("packed refname is dangerous: %s", From patchwork Thu Jan 30 04:07:58 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954280 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B7E70D2FB for ; Thu, 30 Jan 2025 04:06:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209989; cv=none; b=BonWJWk2lk/KJbXvwNXl2P7MP0LA2SnAPnUWFeF3ZEr9bX8imxgzK5Bwy6JGeXAyFksdtP+tMw6MMUzxfe+AlNvQF5S6LG89u82oNvO9/jHpWrcmFmJXZVSfzhOKn1bMRlyxTuR0iI359mROntlLy3ewpQci2hU66vXX305Xu2U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738209989; c=relaxed/simple; bh=5KydgSDNSFGJGhWACXwjH3/gwtzRk9KeT7dcjPeL3kA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EJzSOUcayzBTE6svFhZ3euZmohRHwiHzdxcH7a6+14jsOOPFVFc83V2WFFN5U49vQLMDcXIjrUUOIx3S1DluLe5Kjaqi1ovt5gztlT74SDxhX2gMZBINtwm+fRjgi+6/Q1K3P9SM2jZ24mSkh5v44Bp81+65Ml/c4zOhFhIoMqM= 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=YISn+S/h; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="YISn+S/h" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-219f8263ae0so5912055ad.0 for ; Wed, 29 Jan 2025 20:06:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209986; x=1738814786; 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=c1MEYpqdY2b8II3p/oeipdjqI0geKxZFR3eUllLO6lA=; b=YISn+S/hB0bk64xkvPuC7BmqAP8MIrCoX0X0c0w2vf+1ZGCyYmT6KQCDPSvAuCPUnL uBI3gvFAI2OVwY9y61S+l4e7KA/pH3766EL9lHLnET5HCYZlGTLzztCxyUYPfpJDss/9 ehclT6+FvS6uVb6q+WpHAjQIVteNHLm8KxmhDuwRkbXjgLdlgwkNHQa1oopgfPI4FFUF 4HebWdTqxZOc+PjC7GhPVVTR6Ik5n2hwtKnaK4ewjCEQRJEBb5QLVbzS25zcXI4PHkmI 5XPrsqecU/Qzxh9NfWbDS332uRIWG/Q7OWMNxG9gyiQbOjgcAC2DJJNFuKy0pIfgO4oy PYBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209986; x=1738814786; 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=c1MEYpqdY2b8II3p/oeipdjqI0geKxZFR3eUllLO6lA=; b=RSjfg+U3+R/zyhOKHoz83yH569PqnTVJAG1Yf0xw804ef3d+4q7oN/21u1h0poL9Q5 FtQPsxsaC1+huk9Pdv315nScjUa3AYxRMY4ufVeMi29DcJakBayJ1PrBM8AhITcj8gEU Q4MscjKBKHdm9k9Bt4hrLq385u0oA3xeptAVIz/ykvvOH/URJvBYRzFkeCPHB/Q+NqPK STa+o9cZnX6crn9DmjcychhhuC4mylmDV56o/5nSL6iaryD/n6vdRoHpa/EQE45BQlrT 2J5vYn81aOSRMBYXWJJpBP9SrCHZBPBdV074UCSVhDCiGgrYCdsarC8VrBrXIDzTCE2O LnJg== X-Gm-Message-State: AOJu0Yzqs7rCrlybCyq0SytLnSm4n2tyCBNB8Qn8epsvNCF/W9wL/AIA M5cAmrQr/Kl9He9nYt3hPzl6ihu+jS7Wbygd/MrsB0XURElUEpKRkkelGuDA X-Gm-Gg: ASbGncvdFOrRsCZMt4vL5dzjvdRD26b++QWZBsSMb5GJLHX2fsNOJpIUlnFSDP+5fdn 8b0yAWXF33TOr9j5FfZOd+gCgViUm/G89FyreQR0TvUncGnK8+rUExibyBB/cgX3yC/Ux5RyYuT JYa6ShNU9aMMAzVceC6G13uc2wa+Gv8p+y8D4EVT/pqWHInA2kZWVfa0J7oBkFSVhWakYxsbsIY NRYZrrqiT6wnm66wGuK+bTuRUmP0/P8wJD4p/krgQiK2a0REh+OsvpDhW5iznMForckpw== X-Google-Smtp-Source: AGHT+IHQUYhU91WlwaVB2oFWpyxmMSZfSlQXOBvuXz3K4XtV/r+y8F8I3uUXtH38i+Zozq6fOqfSuA== X-Received: by 2002:a17:902:c409:b0:215:5935:7eef with SMTP id d9443c01a7336-21dd7d78cafmr93711785ad.22.1738209986427; Wed, 29 Jan 2025 20:06:26 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 41be03b00d2f7-acebb3414a4sm349556a12.0.2025.01.29.20.06.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:06:25 -0800 (PST) Date: Thu, 30 Jan 2025 12:07:58 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 6/8] packed-backend: add "packed-refs" entry consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will parse the ref entry to check the consistency. This function has already checked the following things: 1. Parse the main line of the ref entry, if the oid is not correct. It will die the program. And then it will check whether the next character of the oid is space. Then it will check whether the refname is correct. 2. If the next line starts with '^', it will continue to parse the oid of the peeled oid content and check whether the last character is '\n'. We can iterate each line by using the "packed_fsck_ref_next_line" function. Then, create a new fsck message "badPackedRefEntry(ERROR)" to report to the user when something is wrong. Create two new functions "packed_fsck_ref_main_line" and "packed_fsck_ref_peeled_line" for case 1 and case 2 respectively. Last, update the unit test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 ++ fsck.h | 1 + refs/packed-backend.c | 98 ++++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 42 +++++++++++++++ 4 files changed, 143 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 34375a3143..2a7ec7592e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,9 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefEntry`:: + (ERROR) The "packed-refs" file contains an invalid entry. + `badPackedRefHeader`:: (ERROR) The "packed-refs" file contains an invalid header. diff --git a/fsck.h b/fsck.h index 3107a0093d..40126242a4 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 870c8e7aaa..271c740728 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1819,10 +1819,86 @@ static int packed_fsck_ref_header(struct fsck_options *o, const char *start, con return 0; } +static int packed_fsck_ref_peeled_line(struct fsck_options *o, + struct ref_store *ref_store, + struct strbuf *packed_entry, + const char *start, const char *eol) +{ + struct fsck_ref_report report = { 0 }; + struct object_id peeled; + const char *p; + + report.path = packed_entry->buf; + + start++; + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid peeled oid", + (int)(eol - start), start); + } + + if (p != eol) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has trailing garbage after peeled oid '%.*s'", + (int)(eol - p), p); + } + + return 0; +} + +static int packed_fsck_ref_main_line(struct fsck_options *o, + struct ref_store *ref_store, + struct strbuf *packed_entry, + struct strbuf *refname, + const char *start, const char *eol) +{ + struct fsck_ref_report report = { 0 }; + struct object_id oid; + const char *p; + + report.path = packed_entry->buf; + + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid oid", + (int)(eol - start), start); + } + + if (p == eol || !isspace(*p)) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has no space after oid '%s' but with '%.*s'", + oid_to_hex(&oid), (int)(eol - p), p); + } + + p++; + strbuf_reset(refname); + strbuf_add(refname, p, eol - p); + if (refname_contains_nul(refname)) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "refname '%s' contains NULL binaries", + refname->buf); + } + + if (check_refname_format(refname->buf, 0)) { + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_NAME, + "has bad refname '%s'", refname->buf); + } + + return 0; +} + static int packed_fsck_ref_content(struct fsck_options *o, + struct ref_store *ref_store, const char *start, const char *eof) { struct strbuf packed_entry = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; int line_number = 1; const char *eol; int ret = 0; @@ -1843,6 +1919,26 @@ static int packed_fsck_ref_content(struct fsck_options *o, "missing header line"); } + while (start < eof) { + strbuf_reset(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + ret |= packed_fsck_ref_main_line(o, ref_store, &packed_entry, &refname, start, eol); + start = eol + 1; + line_number++; + if (start < eof && *start == '^') { + strbuf_reset(&packed_entry); + strbuf_addf(&packed_entry, "packed-refs line %d", line_number); + ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); + ret |= packed_fsck_ref_peeled_line(o, ref_store, &packed_entry, + start, eol); + start = eol + 1; + line_number++; + } + } + + strbuf_release(&packed_entry); + strbuf_release(&refname); strbuf_release(&packed_entry); return ret; } @@ -1890,7 +1986,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 a7b46b6cb9..e4b4a58684 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -685,4 +685,46 @@ test_expect_success 'packed-refs header should be checked' ' ) ' +test_expect_success 'packed-refs content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + git tag -a annotated-tag-2 -m tag-2 && + + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_2_oid=$(git rev-parse annotated-tag-2) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) && + short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) && + + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s\n" "$short_oid refs/heads/branch-1" >>.git/packed-refs && + printf "%sx\n" "$branch_1_oid" >>.git/packed-refs && + printf "%s refs/heads/bad-branch\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/heads/branch.\n" "$branch_2_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-3\n" "$tag_1_oid" >>.git/packed-refs && + printf "^%s\n" "$short_oid" >>.git/packed-refs && + printf "%s refs/tags/annotated-tag-4.\n" "$tag_2_oid" >>.git/packed-refs && + printf "^%s garbage\n" "$tag_2_peeled_oid" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid + error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\'' + error: packed-refs line 4: badRefName: has bad refname '\'' refs/heads/bad-branch'\'' + error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\'' + error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid + error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\'' + error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\'' + EOF + test_cmp expect err + ) +' + test_done From patchwork Thu Jan 30 04:08:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954281 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (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 B5C32D2FB for ; Thu, 30 Jan 2025 04:06:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738210003; cv=none; b=a7cRSZfcEaIojYoCaHr4GwLLhkrl+7vgncA/ZmJNvEhSVQCaL8/LCazngOhWBo7woLBtAtcBj4MpbjScVox9loZMU+9NyAFaq0P1cT4gkBiMfz/Pxgj9lXTA5637kexUNQ23p5jZPjv9G5XF9+ErA3a0pXmF9uw6gt2ux84Jwqw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738210003; c=relaxed/simple; bh=sAKnlHBdLelImtDnBDP9BsThId+kuih74mMDRTxHNk8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LSdMb6o3y0qkmxd2ztSeMVWkrpsMMUeNqsOcWLDDbVOInOs0uV1V/85tk36HRUZaYA4jd4Gc7Q51qLfmaOPnflE4MBGNkzTU+iGJjWbA0rE6Jah5sSnXwih8XkMh32v/xf1iM0b2pw/ZW8eyis2KAcBCtpAUKl+w0JoKrTdH09c= 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=meH55qRA; arc=none smtp.client-ip=209.85.214.175 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="meH55qRA" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-2161eb95317so5253135ad.1 for ; Wed, 29 Jan 2025 20:06:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738209999; x=1738814799; 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=UGuWiZCus3MjSIoarHcnV3lfcGvXX/ukSiQ2APL+o58=; b=meH55qRAjqpRqnF6em3FV7R9qVc9YPdQq7CuCQqTR1tjclPWBFePLNaSLIiFcx8ld9 5yG61rOmooXooXOe1ZDSoDStaOjYmnYtvEv9sQhJhhunHgTADGdH09KKBb9dQkB2+eKi rlbXHDTh0BBBmCEHtpooqy7ZAGOcE55uea76vceABexkjWBj7ZbEJDFogh2mj8gfXqFs RonPMbULzuT9xKzGJVMdjof4Fnh3iiBPCCB0JUG/cGcW0PFQmncDwOfjvls0C8O78k9h Rd8jydYsxt/gwIckhrse+DqjWnRoXacIluRxGOMZ1h1ilNOekZ1QBmt4UfW6lv11F9/h LlKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738209999; x=1738814799; 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=UGuWiZCus3MjSIoarHcnV3lfcGvXX/ukSiQ2APL+o58=; b=BDhDSCKD4du1WxMHGGgNDEYmjUK1M1zaSVQWBov156EK7j7KSsFLqOgc8IJZpPuUcj an9OrK0NhcZnyzwVys46GJx9ElynBpw+ia8KeqCgb0lOSyo0yKMitOprh9WMK4N4qFkO IeB8XyX26I26yFtDDrmvpVgNL3Uo+RgnhgvR8pvP7+9kOPl7CBJg2moLABIwaS4Zf+eK 1gor+2pd88u4x3hpnXP8atKwyBEJ52mohh5RWKll8b/qEtTt6/8sjHm2s2mmhd4xeIX+ rN5p817G4hQP8Nojp9oy1b7ZVWD1uJdsWJ5e0Q2ac0HrnZ60YwCS0TZKWCbVCceFWO3i +cew== X-Gm-Message-State: AOJu0YwBPrU/fu4iqG5tnLajMSA2j4H4ZEqPZdV1vam0Kuh470qvZQCH Xgz79ZRzPVajNMgyWIJr/c+UAHF4+pOpw3/hLex4AFvmwEuy0/JOzFQNI8h6 X-Gm-Gg: ASbGncvaiFzZIZeFgRj8FwZWRJOdfj2eIjqXZu2ycephcs/ouRo1eODa+cAxiLBNY0s gM3nTLt5WppHb5b0rT0i/amkLSAUUNmWiOrCdnyQZnVJH7U3qwWuXZvPkYk21qOZ3uQgalRBjl5 UCY+5HdXu0jQu8LFsDDocxyiKBFPD26JTPi+5I/6ERuC9/lEyRnl5/N3yqvdBN65Q+l5zAWo+3g vCLygUj3vlksQwHW8X3MFel0eeyxfLxdaaWfWvmEe4u6g+/4/dMfKuE8Pg4Qu0a1kGFQw== X-Google-Smtp-Source: AGHT+IHa+TF8J7uCbqdMUk+fElELcUp+OrZ0JhViEHCNqn6lXX2MpdaWrHjNOQ21nl+Y//KMkfT4RQ== X-Received: by 2002:a17:902:e541:b0:216:282d:c697 with SMTP id d9443c01a7336-21dd7da3eb0mr89659005ad.27.1738209999380; Wed, 29 Jan 2025 20:06:39 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-21de31ee352sm4102315ad.4.2025.01.29.20.06.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:06:38 -0800 (PST) Date: Thu, 30 Jan 2025 12:08:10 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 7/8] packed-backend: check whether the "packed-refs" is sorted Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We will always try to sort the "packed-refs" increasingly by comparing the refname. So, we should add checks to verify whether the "packed-refs" is sorted. We already have code to parse the content. Let's create a new structure "fsck_packed_ref_entry" to store the state during the parsing process for every entry. It may seem that we could just add a new "struct strbuf refname" into the "struct fsck_packed_ref_entry" and during the parsing process, we could store the refname into this structure and we could compare later. However, this is not a good design due to the following reasons: 1. Because we need to store the state across the whole checking lifetime, we would consume a lot of memory if there are many entries in the "packed-refs" file. 2. The most important thing is that we cannot reuse the existing compare functions which cause repetition. So, instead of storing the "struct strbuf", let's use the existing structure "struct snaphost_record". And thus we could use the existing function "cmp_packed_ref_records". However, this function need an extra parameter for "struct snaphost". Extract the common part into a new function "cmp_packed_ref_records" to reuse this function to compare. Then, create a new function "packed_fsck_ref_sorted" to use the new fsck message "packedRefUnsorted(ERROR)" to report to the user. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/packed-backend.c | 100 +++++++++++++++++++++++++++++++--- t/t0602-reffiles-fsck.sh | 38 +++++++++++++ 4 files changed, 135 insertions(+), 7 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 2a7ec7592e..7a11d35c5e 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -190,6 +190,9 @@ `packedRefMissingHeader`:: (INFO) The "packed-refs" file does not contain the header. +`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 40126242a4..0d3d1045ae 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 271c740728..b250f987b2 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. @@ -1768,6 +1774,28 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +struct fsck_packed_ref_entry { + int line_number; + + struct snapshot_record record; +}; + +static struct fsck_packed_ref_entry *create_fsck_packed_ref_entry(int line_number, + const char *start) +{ + struct fsck_packed_ref_entry *entry = xcalloc(1, sizeof(*entry)); + entry->line_number = line_number; + entry->record.start = start; + return entry; +} + +static void free_fsck_packed_ref_entries(struct fsck_packed_ref_entry **entries, int nr) +{ + for (int i = 0; i < nr; i++) + free(entries[i]); + free(entries); +} + static int packed_fsck_ref_next_line(struct fsck_options *o, struct strbuf *packed_entry, const char *start, const char *eof, const char **eol) @@ -1893,13 +1921,60 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, return 0; } +static int packed_fsck_ref_sorted(struct fsck_options *o, + struct ref_store *ref_store, + struct fsck_packed_ref_entry **entries, + int nr) +{ + size_t hexsz = ref_store->repo->hash_algo->hexsz; + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct strbuf refname1 = STRBUF_INIT; + struct strbuf refname2 = STRBUF_INIT; + int ret = 0; + + for (int i = 1; i < nr; i++) { + const char *r1 = entries[i - 1]->record.start + hexsz + 1; + const char *r2 = entries[i]->record.start + hexsz + 1; + + if (cmp_packed_refname(r1, r2) >= 0) { + const char *err_fmt = + "refname '%s' is not less than next refname '%s'"; + const char *eol; + eol = memchr(entries[i - 1]->record.start, '\n', + entries[i - 1]->record.len); + strbuf_add(&refname1, r1, eol - r1); + eol = memchr(entries[i]->record.start, '\n', + entries[i]->record.len); + strbuf_add(&refname2, r2, eol - r2); + + strbuf_addf(&packed_entry, "packed-refs line %d", + entries[i - 1]->line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_UNSORTED, + err_fmt, refname1.buf, refname2.buf); + goto cleanup; + } + } + +cleanup: + strbuf_release(&packed_entry); + strbuf_release(&refname1); + strbuf_release(&refname2); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, const char *start, const char *eof) { struct strbuf packed_entry = STRBUF_INIT; + struct fsck_packed_ref_entry **entries; struct strbuf refname = STRBUF_INIT; + int entry_alloc = 20; int line_number = 1; + int entry_nr = 0; const char *eol; int ret = 0; @@ -1919,7 +1994,13 @@ static int packed_fsck_ref_content(struct fsck_options *o, "missing header line"); } + ALLOC_ARRAY(entries, entry_alloc); while (start < eof) { + struct fsck_packed_ref_entry *entry + = create_fsck_packed_ref_entry(line_number, start); + + ALLOC_GROW(entries, entry_nr + 1, entry_alloc); + entries[entry_nr++] = entry; strbuf_reset(&packed_entry); strbuf_addf(&packed_entry, "packed-refs line %d", line_number); ret |= packed_fsck_ref_next_line(o, &packed_entry, start, eof, &eol); @@ -1935,11 +2016,16 @@ static int packed_fsck_ref_content(struct fsck_options *o, start = eol + 1; line_number++; } + entry->record.len = start - entry->record.start; } + if (!ret) + ret |= packed_fsck_ref_sorted(o, ref_store, entries, entry_nr); + strbuf_release(&packed_entry); strbuf_release(&refname); strbuf_release(&packed_entry); + free_fsck_packed_ref_entries(entries, entry_nr); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index e4b4a58684..9d802d71a9 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -727,4 +727,42 @@ test_expect_success 'packed-refs content should be checked' ' ) ' +test_expect_success 'packed-ref sorted should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname1" >>.git/packed-refs && + printf "%s %s\n" "$branch_1_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname1'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err && + printf "# pack-refs with: peeled fully-peeled sorted \n" >.git/packed-refs && + printf "%s %s\n" "$tag_1_oid" "$refname3" >>.git/packed-refs && + printf "^%s\n" "$tag_1_peeled_oid" >>.git/packed-refs && + printf "%s %s\n" "$branch_2_oid" "$refname2" >>.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: packedRefUnsorted: refname '\''$refname3'\'' is not less than next refname '\''$refname2'\'' + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + test_done From patchwork Thu Jan 30 04:08:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13954282 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B8707D2FB for ; Thu, 30 Jan 2025 04:06:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738210013; cv=none; b=gxyh2WTCDXT+6/Zy+U/2DS7l44lZHwq9dJAFvqx4F8bYULv97Fr6ALeHM/MwSC5qG2nBlDGw4R+rx/pCjzdNHTnIgQUxoKmNF4xIsK6XoKXYIlqhvuBQdeawg9PPtXM01AX2lPLFa3q2e++P00fdWAazPkckAiOms4/fG8M7Z/c= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738210013; c=relaxed/simple; bh=qNjcZDRpsgK0melEBrO3nWr6WvcMd/JYFvdNOs6bLs4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=a29IQMlU794k0cHKfYOB+qghDOZm5K9KzgH8ZMmF8saZRe2N5YnQiV+VbImjJK583Ji8x8Z7+DFyAClVEAneZjmeFY33xmUF4VFv/7V6uu1AWG9SsVacCyiJuTnsNFOZeKsFdh9bNM+noLjbarxq8YFZg/wxrHdgid3PB2/NKTc= 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=Cu1i8bpc; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Cu1i8bpc" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-2166360285dso5244785ad.1 for ; Wed, 29 Jan 2025 20:06:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738210010; x=1738814810; 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=Gb7mfR9eLI9MX3S8Fmi0snuygFRASRa0K/Dt94u/lS0=; b=Cu1i8bpc4SbgdyLZSEa9VzyvzcJghXWQEKvEfeg7LbPNG1C7vdWTFaFuiTSCIa9Ihd t01UuJfojBaspnnr/y/RYnNg5A7ksYripMcAVdNOKFa3fy1RxrLYL6A6lAhoegkch3nU GkVLvLqeTFMg1UQjaDZMBAasT4dkFQzkmZSKk+qY/R7nU8DnONVZbmEojNjjwSG+989q z1a3UgMmAb6sBYh6OUzPgHHnLfdQFGeibaGuFDUzD/jZFWMPdfV4Kt6Yqyyh6eaVy6mv K6ZfUd5w3a59qi9ur2CgnZsP+TksxUlxEA4gozbm6iHsuKtWNki3pug9SdZhIyIEi1eI +hyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738210010; x=1738814810; 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=Gb7mfR9eLI9MX3S8Fmi0snuygFRASRa0K/Dt94u/lS0=; b=CK/4fBhpwutBO3WAHyrFw36y6/yoboFq1fVIzPy3kQIOUFVCmfi2FgBqmE6pVvIkBI NBKzeU/IC+FDfwHNoFmK0lXYs6bs72uy6YMZRoYJiL23owU8oPO0tfwtDwEvW2KFxhoR 7ofEsZk/ACBp7MZ8l3rS7kM7LlYcZ3Gmemmum1pxx00lniO1VieggAMjMW2FMWwk6DRQ Uorj5mSNM2ROtEI3PMs5lPoump9Q1iApoWQr1FcolWWLz/kFAxaz/NNrchFVh9nEvcdS ndg6Mz4Vt/PAUDNH+HW6cb9v9ya3a3P0YHxCe21/782mXLcPeiWkHYd6aHSUkxzaYxs+ DNTw== X-Gm-Message-State: AOJu0Yx7yOoBQKCG9eCVesGWp4IYuLwGbxfHKbBnufc16/dIghNi7b0J YCx8EvkJG9oxIolShvx4PV/7Yenb73GFziQLl01Zm5fYHnW1i9BdpVBNhcl0 X-Gm-Gg: ASbGnct2689KCFI5qCtjE199qVJp/YebAYQrfIp35Ot5Jn9+ABejOVyq3T8rH3dAMN0 OCHg99W3mi+3R/ohOZWlT+l+yYt/bOvO0a9sxjejedVY3lDoO8LRMOkz0/dBt/1MtxnB08PUUnz Q5HHDewZr6fI5S4lk29COTlVL4U8wqU1wD6QiiRzMTjIQUvmgQmNzknJQ1kwnjtgcKEjZJ59nM7 pla75KWNkbluE4LhRQAUoClqZPbdaywTT2JtodBQBXkT1hftDRIh+5J6TpP1dLnWEik7w== X-Google-Smtp-Source: AGHT+IFqx6UyZB2r80RkOpCxlwVporks3uclavCEQA3b77MQ41cBiZlZ1+DcnMgwEgeJVHsq5FvNgA== X-Received: by 2002:a05:6a21:b8a:b0:1ea:e7be:ff27 with SMTP id adf61e73a8af0-1ed7a5b6781mr9495831637.7.1738210010532; Wed, 29 Jan 2025 20:06:50 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-72fe6424facsm342820b3a.39.2025.01.29.20.06.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 20:06:50 -0800 (PST) Date: Thu, 30 Jan 2025 12:08:22 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v2 8/8] builtin/fsck: add `git refs verify` child process Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: At now, we have already implemented the ref consistency checks for both "files-backend" and "packed-backend". Although we would check some redundant things, it won't cause trouble. So, let's integrate it into the "git-fsck(1)" command to get feedback from the users. And also by calling "git refs verify" in "git-fsck(1)", we make sure that the new added checks don't break. Introduce a new function "fsck_refs" that initializes and runs a child process to execute the "git refs verify" command. In order to provide the user interface create a progress which makes the total task be 1. It's hard to know how many loose refs we will check now. We might improve this later. And we run this function in the first execution sequence of "git-fsck(1)" because we don't want the existing code of "git-fsck(1)" which implicitly checks the consistency of refs to die the program. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/fsck.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/builtin/fsck.c b/builtin/fsck.c index 7a4dcb0716..9a8613d07f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -905,6 +905,34 @@ 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; + uint64_t progress_num = 1; + + if (show_progress) + progress = start_progress(r, _("Checking ref database"), + progress_num); + + 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" @@ -970,6 +998,8 @@ int cmd_fsck(int argc, git_config(git_fsck_config, &fsck_obj_options); prepare_repo_settings(the_repository); + fsck_refs(the_repository); + if (connectivity_only) { for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(the_repository,