From patchwork Fri Feb 14 04:51:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974481 Received: from mail-yw1-f180.google.com (mail-yw1-f180.google.com [209.85.128.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 0916F22092 for ; Fri, 14 Feb 2025 04:52:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508730; cv=none; b=aRh6RanIBuB8a41yUs1QH9mw/i4LnvRDWVZHzAcl3GoK9mt0Jj2LDokz3kqIUQTRSH9bcWgYfAcPsd84Ik3/6AwCss/kMbd1B1LU7HWRK69sZ24gEa8tijE3qAZxQFjFd+As11UruyIonU4/RoKmCvQmh7znov+C9FEKjo81FyQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508730; c=relaxed/simple; bh=NiSz0kFXgj2oW62wBNpIa05qAzJdZQp1gZun8gTT0Hs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qTdwe3DPThXnNZth2laCbqIfzUhZ/6/SVxboFyqRP3bf6PHYukH4MvvDQ0L3koi/Y7kIwvvw3m0KmrNXzMcjLlhBJlRIaYqzTbLeFIbLyVDEItos2zhWeWCXtFllr+aZ/XI/hB0lb28I/9aht2V8wNvcs9UMh3pbzwsvkuZyrp8= 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=Ozp0J6eu; arc=none smtp.client-ip=209.85.128.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="Ozp0J6eu" Received: by mail-yw1-f180.google.com with SMTP id 00721157ae682-6f9b1bc594dso15367157b3.0 for ; Thu, 13 Feb 2025 20:52:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508725; x=1740113525; 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=Ozp0J6eukQ4QkMQv+1C27dsFNAv9w/S16lIVyEzNF11+mUOUmV019xF3MfXjulT2qg 5r6jkA8dMY6nT/hqgyXi0Jkhy3RLhOvWOtqxlsN1OzQQ55Mb9C/YOf4/BO0ITrgvabTF ILAYAdqD3bOeVR9vn4bX/W2EkGGfyoF9BcMWRAGabp95MRtxUYONxRsE7V5CJrkhBEw2 Gmi5kiL5U//lLksJgZh2sJb1dx28H1xlOrgccntOUGF89uKAk0Jh2S15woWzol82hI1u zeMewTioNyaZpLglOFXmt5cAS99t2hBW8s92Tx8LN+8tRUaoj43z6+Uk/XFyjmEcCg+d r74A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508725; x=1740113525; 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=XfW/xgJuxYpwLfBaNvvg8sGBm0mQKXicsWCgj9tE3PqOaUO7TSApPK26pl2U7vzHEC wr3sP3FYhe+WHamIdl2qNOxRLIWDVuX4FzFov/ihlh0WRYgsVf6avjo+LF+K9zSEuO6l gygWAOAzxEI7b9bky5ABjZ6PEZg5suOdaEgYQF6lnGcvDFhHDXU4Ogm1S2Jc914DOCJg RYt2QtH5tJy6qzXSDR/W5eZysTLLguFSTbbRf5Ow9zlkusUuvrCspLjVc2l+pj52R4IV i/bYyLab00ZJ7hwsDgA8QWAmo0b0hyMclDSvPYS13HpkYqz6OpsVpAoJn0F9wiFleghD US4A== X-Gm-Message-State: AOJu0YyyT7qws/kDfiKAGjCBmvdVAekXHBnv9PSWZQZV3VGqdOKJMdzX qHzbRfXR05QnV6jd7EFrPk9wt5b53aAeAxMnjZzsmVidE2yEaAXpk4i8DV1k X-Gm-Gg: ASbGncsRqlELMEWqMCHvYT3xMIlfk+MO2Hynw77Dk0kNWxLjOdbW0C3mvsc+hcKcnN7 TLzvadJ9XTnI0BSkPFmEPulz4/divjkPaZAw0tCJFHxoZZbWONhbRPPOtkyyr41hbnzT5UtDsNN ER8OqmKQ0HgpOF6473JH+c2BnFXSmoY2jVfpQY04jSvK6MvtoWS+PgWJAXflQQpjlloRbaCahuk gjBjzT7ttQmX8hGjT/Cm5RqieDyo0G7+Jfwg47UQu6Wsyf3qp5UMp14VJFZBpQiCDx3Eg== X-Google-Smtp-Source: AGHT+IEES82adJQEOcC9lwPNAh/7aghtr437zYSGzDkClBBIJQuO1E9pt7F66l3FsFgzrD8KIvUUbQ== X-Received: by 2002:a05:690c:6f08:b0:6f9:a212:908 with SMTP id 00721157ae682-6fb1f19ba44mr114427057b3.14.1739508724688; Thu, 13 Feb 2025 20:52:04 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 00721157ae682-6fb3619be11sm6166057b3.83.2025.02.13.20.52.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:52:04 -0800 (PST) Date: Fri, 14 Feb 2025 12:51:59 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 1/8] t0602: use subshell to ensure working directory unchanged Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: For every test, we would execute the command "cd repo" in the first but we never execute the command "cd .." to restore the working directory. However, it's either not a good idea use above way. Because if any test fails between "cd repo" and "cd ..", the "cd .." will never be reached. And we cannot correctly restore the working directory. Let's use subshell to ensure that the current working directory could be restored to the correct path. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- t/t0602-reffiles-fsck.sh | 967 ++++++++++++++++++++------------------- 1 file changed, 494 insertions(+), 473 deletions(-) diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index d4a08b823b..cf7a202d0d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -14,222 +14,229 @@ test_expect_success 'ref name should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - - git commit --allow-empty -m initial && - git checkout -b default-branch && - git tag default-tag && - git tag multi_hierarchy/default-tag && - - cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && - git refs verify 2>err && - test_must_be_empty err && - rm $branch_dir_prefix/@ && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && - git refs verify 2>err && - rm $tag_dir_prefix/tag-1.lock && - test_must_be_empty err && - - cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/.lock: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/.lock && - test_cmp expect err && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/$refname: badRefName: invalid refname format - EOF - rm "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + ( + cd repo && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/$refname" && - test_cmp expect err || return 1 - done && + git commit --allow-empty -m initial && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format - EOF - rm "$tag_dir_prefix/multi_hierarchy/$refname" && - test_cmp expect err || return 1 - done && - - for refname in ".refname-starts-with-dot" "~refname-has-stride" - do - mkdir "$branch_dir_prefix/$refname" && - cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && + rm $branch_dir_prefix/@ && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/tag-1.lock && + git refs verify 2>err && + rm $tag_dir_prefix/tag-1.lock && + test_must_be_empty err && + + cp $tag_dir_prefix/default-tag $tag_dir_prefix/.lock && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/$refname/default-branch: badRefName: invalid refname format + error: refs/tags/.lock: badRefName: invalid refname format EOF - rm -r "$branch_dir_prefix/$refname" && - test_cmp expect err || return 1 - done + rm $tag_dir_prefix/.lock && + test_cmp expect err && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname: badRefName: invalid refname format + EOF + rm "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/default-tag "$tag_dir_prefix/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + cp $tag_dir_prefix/multi_hierarchy/default-tag "$tag_dir_prefix/multi_hierarchy/$refname" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/tags/multi_hierarchy/$refname: badRefName: invalid refname format + EOF + rm "$tag_dir_prefix/multi_hierarchy/$refname" && + test_cmp expect err || return 1 + done && + + for refname in ".refname-starts-with-dot" "~refname-has-stride" + do + mkdir "$branch_dir_prefix/$refname" && + cp $branch_dir_prefix/default-branch "$branch_dir_prefix/$refname/default-branch" && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/$refname/default-branch: badRefName: invalid refname format + EOF + rm -r "$branch_dir_prefix/$refname" && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref name check should be adapted into fsck messages' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - git commit --allow-empty -m initial && - git checkout -b branch-1 && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=warn refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/.branch-1: badRefName: invalid refname format - EOF - rm $branch_dir_prefix/.branch-1 && - test_cmp expect err && - - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - git -c fsck.badRefName=ignore refs verify 2>err && - test_must_be_empty err + ( + cd repo && + git commit --allow-empty -m initial && + git checkout -b branch-1 && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=warn refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/.branch-1: badRefName: invalid refname format + EOF + rm $branch_dir_prefix/.branch-1 && + test_cmp expect err && + + cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && + git -c fsck.badRefName=ignore refs verify 2>err && + test_must_be_empty err + ) ' test_expect_success 'ref name check should work for multiple worktrees' ' test_when_finished "rm -rf repo" && git init repo && - - cd repo && - test_commit initial && - git checkout -b branch-1 && - test_commit second && - git checkout -b branch-2 && - test_commit third && - git checkout -b branch-3 && - git worktree add ./worktree-1 branch-1 && - git worktree add ./worktree-2 branch-2 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-3 - ) && - - cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && - cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err && - - for worktree in "worktree-1" "worktree-2" - do + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + ( - cd $worktree && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format - error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format - EOF - sort err >sorted_err && - test_cmp expect sorted_err || return 1 - ) - done + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + for worktree in "worktree-1" "worktree-2" + do + ( + cd $worktree && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err || return 1 + ) + done + ) ' test_expect_success 'regular ref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && - git refs verify 2>err && - test_must_be_empty err && + git refs verify 2>err && + test_must_be_empty err && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content - EOF - rm $branch_dir_prefix/a/b/branch-bad && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - for trailing_content in " garbage" " more garbage" - do - printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + for bad_content in "$(git rev-parse main)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$branch_dir_prefix/a/b/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content + EOF + rm $branch_dir_prefix/a/b/branch-bad && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && cat >expect <<-EOF && - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $branch_dir_prefix/branch-garbage && - test_cmp expect err || return 1 - done && + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + for trailing_content in " garbage" " more garbage" + do + printf "%s" "$(git rev-parse main)$trailing_content" >$branch_dir_prefix/branch-garbage && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\''$trailing_content'\'' + EOF + rm $branch_dir_prefix/branch-garbage && + test_cmp expect err || return 1 + done && + printf "%s\n\n\n" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - '\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err && - printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' + '\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err && + + printf "%s\n\n\n garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage-special && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-garbage-special: trailingRefContent: has trailing garbage: '\'' - garbage'\'' - EOF - rm $branch_dir_prefix/branch-garbage-special && - test_cmp expect err + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err + ) ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -237,99 +244,103 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - bad_content_1=$(git rev-parse main)x && - bad_content_2=xfsazqfxcadas && - bad_content_3=Xfsazqfxcadas && - printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && - printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && - printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && - printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && - printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 - error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 - error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 - warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + bad_content_1=$(git rev-parse main)x && + bad_content_2=xfsazqfxcadas && + bad_content_3=Xfsazqfxcadas && + printf "%s" $bad_content_1 >$tag_dir_prefix/tag-bad-1 && + printf "%s" $bad_content_2 >$tag_dir_prefix/tag-bad-2 && + printf "%s" $bad_content_3 >$branch_dir_prefix/a/b/branch-bad && + printf "%s" "$(git rev-parse main)" >$branch_dir_prefix/branch-no-newline && + printf "%s garbage" "$(git rev-parse main)" >$branch_dir_prefix/branch-garbage && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/a/b/branch-bad: badRefContent: $bad_content_3 + error: refs/tags/tag-bad-1: badRefContent: $bad_content_1 + error: refs/tags/tag-bad-2: badRefContent: $bad_content_2 + warning: refs/heads/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'textual symref content should be checked (individual)' ' test_when_finished "rm -rf repo" && git init repo && branch_dir_prefix=.git/refs/heads && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" + do + printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad && + test_cmp expect err || return 1 + done && - for good_referent in "refs/heads/branch" "HEAD" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && + cat >expect <<-EOF && + warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end + EOF + rm $branch_dir_prefix/branch-no-newline && + test_cmp expect err && - for bad_referent in "refs/heads/.branch" "refs/heads/~branch" "refs/heads/?branch" - do - printf "ref: %s\n" $bad_referent >$branch_dir_prefix/branch-bad && - test_must_fail git refs verify 2>err && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-1 && + test_cmp expect err && + + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + git refs verify 2>err && cat >expect <<-EOF && - error: refs/heads/branch-bad: badReferentName: points to invalid refname '\''$bad_referent'\'' + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines EOF - rm $branch_dir_prefix/branch-bad && - test_cmp expect err || return 1 - done && - - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $branch_dir_prefix/branch-no-newline && - test_cmp expect err && - - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-1 && - test_cmp expect err && - - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-2 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-trailing-3 && - test_cmp expect err && - - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - EOF - rm $branch_dir_prefix/a/b/branch-complicated && - test_cmp expect err + rm $branch_dir_prefix/a/b/branch-trailing-2 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-trailing-3 && + test_cmp expect err && + + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + EOF + rm $branch_dir_prefix/a/b/branch-complicated && + test_cmp expect err + ) ' test_expect_success 'textual symref content should be checked (aggregate)' ' @@ -337,32 +348,34 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && - printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && - printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && - printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && - printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && - printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && - printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && - printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && - - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' - warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end - warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines - warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end - EOF - sort err >sorted_err && - test_cmp expect sorted_err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + printf "ref: refs/heads/branch\n" >$branch_dir_prefix/branch-good && + printf "ref: HEAD\n" >$branch_dir_prefix/branch-head && + printf "ref: refs/heads/branch" >$branch_dir_prefix/branch-no-newline-1 && + printf "ref: refs/heads/branch " >$branch_dir_prefix/a/b/branch-trailing-1 && + printf "ref: refs/heads/branch\n\n" >$branch_dir_prefix/a/b/branch-trailing-2 && + printf "ref: refs/heads/branch \n" >$branch_dir_prefix/a/b/branch-trailing-3 && + printf "ref: refs/heads/branch \n " >$branch_dir_prefix/a/b/branch-complicated && + printf "ref: refs/heads/.branch\n" >$branch_dir_prefix/branch-bad-1 && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/branch-bad-1: badReferentName: points to invalid refname '\''refs/heads/.branch'\'' + warning: refs/heads/a/b/branch-complicated: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-complicated: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-1: refMissingNewline: misses LF at the end + warning: refs/heads/a/b/branch-trailing-1: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-2: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/a/b/branch-trailing-3: trailingRefContent: has trailing whitespaces or newlines + warning: refs/heads/branch-no-newline-1: refMissingNewline: misses LF at the end + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) ' test_expect_success 'the target of the textual symref should be checked' ' @@ -370,28 +383,30 @@ test_expect_success 'the target of the textual symref should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" - do - printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && - git refs verify 2>err && - rm $branch_dir_prefix/branch-good && - test_must_be_empty err || return 1 - done && - - for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" - do - printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' - EOF - rm $branch_dir_prefix/branch-bad-1 && - test_cmp expect err || return 1 - done + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + for good_referent in "refs/heads/branch" "HEAD" "refs/tags/tag" + do + printf "ref: %s\n" $good_referent >$branch_dir_prefix/branch-good && + git refs verify 2>err && + rm $branch_dir_prefix/branch-good && + test_must_be_empty err || return 1 + done && + + for nonref_referent in "refs-back/heads/branch" "refs-back/tags/tag" "reflogs/refs/heads/branch" + do + printf "ref: %s\n" $nonref_referent >$branch_dir_prefix/branch-bad-1 && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-bad-1: symrefTargetIsNotARef: points to non-ref target '\''$nonref_referent'\'' + EOF + rm $branch_dir_prefix/branch-bad-1 && + test_cmp expect err || return 1 + done + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked' ' @@ -399,201 +414,207 @@ test_expect_success SYMLINKS 'symlink symref content should be checked' ' git init repo && branch_dir_prefix=.git/refs/heads && tag_dir_prefix=.git/refs/tags && - cd repo && - test_commit default && - mkdir -p "$branch_dir_prefix/a/b" && - - ln -sf ./main $branch_dir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $branch_dir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $branch_dir_prefix/branch-symbolic && - test_cmp expect err && - - ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref - error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' - EOF - rm $branch_dir_prefix/branch-symbolic-bad && - test_cmp expect err && - - ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref - error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' - EOF - rm $tag_dir_prefix/tag-symbolic-1 && - test_cmp expect err + ( + cd repo && + test_commit default && + mkdir -p "$branch_dir_prefix/a/b" && + + ln -sf ./main $branch_dir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $branch_dir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../logs/branch-escape $branch_dir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: refs/heads/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $branch_dir_prefix/branch-symbolic && + test_cmp expect err && + + ln -sf ./"branch " $branch_dir_prefix/branch-symbolic-bad && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-bad: symlinkRef: use deprecated symbolic link for symref + error: refs/heads/branch-symbolic-bad: badReferentName: points to invalid refname '\''refs/heads/branch '\'' + EOF + rm $branch_dir_prefix/branch-symbolic-bad && + test_cmp expect err && + + ln -sf ./".tag" $tag_dir_prefix/tag-symbolic-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/tags/tag-symbolic-1: symlinkRef: use deprecated symbolic link for symref + error: refs/tags/tag-symbolic-1: badReferentName: points to invalid refname '\''refs/tags/.tag'\'' + EOF + rm $tag_dir_prefix/tag-symbolic-1 && + test_cmp expect err + ) ' test_expect_success SYMLINKS 'symlink symref content should be checked (worktree)' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - main_worktree_refdir_prefix=.git/refs/heads && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - - ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree1_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $worktree2_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && - git refs verify 2>err && - cat >expect <<-EOF && - warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref - EOF - rm $main_worktree_refdir_prefix/branch-symbolic-good && - test_cmp expect err && - - ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref - warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' - EOF - rm $worktree1_refdir_prefix/branch-symbolic && - test_cmp expect err && - - for bad_referent_name in ".tag" "branch " - do - ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + main_worktree_refdir_prefix=.git/refs/heads && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree1_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree1_refdir_prefix/bad-symbolic && + rm $worktree2_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref EOF - rm $worktree2_refdir_prefix/bad-symbolic && + rm $main_worktree_refdir_prefix/branch-symbolic-good && test_cmp expect err && - ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && - test_must_fail git refs verify 2>err && + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + git refs verify 2>err && cat >expect <<-EOF && - warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref - error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' EOF - rm $worktree2_refdir_prefix/bad-symbolic && - test_cmp expect err || return 1 - done + rm $worktree1_refdir_prefix/branch-symbolic && + test_cmp expect err && + + for bad_referent_name in ".tag" "branch " + do + ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err || return 1 + done + ) ' test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && - cd repo && - test_commit default && - git branch branch-1 && - git branch branch-2 && - git branch branch-3 && - git worktree add ./worktree-1 branch-2 && - git worktree add ./worktree-2 branch-3 && - worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && - worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - ( - cd worktree-1 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && - ( - cd worktree-2 && - git update-ref refs/worktree/branch-4 refs/heads/branch-1 - ) && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && - test_must_fail git refs verify 2>err && + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree1_refdir_prefix/bad-branch-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + EOF + rm $worktree1_refdir_prefix/bad-branch-1 && + test_cmp expect err || return 1 + done && + + for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" + do + printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + EOF + rm $worktree2_refdir_prefix/bad-branch-2 && + test_cmp expect err || return 1 + done && + + printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-1/refs/worktree/bad-branch-1: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end EOF - rm $worktree1_refdir_prefix/bad-branch-1 && - test_cmp expect err || return 1 - done && + rm $worktree1_refdir_prefix/branch-no-newline && + test_cmp expect err && - for bad_content in "$(git rev-parse HEAD)x" "xfsazqfxcadas" "Xfsazqfxcadas" - do - printf "%s" $bad_content >$worktree2_refdir_prefix/bad-branch-2 && - test_must_fail git refs verify 2>err && + printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && + git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree-2/refs/worktree/bad-branch-2: badRefContent: $bad_content + warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' EOF - rm $worktree2_refdir_prefix/bad-branch-2 && - test_cmp expect err || return 1 - done && - - printf "%s" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-no-newline && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-no-newline: refMissingNewline: misses LF at the end - EOF - rm $worktree1_refdir_prefix/branch-no-newline && - test_cmp expect err && - - printf "%s garbage" "$(git rev-parse HEAD)" >$worktree1_refdir_prefix/branch-garbage && - git refs verify 2>err && - cat >expect <<-EOF && - warning: worktrees/worktree-1/refs/worktree/branch-garbage: trailingRefContent: has trailing garbage: '\'' garbage'\'' - EOF - rm $worktree1_refdir_prefix/branch-garbage && - test_cmp expect err + rm $worktree1_refdir_prefix/branch-garbage && + test_cmp expect err + ) ' test_done From patchwork Fri Feb 14 04:52:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974482 Received: from mail-yw1-f178.google.com (mail-yw1-f178.google.com [209.85.128.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3F56B78F35 for ; Fri, 14 Feb 2025 04:52:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508743; cv=none; b=AZWRpu/bsxJ8DAnmf55yoFo51gG3ywfrgtSIYW4j+BEgPo1fGAY814VJzkyRznXKy0Mft7WL3KSNjOOGGQ7itTwPrLpbIljm8zQDILY/lFDFsCFtAeZk9BM4FyHrCOIsyJegfzYLR4KB2k2Nux6bjM3y1CJfTuTDLHTIuEqLnRQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508743; c=relaxed/simple; bh=Pw3z9o+kUxJpZxKRgSvXpacK5lCJj5GH/n7+SaPqQLY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=H3/7NQYC5CCkRrkUX93Wmr1FZTf+bdf52cxI0nhvrvrF5hdNqkHvvj2MONGUSHAdC0Jh4w28tTfdazJZCTE5ejOpogZisRxFXJhyIab2eTaHGD1c7VUWntjl1SqKXh+41DLJooovIFuYNlOIUuvsOEqDyDIUEEXlFRvz5hUT3kk= 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=Klbpi4LO; arc=none smtp.client-ip=209.85.128.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Klbpi4LO" Received: by mail-yw1-f178.google.com with SMTP id 00721157ae682-6f9c6d147edso15596917b3.3 for ; Thu, 13 Feb 2025 20:52:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508740; x=1740113540; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=h1Qe1T9rOx9zjzY4gdVYk5gvUXaPpIeOaRjiZtE7aaY=; b=Klbpi4LOSrQUFmtjg+reRIJM3NEPtZO4yt54wA2sm8zJ/YvliQI1xqGGBgNVb16roY sYG8lottg+DveFrxbEcKnGVb4l6acl1K9ZpkRhZpbv/CBWcRjpUNbm2Gk2G592fGoM8E L1CNtWMe8P6nAxiumTdTkyfkZkXl+FByNC+/imc0T6z6bG9QiDg854ZA7ZAmnd3BdrTk Q66aR/pUB4tm6XAuI0xiqLn/1NTssS2Ta9bdUXurbwzGoxICXds80kP/jBXwOX3Bxdsp bD6kO4V9YaiuW+t342xsCgiea9hvgfETgc0tx623YW/qHnQavNck/uvlH2SgZBNvQC20 eK4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508740; x=1740113540; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=h1Qe1T9rOx9zjzY4gdVYk5gvUXaPpIeOaRjiZtE7aaY=; b=LYsngaRgnp9voCtQoGetCejURt4HLHkX+SEdESD60EVVuy6gbZ3Z/rrue6rmwNooM3 wkFAAn+tMZ9Cv6y044fovXBzkalX9jRXz1nECnv+QdRGNMxTb3zWj69xX1HqVayY+5/J aIJ6MXyaOB9HagczXQR6WxjRF0lKBCsehW265DGACKePv6LJrOJYqQTdtOUkJBD25LiC DOUFxE3ql/ZoQ5pkVn8ZIyTHJCf3pIAJjeRWikp6OqZU3HxtuZas/7W9TqBNIBcxuM4P 4zWgmWvPp/ivjmbTppgO+7+aVE34L+Atd64gWnxwwoABN2vHKR+q9ya0gxjv2ORH+n2R YT3A== X-Gm-Message-State: AOJu0YxBvUb8X6ah+CR0KAnLj3ffbHdRfkDHUad+XKNcW9aLNw04Yv0R pzXQw/h5kGHJA677WAjp/YZL9hBUjnMn3lmWrG+Y+tPskCLou3H80g0hJSkC X-Gm-Gg: ASbGncv3S3mluoIPuOxrfjojj4lAG+K+T7Wde1ks8i8K2WPA0oRL/Id8sIW1CZ3ogkz jdnIMp/UvFwPBC0NXqG6Ibqa1BGa0K9KlxIj0V9UyFcaODIhKWaW1Hr7KJJpsArfB2j9sSzYP4P U0CEpZARqCVRZubCQJL+h8gN9Kn516zGVMbdrT2/Fw42SYD+k9wmcwwbsGwoN6R29OSHgDxw8PO SXAcs7oNdm1cuMY6tstcAwWgENBrHKpHi30w6BlXWHV+QfLcjbZwxF2D9LGQmzR6VXWhg== X-Google-Smtp-Source: AGHT+IHieUjAeb932Wst7bHhZGSZrEgiCCTXjlu8tnyC7En7whkN6VgT+DQBk9+fmk4tfK8fxmIipQ== X-Received: by 2002:a05:690c:6911:b0:6f9:ad48:a3c7 with SMTP id 00721157ae682-6fb1f1e5e09mr94842957b3.21.1739508740305; Thu, 13 Feb 2025 20:52:20 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 3f1490d57ef6-e5dade8aa10sm803879276.12.2025.02.13.20.52.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:52:19 -0800 (PST) Date: Fri, 14 Feb 2025 12:52:14 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 2/8] builtin/refs: get worktrees without reading head information Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c", there are some functions such as "create_snapshot" and "next_record" which would check the correctness of the content of the "packed-ref" file. When anything is bad, the program will die. It may seem that we have nothing relevant to above feature, because we are going to read and parse the raw "packed-ref" file without creating the snapshot and using the ref iterator to check the consistency. However, when using "get_worktrees" in "builtin/refs", we would parse the "HEAD" information. If the referent of the "HEAD" is inside the "packed-ref", we will call "create_snapshot" function to parse the "packed-ref" to get the information. No matter whether the entry of "HEAD" in "packed-ref" is correct, "create_snapshot" would call "verify_buffer_safe" to check whether there is a newline in the last line of the file. If not, the program will die. Although this behavior has no harm for the program, it will short-circuit the program. When the users execute "git refs verify" or "git fsck", we should avoid reading the head information, which may execute the read operation in packed backend with stricter checks to die the program. Instead, we should continue to check other parts of the "packed-refs" file completely. Fortunately, in 465a22b338 (worktree: skip reading HEAD when repairing worktrees, 2023-12-29), we have introduced a function "get_worktrees_internal" which allows us to get worktrees without reading head information. Create a new exposed function "get_worktrees_without_reading_head", then replace the "get_worktrees" in "builtin/refs" with the new created function. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 2 +- worktree.c | 5 +++++ worktree.h | 6 ++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/builtin/refs.c b/builtin/refs.c index a29f195834..55ff5dae11 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -88,7 +88,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix, git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - worktrees = get_worktrees(); + worktrees = get_worktrees_without_reading_head(); for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); diff --git a/worktree.c b/worktree.c index 248bbb39d4..89b7d86cef 100644 --- a/worktree.c +++ b/worktree.c @@ -175,6 +175,11 @@ struct worktree **get_worktrees(void) return get_worktrees_internal(0); } +struct worktree **get_worktrees_without_reading_head(void) +{ + return get_worktrees_internal(1); +} + const char *get_worktree_git_dir(const struct worktree *wt) { if (!wt) diff --git a/worktree.h b/worktree.h index 38145df80f..1ba4a161a0 100644 --- a/worktree.h +++ b/worktree.h @@ -30,6 +30,12 @@ struct worktree { */ struct worktree **get_worktrees(void); +/* + * Like `get_worktrees`, but does not read HEAD. This is useful when checking + * the consistency, as reading HEAD may not be necessary. + */ +struct worktree **get_worktrees_without_reading_head(void); + /* * Returns 1 if linked worktrees exist, 0 otherwise. */ From patchwork Fri Feb 14 04:52:28 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974483 Received: from mail-yb1-f176.google.com (mail-yb1-f176.google.com [209.85.219.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A9BDA18A6DE for ; Fri, 14 Feb 2025 04:52:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508757; cv=none; b=jSeAZzwgg0e/tup0ItMEThDU+uEI9bgGCqL26+fmpQCkpnuBY/PRbicJQKjM2O92d9sDPIXG+GFfO3/1MRUQoArfETGdq+y62oGIDNic6jHcvLJdIbfLcJ2jr8MIdcJZnEf0euqUy1hUfbdSfJt7mNozg1Y4/jm3nv18zu5lZHo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508757; c=relaxed/simple; bh=hvB8RSAeuHpGr5D4Vm3uLHj1yMDuoW9Kh8KYzt6v5wo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WxuiKx2Ouls8q9Ip9jfJCM4Q+bnHD0p/1uXV/OpohLI+St7BN6ltSd5YqvogH7HUQYCIwg3zvVDc/ISC7foP21+hU80e90/Tm9TMC/X97QA8UlQd6SqvGEatHhEtEzPPhSOWncQ8EdNtZN6rrc3SbFd3p3+lOY3JZnPj/u4kbQ4= 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=M2998zJc; arc=none smtp.client-ip=209.85.219.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="M2998zJc" Received: by mail-yb1-f176.google.com with SMTP id 3f1490d57ef6-e3c8ae3a3b2so1239959276.0 for ; Thu, 13 Feb 2025 20:52:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508754; x=1740113554; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=QScxn0YiELptPi/YqbglTfJ8vukbjirrSQIaMxw/KoY=; b=M2998zJco0I6lLFE8uASbL33dgBPhrbZjV5poVV7jOYURTE3TAXRCTIEKkGlsRdSAz tn/YVAMA41+LLCgbzUbCP2mPJ9ahoVdFxAl3bFnGOPYTr+XpzVDDsqQ3WFnHmiGG97lz Adv0nGzjG89ARaDXaWzo+KbG7h0iWawfePmyp+sOzaVOZ5433wYlbG/HRZgSDAq17nYz w27/Q3WsetiQeZiGpBoSSptna+HAZdXFP5UoOew1h4rNWklBE4fdfk2MfhVsptNVoDuY Pb5QbSAnaMJLXLBBwnE7TZBgvWjrrZOvIP2XPxXRmun+hFEKiKn4UTOev/iR9J9MmNiX dZ4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508754; x=1740113554; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=QScxn0YiELptPi/YqbglTfJ8vukbjirrSQIaMxw/KoY=; b=cZ2GXyb5fmo1SECH4YSYyeM8izdQ0nTbPIih0pkylmNfZvU87UQKrkUM6UGnQsPU7H xidkvN5C84kx6dbV+fGfPyrco0KiqBteGQt1hEDkV2UQf4PiDywbDq6Qw9KBBk2m2TL7 2IJJdTeqCzmiQPZUNetC7syscTYGGzEz0obct4eR7L3Luinnh8KcAQ3OMVwAAXFpIA9u NizHO1neWIlaDULq9gYQAk4tVNn4UkBoUF2UHBdF6k137b9LCoZC2pL2swvynsYUU1/p QDDcCB62LGja4sxgcnYOx9RmhgGpH3t1CCpChA9hvip+Q1VWx6OxLiMdVVm22sEVYOnh b7TQ== X-Gm-Message-State: AOJu0YzpkNT6/PbQKbR5iL8uOmhhB4Hmop1Iw6IAmEKd0UdbAJHGS34Y m99cg9T/Poc0u9uYMzoL0yEgANIZz4JfybNc/BcdiHvhESRxmQhPk6HH5lwP X-Gm-Gg: ASbGnctq+7czfya9UlUAtFwe+LzZLtTKysBgoMmzgYCHwLaI5HBJ8UKP3vnPMe+fDK8 iTk30eufvr0spIejGRw5mF4DoQCQs1uSvnsspYwO/vbT/rnDaV3ietGghN6bzxEIBsCiMbB6Pjb yLk7Z+MwehGsacQpEBW0XAsY1CGcwYXeLHRuxo+vPFz8DnVaO+LI5TIVdFHyvIYE2n38FX6pP/a Jmji2tq+u8mc6M6Hh92MnJrLb09ZFl7yJGQeIIfXZERmI5jKIxgFt9ilmVGTBDbY7DtoQ== X-Google-Smtp-Source: AGHT+IFB/ysADCQmA27OIrYmaEbTK/xctMjpq60QINYJSAQGG1WAI64j8ofbMBFNx9TgB9vv8J4Ehg== X-Received: by 2002:a05:6902:1704:b0:e5d:b88a:5536 with SMTP id 3f1490d57ef6-e5db88a57a4mr3578410276.44.1739508754202; Thu, 13 Feb 2025 20:52:34 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 3f1490d57ef6-e5dae0db1b8sm781423276.38.2025.02.13.20.52.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:52:33 -0800 (PST) Date: Fri, 14 Feb 2025 12:52:28 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 3/8] packed-backend: check whether the "packed-refs" is regular file Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Although "git-fsck(1)" and "packed-backend.c" will check some consistency and correctness of "packed-refs" file, they never check the filetype of the "packed-refs". The user should always use "git pack-refs" command to create the raw regular "packed-refs" file, so we need to explicitly check this in "git refs verify". We could use "open_nofollow" wrapper to open the raw "packed-refs" file. If the returned "fd" value is less than 0, we could check whether the "errno" is "ELOOP" to report an error to the user. Reuse "FSCK_MSG_BAD_REF_FILETYPE" fsck message id to report the error to the user if "packed-refs" is not a regular file. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 39 +++++++++++++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 22 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7b6f74b6e..6401cecd5f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -4,6 +4,7 @@ #include "../git-compat-util.h" #include "../config.h" #include "../dir.h" +#include "../fsck.h" #include "../gettext.h" #include "../hash.h" #include "../hex.h" @@ -1748,15 +1749,45 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } -static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED, +static int packed_fsck(struct ref_store *ref_store, + struct fsck_options *o, struct worktree *wt) { + struct packed_ref_store *refs = packed_downcast(ref_store, + REF_STORE_READ, "fsck"); + int ret = 0; + int fd; if (!is_main_worktree(wt)) - return 0; + goto cleanup; - return 0; + if (o->verbose) + fprintf_ln(stderr, "Checking packed-refs file %s", refs->path); + + fd = open_nofollow(refs->path, O_RDONLY); + if (fd < 0) { + /* + * If the packed-refs file doesn't exist, there's nothing + * to check. + */ + if (errno == ENOENT) + goto cleanup; + + if (errno == ELOOP) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs"; + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_FILETYPE, + "not a regular file"); + goto cleanup; + } + + ret = error_errno(_("unable to open %s"), refs->path); + goto cleanup; + } + +cleanup: + return ret; } struct ref_storage_be refs_be_packed = { diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index cf7a202d0d..42c8d4ca1e 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -617,4 +617,26 @@ test_expect_success 'ref content checks should work with worktrees' ' ) ' +test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git pack-refs --all && + + mv .git/packed-refs .git/packed-refs-back && + ln -sf packed-refs-bak .git/packed-refs && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: packed-refs: badRefFiletype: not a regular file + EOF + rm .git/packed-refs && + test_cmp expect err + ) +' + test_done From patchwork Fri Feb 14 04:52:43 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974484 Received: from mail-yw1-f182.google.com (mail-yw1-f182.google.com [209.85.128.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9177D19005D for ; Fri, 14 Feb 2025 04:52:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508772; cv=none; b=Y3UGzQg2q0JrPXDXJ1QYKXZeZuxdqKYYnkkC75oxxvVVhCmgizl7bB4nxNHFg5erq4AXydu60oygBZWYkZXEHxQZts/9jvMKrlHTgEPm19tjTMk0uc2rADAXSnfsEcKWYW1YfgDenFnqLOLnEMU9B6LdAxvN+bLnyCZULvYL1ak= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508772; c=relaxed/simple; bh=XgglGyyHVjOukLc+yaEc2H0wQABeyPTLor2XoYMAWBo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=M8AvwTounpFsMq2F7R2l7f7638Pzo2XOs1Fly23gbc6+dUIH7/60aA4z1l8uRj54RwLLtFq5tVHOnBxxk4C3ptH15vdUckTY+R4uBQrd+KQdGBlydu8gjpG6P/gHQJtIlmD4plIIbCAGzJUwy9J3kTSQsyi4Bf37bM6hFPnRcB4= 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=EaPDGs29; arc=none smtp.client-ip=209.85.128.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="EaPDGs29" Received: by mail-yw1-f182.google.com with SMTP id 00721157ae682-6f4b266d333so12704377b3.2 for ; Thu, 13 Feb 2025 20:52:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508769; x=1740113569; 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=rH1PScBOe9CxDRnikGkIPQKor90GpTZhqHywp/brHlY=; b=EaPDGs29v/tVWe3EE5sAtytCICC9tDK7ejLvLa2S1I/4iQ61Jtf6Zsvso5VZh7lTgf +H0kX+jccsCzgWzHLMiX+AXhHPH8/A5lNbOzDbtRgf99nl/ivqxXA1k93QW2R70Rk0NI zpR6avYCSgu8nIDGu1JZeBnIXn20mY699JFd9aOMkiuzEqVI1KZXT6m1n3xa5OfFG1su 35GG3dm8ciETTI6fo/cZMVOYsumJ4i7qGzPt/yudb26J1Jw7vqkbwL4TLcEuWutjFZ4V srXfr4za6b1e18itwXphLmh5BBIc+IGWhlduMahTMRP6GBJuUMzQWb/ZJP2Kp6RCJYDS YxYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508769; x=1740113569; 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=rH1PScBOe9CxDRnikGkIPQKor90GpTZhqHywp/brHlY=; b=dmgRAdxP3nKxZu4k7qcYliY+zhrUrUxaZEOo7+HpC6m/kMQOfoEHllFq/l0DR+FFEU 6UWREs9zQ2lLxLGL0HMuKahc/LrOQKBbPD7SDQaGSbeYIU16hAgbMZeFlVTXmw6isb7u JqJjMfm6g+c2ZcFzAdW0WKxn4g/re5oIQOuuiXWkjvAMItMY+K8BT4ffExP6q9NPy1RF IJRTPv61BLW2C9TpiFOxyKB1B4jN5vqZM5Tm6WyiL8A9KObncGOn0y0T2kCKOVzDoOB4 gf2Mg9KAAwa0Rc24423MflQMKmLCEkOZcCfzievXib9SN8cNMlExessuQxDCDqQfp4iQ +zYw== X-Gm-Message-State: AOJu0YzBWH+RgmVkYTah2nHH2SBmgiLevFMSChOeAdQU/wrPmpw3M4Dx vCSxDSs6L1WoXLdok2/ZomTiovJSZKYW1HnR3QrtRy6ks20Di3JrpY4pB9Jg X-Gm-Gg: ASbGncssD+rhGCDz1t1f+j2EkU+MlwGB4EPUhtiTBOA4dBsGP250pMT7LfC4csw0R7U KtVz4dGtTWXtohU7E7Fn3+njqfz6Adm514eKmz3zwcxkGKnYU55k7/3rK7WBGcQ0sVY7KVzUftH +Ey69tybcHclkhjUVhwckX3hXnnYqd5WNBetwGORsTofsu/J2KNMrBLj/mLIzbRyU7x832Fg1Dl L7pL9cJmNsbD49xh1vtiIADw/1QWxozuN0tBrAicZ1mgimwwdqIK3LxFXwa3aLa+cOuBg== X-Google-Smtp-Source: AGHT+IHPReLrzwMo8e6q7a44PXc7W0vg1xMyzJDPeoermgPq7xOLUvWVeM+lDC/9OumHZEN7bAZktg== X-Received: by 2002:a05:690c:6f0c:b0:6f9:43d4:44ee with SMTP id 00721157ae682-6fb1f683367mr115652447b3.30.1739508769008; Thu, 13 Feb 2025 20:52:49 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 00721157ae682-6fb361db9e8sm6189027b3.125.2025.02.13.20.52.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:52:47 -0800 (PST) Date: Fri, 14 Feb 2025 12:52:43 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 4/8] packed-backend: add "packed-refs" header consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: In "packed-backend.c::create_snapshot", if there is a header (the line which starts with '#'), we will check whether the line starts with "# pack-refs with:". Before we port this check into "packed_fsck", let's fix "create_snapshot" to check the prefix "# packed-ref with: " instead of "# packed-ref with:" due to that we will always write a single trailing space after the colon. However, we need to consider other situations and discuss whether we need to add checks. 1. If the header does not exist, we should not report an error to the user. This is because in older Git version, we never write header in the "packed-refs" file. Also, we do allow no header in "packed-refs" in runtime. 2. If the header content does not start with "# packed-ref with: ", we should report an error just like what "create_snapshot" does. So, create a new fsck message "badPackedRefHeader(ERROR)" for this. 3. If the header content is not the same as the constant string "PACKED_REFS_HEADER". This is expected because we make it extensible intentionally. So, there is no need to report. As we have analyzed, we only need to check the case 2 in the above. In order to do this, read the "packed-refs" file via "strbuf_read". Like what "create_snapshot" and other functions do, we could split the line by finding the next newline in the buffer. When we cannot find a newline, we could report an error. So, create a function "packed_fsck_ref_next_line" to find the next newline and if there is no such newline, use "packedRefEntryNotTerminated(ERROR)" to report an error to the user. Then, parse the first line to apply the checks. Update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 8 ++++ fsck.h | 2 + refs/packed-backend.c | 75 ++++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 52 ++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index b14bc44ca4..11906f90fd 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,10 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefHeader`:: + (ERROR) The "packed-refs" file contains an invalid + header. + `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. @@ -176,6 +180,10 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`packedRefEntryNotTerminated`:: + (ERROR) The "packed-refs" file contains an entry that is + not terminated by a newline. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index a44c231a5f..67e3c97bc0 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ @@ -53,6 +54,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE, ERROR) \ FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ + FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6401cecd5f..ff74ab915e 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); @@ -1749,12 +1749,76 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s return empty_ref_iterator_begin(); } +static int packed_fsck_ref_next_line(struct fsck_options *o, + unsigned long line_number, const char *start, + const char *eof, const char **eol) +{ + int ret = 0; + + *eol = memchr(start, '\n', eof - start); + if (!*eol) { + struct strbuf packed_entry = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + + strbuf_addf(&packed_entry, "packed-refs line %lu", line_number); + report.path = packed_entry.buf; + ret = fsck_report_ref(o, &report, + FSCK_MSG_PACKED_REF_ENTRY_NOT_TERMINATED, + "'%.*s' is not terminated with a newline", + (int)(eof - start), start); + + /* + * There is no newline but we still want to parse it to the end of + * the buffer. + */ + *eol = eof; + strbuf_release(&packed_entry); + } + + return ret; +} + +static int packed_fsck_ref_header(struct fsck_options *o, + const char *start, const char *eol) +{ + if (!starts_with(start, "# pack-refs with: ")) { + struct fsck_ref_report report = { 0 }; + report.path = "packed-refs.header"; + + return fsck_report_ref(o, &report, + FSCK_MSG_BAD_PACKED_REF_HEADER, + "'%.*s' does not start with '# pack-refs with: '", + (int)(eol - start), start); + } + + return 0; +} + +static int packed_fsck_ref_content(struct fsck_options *o, + const char *start, const char *eof) +{ + unsigned long line_number = 1; + const char *eol; + int ret = 0; + + ret |= packed_fsck_ref_next_line(o, line_number, start, eof, &eol); + if (*start == '#') { + ret |= packed_fsck_ref_header(o, start, eol); + + start = eol + 1; + line_number++; + } + + return ret; +} + static int packed_fsck(struct ref_store *ref_store, struct fsck_options *o, struct worktree *wt) { struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); + struct strbuf packed_ref_content = STRBUF_INIT; int ret = 0; int fd; @@ -1786,7 +1850,16 @@ static int packed_fsck(struct ref_store *ref_store, goto cleanup; } + if (strbuf_read(&packed_ref_content, fd, 0) < 0) { + ret = error_errno(_("unable to read %s"), refs->path); + goto cleanup; + } + + ret = packed_fsck_ref_content(o, packed_ref_content.buf, + packed_ref_content.buf + packed_ref_content.len); + cleanup: + strbuf_release(&packed_ref_content); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 42c8d4ca1e..30be1982df 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 Fri Feb 14 04:52:54 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974485 Received: from mail-yw1-f181.google.com (mail-yw1-f181.google.com [209.85.128.181]) (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 7DE0919415E for ; Fri, 14 Feb 2025 04:53:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508783; cv=none; b=eZ3jKm5pDEYTD4S6NqUYFnKd5FwKFV4SP5VicPvP4qitcsZuOpXSaWWnQt0UbrHfpcfgFVJLvjpleDrMXUrQY0stFKRLOcxu88/FvUM9l2dmPtcbOhqw80kHVIJgbg+qJbAGMgVdl/S/WcisjJRFqr8r6xnCuu3oWh+zWvOVJHw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508783; c=relaxed/simple; bh=hPMn6DD0ADH4YnGuaYhRQGtOKD8bax7WYvj5uffBL1Q=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=RAwbp+djyuhmn9BDx61ia16j+H4oYR0W2IgNZPL+WTJ9ktblLJeLDFowHhpWUu6Rr7pSJ7uPLqXVl+fWeMa2/zsgG2g+14LYWi21CqRnlI/K1JA/ZXZPnCumlu1T/8j7dQR8CrzaEJoRgXODzZFNQsGXm2ER1YxEWcwdcE6ENXY= 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=Ft+EYfh1; arc=none smtp.client-ip=209.85.128.181 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="Ft+EYfh1" Received: by mail-yw1-f181.google.com with SMTP id 00721157ae682-6f4bc408e49so13277167b3.1 for ; Thu, 13 Feb 2025 20:53:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508780; x=1740113580; 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=xjJLRkmge/Qhg78bYF2NZXzaCqee1nKnJIaYhGxD6tU=; b=Ft+EYfh1zGsnxthM2zKX1soCTO3NqDiF4melKDLjVPU/E5IruYmgheo3i9wIf6PHhh AbWB7IxMMhJFwOZtQsI8fHr/pRu/8up4/T4RYuxvdW1BfCRt+tsGGWRG+0SPdjK/aWkr lxign/BB3bN6JtdZHrVrOiC6/8wmEIZJOK8Y9bLxqoGUDTGJXk7XLlsKGcwhEZ98lqm8 wj5N2JgaxS6r8BaOS9v9dKftvo6ZJbALGmqF/5T/oB9DQTvHdBz95m/MGNr1i4bcU1k+ ZitTmDyJpMY9BcvLWNA11vzfPgRWqEP9yvyDKKSV8WQ942fBHlZ73/8B0NndyXusj+oY 1BKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508780; x=1740113580; 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=xjJLRkmge/Qhg78bYF2NZXzaCqee1nKnJIaYhGxD6tU=; b=ajVAI3+ObFhAf6Kp127tdPJ1pM9J+9SHKGPUWCa8nJZXJwDPsH/7FWnPZe261b4DlE /abobz5RGhZfvYBGE3VEOppG601+klaBbyuW9royELegI1h/zZQwHLqGQNuSX8tUUIax c1SeZD4iwkNqJaBh9QSjFedDv8sD/8q63RatfZn+MtnXdgUFWfaysUD1aF++HYiZ0Ynu 9tk7l60G3WrNH473dE+8arzKJLr71/+mKgenzNK6iwZRPbAZ77Xfq5ZIizcGgkbaO/NX 9CGjIizqA7MYSzhmB7kiw5n5SlIkaTsTLvxG+OVpQC0LaykXlZJIw5sfq4uG4yvPO8U1 jK9Q== X-Gm-Message-State: AOJu0YxCKTTTVZ2P3S/YGArpQtW3pZIHq7nXjvHk7vRP/sGTXYuTHeIA fjmViJaOXxeJT/7HYT6dfTaHnqmzaPU1RIGyiHqEgcUeAa04RNwPSgxwKrZw X-Gm-Gg: ASbGncuWhTSWrNP/rrXA1zHl/80gQiASyK5TdpfPH928wNGbaOwasA+EFacKEVDy01i zSNW32VNIb9b5qdVv1ajop9yk7ytRMa7P3LM2HHUBSRB7nen6vwea/93O7x8tdrfg3QQfNqRmxk r7gZoW4sppRBbmtsdulECFlnzeR5z0HDkAc2SPbZUQEm8FakaZV6FTOohrFduZNp/89OdRqSGQq bv6BMX/BmqItHAR89f1PMYr7ydZ6Hq1nA8W0l7k+Vi1Dm59HGSvRHPGxaB1y2Rkbjtd2A== X-Google-Smtp-Source: AGHT+IHbIBbEXSY0CNUzb+xrCaPBcUk2v9WSsPC+1QPxl5RqecVVdKTjzhe9UIcWhCiYJVdajzALQw== X-Received: by 2002:a05:690c:6988:b0:6f6:cad6:6b5a with SMTP id 00721157ae682-6fb1f19ba28mr122491737b3.13.1739508779883; Thu, 13 Feb 2025 20:52:59 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 00721157ae682-6fb35d58720sm6229387b3.22.2025.02.13.20.52.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:52:58 -0800 (PST) Date: Fri, 14 Feb 2025 12:52:54 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 5/8] packed-backend: check whether the refname contains NUL characters Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will use "check_refname_format" to check the consistency of the refname. If it is not OK, the program will die. However, it is reported in [1], we cannot catch some corruption. But we already have the code path and we must miss out something. We use the following code to get the refname: strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf In the above code, `p` is the start pointer of the refname and `eol` is the next newline pointer. We calculate the length of the refname by subtracting the two pointers. Then we add the memory range between `p` and `eol` to get the refname. However, if there are some NUL characters in the memory range between `p` and `eol`, we will see the refname as a valid ref name as long as the memory range between `p` and first occurred NUL character is valid. In order to catch above corruption, create a new function "refname_contains_nul" by searching the first NUL character. If it is not at the end of the string, there must be some NUL characters in the refname. Use this function in "next_record" function to die the program if "refname_contains_nul" returns true. [1] https://lore.kernel.org/git/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/ Reported-by: R. Diez Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/packed-backend.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index ff74ab915e..692e315e41 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 Fri Feb 14 04:53:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974486 Received: from mail-yw1-f174.google.com (mail-yw1-f174.google.com [209.85.128.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F0175198A11 for ; Fri, 14 Feb 2025 04:53:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508794; cv=none; b=uiUM2a4gDkiCgkKvKm/XAzoiI7LsFdsdQEqSYL+UljrwM9CYC2YWHXvweT2EUd4SD2hjrm307mkG/pwLySYoJ0ZJDaRCHGRbyFDSDAUwCHGEcgfClp1SZOLkQVsShQ22ruXs2MbUHreGvS3AUEfA18G2/rarr7f2hmKPnoDG5R0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739508794; c=relaxed/simple; bh=Dq0fFNqhuZ8Mf1OZq3oGBtBNi86EQjfKyt6RIrUBA2E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=luUB8dLJuJ2TB/Vk/BOIyI5G4+jdBW7xrcg5WdasQqNzuxD+y2w/74+yjMv6o4N5lKfdY4JAm2JwpQgQGwGnqdIpsQi3ZJy/ascKEq/aejCQODqExvsFHKNfVkipoW4VxZr4gpx+T55VTRO3cIQITvRjU7AS0gVwpX003ex3FSI= 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=OLzaZbt0; arc=none smtp.client-ip=209.85.128.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="OLzaZbt0" Received: by mail-yw1-f174.google.com with SMTP id 00721157ae682-6f88509dad2so15426717b3.3 for ; Thu, 13 Feb 2025 20:53:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739508791; x=1740113591; 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=G6DzM86FhImPk2pyLp6YfFF0PKNAyeH8MQS6ZXzi6qM=; b=OLzaZbt0Mu3io3K8CLJlb3947PBT6BS7Bn5bxAumsvfOiLBu19YuC8GKg8rAQdi5im jMYK4Pj70Ozx1BOXEfA+Yxsc1xnkymH2YI45/3cskCKakqoAUtHJLMmu8O2S1qkC6BEr xrFwQ8ywViluh4kgTya/WPmVs95WQFymDFWbKBfhIZo3yUJfCGahQbiTZlquSgXn4pfH x2xUmk2GENkDeq+Ysu5bcKGoWYH21kTLz8eNLnNVjdRx+zZwegHzC0dmJnQ/d29vFxIm GOKFzfd0IQ0LweQ7WDcaEgyEUmN9deTVInyPE2ky4hLCjv2zbRvqx3bnXoBq7ZJb5Zou AyIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739508791; x=1740113591; 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=G6DzM86FhImPk2pyLp6YfFF0PKNAyeH8MQS6ZXzi6qM=; b=AtEh6X5NuiuCl8ziOATfC7Vn/vR+mUhe4925ITNCChRt1UXlkPH5i5h3gvGZlQiw6T zXocBjhSvfnwrpDGj74sqr/AcIVAxSjFkfKpeJHT3L0KaVUuWWsyb/bocSEH8ylm4SkG PILX2aSaBq4rO25a3WpE22PQfU4I8Zyz63yO7TZiOHWfzNxEhiyZ/VIRQEptXdG9NURZ 4AwI9s/ix2tTcHxiFgU/9BvM6VOX//4+VIQnGR9+a8Um698KSQy3mn+NlTRBFBI7ogyl L9Xt4wylYPhypqSBpbq1UqONnVhIoa1vLcBDaDVWeA25KucJCrmA+WRF5XWT2i6fMvow m2Ng== X-Gm-Message-State: AOJu0YyynqYGxCmo6Nc9JSdjKeNwmVJZx6UuP81FAHso4Tp68zc98bxU kb9+zWM3THhUj9re0gBXMCE+0jOdgQYdHeH7qYhGsYXKMIlRSWqnsFLB252k X-Gm-Gg: ASbGncup5xiRogsSdEvvpEVPNRc74TbSrI+1/CSoCQQ9lLQCp45E441aNTRA/RXBO05 otzAla08VrRdzo7IvB8Od42x7NwWlKAa+qnUStVpIRoQsx/WQy9FXlJPbvqplBiW/kXxfbd5aEu khexRxifNg3HGLjLy81BNThAQAkLW0k4GV+/wvtf80/zrahxWZKKbzfDgaycNDSu7vZIlyLzthm TVTjcaQTJN2PpzIlKL1SfFQWdhWBf3bMiWOMZdfqy64JexLq2NGEvewJELET3ncw1BUSA== X-Google-Smtp-Source: AGHT+IFXB5wyTU4EOPLvpiEpMIgnBjkngkmXr4lguP1OycEm4kkyO/XTLtXrt5fmDMquV/v9jAENbg== X-Received: by 2002:a05:690c:b96:b0:6ef:77e3:efe6 with SMTP id 00721157ae682-6fb1f5a69e0mr109044877b3.13.1739508791020; Thu, 13 Feb 2025 20:53:11 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 00721157ae682-6fb360a7502sm6200957b3.64.2025.02.13.20.53.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:53:09 -0800 (PST) Date: Fri, 14 Feb 2025 12:53:05 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 6/8] packed-backend: add "packed-refs" entry consistency check Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: "packed-backend.c::next_record" will parse the ref entry to check the consistency. This function has already checked the following things: 1. Parse the main line of the ref entry to inspect whether the oid is not correct. Then, check whether the next character is oid. Then check the refname. 2. If the next line starts with '^', it would continue to parse the peeled oid and check whether the last character is '\n'. As we decide to implement the ref consistency check for "packed-refs", let's port these two checks and update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/packed-backend.c | 121 +++++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 44 +++++++++++++ 4 files changed, 168 insertions(+), 1 deletion(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 11906f90fd..02a7bf0503 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -16,6 +16,9 @@ `badObjectSha1`:: (ERROR) An object has a bad sha1. +`badPackedRefEntry`:: + (ERROR) The "packed-refs" file contains an invalid entry. + `badPackedRefHeader`:: (ERROR) The "packed-refs" file contains an invalid header. diff --git a/fsck.h b/fsck.h index 67e3c97bc0..14d70f6653 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ + FUNC(BAD_PACKED_REF_ENTRY, ERROR) \ FUNC(BAD_PACKED_REF_HEADER, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ FUNC(BAD_REF_CONTENT, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 692e315e41..5d1dcfec6f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1812,9 +1812,113 @@ 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 +1931,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; } @@ -1873,7 +1992,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 30be1982df..058a783cb7 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 Fri Feb 14 04:59:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974488 Received: from mail-yw1-f174.google.com (mail-yw1-f174.google.com [209.85.128.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B441322071 for ; Fri, 14 Feb 2025 04:59:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739509174; cv=none; b=kc2iKmvYTMhaJmA61MgotPKoZ3z6xyq5mC7BKMZEmPeZaLr+Xlq9fwxu8pAffuzIpyTco7MgM6BH3QqsCTYtsjJbohoyShjWvdu1eT+mrFgYXwKFfqTc+82cq4/2JMKgQbQuvph3AaurJavUqIxxWqe5udsDD4SXk0L71UVRpes= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739509174; c=relaxed/simple; bh=GyRDBQRu8dqqQsFxIEBdlzUpxxgK4m9X0l9krBfTdr8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=glk9qovcBDor1H/q5q0vfu4O3WFRugc0pFwGKM6OiwZZ4pTC0u+4RmCtWVvG+gKaUG/6/NgzombTNsN/WtbJcb5+2070ylzEZ/e4D4ivefkrShJyWjijMu2fZnaawK7CLpkvgrONbOzc2kXk6ecvQJfMkHWKS3+MjPNpi/h9pu4= 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=K+MY5kOL; arc=none smtp.client-ip=209.85.128.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="K+MY5kOL" Received: by mail-yw1-f174.google.com with SMTP id 00721157ae682-6fb2a6360efso12277797b3.0 for ; Thu, 13 Feb 2025 20:59:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739509171; x=1740113971; 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=rgoFDMJFqLrwpEy5c7lw++17FKEsQAjW/XXdnX8ba/g=; b=K+MY5kOLddLCTq3w+rn78QoH76K2esY2zySBO3H3G53dpeA8INCqiRX+11oR5ZEkIN zL889UkaAckEH2Kjp6FwXHmOkR2no3s6szMr/kvPWxEz+BPMt8XsanlFwEP44tpZo1Jo ScrW5ngaj6LxKgZZ7QZ5l+CNBAkbF9JGY7lMXgjdV/2stcfiIiu+nK5o6U3wo6SyrGAu lDZ+kyqp1MVzBEgmtSfFvBIDHH+l2Cg8NCUEt19al/b4QWZD0uln4hQz5lN3a6K7SvKv BXd/A6OEgylJBs1/GpeRYka+JgMxzM6a1Vqiun3/JpPm3asXD2VDwRyiuAjw18cYb14j K/FA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739509171; x=1740113971; 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=rgoFDMJFqLrwpEy5c7lw++17FKEsQAjW/XXdnX8ba/g=; b=OWxGtw19TbAvfDaJ1f2OlDT4IYZNKCnVN8jCxRe8JXt8gSfj530qc6cnGVxgxvVfQ/ cryaQ7+1Gjs4fyOl/GWm0FtMKqi5ZLbZv3UjUIFfmVv6cfL3+Q8e8Z3UyO/lvjd7t+e2 4sM1fiA5LFfiQQanMvfIJxk30iAZB7Ml6CR4BSW5HHXyc/7a4pTuDvErUISZ52rrYQmG 7+D6gbVUPua7mVmuWTmScp6dofg8CDvbdypKCZVBPh8B4p/QCwr8/9Nmnx5vhNHsRLYg LOCM6YAEXZEBR5RVX5W9D2P0b3EA8LvQNiuJ0kCP0BQ7qE2i2YyrkgKJggcGtQZxwzEr fgEg== X-Gm-Message-State: AOJu0YwYkSZTo+8Kd+c49+MtFpzQrlKmSJipPpXNxHx0+SRFjnTeR421 I5pS0JgIRqp/4JEv8i5ueiuXLY7+Z/FiPztkkY6BeegWP/6vU60cWGibXr4W X-Gm-Gg: ASbGncurmA1qc3wTBMLMYX6LSCgWY2vx2Zsa3VMaz/jRVeNqTG+MXCVPKf77sFEUABd 0mj2HohJc5qdF8/l8erjs4epCtiNxyYDhCfAKeIO0RXsVJKRR5twlt+3nLGO2vrxaYNlOfP64p5 zaot/rJcS40e9Xv2+1IEFjepQP1Ijb5DF9Rs7opKc9PNajj28qg2X/4mRNmU51bbV9Pq5ao30tT xkq8csfe8zfwo+8ZNDRIY/MK/HAWBe1SKvg3+gKofgiWEdKxB3D/toRuCulm+EMDhUORg== X-Google-Smtp-Source: AGHT+IH7Jb2q8eVxkdCP+hFSHXhMW/nuae/OBIncmB6CwZtPzP/iPe9WxVZxkApHUQ1wXy2yKpySjw== X-Received: by 2002:a05:6902:1107:b0:e57:3a77:99b9 with SMTP id 3f1490d57ef6-e5da04b35a1mr7754278276.41.1739509170717; Thu, 13 Feb 2025 20:59:30 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 3f1490d57ef6-e5dadeca97bsm789537276.32.2025.02.13.20.59.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 20:59:29 -0800 (PST) Date: Fri, 14 Feb 2025 12:59:24 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 7/8] packed-backend: check whether the "packed-refs" is sorted Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: When there is a "sorted" trait in the header of the "packed-refs" file, it means that each entry is sorted increasingly by comparing the refname. We should add checks to verify whether the "packed-refs" is sorted in this case. Update the "packed_fsck_ref_header" to know whether there is a "sorted" trail in the header. 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.txt | 3 + fsck.h | 1 + refs/packed-backend.c | 116 +++++++++++++++++++++++++++++----- t/t0602-reffiles-fsck.sh | 87 +++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 16 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 02a7bf0503..9601fff228 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -187,6 +187,9 @@ (ERROR) The "packed-refs" file contains an entry that is not terminated by a newline. +`packedRefUnsorted`:: + (ERROR) The "packed-refs" file is not sorted. + `refMissingNewline`:: (INFO) A loose ref that does not end with newline(LF). As valid implementations of Git never created such a loose ref diff --git a/fsck.h b/fsck.h index 14d70f6653..19f3cb2773 100644 --- a/fsck.h +++ b/fsck.h @@ -56,6 +56,7 @@ enum fsck_msg_type { FUNC(MISSING_TYPE_ENTRY, ERROR) \ FUNC(MULTIPLE_AUTHORS, ERROR) \ FUNC(PACKED_REF_ENTRY_NOT_TERMINATED, ERROR) \ + FUNC(PACKED_REF_UNSORTED, ERROR) \ FUNC(TREE_NOT_SORTED, ERROR) \ FUNC(UNKNOWN_TYPE, ERROR) \ FUNC(ZERO_PADDED_DATE, ERROR) \ diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 5d1dcfec6f..391efced54 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, @@ -1914,8 +1934,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; @@ -1925,7 +2005,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++; @@ -1956,6 +2036,7 @@ static int packed_fsck(struct ref_store *ref_store, struct packed_ref_store *refs = packed_downcast(ref_store, REF_STORE_READ, "fsck"); struct strbuf packed_ref_content = STRBUF_INIT; + unsigned int sorted = 0; int ret = 0; int fd; @@ -1992,8 +2073,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 058a783cb7..f305428f12 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 Fri Feb 14 04:59:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13974489 Received: from mail-yw1-f180.google.com (mail-yw1-f180.google.com [209.85.128.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 F04B5155335 for ; Fri, 14 Feb 2025 05:00:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739509208; cv=none; b=dhV0o0VLQIyLys5C+5kOb8t2wmyqoB9qWKsbiOkDkOJM2PJr7dCIictpW2VjGxIfYW9s/nmrhdDatFD2GFdI83Cqdlq3yiEc+h19TL4EC24TC0BPVGSNEJtGTBp7IUpSxVIhXvZYwmg7nS2HObzZ5ad9bGLNQ23gR8V5UqAB+dU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1739509208; c=relaxed/simple; bh=bOH8EXS/qoID+eTMIaUS5T2Cf4HXM1yFX6eN6nldINI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=emwyxYVykTCVuzTIzv1bWg0MWpquMnZCxY5w8fIhSSqrmRBOXdL2z/wgHKVwbm/itzCdrIWOTw1TQYcBe+Aq91fl0GPiC+ku6UiC7GNmMupW+829icM6a163JJ+z9v0pnB1KEnAbSkQ2siffONE4j/93HKXTpOlal8hcU9xCTrA= 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=A+DsoX5t; arc=none smtp.client-ip=209.85.128.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="A+DsoX5t" Received: by mail-yw1-f180.google.com with SMTP id 00721157ae682-6ef7c9e9592so14069297b3.1 for ; Thu, 13 Feb 2025 21:00:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739509205; x=1740114005; 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=PFe8USlvzqm0SFaL8IfMaARwBqQYjT6+vEiHsm5EcBI=; b=A+DsoX5tp+9502e9wllaJtWuL4tDkH/AMUoO2nR9gk5KNumJx5M4ZYu3yGW72s+i1N 791GeC7xk0jfvXcF9oL+6Ff3Yin4CCHqEsmCEJcoJs4Xc/WUiPAVQ4CSq55kaOi7EF8V sq9jPO6jzT9pYXt/Fp9YoUY0/GKdU9dhiyaDuqfXMTpfmQD1XRSH6zQwc13Yu+hcJytd aTU09DN/aRQ0Unr2VvNLAioGyBZ6V2vlWEw78UV1Drm/LoQuUzmoJiY6mlqjmyShxklm RMJJNmJydC26LoeT4ywzN4QbtG6/9PavHtXsNs7voDuYqXtb6l5rf6coK4gdLwlV4Rnb F8gQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739509205; x=1740114005; 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=PFe8USlvzqm0SFaL8IfMaARwBqQYjT6+vEiHsm5EcBI=; b=uOMqnIKpyShFoQTtU2S1erdiSMwCZocdAbGVhBDzmdONKc9B1SgmCPo5KrN8Gw0peI L8JEEOATQEGNJOsxvPOfTwSKXzkDL4s9DldgCqZEzSAbe9c/UF3hZkFAva+oMRuClgBg FdFVOtR74cQfHeZ87xSWmjkQ0hH1YDoIxi4W+5nAEH+lmj65poXirtApNGQSnyaZ3/55 8eVdANhvrjHmC952F0GYXyX5dXFfzjV1wGjpYXzNPG6IyRGrikCk6t1GxbriIMs+XMRP 0GRHXcBlex9cxYkCQHaTurFyzyc3HKm43/45+A+i/WoqBCg6iwOM3fQSQDzFMcPfy5I6 WRMw== X-Gm-Message-State: AOJu0Ywxn2Uwyx9kXzo6L8/IpQufC0QZvbT7IBKoBquGdDWzMN17a7wn 8gYKswm3oVajmqeKiItBoveDnb8HK2e4w9AKzG8DR6UG9UIEV90Z0m0VFwsP X-Gm-Gg: ASbGnctmKuhSDFjAbafebSALQ3/QY5S2rFA4Shhuvd0fQ33v3VHn1qWuXAz22XQfCn+ vwUyX01Knjy7Eg9svl3yhMKzqyv9mHi6sdobAqVbf4q6qDGJoh4rkvAVf25uGonQ2DSv/tWXdNx 8Zy6jIE/foL0AqD/NggwSR16p0Qd8SCh1/oVkmWz+H8fnSpsa3MNWtHd5iFMEODY6WImBUmC6W4 wh4u9raRaJ5rYL+R7J6wep3RzUeY3SCZMUXMrR+iPJ6ACNUUuIBKdcCFg0SjOudPwY6IA== X-Google-Smtp-Source: AGHT+IFS5W3LxvxUUJ5MEBX8v9p3H7xemezSvLFgVtfVC9WFACBcDi9IgNwCOsQjD6ucpwZg+/Y2Gg== X-Received: by 2002:a05:690c:700e:b0:6ef:64e8:c708 with SMTP id 00721157ae682-6fb32ca520amr56914427b3.17.1739509205482; Thu, 13 Feb 2025 21:00:05 -0800 (PST) Received: from localhost ([2604:5040:11:69e::e973]) by smtp.gmail.com with UTF8SMTPSA id 00721157ae682-6fb361d037esm6083857b3.118.2025.02.13.21.00.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Feb 2025 21:00:04 -0800 (PST) Date: Fri, 14 Feb 2025 12:59:59 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano , Michael Haggerty Subject: [PATCH v4 8/8] builtin/fsck: add `git refs verify` child process Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: At now, we have already implemented the ref consistency checks for both "files-backend" and "packed-backend". Although we would check some redundant things, it won't cause trouble. So, let's integrate it into the "git-fsck(1)" command to get feedback from the users. And also by calling "git refs verify" in "git-fsck(1)", we make sure that the new added checks don't break. Introduce a new function "fsck_refs" that initializes and runs a child process to execute the "git refs verify" command. In order to provide the user interface create a progress which makes the total task be 1. It's hard to know how many loose refs we will check now. We might improve this later. Then, introduce the option to allow the user to disable checking ref database consistency. Put this function in the very first execution sequence of "git-fsck(1)" due to that we don't want the existing code of "git-fsck(1)" which would implicitly check the consistency of refs to die the program. Last, update the test to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/git-fsck.txt | 7 ++++++- builtin/fsck.c | 33 +++++++++++++++++++++++++++++++- t/t0602-reffiles-fsck.sh | 39 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt index 5b82e4605c..5e71a29c3b 100644 --- a/Documentation/git-fsck.txt +++ b/Documentation/git-fsck.txt @@ -12,7 +12,7 @@ SYNOPSIS 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs] [--[no-]full] [--strict] [--verbose] [--lost-found] [--[no-]dangling] [--[no-]progress] [--connectivity-only] - [--[no-]name-objects] [...] + [--[no-]name-objects] [--[no-]references] [...] DESCRIPTION ----------- @@ -104,6 +104,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 f305428f12..22bd847782 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