From patchwork Tue Feb 25 13:21:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990014 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 CE42720AF88 for ; Tue, 25 Feb 2025 13:21:06 +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=1740489669; cv=none; b=LgUgWSlXJIvxNTpHdZDYDOc/65/NFAArICwraM5Adoqk+klex8gKVy2hCq2z03ZPSISic4gj/uU7qJLTbFB57ycpQnUJewTZK/NyG7J6UoFwFSI8atHpVlnA+u3cOQqGYUCGC9votKtVX2cD9fzkpWSo40Jnuj9IWf9+xiSJZ0Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489669; c=relaxed/simple; bh=NiSz0kFXgj2oW62wBNpIa05qAzJdZQp1gZun8gTT0Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TGCt1mbuPSjkhyWiAJUFeNy1D1s60I2hK/18LFNb0W9ldFMlqf0KAg2OeKR7ML8xUKk6LwTlcdd/aEIyR36+JY8n51D81R27nH9DBxT67De0TTZ147oEFzgJFrWTu09oVxTrmeCFdDU21Pxl/VRpQfB6GqL1lNDtDzTs+LmwIUc= 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=PP0YL6FW; 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="PP0YL6FW" Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-220ec47991aso76948455ad.1 for ; Tue, 25 Feb 2025 05:21:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489665; x=1741094465; 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=PP0YL6FWRWbTGpp09xwac8RXTzZ9DMz6llR6VUopJPKD3IxY8fIkRnuTEc8t+DmOXB SIeD48ye1K7ZOcF4WxbHD1vlL4sZo3JwW0lsxp1xArRAvR/XCAQ7xX1cbK8nW7Yw1vY+ 4WQ0Bc2jt+d6KArjxUi8QfD9QBbZRvYnbR1qm5xIFiuxdlDPgC0q/pnyRA71etlGjbbY jE9bhx7lxJ50Ig0l2twH91u+RWapAXBFnaeYGHwOKXojTfttnKmFtOKCOkdMPxnfM+HP XVA+9CxVmqmEaWI+qFXu3dAyfhwq3npf7BSwjugibTmhp1t5YugjTViTxeSeuYqdj629 gNJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489665; x=1741094465; 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=A/Pi35bv0cnkuKy8hpgT2+QJP5keHlOzgySlL7etvZOCXPGK6OBhr7R8Wi8Dm1wMIi pgNARA0XxDBekEtQo7O7DMhQ4oHofXhIX4uscIxZOpJjmkcoHDVEtluZ8LSNq5/Ouhxf HO1o3ywIBSF2gIuH8oFC8RGTJnyxYCWC9+yWQgYUSOUYlHhqdWavBNi9FQdGH6n2hjXl ZTnDKVjAmfneQHV4bkPcLI0mMS/Nu2B54J7o3MQzF7NZfTkji/3ICJb9fUdvkQfFH6bx iVwEi5tKBV/+c6J4omzP28DcT0MMoOVqsqh3wes9lY0o8/TT34/I62QLUc5Xo8M+1qxb E6Fw== X-Gm-Message-State: AOJu0YwXk+D3eiQtAv6mkqG1FNXuy257F05T6Z2vgRPWJotHUh2Qrsyc C3CQYsuygpEanSke7pyXJgKL1Amjw1K4hLAWaoha1JPRm5pE3GMfCACj2w== X-Gm-Gg: ASbGncs/O+z7tQhwcLt3+wYKrCQuIL7SzWi/tRfZ3TgpWhNUjUD9ItNUeWoLM9bLIln 3dVHM4aJx/Qrj4IPEUrAQjwDPwhvtK4KcwmlbIp2qK6NeuadM8CRvcfBFQ7cUqiU1M+/5UnzzJz 6M38OSzUCsx5URa5YyYf29iLDUpezER/W9R7kWjh60QWgG9ltUEY6N+/47f/BvB5s0luOJYqmTE 8HJRAQlZ+4CDfTO7MUTwCayharNKIVIhqrmTMI3LfgrZeHAaFDs9WMGOB2iih+pwVaa9+jzLgGt KpiusgAXjyhTle+TJJfdIQ== X-Google-Smtp-Source: AGHT+IEMD9dGNRWWkA4587niBE9LuYXXYHNd9/lbZz+MbMq8l1Imr5lgkvjESQDn7tPR30FkUoy1xQ== X-Received: by 2002:a05:6a20:72a3:b0:1ee:6b39:c386 with SMTP id adf61e73a8af0-1eef3c77081mr30755793637.13.1740489665065; Tue, 25 Feb 2025 05:21:05 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-7347a7f9aafsm1468218b3a.105.2025.02.25.05.21.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:04 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:12 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 1/9] t0602: use subshell to ensure working directory unchanged Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: For every test, we would execute the command "cd repo" in the first but we never execute the command "cd .." to restore the working directory. However, it's either not a good idea use above way. Because if any test fails between "cd repo" and "cd ..", the "cd .." will never be reached. And we cannot correctly restore the working directory. Let's use subshell to ensure that the current working directory could be restored to the correct path. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- t/t0602-reffiles-fsck.sh | 967 ++++++++++++++++++++------------------- 1 file changed, 494 insertions(+), 473 deletions(-) diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index d4a08b823b..cf7a202d0d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -14,222 +14,229 @@ test_expect_success 'ref name should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - - git commit --allow-empty -m initial && - git checkout -b default-branch && - git tag default-tag && - git tag multi_hierarchy/default-tag && - - cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && - git refs verify 2>err && - test_must_be_empty err && - rm $branch_dir_prefix/@ && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && - git refs verify 2>err && - rm $tag_dir_prefix/tag-1.lock && - test_must_be_empty err && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/.lock: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/.lock && - test_cmp expect err && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/$refname: badRefName: invalid refname format - EOF - rm "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + ( + cd repo && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + git commit --allow-empty -m initial && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/multi_hierarchy/$refname" && - test_cmp expect err || return 1 - done && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - mkdir "$branch_dir_prefix/$refname" && - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && + rm $branch_dir_prefix/@ && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && + git refs verify 2>err && + rm $tag_dir_prefix/tag-1.lock && + test_must_be_empty err && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/$refname/default-branch: badRefName: invalid refname format + error: refs/tags/.lock: badRefName: invalid refname format EOF - rm -r "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done + rm $tag_dir_prefix/.lock && + test_cmp expect err && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname: badRefName: invalid refname format + EOF + rm "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/multi_hierarchy/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + mkdir "$branch_dir_prefix/$refname" && + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname/default-branch: badRefName: invalid refname format + EOF + rm -r "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref name check should be adapted into fsck messages' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - git commit --allow-empty -m initial && - git checkout -b branch-1 && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=warn refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/.branch-1: badRefName: invalid refname format - EOF - rm $branch_dir_prefix/.branch-1 && - test_cmp expect err && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=ignore refs verify 2>err && - test_must_be_empty err + ( + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=warn refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/.branch-1: badRefName: invalid refname format + EOF + rm $branch_dir_prefix/.branch-1 && + test_cmp expect err && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=ignore refs verify 2>err && + test_must_be_empty err + ) ' test_expect_success 'ref name check should work for multiple worktrees' ' test_when_finished "rm -rf repo" && git init repo && - - cd repo && - test_commit initial && - git checkout -b branch-1 && - test_commit second && - git checkout -b branch-2 && - test_commit third && - git checkout -b branch-3 && - git worktree add ./worktree-1 branch-1 && - git worktree add ./worktree-2 branch-2 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && - - cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && - cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err && - - for worktree in "worktree-1" "worktree-2" - do + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + ( - cd $worktree && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err || return 1 - ) - done + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + for worktree in "worktree-1" "worktree-2" + do + ( + cd $worktree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err || return 1 + ) + done + ) ' test_expect_success 'regular ref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && - git refs verify 2>err && - test_must_be_empty err && + git refs verify 2>err && + test_must_be_empty err && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/a/b/branch-bad && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - for trailing_content in " garbage" " more garbage" - do - printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && cat >expect <<-EOF && - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $branch_dir_prefix/branch-garbage && - test_cmp expect err || return 1 - done && + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - '\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err && - printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + '\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - garbage'\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err + ) ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -237,99 +244,103 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - bad_content_1=$(git rev-parse main)x && - bad_content_2=xfsazqfxcadas && - bad_content_3=Xfsazqfxcadas && - printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && - printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && - printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 - error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 - error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + bad_content_1=$(git rev-parse main)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && + printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && + printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 + error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 + error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'textual symref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" + do + printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for good_referent in "refs/heads/branch" "HEAD" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" - do - printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && - - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-1 && - test_cmp expect err && - - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-2 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-3 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-complicated && - test_cmp expect err + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err + ) ' test_expect_success 'textual symref content should be checked (aggregate)' ' @@ -337,32 +348,34 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && - printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'the target of the textual symref should be checked' ' @@ -370,28 +383,30 @@ test_expect_success 'the target of the textual symref should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && - git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && - - for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" - do - printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' - EOF - rm $branch_dir_prefix/branch-bad-1 && - test_cmp expect err || return 1 - done + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" + do + printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err || return 1 + done + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked' ' @@ -399,201 +414,207 @@ test_expect_success SYMLINKS 'symlink symref content should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - ln -sf ./main $branch_dir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $branch_dir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $branch_dir_prefix/branch-symbolic && - test_cmp expect err && - - ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref - error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' - EOF - rm $branch_dir_prefix/branch-symbolic-bad && - test_cmp expect err && - - ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref - error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' - EOF - rm $tag_dir_prefix/tag-symbolic-1 && - test_cmp expect err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + ln -sf ./main $branch_dir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $branch_dir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' + EOF + rm $branch_dir_prefix/branch-symbolic-bad && + test_cmp expect err && + + ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref + error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' + EOF + rm $tag_dir_prefix/tag-symbolic-1 && + test_cmp expect err + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked (worktree)' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - main_worktree_refdir_prefix=.git/refs/heads && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - - ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree1_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree2_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $main_worktree_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $worktree1_refdir_prefix/branch-symbolic && - test_cmp expect err && - - for bad_referent_name in ".tag" "branch " - do - ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + main_worktree_refdir_prefix=.git/refs/heads && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree1_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree2_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree2_refdir_prefix/bad-symbolic && + rm $main_worktree_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' EOF - rm $worktree2_refdir_prefix/bad-symbolic && - test_cmp expect err || return 1 - done + rm $worktree1_refdir_prefix/branch-symbolic && + test_cmp expect err && + + for bad_referent_name in ".tag" "branch " + do + ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && - test_must_fail git refs verify 2>err && + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $worktree1_refdir_prefix/bad-branch-1 && - test_cmp expect err || return 1 - done && + rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && - test_must_fail git refs verify 2>err && + printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' EOF - rm $worktree2_refdir_prefix/bad-branch-2 && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $worktree1_refdir_prefix/branch-no-newline && - test_cmp expect err && - - printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - EOF - rm $worktree1_refdir_prefix/branch-garbage && - test_cmp expect err + rm $worktree1_refdir_prefix/branch-garbage && + test_cmp expect err + ) ' test_done From patchwork Tue Feb 25 13:21:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990015 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 5FCFE267721 for ; Tue, 25 Feb 2025 13:21:17 +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=1740489678; cv=none; b=S+QN4eIZ4hxc27gfP0wOQAyIxL1cKRRtK03IVQA8QUyLQPYomRAsIO3v0loKgKdDCnEytSdla801QFYlDO3DMiYu4FZbm+MxtUAXWzW5Kg7Nm9crvxRdWNK6Kefv24kK/aFWhDAPB4jMgWcDg7iR2c5EvF5FBit899tN/Q7qP+U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489678; c=relaxed/simple; bh=91zfuoLoQwgrw0TlcEswgv67DDwoA1hs2npm5rnt8rU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oKJCEvW0cNIuuz/uzmdqdDefSz8iEYzJXcBb3Mk6m4YGRWhC+PSjPI0iK9x5tSqrqnZsx1v4LhH7//HKtxQ+kvCc2UvV7cFVLMABuXoue7V/2U9ql2LZjvYuM52knp0k5kbD/+pG9YFcZCDBeC7GwnOpOuZNV8pATTs210EmjJ0= 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=cPpcU91d; 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="cPpcU91d" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-220c8cf98bbso31793245ad.1 for ; Tue, 25 Feb 2025 05:21:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489676; x=1741094476; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=kn5rEPmZULamx/qgElC3YO1aE9Gj/Fu7ltZCF1dh5ds=; b=cPpcU91dF4+lXnRxDpzPo3SNPihV4xsJDsExzodWFTDOYSpxa4u//0GiRx5CeEg/gj RIi5hFYTkzyF7QbXT1pCCl2KOA/LfdXKex9PqGk8dJWvKmczS8WXPTnPvqUl9o39OiTv lkQyg0IcOEu/Ek2/syYR7lOu9U2Y2u0GadV28aeNdU4bfj103inF5HAbWshDPDToIq1g k7u/kc/MwUMx9tFPxftnTksP6X/hQyzcGJ87PvnFRHobP40PxV5rjYxwYopMidM4s+rh klTvw611Tvn21aJTBNaGS8nXSbtULf2X3ZY/uC6j5Teq8xCvUKXTUq/P7/bwdw6RgXMb 8X9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489676; x=1741094476; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kn5rEPmZULamx/qgElC3YO1aE9Gj/Fu7ltZCF1dh5ds=; b=UuEvgY84BXGmNXqWeYANVrtzbQVS073wrF8z8NC6zPenWSAzd7aBLfKGz5vsNO58pK C8PpoF6gGoou10Nb5CYE+rXngOhcY2pG9ITBi5yqHbldT4QPHqpW7I7TduHclHmpkpZ3 Y29F0Ui+zVWIHd8hCipPRMGeuRYTGQ+1g5uBhp2bal4qdNwz81di5ZDFzvXIHDFg2Rwv 8DzHGyb7YiygyaKLs513PiptprQ37Q74FWETx/GYBp3t67F69XDC60B2+aQC6QvWCOXe ofvRB2vDJjCzfgb1+kFubYoErCHsQUL5Tg4MZ+JkNZ+h8YxYkWKDujhA3gqAhOqXq8gN 1dmA== X-Gm-Message-State: AOJu0Yyh4t06C3hAKF+HuI7ri9JgH+afFKaKbWn0nQN0eXNCLJyRDSi/ 8KRITbF/FIu8r4wORT0UhcEAYfdV+p7XG09ilR0I8793627dV0pMmdV/ow== X-Gm-Gg: ASbGncuGQgUqyMP7nsOtGt3nqR2Oxt32bbLNwRWg1BWkU5jh3Ax4zODGvNGlYYyhA3q J7d04aw63Uzh0rp2IjrhPjZzpmM75oZNq4ndTQKD4PhuyRQ4drwtBReAidQffLcLxhPzJ0MugWq v0xiYW6kBZQyG0ZWSefe093LrziII65SkhBEGs5B43+BogLjtIGUBLHX/PXcExlZzLCiHme+64s ugZqiSwwGiWhMz/iXheTEeFCNwpTOYFJIq3tUriUfWHd0pMeEemoKKNrHexBfGtfzyecL8Rj4oU N2k0ykS+YxNY+JStS++y2g== X-Google-Smtp-Source: AGHT+IHZBHdR5KZKqlLEfbd0hcDwnYfADbG3LVUWdpQIVWq9Upw0EsCQJ5zf4/GGVPVBZqTkWbDIwA== X-Received: by 2002:a17:903:2282:b0:216:644f:bc0e with SMTP id d9443c01a7336-221a10dddbbmr309157255ad.24.1740489676056; Tue, 25 Feb 2025 05:21:16 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a0a3ca8sm13930935ad.169.2025.02.25.05.21.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:15 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:23 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 2/9] builtin/refs: get worktrees without reading head information Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c", there are some functions such as "create_snapshot" and "next_record" which would check the correctness of the content of the "packed-ref" file. When anything is bad, the program will die. It may seem that we have nothing relevant to above feature, because we are going to read and parse the raw "packed-ref" file without creating the snapshot and using the ref iterator to check the consistency. However, when using "get_worktrees" in "builtin/refs", we would parse the "HEAD" information. If the referent of the "HEAD" is inside the "packed-ref", we will call "create_snapshot" function to parse the "packed-ref" to get the information. No matter whether the entry of "HEAD" in "packed-ref" is correct, "create_snapshot" would call "verify_buffer_safe" to check whether there is a newline in the last line of the file. If not, the program will die. Although this behavior has no harm for the program, it will short-circuit the program. When the users execute "git refs verify" or "git fsck", we should avoid reading the head information, which may execute the read operation in packed backend with stricter checks to die the program. Instead, we should continue to check other parts of the "packed-refs" file completely. Fortunately, in 465a22b338 (worktree: skip reading HEAD when repairing worktrees, 2023-12-29), we have introduced a function "get_worktrees_internal" which allows us to get worktrees without reading head information. Create a new exposed function "get_worktrees_without_reading_head", then replace the "get_worktrees" in "builtin/refs" with the new created function. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 2 +- worktree.c | 5 +++++ worktree.h | 8 ++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin/refs.c b/builtin/refs.c index a29f195834..55ff5dae11 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -88,7 +88,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix, git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - worktrees = get_worktrees(); + worktrees = get_worktrees_without_reading_head(); for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); diff --git a/worktree.c b/worktree.c index d4a68c9c23..d23482a746 100644 --- a/worktree.c +++ b/worktree.c @@ -198,6 +198,11 @@ struct worktree **get_worktrees(void) return get_worktrees_internal(0); } +struct worktree **get_worktrees_without_reading_head(void) +{ + return get_worktrees_internal(1); +} + const char *get_worktree_git_dir(const struct worktree *wt) { if (!wt) diff --git a/worktree.h b/worktree.h index 38145df80f..a305c7e2c7 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,14 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. Skip reading HEAD allows to + * get the worktree without worrying about failures pertaining to parsing + * the HEAD ref. This is useful in contexts where it is assumed that the + * refdb may not be in a consistent state. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Tue Feb 25 13:21:32 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990016 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 6E40826868B for ; Tue, 25 Feb 2025 13:21:25 +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=1740489686; cv=none; b=kknEEteP8DB3Tn3UIH0bBvbIPy4nz1Pkog6iXSvkvfJn5P6zr5bcxJslkNf0ZaWzAm+UfKt13IRQSIs+hMkzNSWprLEIUCIXZLHZ4abWwRWB877fbUKRodXEPpitBGwyH8ncc/++v960A9TCzA9dKENYcD7k3MCc4bY9kaZQy+Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489686; c=relaxed/simple; bh=NEZrap5uRCMIn+O72hexQHgNj3Q2aEWqRNryyWSjQlk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=q+VObqeXcS4xFanrE/nkLPx/cmV+ufA9Ks3kIJPLOtbMB3Ks9/flrnKDC1qCcCCGAn61TWlPhqI0G9EX/9m0xFVqBw7WfGe/5MznjYWdXPa8/Sj/xlqfwdLMuwvHLjGBHk7YWK3dOmK4hiluBkxZJKIvSZjF2oEa98Zo3HGBqBg= 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=KtzYkqEx; 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="KtzYkqEx" Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-221050f3f00so125452495ad.2 for ; Tue, 25 Feb 2025 05:21:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489684; x=1741094484; 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=+Zk38zjNcqxn99TYghs3Li7mlOd2BXLB5nECBVoJzWw=; b=KtzYkqEx34NfgYNW6szhuFRCVQMtjUhTKib4pqURkp91VAJFw9b6a+d00gV+AigYDp dvdXEZjN7mRptN7OmQbWOVKYf7xKdFb+v+R8XL/Q6VtMgjSSyQ7Jr4R40uWuvPcK4SlD sa5wPHnHW6cbOo/YkZ0Yv22U0uYHCc1zBUbejUofUhgVWHs1lLUGj/zeivh6e2ek9tb/ eAQQogrCjr0RZXperKBl/OP3U0KOz1sRdrl2JzUHyrRTrVjL2XBFlcRNiFeZKvwixJz5 PkWBi8FzfcnErJ6LvQwCGtw8lZsruuIIFVjJwF2dPf3ObqR9ZsMTLF2C2Pvuw+i2/6r+ fAlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489684; x=1741094484; 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=+Zk38zjNcqxn99TYghs3Li7mlOd2BXLB5nECBVoJzWw=; b=mcCGF7AgE01TmqWN+CpKdHywKWgFo6Xq3a/QYq+tm6TeqK4BGKAxtHP6J/g0J/u6xo WclYsw7bc37XlljddG04AeBezjgdOXPMhbbtTAMaMVSNe2FpuR/7IUbsKcOQdJLmArw9 pweQU/lXuEcSARTdXFrECqrAFFptiuWYMOJBdl2wrIIMJATcqBWQOEKI90QjMjhbE//a LsO2btAeXEOt45QzAd+JTnuOUHNFdNHvTOuOYSnhx4Ny77n07HCL0jV2Rcc43teifE1X rWmONDJP/1XDJTxfebnKMIrd5Tqio59NZ/yOTvAGCsmrb6xlpyuPVPmQ2BkNSvwdlbCS O2Rg== X-Gm-Message-State: AOJu0Yz8VsUrbEMKyzSSfNME5Pf5HW15m8BLbh7jJ0EAHaNftGxak6xl LqTW5zZc/adT62qQQWGGxPN+LPXJOr/2u4+dn5v/0ooA9nbsfNo+0Xs//w== X-Gm-Gg: ASbGncvqVUkbEj3rt18sIvOR98IpNJ4/OBkPq3C+wQcMJDuYCrcE372iNCz/dNzz8eH b9gIUJk8gltkOhk1mMayCgbReluF9VBqv96436SuQHpdw3V8jzbg03aHlr2X/JUdKN3UBfVi073 Er9MA+Z7umXHdTktInVIVcysJiFGb4LCAZ8qIrwc6X6Githj2ZyUWZSDlc9dy+wVLHPYa1QnYMq O+2gZX11M7FWXZjeoTEHK8UQZZpxnLwKk1ZRk6tRZ3wRcINja/MYLsBF9uc8xhDmcb5ux7yn0sM XHRXl3j9MKjs3le8gCmYBg== X-Google-Smtp-Source: AGHT+IEqrO6oiAqnuvvaa5KoOaG1P9CyzhDgJBWEAcNLzHfDZe7MmfZ3CyUx44TkpY7KHmB47DDtNA== X-Received: by 2002:a17:902:ec82:b0:220:cd5f:9d76 with SMTP id d9443c01a7336-22307e791d6mr40053015ad.50.1740489684216; Tue, 25 Feb 2025 05:21:24 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a0a617dsm13704205ad.183.2025.02.25.05.21.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:23 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:32 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 3/9] packed-backend: check whether the "packed-refs" is regular file Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Although "git-fsck(1)" and "packed-backend.c" will check some consistency and correctness of "packed-refs" file, they never check the filetype of the "packed-refs". Let's verify that the "packed-refs" has the expected filetype, confirming it is created by "git pack-refs" command. Use "lstat" to check the file mode. If we cannot check the file status due to there is no such file this is OK because there is a possibility that there is no "packed-refs" in the repo. 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 | 37 +++++++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 22 ++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e..6c118119a0 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,43 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } -static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED, +static int packed_fsck(struct ref_store *ref_store, + struct fsck_options *o, struct worktree *wt) { + struct packed_ref_store *refs = packed_downcast(ref_store, + REF_STORE_READ, "fsck"); + struct stat st; + int ret = 0; if (!is_main_worktree(wt)) - return 0; + goto cleanup; - return 0; + if (o->verbose) + fprintf_ln(stderr, "Checking packed-refs file %s", refs->path); + + if (lstat(refs->path, &st) < 0) { + /* + * If the packed-refs file doesn't exist, there's nothing + * to check. + */ + if (errno == ENOENT) + goto cleanup; + ret = error_errno(_("unable to stat '%s'"), refs->path); + goto cleanup; + } + + if (!S_ISREG(st.st_mode)) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_FILETYPE, + "not a regular file"); + goto cleanup; + } + +cleanup: + 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..e65ca341cd 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-back .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 Tue Feb 25 13:21:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990017 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AEB6C269AE0 for ; Tue, 25 Feb 2025 13:21:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489697; cv=none; b=jODR8PIlETaBdydMbN5tMlb5mkyyJuUZsXAxdoyx31nVDecfMKrSiAkcJ3D229Nba/0Fi3WhuwAwpWyLbd3WhwMp2tRfxY1aWKYCunKbBsi5DqQmQsYAgObuCfs6jf/vpabo6VBIwRt48gUxZnTkTeCb1hREVh2g/h0B1MfEHF0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489697; c=relaxed/simple; bh=zB2ggN/Sn2QaoufTpsvQ98G7JQtHsqlNqmJ6V1c2Q0s=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hO7Cs8NKlVHw4uNIQaM4qrRW7GVnKsit6ESMaO/bkaPCtltFnaTNpLYNbs+r6EE0dON5EDjDEmmfPcyJ2a6ZGx409Vm7wLcqry3PMhpWDw6irn5i9kGUK0OF4WcJ/MJIKXP6nhiWWEgPL1jJfQBOhov4V2Cj/Q0MBO3vY5vSiks= 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=CNGdGYXG; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="CNGdGYXG" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2fa8ada6662so11079300a91.1 for ; Tue, 25 Feb 2025 05:21:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489693; x=1741094493; 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=RvauXIW4rY6MIUf51ccjmKEOY4tK+Uf8uEi09qLgXDA=; b=CNGdGYXGAy09gQ8dRPum8RtpHo+aGYSOfHX8r/a43cXupl76zHGXmUmbhJqLZuBsHF ZvLVmiANvzkLOzAHWkBFsWJVYHQ7FUCSInvnDjV94ttTfYjg6xVQPLwJfwXDhYUquKju f0nVd3NAS5aGGFdrAqyJLIlqoi8GRbPlR8azk+vpQtf43BsElr/T0LWVSQoUPHU6gfrN MIbCI/t2jhj0FSTg7r/3c07bbOILW7Nf4+dgrGXOawbHAbKtQw1cB3HgI3KtL7250HWd 8DRsbkzjuuLWGUXNwhELT6QN70HCNH3VcEZNSioVF7AueC092K/DrN/yYuRiqrCabJq2 XCAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489693; x=1741094493; 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=RvauXIW4rY6MIUf51ccjmKEOY4tK+Uf8uEi09qLgXDA=; b=IPo+29eCY+8o2m/DCYTqybW9u/IT/yhgvmFFqYMmEVITGHEabKO1zPuf/bFqZLLZCK FGpKQSoHzN9CuvGOJb8s0DLG3IzK5AUBt00hxQZDDwRiSxgILL0V9DChGWhrJziKdyij dVbkCxZMCW5jq7HQ46KvuZb6w+rSqMwaygcBk+ntADTk6OA/G+jK3bjWtXqr0upbMa0w GqzDemcU/eAZEHFrEB4XWNqWZGWkqe28jL0aLfaW7Ltcw4pM882y8mruTgcsZ0xtly8h +LyhC25n9wYYxEdeEw7gOV/ewpsYXooypkyocWbajbYGm1zDYlf855I7yd36wPbNspjw 8Zvw== X-Gm-Message-State: AOJu0YwcbESn00hc7UEWS4sLbbdmwW6kdQXQ5SqYTDezAAUOd5dA4Tjk 58DBy16Tt+6BpLEjzFWrPCGFsD0kKN2xnLRdgvw0XiF+tHAm5HJfGLP8WQ== X-Gm-Gg: ASbGncuiqa7asbgPVu6QG/XAVkOXxN6VABTq5uo0MNyM5YwSh3gLRN1Oen6sX7Dzq8O mnPMzRJaiKeJLXJeXDxrYsl6aIBXsR6owzz2Eqb2tODL7u+jhrLn0UXMVNJr6A+ip2AXzgs3dTT cR1FHNxBO16YRQqsAcNoNFVFyHrY8IF2MGlwxJKVpejdYYa1SwQgyDHGX2yShZm+22utfRfPezz 4kdjGFbFgzGaWlG5+18JPxvxf3/imhIvUG0qERdVVh3eka/GwfRDAD6yZaDuoUigJO81ywrvBpG WdahuizeCSi8pcPLsqQ1DA== X-Google-Smtp-Source: AGHT+IF1Z0GFjcGasZrUk3ddPLqurgyxTd1BOYMA0grmvDYTU7nkZN9M2Rwk12gnDp6vJCTPspQxPw== X-Received: by 2002:a17:90b:4d0d:b0:2ee:aef4:2c5d with SMTP id 98e67ed59e1d1-2fce8724453mr24609451a91.26.1740489693419; Tue, 25 Feb 2025 05:21:33 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id 98e67ed59e1d1-2fe6a39aa3csm1521156a91.10.2025.02.25.05.21.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:32 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:41 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 4/9] packed-backend: check if header starts with "# pack-refs with: " Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We always write a space after "# pack-refs with:". However, when creating the packed-ref snapshot, we only check whether the header starts with "# pack-refs with:". However, we need to make sure that we would not break compatibility by tightening the rule. The following is how some third-party libraries handle the header of "packed-ref" file. 1. libgit2 is fine and always writes the space. It also expects the whitespace to exist. 2. JGit does not expect th header to have a trailing space, but expects the "peeled" capability to have a leading space, which is mostly equivalent because that capability is typically the first one we write. It always writes the space. 3. gitoxide expects the space t exist and writes it. 4. go-git doesn't create the header by default. So, we are safe to tighten the rule by checking whether the header starts with "# pack-refs with: ". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6c118119a0..9dabb5e556 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -694,7 +694,7 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs) tmp = xmemdupz(snapshot->buf, eol - snapshot->buf); - if (!skip_prefix(tmp, "# pack-refs with:", (const char **)&p)) + if (!skip_prefix(tmp, "# pack-refs with: ", (const char **)&p)) die_invalid_line(refs->path, snapshot->buf, snapshot->eof - snapshot->buf); From patchwork Tue Feb 25 13:21:49 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990018 Received: from mail-pj1-f54.google.com (mail-pj1-f54.google.com [209.85.216.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5979220AF88 for ; Tue, 25 Feb 2025 13:21:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489706; cv=none; b=Zc721h+1X1NeUV+0gNe/Uz0e1ZRyLxonSFHCUVaWmYuvtpZWV+ST8zClceXB+fnVjrQYkeukWFtPtap2erXNEjO2OuJmpBJ9uL/XEmIdgrLAomMx8Q9/sPScRhOwEylXswb6zG86pFaoUXt0zZ9l0MHvTkhC6sv2EzajkODHni8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489706; c=relaxed/simple; bh=7D4pyMk43CFyX4TRC0Q3lWp57ydTVz6uDa7zDP8VRGA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=P/SgPD/NWwPtnBCS0i/KUxVuBI1vXY2t3RkPulJ+u1Lk4lqIt7jsdLFK4Mh+Uqwk/58XyfFtw+ymz6P9+/jnoevhk+TXAO1WzKN7MwrQi3EdGoeCfU8OiCRo2vtyZCYn54UEbhwomXC4wEZgZDZeS8zWuXQrDiaNAPN2edJR+2Q= 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=HASzTHwe; arc=none smtp.client-ip=209.85.216.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HASzTHwe" Received: by mail-pj1-f54.google.com with SMTP id 98e67ed59e1d1-2fbffe0254fso11261258a91.3 for ; Tue, 25 Feb 2025 05:21:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489702; x=1741094502; 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=7AH97J9VkX6zmmeh4ntf0OO0BnIyRiq2aYVrrWC6hOE=; b=HASzTHweIhDJTaoboljTVajm3T1HhFQJdUrzWbkHB2J4wEu1wzdKqB08HJkmNnuhsK bndTDMHur6rLAS5p3QJdhqTiwUyOm82OrRy5P+8P0mKmHze8OJH+BeuWZevI8M9VSA6v ZZij3PvqkKyo8W0P1UgmrtGMYHNmGWxQVF0GWmjmkvmquoS3HoOzyrSLeAnKuMGuMj+b ZY9Yrz0fvWAMYAqVdGYSCW2EOt5RBIwRkbO/f2/DouXhtx8lsW4Yyzgj+xVlVM84u9Ai BKfW1zX/NicReeIPyyOYZIbCKBHAQ1PbMH/gsaF/Gj0odtbIXKHgq633l++ZjQzuJEe8 NAGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489702; x=1741094502; 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=7AH97J9VkX6zmmeh4ntf0OO0BnIyRiq2aYVrrWC6hOE=; b=E3LIQGBsHxN4MCmG7wopeoosI114iOE/5lR4F1c6awKwYFSuO6o2SxveQ/Z/1xzb6J 5DinKRRdfek3Y1u4oTZeNbdKTJhUCCkYnA6CocbzZMOFNcq0R1ztWBNEcKPkCkeLTwxm HCSmdxufwyMZHneIWPJXmIYjxKXhpaBt2aTh+PbpVYHwTWWcbAgcd5hNNtNOLqxFAjo+ OQTy/4+G5rSv0BZIoRug8ITS1Nbsonzj2IxeuOnK3FIFABrwwK8Jujwcuoud0UQ75H7l uDYJZeUczq4+xNZHBp+p4mAAd84BuQZFSIp2KekiPdtwa3CwN0BOOZaI0WtNYFN6TCtO lmMg== X-Gm-Message-State: AOJu0YxRDT4CSN9VbJxS9NsLiOxPofBLr2surYCegawWnpaj6uMlH3eZ Jykl5+hkcTtp0JU5b4ydM4DzM1Q6+1GYFSokeBHkaZRXsSSSyg+2l7DuoQ== X-Gm-Gg: ASbGncuRxtJqZinioCFlA8DCtVfLmuCoLO0qQ99M/Nb1OK1PrwSrFNjCHK7PkQO2zla OGCh6ky0zDxV5srN7PKFUh4ipSJBcExjGJaEsKhlrcAtX2UBr7c7yoW14mquzDBEMauj/6xrHTH wVssiYTMGlHex3vL6aSfMm1ewh2rnRI168ruw4QVR4Xs2zNP1qGhxTsyq6Qa8zqi89WJq4r+qO2 1cK+ntS4igmlhYsE/kDhFL/OOWYQnpZ1ef2Sgax2hq0nKWyQ2iDB0EqWvytz4BrhHhqFAUVlmwQ pfThdjmqfGg5hx6yaP9+xQ== X-Google-Smtp-Source: AGHT+IE6tkC25JHgBrx9a6pebbRsisXyoOjmAAgNzXW8azv11Wcfl98wYOuRxm7bDMBHB9dPGhDgyQ== X-Received: by 2002:a17:90b:1a89:b0:2ee:c9b6:c267 with SMTP id 98e67ed59e1d1-2fce78a8a38mr31445315a91.9.1740489702036; Tue, 25 Feb 2025 05:21:42 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a000a2csm13935485ad.32.2025.02.25.05.21.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:41 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:49 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 5/9] packed-backend: add "packed-refs" header consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with: ". However, we need to consider other situations and discuss whether we need to add checks. 1. If the header does not exist, we should not report an error to the user. This is because in older Git version, we never write header in the "packed-refs" file. Also, we do allow no header in "packed-refs" in runtime. 2. If the header content does not start with "# packed-ref with: ", we should report an error just like what "create_snapshot" does. So, create a new fsck message "badPackedRefHeader(ERROR)" for this. 3. If the header content is not the same as the constant string "PACKED_REFS_HEADER". This is expected because we make it extensible intentionally and runtime "create_snapshot" won't complain about unknown traits. In order to align with the runtime behavior. There is no need to report. As we have analyzed, we only need to check the case 2 in the above. In order to do this, use "open_nofollow" function to get the file descriptor and then read the "packed-refs" file via "strbuf_read". Like what "create_snapshot" and other functions do, we could split the line by finding the next newline in the buffer. When we cannot find a newline, we could report an error. So, create a function "packed_fsck_ref_next_line" to find the next newline and if there is no such newline, use "packedRefEntryNotTerminated(ERROR)" to report an error to the user. Then, parse the first line to apply the checks. Update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.adoc | 8 +++ fsck.h | 2 + refs/packed-backend.c | 94 ++++++++++++++++++++++++++++++++++ t/t0602-reffiles-fsck.sh | 52 +++++++++++++++++++ 4 files changed, 156 insertions(+) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index b14bc44ca4..11906f90fd 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -16,6 +16,10 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefHeader`:: + (ERROR) The "packed-refs" file contains an invalid + header. + `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. @@ -176,6 +180,10 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`packedRefEntryNotTerminated`:: + (ERROR) The "packed-refs" file contains an entry that is + not terminated by a newline. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index a44c231a5f..67e3c97bc0 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ @@ -53,6 +54,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE, ERROR) \ FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ + FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 9dabb5e556..4891c86a5a 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1749,13 +1749,78 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +static int packed_fsck_ref_next_line(struct fsck_options *o, + unsigned long line_number, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED, + "'%.*s' is not terminated with a newline", + (int)(eof - start), start); + + /* + * There is no newline but we still want to parse it to the end of + * the buffer. + */ + *eol = eof; + strbuf_release(&packed_entry); + } + + return ret; +} + +static int packed_fsck_ref_header(struct fsck_options *o, + const char *start, const char *eol) +{ + if (!starts_with(start, "# pack-refs with: ")) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs.header"; + + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_HEADER, + "'%.*s' does not start with '# pack-refs with: '", + (int)(eol - start), start); + } + + return 0; +} + +static int packed_fsck_ref_content(struct fsck_options *o, + const char *start, const char *eof) +{ + unsigned long line_number = 1; + const char *eol; + int ret = 0; + + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } + + return ret; +} + static int packed_fsck(struct ref_store *ref_store, struct fsck_options *o, struct worktree *wt) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); + struct strbuf packed_ref_content = STRBUF_INIT; struct stat st; + int fd; int ret = 0; if (!is_main_worktree(wt)) @@ -1784,7 +1849,36 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } + /* + * There is a chance that "packed-refs" file is removed or converted to + * a symlink after filetype check and before open. So we need to avoid + * this race condition by opening the file. + */ + fd = open_nofollow(refs->path, O_RDONLY); + if (fd < 0) { + 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; + } + } + + 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 e65ca341cd..e055c36e74 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -639,4 +639,56 @@ test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' ) ' +test_expect_success 'packed-refs header should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + git refs verify 2>err && + test_must_be_empty err && + + for bad_header in "# pack-refs wit: peeled fully-peeled sorted " \ + "# pack-refs with traits: peeled fully-peeled sorted " \ + "# pack-refs with a: peeled fully-peeled" \ + "# pack-refs with:peeled fully-peeled sorted" + do + printf "%s\n" "$bad_header" >.git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs.header: badPackedRefHeader: '\''$bad_header'\'' does not start with '\''# pack-refs with: '\'' + EOF + rm .git/packed-refs && + test_cmp expect err || return 1 + done + ) +' + +test_expect_success 'packed-refs missing header should not be reported' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + printf "$(git rev-parse HEAD) refs/heads/main\n" >.git/packed-refs && + git refs verify 2>err && + test_must_be_empty err + ) +' + +test_expect_success 'packed-refs unknown traits should not be reported' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + + printf "# pack-refs with: peeled fully-peeled sorted foo\n" >.git/packed-refs && + git refs verify 2>err && + test_must_be_empty err + ) +' + test_done From patchwork Tue Feb 25 13:21:57 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990019 Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) (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 4B1F2268FD2 for ; Tue, 25 Feb 2025 13:21:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.49 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489712; cv=none; b=N7mL5czgURc/Dd8SSdQCShtSIgA87BBSGpkQJI+CIXrV//ZlxJlT/pqaSX5PIPH8VlUNrwXI984Hf+zN4NUk3Pf5zPZqwS/QIpAIwnegKpWHiwP3/MrVwTPUkcRzqymitdmLySbX8XkfVZn1aDUo30w/O9x7ko3REu+vyyEtjnc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489712; c=relaxed/simple; bh=dKAVAYc+7Bgw7JZdcIMaQWdJdidIWFcAyAVzpmecRMA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DoFc8m/K3LrsUC73cIAUUPOgq+MTCmaE7zw/l4gmiOOwEAARoeAh6DfIdwTR7qewunFydUmUDcgYn7YBDcTzSOIOWNF4bLS45sO1N6I8IGfSf5k17m53VAhYP0f7HBWusA64DYqLgCRO1E0eoLqjkJN7/4U/ElTNodbqWz0hMPg= 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=V9lGVn12; arc=none smtp.client-ip=209.85.216.49 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="V9lGVn12" Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2fc1843495eso8250347a91.1 for ; Tue, 25 Feb 2025 05:21:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489710; x=1741094510; 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=3LmQ2YImWfbSfaaB+1MToiKl1REfwyXXn34jm8/oJLw=; b=V9lGVn12IzCMLkKmmUD1ebSUeW//5K/N9cBbWx8VIozQrvXjelrdh/ckVq12f64iiN mti+BxpvI9NngvNTTezS0t53azrGKaR9tkYDMsyH6ICl+vHB3/BiKWnF7ACfaKQYB6IA TNXkIL3TQ/iXhgic4u2bu1xwU6neftRZztFe3VccQ73WnHb2pONARDVf4uQF4EXIgty4 bgYNRN0/q1BhnVvY4ntDS0Raz5Dn7+8/ACtj2ke1mjWqsp7L9g9v3VTD4NC52C86GvOX lSIeYKbn91ShivZ5dzhS1CW30OhE5EKc7LrKLhPe8Z/aMKr4U2h4J9PZnWkRNx58K3sK S6Gw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489710; x=1741094510; 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=3LmQ2YImWfbSfaaB+1MToiKl1REfwyXXn34jm8/oJLw=; b=foF4E0cnGOs/06ZNJasXQhmjf1waSYievBMbQelNmb1qZD/W/1fcNvmh64lwPWa7mx +PgOsRc73/Rq8ix3AyE7v1KZ+QXXw6N0+W/JZ8ub4NIyDk9uCanICfInAZ+NOzfqoRJf jV8/ecdmPkQou8iCjySiKjsdTWiqy7NM6YL7mFVoN0TvMeKupWaiIAtIhEkLdlXedR+e OnkOQDJicEvsgzZJb17NDoZMHEOFZc69xz2e6/qtSsPNQ1FFV6mnpxKIgJSyAWsG5KnK +7cFMoy8TuNyeSkTTwo4MfFvhQ3uB9cyWyDkOJ8IeIJpsalGjaskYAEPFUCaK36kQldz GJAw== X-Gm-Message-State: AOJu0YyOe2gw9K5YQb/ycumfDkwa/VRLf0BguKG3rddkgshDpwhCie6F URTBNKxA+hy6Y+82fYNSLgcH5p7qLM7beNNouXiP0wlYC4XzD4fsJpux6g== X-Gm-Gg: ASbGncsfJEqCvuFdXkiCeKyNz/P3hIwHTbGZJWTrGdN7UkALRgQZOd7/0nIp43+hfET fmvINqu/NOIWHRZU3OgJViJU2Prbnw+IXuasoQxBzqqupuuYFiMFyRd0T6P/9uUqQ11GFHASQRy VxpN5vHvJjU2hP3aVaNHIcU7oYY9n+zrT/mG31b2AXiqnpJEh7CzHJUh2fj2inEvL7jPOKSKcWH 9ZR/dPufnsYq6m255ECv+wV5Hgeu0BwPgx27uuzBmAEHxYw2HZ0qNskm7FZ2MjcF50pal8FkKVk eoyxM2r+rw50P0lbpsd0yg== X-Google-Smtp-Source: AGHT+IEbmnizJr2fXSYkiTKZ51NwPlEtexYCebQR5JBfqcNA68kX5GpOzq05VNIWzjRXziAac6munQ== X-Received: by 2002:a17:90b:4a41:b0:2f4:47fc:7f18 with SMTP id 98e67ed59e1d1-2fce78a2965mr29864078a91.10.1740489710149; Tue, 25 Feb 2025 05:21:50 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a092ee5sm13860485ad.133.2025.02.25.05.21.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:49 -0800 (PST) Date: Tue, 25 Feb 2025 21:21:57 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 6/9] packed-backend: check whether the refname contains NUL characters Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will use "check_refname_format" to check the consistency of the refname. If it is not OK, the program will die. However, it is reported in [1], we cannot catch some corruption. But we already have the code path and we must miss out something. We use the following code to get the refname: strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf In the above code, `p` is the start pointer of the refname and `eol` is the next newline pointer. We calculate the length of the refname by subtracting the two pointers. Then we add the memory range between `p` and `eol` to get the refname. However, if there are some NUL characters in the memory range between `p` and `eol`, we will see the refname as a valid ref name as long as the memory range between `p` and first occurred NUL character is valid. In order to catch above corruption, create a new function "refname_contains_nul" by searching the first NUL character. If it is not at the end of the string, there must be some NUL characters in the refname. Use this function in "next_record" function to die the program if "refname_contains_nul" returns true. [1] https://lore.kernel.org/git/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/ Reported-by: R. Diez Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4891c86a5a..a74ee57776 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -494,6 +494,21 @@ static void verify_buffer_safe(struct snapshot *snapshot) last_line, eof - last_line); } +/* + * When parsing the "packed-refs" file, we will parse it line by line. + * Because we know the start pointer of the refname and the next + * newline pointer, we could calculate the length of the refname by + * subtracting the two pointers. However, there is a corner case where + * the refname contains corrupted embedded NUL characters. And + * `check_refname_format()` will not catch this when the truncated + * refname is still a valid refname. To prevent this, we need to check + * whether the refname contains the NUL characters. + */ +static int refname_contains_nul(struct strbuf *refname) +{ + return !!memchr(refname->buf, '\0', refname->len); +} + #define SMALL_FILE_SIZE (32*1024) /* @@ -895,6 +910,9 @@ static int next_record(struct packed_ref_iterator *iter) strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf; + if (refname_contains_nul(&iter->refname_buf)) + die("packed refname contains embedded NULL: %s", iter->base.refname); + if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { if (!refname_is_safe(iter->base.refname)) die("packed refname is dangerous: %s", From patchwork Tue Feb 25 13:22:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990020 Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) (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 45C5C2690FB for ; Tue, 25 Feb 2025 13:21:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.170 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489720; cv=none; b=oiM2kJPTPQJHgRnVCQgeIB2ulqG+2Z870zQ32HotdOlQXL3cWpaaeWtHWSawCcIeUT5lENI7rQf4cZCI3vUlXoxgJ/0+r5ZOZb720sQDyuZtZDVfkgkFR20VqO8aDfM6BWSg3AbiL6Xs1mEhPQ3qQV/uJC+2irVSnUrv2dfFBfg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489720; c=relaxed/simple; bh=HOt5klsXveqjXI7FvJixxV7FTi5vs390BvPChM0tLqY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=dCQ9BmQVjyPzgJW1fn7VdfGyeBfTg0oB5sKSFlk7jcCP0nuuPnixcEerzrA/0quQzaDxbMk3oRGZVN2huObLgz96jnsSOlLs+rr64Mp5+Xh+nlGL4qM7AlAhyFgwtkCV5DAH5xsPapgg+mJg8ykhCDWhxcW1Eg/I6FNu9bD8pQc= 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=VJ7jYj0I; arc=none smtp.client-ip=209.85.214.170 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="VJ7jYj0I" Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-2212a930001so39184525ad.0 for ; Tue, 25 Feb 2025 05:21:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489718; x=1741094518; 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=wkSQSgV+yPru8b7qwmjDdQtnz76oZC5o0v/SzXOEchI=; b=VJ7jYj0IVuKo0mTYwrMCzcRGkGpcVE6SYZVnOlHLAZm/7caV+mQaJWtxQETTRi/GLo oVp23yiShcEGZf+/tik02SEXazGqRdpwzPfP9I5k3DauwiYVZbKUqathfg61V+4klWxN V/c+BdRn34kHlf3uUjLZIH7EGjdv6ekn50kiMCaZtWYsPkPZvEyuYqrCNhag6M1gLNKv wfnqn3Nowj8LKSz47L+wzNcoSAPurgD3TmxPdhHK+sW61lYKCgXv6Qsc/Yd6LUtfGTd4 Sw/Un5Wsu05fbrhQV+sSudsDDrax7rFiNlZD789mKxkGXmKqBzJgevvpJrID3TFh21pC gsrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489718; x=1741094518; 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=wkSQSgV+yPru8b7qwmjDdQtnz76oZC5o0v/SzXOEchI=; b=UEAz8j+8W4ZhAkbPN9AqrNDg5eHFc+R6TZEbcPyn+DeBpgYb6HuFXzSgQH1jLpHWed +iz2tAwdLxvsmj4/IQgaXj33XK1B+5fXaJ6VmkjQH1f5qZBJN5/9DH2HOVq+/Xz1Z30p Jz5mryAW4YcHXUBHi+dDbUbFnxNgOqklqRSZxCD2Jn/ur8l60y+iByPzmJrXqD/p2c7r 63V82a9od4RQzTKUZ5hZi4PuXY9hMXx1UfEWgiE/VYsDsUJsKAEM8Cuq/c1TMGbYcj96 vQ5fBm7UlPkRha8KHZoYSZPi90iH5Wqd2U4Tg4V3MQIvPLsylWBUmom0/X4hmrX4/77n xmgQ== X-Gm-Message-State: AOJu0YyJkwp6+SIdA39ugKMFxF0znyAsIrLL7JwueErwoXziyN7nQotO k3V6ELFsOH2obtLWZNtTPB8D1+kxxT5zD2B7XONrZtt5g7ShVBDIfc8MgQ== X-Gm-Gg: ASbGncvu42EdquoMeGQIOqvMobVTUSW24yAg7o+PLwOwtW25KvhXG63SPcAALztAGVX xUmVdmHwdBOu8wi1seNr/OjRK1GYJwcX/kaPaPHBT6wc9pauznf1I2IdKxgPPP1KCjxTZPLaiGL zORUWuaeIAtjUFn+FdPV9NfmrzLwwGMpNDM9l2PbUNk7yb7NmoEBrIq2HGKU54dIYAyF2aSQ/vF Mh6aABY00f7RhiWG3bL6mPHDaCwaH1te4vX8CqH8pSpgnCIuIo5W1W3Dt/RvkyEfpFYrZAVKc2G uC//dF7oTPVqSUl/6nhQAA== X-Google-Smtp-Source: AGHT+IE3J15VKak6lipU8MNo16ICTY/JiD9SrQQIETE/06uxFpUOR4DyYVPx28ahboH+vKBf3qvZ1w== X-Received: by 2002:a17:902:ce82:b0:221:283:5884 with SMTP id d9443c01a7336-221a10df197mr283411115ad.29.1740489717982; Tue, 25 Feb 2025 05:21:57 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a00093esm13833645ad.18.2025.02.25.05.21.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:21:57 -0800 (PST) Date: Tue, 25 Feb 2025 21:22:05 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 7/9] packed-backend: add "packed-refs" entry consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will parse the ref entry to check the consistency. This function has already checked the following things: 1. Parse the main line of the ref entry to inspect whether the oid is not correct. Then, check whether the next character is oid. Then check the refname. 2. If the next line starts with '^', it would continue to parse the peeled oid and check whether the last character is '\n'. As we decide to implement the ref consistency check for "packed-refs", let's port these two checks and update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.adoc | 3 + fsck.h | 1 + refs/packed-backend.c | 122 ++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 44 ++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index 11906f90fd..02a7bf0503 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -16,6 +16,9 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefEntry`:: + (ERROR) The "packed-refs" file contains an invalid entry. + `badPackedRefHeader`:: (ERROR) The "packed-refs" file contains an invalid header. diff --git a/fsck.h b/fsck.h index 67e3c97bc0..14d70f6653 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_ENTRY, ERROR) \ FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a74ee57776..dd3f7ab255 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1812,9 +1812,114 @@ static int packed_fsck_ref_header(struct fsck_options *o, return 0; } +static int packed_fsck_ref_peeled_line(struct fsck_options *o, + struct ref_store *ref_store, + unsigned long line_number, + const char *start, const char *eol) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object_id peeled; + const char *p; + int ret = 0; + + /* + * Skip the '^' and parse the peeled oid. + */ + start++; + if (parse_oid_hex_algop(start, &peeled, &p, ref_store->repo->hash_algo)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid peeled oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p != eol) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has trailing garbage after peeled oid '%.*s'", + (int)(eol - p), p); + goto cleanup; + } + +cleanup: + strbuf_release(&packed_entry); + return ret; +} + +static int packed_fsck_ref_main_line(struct fsck_options *o, + struct ref_store *ref_store, + unsigned long line_number, + struct strbuf *refname, + const char *start, const char *eol) +{ + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct object_id oid; + const char *p; + int ret = 0; + + if (parse_oid_hex_algop(start, &oid, &p, ref_store->repo->hash_algo)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "'%.*s' has invalid oid", + (int)(eol - start), start); + goto cleanup; + } + + if (p == eol || !isspace(*p)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "has no space after oid '%s' but with '%.*s'", + oid_to_hex(&oid), (int)(eol - p), p); + goto cleanup; + } + + p++; + strbuf_reset(refname); + strbuf_add(refname, p, eol - p); + if (refname_contains_nul(refname)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_ENTRY, + "refname '%s' contains NULL binaries", + refname->buf); + } + + if (check_refname_format(refname->buf, 0)) { + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_NAME, + "has bad refname '%s'", refname->buf); + } + +cleanup: + strbuf_release(&packed_entry); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, + struct ref_store *ref_store, const char *start, const char *eof) { + struct strbuf refname = STRBUF_INIT; unsigned long line_number = 1; const char *eol; int ret = 0; @@ -1827,6 +1932,21 @@ static int packed_fsck_ref_content(struct fsck_options *o, line_number++; } + while (start < eof) { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_main_line(o, ref_store, line_number, &refname, start, eol); + start = eol + 1; + line_number++; + if (start < eof && *start == '^') { + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + ret |= packed_fsck_ref_peeled_line(o, ref_store, line_number, + start, eol); + start = eol + 1; + line_number++; + } + } + + strbuf_release(&refname); return ret; } @@ -1892,7 +2012,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 e055c36e74..7421cc1e7f 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -691,4 +691,48 @@ test_expect_success 'packed-refs unknown traits should not be reported' ' ) ' +test_expect_success 'packed-refs content should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + git tag -a annotated-tag-2 -m tag-2 && + + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_2_oid=$(git rev-parse annotated-tag-2) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) && + short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $short_oid refs/heads/branch-1 + ${branch_1_oid}x + $branch_2_oid refs/heads/bad-branch + $branch_2_oid refs/heads/branch. + $tag_1_oid refs/tags/annotated-tag-3 + ^$short_oid + $tag_2_oid refs/tags/annotated-tag-4. + ^$tag_2_peeled_oid garbage + EOF + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 2: badPackedRefEntry: '\''$short_oid refs/heads/branch-1'\'' has invalid oid + error: packed-refs line 3: badPackedRefEntry: has no space after oid '\''$branch_1_oid'\'' but with '\''x'\'' + error: packed-refs line 4: badRefName: has bad refname '\'' refs/heads/bad-branch'\'' + error: packed-refs line 5: badRefName: has bad refname '\''refs/heads/branch.'\'' + error: packed-refs line 7: badPackedRefEntry: '\''$short_oid'\'' has invalid peeled oid + error: packed-refs line 8: badRefName: has bad refname '\''refs/tags/annotated-tag-4.'\'' + error: packed-refs line 9: badPackedRefEntry: has trailing garbage after peeled oid '\'' garbage'\'' + EOF + test_cmp expect err + ) +' + test_done From patchwork Tue Feb 25 13:22:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990021 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 324E62690FB for ; Tue, 25 Feb 2025 13:22:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489729; cv=none; b=PsuVMpohfUNVwvIo2FLhT7v7cGtadH+pF7iXQaotFMH9Dw2g0EhyMRFtKgM8bkA+50O2ZPgCuGStNi4bZoZoh21kfYBd5mOLH86ggY9L+J0+vZifzfZCANp4VM8SnMzT0tWfA80bykX7PUAQaqLqKu07dLnipKJIsm8gzuM1FJk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489729; c=relaxed/simple; bh=FLawI1IqemeQ0C3qJhwMcQD0vQO/C6K/5LuerCpJOJY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lKJE9dmUp3HElHQP2EPbCVikEwPI17CgcuoaUzfdyEvrANM3kuYUWBVOx5ugGIAwBke2Tacl8qkXH39EU5Zo1W9NRzEOj54G2ksMWAcxJbfuKMiBL7t1C0TR+Uh3wYwP1EvQ7Vw/ffnTGJJLL3SLGzM4luAeYHblWWWDqeGJoW0= 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=nslXSsss; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="nslXSsss" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-2fc3027c7aeso11101025a91.0 for ; Tue, 25 Feb 2025 05:22:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489727; x=1741094527; 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=n1BBMBraoYNVNsUG7OjdlmFnM3Vqo840s8IXBSAZAE0=; b=nslXSsssRbrX5MkAHXHAB3Heib+2gdN4m4DdsKVVMCdaaNwgeE/oR5ZZSOozVVDjEW cMJQ1vHSaqQSch8zOnIYZ6oy3oIw9ufrf0Z5u9Zl0xW7FIR2mlpX/UezEBKwwC5Tykqu aLZ5+Fch5VRmm1AD7MKpkxiYv2RooLUYCZfyI07yGxsY2JoDA126c9qpqvZBzKupqBM8 d68VkpIaPrT4gqtRoJvF9VQTVSjDDpIdV9whudOwi8WaaIuzetGDHr8swLcANZgPWT/Z pyJ7dye1HHfBcR/S78EHilrnYurUgMeyG/q+WrBrAQ0qhH0+dcTMzk3eFoQaVy92+5vB YO6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489727; x=1741094527; 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=n1BBMBraoYNVNsUG7OjdlmFnM3Vqo840s8IXBSAZAE0=; b=Ncny0U6YOGNAmjWmltOjb4/4zBq+g4jG1bJNgUF2p5IJF37dnhSgNZsafv9vAq+bqB yr2WxOP4Js35joPTrFUDLpyc8gmcQIItwrdFYF/5XX6wTN9irQI8tlyn4Rk4YRQmCvOl s245Q3pH8PheHd0RqSgeguWRAP04hNiF+l4coku6dNzZOZOFDEhUvtEYa95SjMzKGrhp e6BTDIHnyozf4hrIwwNXCFRIHyRGQPPUJYBRvpwZEPLZBfYgcYKptNwRiygU1yBbLOyx X8m+pwDJWLUEuvBCvD5+B/2mM29ujXZlvrGswMbkeU6EJNPvbLfp00WXQ+mxukufQuse hgpA== X-Gm-Message-State: AOJu0YyH9skCrvrRnVTfMZOZ4kHQPa+wLarW6TkcWcDIgWJ/Y4ygoc/x Cv5YhY9pLPh0zjc5rkblIt0NZxsLK36gjaT70qnfPe2vVYB7Fqzfts8+vg== X-Gm-Gg: ASbGncuFKaT53KDmm/ZKRJqGcdlJhtouI/iYwdCDGZOzpGUVmR21kf+s1wlaLG/HmmS yATapMOFwLkdHblT1lkDTmmR7IuX2k2+7/p292QKgYV2IgJcxYYH9QHj4qciIthfO4yk7JYgCSG /KLuszuZdp5O8vbslSz27JkLiOtt3VjcTxV2br9mPNreqsb5zowbtsE5N/vjm4hYbbDcyykxah1 Nz9p1JfdydRHqERaTPKhy1LJ+/1oCVIkrqeyUY9qUCpcg2zwf/1QEHgLvAGmtqdVTmaxfGsdzcE O8neR4IK/2fzTZoxsQnOsQ== X-Google-Smtp-Source: AGHT+IE22+uZrPAw8nOy3+xHZQtfgNfU671N4bmVkbFXmxYL4FUS09xUdARuUsAQ9wwLy6KivxH3Cg== X-Received: by 2002:a17:90b:3b88:b0:2ee:c4f2:a76d with SMTP id 98e67ed59e1d1-2fce86d0e7dmr24532803a91.21.1740489726974; Tue, 25 Feb 2025 05:22:06 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d9443c01a7336-2230a0b0dcfsm13664575ad.236.2025.02.25.05.22.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:22:06 -0800 (PST) Date: Tue, 25 Feb 2025 21:22:14 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 8/9] packed-backend: check whether the "packed-refs" is sorted Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When there is a "sorted" trait in the header of the "packed-refs" file, it means that each entry is sorted increasingly by comparing the refname. We should add checks to verify whether the "packed-refs" is sorted in this case. Update the "packed_fsck_ref_header" to know whether there is a "sorted" trail in the header. It may seem that we could record all refnames during the parsing process and then compare later. However, this is not a good design due to the following reasons: 1. Because we need to store the state across the whole checking lifetime, we would consume a lot of memory if there are many entries in the "packed-refs" file. 2. We cannot reuse the existing compare function "cmp_packed_ref_records" which cause repetition. Because "cmp_packed_ref_records" needs an extra parameter "struct snaphost", extract the common part into a new function "cmp_packed_ref_records" to reuse this function to compare. Then, create a new function "packed_fsck_ref_sorted" to parse the file again and user the new fsck message "packedRefUnsorted(ERROR)" to report to the user if the file is not sorted. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.adoc | 3 + fsck.h | 1 + refs/packed-backend.c | 118 ++++++++++++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 87 ++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 17 deletions(-) diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index 02a7bf0503..9601fff228 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -187,6 +187,9 @@ (ERROR) The "packed-refs" file contains an entry that is not terminated by a newline. +`packedRefUnsorted`:: + (ERROR) The "packed-refs" file is not sorted. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index 14d70f6653..19f3cb2773 100644 --- a/fsck.h +++ b/fsck.h @@ -56,6 +56,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ + FUNC(PACKED_REF_UNSORTED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index dd3f7ab255..75f28e283a 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -300,14 +300,9 @@ struct snapshot_record { size_t len; }; -static int cmp_packed_ref_records(const void *v1, const void *v2, - void *cb_data) -{ - const struct snapshot *snapshot = cb_data; - const struct snapshot_record *e1 = v1, *e2 = v2; - const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; - const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; +static int cmp_packed_refname(const char *r1, const char *r2) +{ while (1) { if (*r1 == '\n') return *r2 == '\n' ? 0 : -1; @@ -322,6 +317,17 @@ static int cmp_packed_ref_records(const void *v1, const void *v2, } } +static int cmp_packed_ref_records(const void *v1, const void *v2, + void *cb_data) +{ + const struct snapshot *snapshot = cb_data; + const struct snapshot_record *e1 = v1, *e2 = v2; + const char *r1 = e1->start + snapshot_hexsz(snapshot) + 1; + const char *r2 = e2->start + snapshot_hexsz(snapshot) + 1; + + return cmp_packed_refname(r1, r2); +} + /* * Compare a snapshot record at `rec` to the specified NUL-terminated * refname. @@ -1797,19 +1803,33 @@ static int packed_fsck_ref_next_line(struct fsck_options *o, } static int packed_fsck_ref_header(struct fsck_options *o, - const char *start, const char *eol) + const char *start, const char *eol, + unsigned int *sorted) { - if (!starts_with(start, "# pack-refs with: ")) { + struct string_list traits = STRING_LIST_INIT_NODUP; + char *tmp_line; + int ret = 0; + char *p; + + tmp_line = xmemdupz(start, eol - start); + if (!skip_prefix(tmp_line, "# pack-refs with: ", (const char **)&p)) { struct fsck_ref_report report = { 0 }; report.path = "packed-refs.header"; - return fsck_report_ref(o, &report, - FSCK_MSG_BAD_PACKED_REF_HEADER, - "'%.*s' does not start with '# pack-refs with: '", - (int)(eol - start), start); + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_HEADER, + "'%.*s' does not start with '# pack-refs with: '", + (int)(eol - start), start); + goto cleanup; } - return 0; + string_list_split_in_place(&traits, p, " ", -1); + *sorted = unsorted_string_list_has_string(&traits, "sorted"); + +cleanup: + free(tmp_line); + string_list_clear(&traits, 0); + return ret; } static int packed_fsck_ref_peeled_line(struct fsck_options *o, @@ -1915,8 +1935,68 @@ static int packed_fsck_ref_main_line(struct fsck_options *o, return ret; } +static int packed_fsck_ref_sorted(struct fsck_options *o, + struct ref_store *ref_store, + const char *start, const char *eof) +{ + size_t hexsz = ref_store->repo->hash_algo->hexsz; + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + struct strbuf refname1 = STRBUF_INIT; + struct strbuf refname2 = STRBUF_INIT; + unsigned long line_number = 1; + const char *former = NULL; + const char *current; + const char *eol; + int ret = 0; + + if (*start == '#') { + eol = memchr(start, '\n', eof - start); + start = eol + 1; + line_number++; + } + + for (; start < eof; line_number++, start = eol + 1) { + eol = memchr(start, '\n', eof - start); + + if (*start == '^') + continue; + + if (!former) { + former = start + hexsz + 1; + continue; + } + + current = start + hexsz + 1; + if (cmp_packed_refname(former, current) >= 0) { + const char *err_fmt = + "refname '%s' is less than previous refname '%s'"; + + eol = memchr(former, '\n', eof - former); + strbuf_add(&refname1, former, eol - former); + eol = memchr(current, '\n', eof - current); + strbuf_add(&refname2, current, eol - current); + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_UNSORTED, + err_fmt, refname2.buf, refname1.buf); + goto cleanup; + } + former = current; + } + +cleanup: + strbuf_release(&packed_entry); + strbuf_release(&refname1); + strbuf_release(&refname2); + return ret; +} + static int packed_fsck_ref_content(struct fsck_options *o, struct ref_store *ref_store, + unsigned int *sorted, const char *start, const char *eof) { struct strbuf refname = STRBUF_INIT; @@ -1926,7 +2006,7 @@ static int packed_fsck_ref_content(struct fsck_options *o, ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); if (*start == '#') { - ret |= packed_fsck_ref_header(o, start, eol); + ret |= packed_fsck_ref_header(o, start, eol, sorted); start = eol + 1; line_number++; @@ -1957,9 +2037,10 @@ static int packed_fsck(struct ref_store *ref_store, struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); struct strbuf packed_ref_content = STRBUF_INIT; + unsigned int sorted = 0; struct stat st; - int fd; int ret = 0; + int fd; if (!is_main_worktree(wt)) goto cleanup; @@ -2012,8 +2093,11 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } - ret = packed_fsck_ref_content(o, ref_store, packed_ref_content.buf, + ret = packed_fsck_ref_content(o, ref_store, &sorted, packed_ref_content.buf, packed_ref_content.buf + packed_ref_content.len); + if (!ret && sorted) + ret = packed_fsck_ref_sorted(o, ref_store, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); cleanup: strbuf_release(&packed_ref_content); diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 7421cc1e7f..28dc8dcddc 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -735,4 +735,91 @@ test_expect_success 'packed-refs content should be checked' ' ) ' +test_expect_success 'packed-ref with sorted trait should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + EOF + git refs verify 2>err && + rm .git/packed-refs && + test_must_be_empty err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $branch_2_oid $refname1 + EOF + git refs verify 2>err && + rm .git/packed-refs && + test_must_be_empty err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $branch_2_oid $refname1 + $branch_1_oid $refname2 + $tag_1_oid $refname3 + EOF + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 3: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname1'\'' + EOF + rm .git/packed-refs && + test_cmp expect err && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled sorted + $tag_1_oid $refname3 + ^$tag_1_peeled_oid + $branch_2_oid $refname2 + EOF + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs line 4: packedRefUnsorted: refname '\''$refname2'\'' is less than previous refname '\''$refname3'\'' + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + +test_expect_success 'packed-ref without sorted trait should not be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git tag -a annotated-tag-1 -m tag-1 && + branch_1_oid=$(git rev-parse branch-1) && + branch_2_oid=$(git rev-parse branch-2) && + tag_1_oid=$(git rev-parse annotated-tag-1) && + tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) && + refname1="refs/heads/main" && + refname2="refs/heads/foo" && + refname3="refs/tags/foo" && + + cat >.git/packed-refs <<-EOF && + # pack-refs with: peeled fully-peeled + $branch_2_oid $refname1 + $branch_1_oid $refname2 + EOF + git refs verify 2>err && + test_must_be_empty err + ) +' + test_done From patchwork Tue Feb 25 13:22:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13990022 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (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 56E1326868B for ; Tue, 25 Feb 2025 13:22:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489738; cv=none; b=W40VqnbgfnOnnRhbEu5MIV72Mfwyn2rDsn/2GCw96HgYx/ngZYHNX4bn8sPuAqmuwLhigcuKE3PBFq0e+qyJ55Ap/3e98d+9tgu/DzPvUXtT0KW5uvdJAhTQehtHenlVO7GfyU4raVSKX/NUN1koKuXSDi/i+MQfJ6ceJIkw/z4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740489738; c=relaxed/simple; bh=wPaIqmLHwWRvrbBUYCny4tdq5axRNe06u08RN96g8oI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=g31ztEN9q5JX849ua5Sb8EBWCDnRY2SsaY8YUlgbXG59WrFVS/+4nI4xIsTIJyZjfvIwQozEwjRltStTTfzuvsE1SjV1ntKvrFfmKOXRNoUZw6XJ5pvjSH5HvipaKf8lwZXyARBLLYKXisP37lqgeroJ2qqgLiwsrkXo6SGSohM= 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=QVFEpe30; arc=none smtp.client-ip=209.85.214.173 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="QVFEpe30" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-220dc3831e3so102038295ad.0 for ; Tue, 25 Feb 2025 05:22:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740489735; x=1741094535; 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=ZdXrc7vhpoU7P9lSCnZihSpFpzQm1O5gdN7HPnT6JKs=; b=QVFEpe30RrX05YNK5L0zZpP2U+Dz4/rDgtH2Y8jU21aPZeB8wKwD2nlCXVXsiWrt8M 9osmLqjeZDtpYU+wY/terwT5NOIUUf7tDsUwS2f/YIo1p0eMzSa1+AFI2Br/Fj47L6MZ C/YZn8uET3MU7tCmbSNMCW0Gf6sP47iH74R/gcgcYy8ph8r99gIIvHwZw5r+orcgjCH1 NwZh21fR1ViuHbr9nWA9KEXA4/3S4fYN8ixqICdhkJSkSpRooMUbz8uAsn5W2RQKTfru NUdMYT4Z0daF9X2GQTZ1App82hrArl+fwDuxSNkfD3CX0N9E76jWo64v61TMOzHsZefi spKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740489735; x=1741094535; 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=ZdXrc7vhpoU7P9lSCnZihSpFpzQm1O5gdN7HPnT6JKs=; b=YJ66aoG8F7RQT7/4kFXFZw1MtCQAPELqseYDUPzBcKsLOTNqMtXLdbV5hSsfrIwxzB alFj7JOc6i0Lbc4PULCp7jrpvVtj9oGFfJC5V9BF4SM3nR9C3HIXLiag0YgCXdt13yGv N9ev4I/ioIaF+ClUuMkWG5xf6V4G4kcoUoi1xFI2b8L+DzrsnK6rnxrWEAKSiFBSh8G6 i8Op7elFy9qExGweXh2ohuX473U2Pimx8Ptt57TybdAwRT+uUqW/IdWogEF/qfYUep7x rXkMIVTwtGWS/9Eixu8sytdn4t9nvb4uDZj3b9bGjBhFy8x4NP9jEmYVxorTsNuxNLSF kyOA== X-Gm-Message-State: AOJu0Yx/SR0JutcH2fNqwV/y58XOjBn53L5TBzUOMiRFqafWJmbkgFXN OBfZh9yl+u0n55TSzMlD3rm9zlHFrl072ZnGUjm/RXGO4Se+SOCQveHKmQ== X-Gm-Gg: ASbGncsmyI6HViT8AS6m4JqxCE2thERUTTaXny8qoWkHmwaQwYknDMAAtth/zYzTtrn BVJQgrOwQMCf3UWiSwyqXmDFb0Is2IbTwUuKO98nbqnc6G+4LXfEYKHg17Td8eTOeqbN1J5dLU+ p3RiBh7fns/J0T073fd31jkamVC4jDneuLf7tZmcRGUAREX6wpWFgndGlCA8wMgvIMfEENYmC2U jWfU3CuZzxruszYqxVXtv2ypkb+RJqFMS0HCxVU8Igao5sIN1ZvsEBDktQl0ub3XZwCuH8XyJ1N BZFt4fgpV5UxmTDocuJKvA== X-Google-Smtp-Source: AGHT+IG1TC6g2AdaMBxDMsS1XHjpSGF3bpq+5YCLl1TZtv1Mk1HULTSeFXvTmQFAQ5Ocb45SlwWQKg== X-Received: by 2002:a05:6a00:991:b0:732:6276:b46c with SMTP id d2e1a72fcca58-73425a1fab8mr29216580b3a.0.1740489734951; Tue, 25 Feb 2025 05:22:14 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with UTF8SMTPSA id d2e1a72fcca58-7347a6f7c1csm1462874b3a.49.2025.02.25.05.22.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Feb 2025 05:22:14 -0800 (PST) Date: Tue, 25 Feb 2025 21:22:22 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v6 9/9] builtin/fsck: add `git refs verify` child process Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: At now, we have already implemented the ref consistency checks for both "files-backend" and "packed-backend". Although we would check some redundant things, it won't cause trouble. So, let's integrate it into the "git-fsck(1)" command to get feedback from the users. And also by calling "git refs verify" in "git-fsck(1)", we make sure that the new added checks don't break. Introduce a new function "fsck_refs" that initializes and runs a child process to execute the "git refs verify" command. In order to provide the user interface create a progress which makes the total task be 1. It's hard to know how many loose refs we will check now. We might improve this later. Then, introduce the option to allow the user to disable checking ref database consistency. Put this function in the very first execution sequence of "git-fsck(1)" due to that we don't want the existing code of "git-fsck(1)" which would implicitly check the consistency of refs to die the program. Last, update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/git-fsck.adoc | 7 ++++++- builtin/fsck.c | 33 ++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 39 +++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/Documentation/git-fsck.adoc b/Documentation/git-fsck.adoc index 8f32800a83..11203ba925 100644 --- a/Documentation/git-fsck.adoc +++ b/Documentation/git-fsck.adoc @@ -12,7 +12,7 @@ SYNOPSIS 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] [--[no-]full] [--strict] [--verbose] [--lost-found] [--[no-]dangling] [--[no-]progress] [--connectivity-only] - [--[no-]name-objects] [...] + [--[no-]name-objects] [--[no-]references] [...] DESCRIPTION ----------- @@ -104,6 +104,11 @@ care about this output and want to speed it up further. progress status even if the standard error stream is not directed to a terminal. +--[no-]references:: + Control whether to check the references database consistency + via 'git refs verify'. See linkgit:git-refs[1] for details. + The default is to check the references database. + CONFIGURATION ------------- diff --git a/builtin/fsck.c b/builtin/fsck.c index 7a4dcb0716..f4f395cfbd 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -50,6 +50,7 @@ static int verbose; static int show_progress = -1; static int show_dangling = 1; static int name_objects; +static int check_references = 1; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 #define ERROR_PACK 04 @@ -905,11 +906,37 @@ static int check_pack_rev_indexes(struct repository *r, int show_progress) return res; } +static void fsck_refs(struct repository *r) +{ + struct child_process refs_verify = CHILD_PROCESS_INIT; + struct progress *progress = NULL; + + if (show_progress) + progress = start_progress(r, _("Checking ref database"), 1); + + if (verbose) + fprintf_ln(stderr, _("Checking ref database")); + + child_process_init(&refs_verify); + refs_verify.git_cmd = 1; + strvec_pushl(&refs_verify.args, "refs", "verify", NULL); + if (verbose) + strvec_push(&refs_verify.args, "--verbose"); + if (check_strict) + strvec_push(&refs_verify.args, "--strict"); + + if (run_command(&refs_verify)) + errors_found |= ERROR_REFS; + + display_progress(progress, 1); + stop_progress(&progress); +} + static char const * const fsck_usage[] = { N_("git fsck [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n" " [--[no-]full] [--strict] [--verbose] [--lost-found]\n" " [--[no-]dangling] [--[no-]progress] [--connectivity-only]\n" - " [--[no-]name-objects] [...]"), + " [--[no-]name-objects] [--[no-]references] [...]"), NULL }; @@ -928,6 +955,7 @@ static struct option fsck_opts[] = { N_("write dangling objects in .git/lost-found")), OPT_BOOL(0, "progress", &show_progress, N_("show progress")), OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), + OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), OPT_END(), }; @@ -970,6 +998,9 @@ int cmd_fsck(int argc, git_config(git_fsck_config, &fsck_obj_options); prepare_repo_settings(the_repository); + if (check_references) + fsck_refs(the_repository); + if (connectivity_only) { for_each_loose_object(mark_loose_for_connectivity, NULL, 0); for_each_packed_object(the_repository, diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 28dc8dcddc..42e8a84739 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -822,4 +822,43 @@ test_expect_success 'packed-ref without sorted trait should not be checked' ' ) ' +test_expect_success '--[no-]references option should apply to fsck' ' + test_when_finished "rm -rf repo" && + git init repo && + branch_dir_prefix=.git/refs/heads && + ( + cd repo && + test_commit default && + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck --references 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse HEAD)$trailing_content" >$branch_dir_prefix/branch-garbage && + git fsck --no-references 2>err && + rm $branch_dir_prefix/branch-garbage && + test_must_be_empty err || return 1 + done + ) +' + test_done