From patchwork Sun Nov 10 12:09:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869791 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1BBF41537C6 for ; Sun, 10 Nov 2024 12:09:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240563; cv=none; b=KC9rzFVcf0wzgMeksNenJkC2Hc2PN2qx6/NUJG9u3aHXRlq216lmteADkSfvvqD9lsNvHMdRjPW/2Xix8JhUbCEPUq1AH3m2aZ5GI9lK0Datbxh9J204T/YR9eWlp+uWZLPvwhXrLy9DVjz1sRb5t0rjGrDln0zsRXU0xvVlYzg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240563; c=relaxed/simple; bh=Iq240slf6F8USCuNlFVvJmIT6fjkny4AKF4dW34UQj8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qgbqi3T78en7Gz6JJdUQBxh8hJlEIqV4Z8RN0H/UnqiRxmqon9DbHDY/+Hq2+tywcxyy/rR1Zs475TzGjY+w8crlhCBixebNrICsuiXztDgtkkWhFLBNEqSlA+yJsHJGoPNlfJDZQBWJba9619a/A0GnVNsQ4+9zAM6Qm4rKxgk= 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=AlFDep5j; arc=none smtp.client-ip=209.85.214.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="AlFDep5j" Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-20cb7139d9dso33934385ad.1 for ; Sun, 10 Nov 2024 04:09:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240560; x=1731845360; 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=Vc/fmkvpcUY3It7x/Wyto6u14vgd1/H96CWyWXBd6jA=; b=AlFDep5jmRVjFipsOxRTZlY724iMsVWrT8GBCZVO/L2W2IeS4S5G97l16pFQHJoYKf dAW310+F1n18a2a4xC71OvNtyt8teYvBP6jei8bm+3lcqtWO2OYkT5dEHsGrXlMf2PLY ixFZfVtHq2+akJxDtNz1dQTXOqRYNswhiAYZ1AJfDvGba4cdDYdCXcjHW56hg8oTVJss 3sYC1lYA2DcMUn0r0BNgo+LQEOYGXs6pjU9HkgiUHmoCAwlT11SaCvy2iICS9VhcFYRW eQTftvViVpjOcaKNqYvpBamrrhbAX/QKVOrWa7cz6Rlip/N+9bmSk2ms4N88iCcVIC9y XLFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240560; x=1731845360; 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=Vc/fmkvpcUY3It7x/Wyto6u14vgd1/H96CWyWXBd6jA=; b=V9j2pJZ9S1liGNsX3SdImyRQUEMXPjmga96Aosk9FN1z0mHVoE+0iGztjVirgEJ28z dHeUH4TXtklW0FZAGffvzzwZGwQirG/Q1JGjNWxytLSJlb5DxzsK842ClEXmTOkI7Aiy /L+M9H9ykSNl4aJ2Xpt6jptDOiZs5/Da9avQUWXFBOcyVBHuUw9GUSIWgMhneq+zUlvd bl/IIS2uIHKvuDmN8NpptSqNtdfiK2ReZUY31V3NkComNRD5qAz75et739Bdg4xpVNAY aIZZCG70fO9WPDMgZh44F2m2dsRIrhmqMTk+RI65AMKx/BfkRL6+QuC7BE+1c36fF04X CY2Q== X-Gm-Message-State: AOJu0YyogV9TcjIlFdQ3S3hI5wRtUomZCcRIfurqPVcu0rTztIntdasu vboP1FwZh/mCv9+oSrzEfGcqPF6nwz+yx6pBVx8oIu3Hk0CvA7q9ssQ9UqHKunI= X-Google-Smtp-Source: AGHT+IGixWoTKRDkboKsmpoh02lykIpgtdNTwU1tx5wz/mx/NZJKGzCU4h1WNk0AkjF5qqE8IGjvVg== X-Received: by 2002:a17:902:f685:b0:20b:951f:6dff with SMTP id d9443c01a7336-21183b91f73mr132594575ad.0.1731240560561; Sun, 10 Nov 2024 04:09:20 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21177e41902sm56953275ad.121.2024.11.10.04.09.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:09:20 -0800 (PST) Date: Sun, 10 Nov 2024 20:09:18 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 1/9] ref: initialize "fsck_ref_report" with zero 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 "fsck.c::fsck_refs_error_function", we need to tell whether "oid" and "referent" is NULL. So, we need to always initialize these parameters to NULL instead of letting them point to anywhere when creating a new "fsck_ref_report" structure. The original code explicitly initializes the "path" member in the "struct fsck_ref_report" to NULL (which implicitly 0-initializes other members in the struct). It is more customary to use "{ 0 }" to express that we are 0-initializing everything. In order to align with the codebase, initialize "fsck_ref_report" with zero. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0824c0b8a9..03d2503276 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3520,7 +3520,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, goto cleanup; if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { - struct fsck_ref_report report = { .path = NULL }; + struct fsck_ref_report report = { 0 }; strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; From patchwork Sun Nov 10 12:09:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869792 Received: from mail-pf1-f180.google.com (mail-pf1-f180.google.com [209.85.210.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 2E24D1537C6 for ; Sun, 10 Nov 2024 12:09:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240571; cv=none; b=tGmeBuYaYlM+cHb88cK2LCOuCRwMXQfTftPfNuO2BxeF4w7a9fhSHK/3JA3C96G+EltJAxAO4jPG63sxrdr6m4ZzOrspPOt7bvcoPzbmn0STg0moZpbb1m5ch4btOCS5etx7k4mK2R0LCySuHYdYnRFc23B/6aSz0PH3BJIBhsE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240571; c=relaxed/simple; bh=4iAJA46zpl/v6qmBKcIfVl8MSuXSf8PZ1daMDFkHi9A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Xfp3IPXXtTZvoc4y6SkSUFlUg37t9F4bOukVM5DAegQHpEVLwSi0Mhu3pSM+4Sf70eXjnUONJZlbslfYMiZ1mtSJxA/lthZQD5R7In1nf2ylx418MaZeqpeSnxJLi4Z6R7rJivEkpyNu2gMSgOwFuJUo+2Ybkduxso5OTvEH4uE= 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=CeOhF2mR; arc=none smtp.client-ip=209.85.210.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="CeOhF2mR" Received: by mail-pf1-f180.google.com with SMTP id d2e1a72fcca58-71ec997ad06so2968103b3a.3 for ; Sun, 10 Nov 2024 04:09:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240569; x=1731845369; 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=ML4KPi4wQjf1qC2Qte9G4jPx11iLAGGFB8tIl6WAl74=; b=CeOhF2mR4dMHg8i5GkdX21EMJpLctEqR7lobEmuCP6q+Du3Xg+VD8Zw+zTYeX9Wqsc Oa0o7NkUDo9/WLlnS5JuL4+W5h4Xzf3DCkDq+sIosDk17xCTDHSaJyZe1cBUOxLHDGJ1 bEarKNKYfzV6Kap+IX109kgjiL7oS6I2Wolty3Xthg190mPaa3MaPStd10WKa+ObG2nf fJwgovz8BJJNE+HQV8bjtMR4lP662d2rl3NgdVdHp2pFmCwnOlhkqi6Vc1gmJCNCDHD8 ANQ/g1dc9QppnKCicxXGke1mEkfrxAEGR412boaBAGpC4Kp3JUddIRHg5w/lMYSx2gS1 RMxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240569; x=1731845369; 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=ML4KPi4wQjf1qC2Qte9G4jPx11iLAGGFB8tIl6WAl74=; b=OIGJgNUmoAoZLTiXZq6KJn1+eBwX7s2+iwHL716s3JANBmrpcJTjrEtVCZyXCenn52 kM68iGXVOeAal8LzVyV16IyR5a7Yg6M3SjpxLcPh+dZK2fEzPdhsWNOz+7QJbKad/OHP 4ek2VLBp9lq4eByuim8DgwMedGNhasrcE8HSJDOJBBSnaUdLEU5zdwpKwSjmQ3me4TGe ikugf0wcH+vbZwd7YH9PmOplFDanknK3eV1o+TOF8PjH46MAn71QQ449xpAZ+CO0bdIb 5wiv9UP/2Te+l/dxn1uij9kVmWbMGBN70CBtHHwjCX0nFcOXSyNJHoIf5OpYRMtiPkXs rFkA== X-Gm-Message-State: AOJu0Yxpltgy5/d4Pe9r9EK3+ErTHZ9aINz+PWKuzNvghFPQVMX+V36j zODj6CKc50atXPILS5+mipLr6TvjkF0pUrZ+ehoED0v5fTn/Cj/1e3+FbQN/jX8= X-Google-Smtp-Source: AGHT+IGphGC7NA2ZZQ2YdOBN413MgZ56iE8TsUJ6YqWPocMB0B3xz9JCft2Rj7E/n8KCJtrPqqb+aQ== X-Received: by 2002:a05:6a00:139a:b0:71e:6ec9:fcda with SMTP id d2e1a72fcca58-7241337ff37mr11320381b3a.19.1731240568517; Sun, 10 Nov 2024 04:09:28 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7240799bb25sm7238949b3a.109.2024.11.10.04.09.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:09:28 -0800 (PST) Date: Sun, 10 Nov 2024 20:09:26 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 2/9] ref: check the full refname instead of basename 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 "files-backend.c::files_fsck_refs_name", we validate the refname format by using "check_refname_format" to check the basename of the iterator with "REFNAME_ALLOW_ONELEVEL" flag. However, this is a bad implementation. Although we doesn't allow a single "@" in ".git" directory, we do allow "refs/heads/@". So, we will report an error wrongly when there is a "refs/heads/@" ref by using one level refname "@". Because we just check one level refname, we either cannot check the other parts of the full refname. And we will ignore the following errors: "refs/heads/ new-feature/test" "refs/heads/~new-feature/test" In order to fix the above problem, enhance "files_fsck_refs_name" to use the full name for "check_refname_format". Then, replace the tests which are related to "@" and add tests to exercise the above situations using for loop to avoid repetition. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 7 ++- t/t0602-reffiles-fsck.sh | 92 ++++++++++++++++++++++++---------------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 03d2503276..b055edc061 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3519,10 +3519,13 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock")) goto cleanup; - if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { + /* + * This works right now because we never check the root refs. + */ + strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); + if (check_refname_format(sb.buf, 0)) { struct fsck_ref_report report = { 0 }; - strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); report.path = sb.buf; ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_NAME, diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 71a4d1a5ae..2a172c913d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -18,63 +18,81 @@ test_expect_success 'ref name should be checked' ' cd repo && git commit --allow-empty -m initial && - git checkout -b branch-1 && - git tag tag-1 && - git commit --allow-empty -m second && - git checkout -b branch-2 && - git tag tag-2 && - git tag multi_hierarchy/tag-2 && + git checkout -b default-branch && + git tag default-tag && + git tag multi_hierarchy/default-tag && - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: 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/@ && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/heads/@: badRefName: invalid refname format - EOF + cp $branch_dir_prefix/default-branch $branch_dir_prefix/@ && + git refs verify 2>err && + test_must_be_empty err && rm $branch_dir_prefix/@ && - test_cmp expect err && - cp $tag_dir_prefix/multi_hierarchy/tag-2 $tag_dir_prefix/multi_hierarchy/@ && - test_must_fail git refs verify 2>err && - cat >expect <<-EOF && - error: refs/tags/multi_hierarchy/@: badRefName: invalid refname format - EOF - rm $tag_dir_prefix/multi_hierarchy/@ && - test_cmp expect err && - - cp $tag_dir_prefix/tag-1 $tag_dir_prefix/tag-1.lock && + 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/tag-1 $tag_dir_prefix/.lock && + 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 + 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 && - tag_dir_prefix=.git/refs/tags && cd repo && git commit --allow-empty -m initial && git checkout -b branch-1 && - git tag tag-1 && - git commit --allow-empty -m second && - git checkout -b branch-2 && - git tag tag-2 && cp $branch_dir_prefix/branch-1 $branch_dir_prefix/.branch-1 && git -c fsck.badRefName=warn refs verify 2>err && @@ -84,7 +102,7 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' rm $branch_dir_prefix/.branch-1 && test_cmp expect err && - cp $branch_dir_prefix/branch-1 $branch_dir_prefix/@ && + 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 ' From patchwork Sun Nov 10 12:09:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869793 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CF9F81537C6 for ; Sun, 10 Nov 2024 12:09:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240579; cv=none; b=qX8/AvL7HVMByfvREbnckOQTg0nKsnI+kSWu7qMAs7mJetOk/GYHsFfV2Pfpg+IsgcFi+7a7qTAYgOIEIL643iiBqqLraQLbyf4+HCSmW1jarepdr1aBNa1k7VnWp19bv4Lzq/x5Kqayy1I4AsLe8J6IKzeDfIVzOwmgT7uofso= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240579; c=relaxed/simple; bh=5V0XiAbP8HWSYG2I6t6wyCWdiAvyF9V1n8no7T7g7Rw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=iHkN3uFBL1Y7P7asWECRV3qvpDm7uUHSgfQWgEMVGvp1CPZec0EkI1pD9/Bv/hA/f5itLK4Xd+9bxlYtGuUaNKJLe4S3yO3cfs9bWYUuLIYkLWFBV/6wFM89F1jYXqORqwjCs09wzSyPclV9y/QQJ3ZTLOY0WrNCZ6BS+sGUggg= 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=Pavg27DQ; arc=none smtp.client-ip=209.85.214.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Pavg27DQ" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-20ca1b6a80aso39324845ad.2 for ; Sun, 10 Nov 2024 04:09:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240577; x=1731845377; 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=zJ75sjuyqz5k++4m10zIcF0Gbi0g1hHVhXO/JjRwBWE=; b=Pavg27DQqIUZMVH5qkVOQGGjkoJy2tXN0tRXWBTPvS+2TP3yzD22Fx62GXST+UkRkU TWlNabUr0CzsStcZZjjWGEc8KSfbjiiR93eI6nxHDgOHsboDaq+BdtixQikwkqPLcMtV l+tmGmBHRtmDc/mjJPg1OZgJ9oQjZHT0gzU4u07x7LZFvcOnt+aHu18zzbrIEAWnGBE6 O/l5d4xZt1VqSO9G7G+abr/wHF3jU5cBzVLOQck96Ln1FuwmM0eaYOd1sMyK5hgBrfZy ban2S/vAPuFBHm0wy8DNmHRDOCt9Y3sIncDOu0PBlWlz4tN5SEagKDnM8bnAdlZUsA3x jWkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240577; x=1731845377; 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=zJ75sjuyqz5k++4m10zIcF0Gbi0g1hHVhXO/JjRwBWE=; b=GcjC4S9rSKA1VM6eKzpibpe6J715hR41P29Fv143dAeQN3+eKVk1bwNfJ9gUyNZiho iMbow5aPGjEFUzZuIOgS7FKxqtoSvoOQ8vWxiRus+Qb3b+wmGLUoP8qJQ6DSJreJr1op EzzJZl4V/Bd/52zF4XK68qhYr5S4uf31cSSndNjodNMQeQLyR4aOyuPG+4Kkxsfh0uf6 JUwpAkczj4mBN8w5/mBhdydLW7ft1L01o/PHajYqIPI+EpkprVUvJQiQehD9XadmJl2k aoDqUFK+h1NG6aIYfK3/fKRU9peoCCYh15p6y9L0ed8bKOy7yli6kh10qAfgrX24jxSm XIHA== X-Gm-Message-State: AOJu0Yw1/5p8AU0E//aMbeib+Qy8FXbT8DOpcYx0HvX4NEFRY6aaSeo1 nMuXB7pk/Xxi4qU0xn0WqX9PSDhRgz4cFBPbxOfL6Mt4aLP8AqaAkAmDgULLwSQ= X-Google-Smtp-Source: AGHT+IGTfseGN89g/G/jKpgKcyORiO4MWnmLVSKgzJP9pYeu5D6jm6/aM18HDttyUHNWhI+Snb0Z2Q== X-Received: by 2002:a17:902:f707:b0:20c:79f1:fee9 with SMTP id d9443c01a7336-21183c7df2cmr134164075ad.11.1731240576620; Sun, 10 Nov 2024 04:09:36 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21177e6abb0sm58893595ad.244.2024.11.10.04.09.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:09:36 -0800 (PST) Date: Sun, 10 Nov 2024 20:09:34 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 3/9] ref: initialize ref name outside of check functions Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We passes "refs_check_dir" to the "files_fsck_refs_name" function which allows it to create the checked ref name later. However, when we introduce a new check function, we have to allocate redundant memory and re-calculate the ref name. It's bad for us to allocate redundant memory and duplicate logic. Instead, we should allocate and calculate it only once and pass the ref name to the check functions. In order not to do repeat calculation, rename "refs_check_dir" to "refname". And in "files_fsck_refs_dir", create a new strbuf "refname", thus whenever we handle a new ref, calculate the name and call the check functions one by one. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- refs/files-backend.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index b055edc061..8edb700568 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3501,12 +3501,12 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store, */ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, struct fsck_options *o, - const char *refs_check_dir, + const char *refname, struct dir_iterator *iter); static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, - const char *refs_check_dir, + const char *refname, struct dir_iterator *iter) { struct strbuf sb = STRBUF_INIT; @@ -3522,11 +3522,10 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, /* * This works right now because we never check the root refs. */ - strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path); - if (check_refname_format(sb.buf, 0)) { + if (check_refname_format(refname, 0)) { struct fsck_ref_report report = { 0 }; - report.path = sb.buf; + report.path = refname; ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_NAME, "invalid refname format"); @@ -3542,6 +3541,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, const char *refs_check_dir, files_fsck_refs_fn *fsck_refs_fn) { + struct strbuf refname = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; struct dir_iterator *iter; int iter_status; @@ -3560,11 +3560,15 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, continue; } else if (S_ISREG(iter->st.st_mode) || S_ISLNK(iter->st.st_mode)) { + strbuf_reset(&refname); + strbuf_addf(&refname, "%s/%s", refs_check_dir, + iter->relative_path); + if (o->verbose) - fprintf_ln(stderr, "Checking %s/%s", - refs_check_dir, iter->relative_path); + fprintf_ln(stderr, "Checking %s", refname.buf); + for (size_t i = 0; fsck_refs_fn[i]; i++) { - if (fsck_refs_fn[i](ref_store, o, refs_check_dir, iter)) + if (fsck_refs_fn[i](ref_store, o, refname.buf, iter)) ret = -1; } } else { @@ -3581,6 +3585,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, out: strbuf_release(&sb); + strbuf_release(&refname); return ret; } From patchwork Sun Nov 10 12:09:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869794 Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C5C281537C6 for ; Sun, 10 Nov 2024 12:09:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.177 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240588; cv=none; b=g8cceOFJGPEiIXE1xqi5oxGXOOolYRsFR8xiHQaNxkOpCIBYG2rjzEuvMmelEhP59i3/c9QDl3J5sfcBgtwuj+IdwkgLz0IswyJm+7FeiCAvS4UQr1PGtp9y5rUkB255+z0UL1PHiOrlMXSgxQgLB3YUa2r+uGNVUgkmDcNdt3c= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240588; c=relaxed/simple; bh=UMmNFe+UimwvkhTx+Oxb3XjhyVe27DK+GxSHcDqvSRw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PLvy0NlFULQL1RJxaDIn5qzI6OrJnX4fM4empOPnhmFeRbxmVYp68pEWROGgeXkFMEPF2jkCGWlLNUl+VzGhQ/uLiCv5y1VopUMjSZ4yjxvOxzr+JdO+LCMTzbRlWk6sSoxq3mYmTojh9XSQP8Zg4r2VIPbScgppCl/+EHrXw8E= 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=Pb4zPfeV; arc=none smtp.client-ip=209.85.210.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Pb4zPfeV" Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-71e52582cf8so2905969b3a.2 for ; Sun, 10 Nov 2024 04:09:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240586; x=1731845386; 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=oCrwS1CZXCGmjcEuTdAojT1OFTke1dmbq+oiF708lmM=; b=Pb4zPfeVOXtR2yHdJUIDK2H/krn8dc//ndqHQ76t0El6bDpcd+2bVDNLrZddF8kLy1 Dyw0U/N+WF8DClvTvuOGoFmDzX1rGF9O7zxjh2sZwY7uf4elDduIncWhZObHo3vlsx+M T470psbBS7/4S+pniLT/CR2kZ0WLBD6WBHj0tFqBslwIyzvCBtnXQRfTogjERIcQ3069 VOonyzgAC+TKMGLzxk9t1sQQfCkRR1KG6+O7blicHpG5mnlN2ixHWHdVZJqdCF99IkzG pSDqEV7DG7mNpmxgihsscknuH9WsGNaVmv0n9jpySd+joOS9Y+nD9n/D5J+2l+AHPy7N wSVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240586; x=1731845386; 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=oCrwS1CZXCGmjcEuTdAojT1OFTke1dmbq+oiF708lmM=; b=GhVN2Hdzpk3P2LXTwZRXmiBvPJm4U8U+1ClEvI3TJWoFQyFZOEGDYVg56kELt5Tmhy KX1rEbzhCV9NT5G+pPKU7EFIZ/e6ZITHXRlKxcsfH5bNuK5Doy7D+/GF4CC4x0SDbhWR Kh+i3x3dbX1Cv9ul/kyePsFbyw86suj3uioN5smS8wKK9cHePPFcZlb3EWr4/cREjnP7 161raxnUOi5Of4mHuMQ64NaHHbKPYUI/38z2NJp0OySoMDaVY3FmLsUFMUgMGQBYmbJ8 op0Ty2WJXcTQhhJd2t5JB1dJGjL1Yfoj4Y4FvxI2fuNvpy1ABV8CmejYMVOk1UvTEcPp M87w== X-Gm-Message-State: AOJu0YxYbDlgv6XIYg5kmfIUr68cx2OOvFEZ+rSgiuEkHobVHdTwO4Wq eVRpStejqgGYJu+/4OnOmsuv0iJUU9TBd2e9Ow2zXhjB8IJSLS6ipnoLEsMqCZw= X-Google-Smtp-Source: AGHT+IHZ9kL15yvxR1VsKu7JTryN2M2frqYw5bdB69a6+X0O8kMXjmy5LRd29k8333X9YmwNGPJdnA== X-Received: by 2002:a05:6a20:84a5:b0:1dc:ef:5a85 with SMTP id adf61e73a8af0-1dc22af3a70mr9887177637.32.1731240585502; Sun, 10 Nov 2024 04:09:45 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7f41f643a60sm6616704a12.56.2024.11.10.04.09.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:09:45 -0800 (PST) Date: Sun, 10 Nov 2024 20:09:43 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 4/9] ref: support multiple worktrees check for refs Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already set up the infrastructure to check the consistency for refs, but we do not support multiple worktrees. However, "git-fsck(1)" will check the refs of worktrees. As we decide to get feature parity with "git-fsck(1)", we need to set up support for multiple worktrees. Because each worktree has its own specific refs, instead of just showing the users "refs/worktree/foo", we need to display the full name such as "worktrees//refs/worktree/foo". So we should know the id of the worktree to get the full name. Add a new parameter "struct worktree *" for "refs-internal.h::fsck_fn". Then change the related functions to follow this new interface. The "packed-refs" only exists in the main worktree, so we should only check "packed-refs" in the main worktree. Use "is_main_worktree" method to skip checking "packed-refs" in "packed_fsck" function. Then, enhance the "files-backend.c::files_fsck_refs_dir" function to add "worktree//" prefix when we are not in the main worktree. Last, add a new test to check the refname when there are multiple worktrees to exercise the code. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- builtin/refs.c | 10 ++++++-- refs.c | 5 ++-- refs.h | 3 ++- refs/debug.c | 5 ++-- refs/files-backend.c | 17 ++++++++++---- refs/packed-backend.c | 8 ++++++- refs/refs-internal.h | 3 ++- refs/reftable-backend.c | 3 ++- t/t0602-reffiles-fsck.sh | 51 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 90 insertions(+), 15 deletions(-) diff --git a/builtin/refs.c b/builtin/refs.c index 24978a7b7b..394b4101c6 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -5,6 +5,7 @@ #include "parse-options.h" #include "refs.h" #include "strbuf.h" +#include "worktree.h" #define REFS_MIGRATE_USAGE \ N_("git refs migrate --ref-format= [--dry-run]") @@ -66,6 +67,7 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) static int cmd_refs_verify(int argc, const char **argv, const char *prefix) { struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT; + struct worktree **worktrees; const char * const verify_usage[] = { REFS_VERIFY_USAGE, NULL, @@ -75,7 +77,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "strict", &fsck_refs_options.strict, N_("enable strict checking")), OPT_END(), }; - int ret; + int ret = 0; argc = parse_options(argc, argv, prefix, options, verify_usage, 0); if (argc) @@ -84,9 +86,13 @@ 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); - ret = refs_fsck(get_main_ref_store(the_repository), &fsck_refs_options); + worktrees = get_worktrees(); + for (size_t i = 0; worktrees[i]; i++) + ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), + &fsck_refs_options, worktrees[i]); fsck_options_clear(&fsck_refs_options); + free_worktrees(worktrees); return ret; } diff --git a/refs.c b/refs.c index 5f729ed412..395a17273c 100644 --- a/refs.c +++ b/refs.c @@ -318,9 +318,10 @@ int check_refname_format(const char *refname, int flags) return check_or_sanitize_refname(refname, flags, NULL); } -int refs_fsck(struct ref_store *refs, struct fsck_options *o) +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt) { - return refs->be->fsck(refs, o); + return refs->be->fsck(refs, o, wt); } void sanitize_refname_component(const char *refname, struct strbuf *out) diff --git a/refs.h b/refs.h index 108dfc93b3..341d43239c 100644 --- a/refs.h +++ b/refs.h @@ -549,7 +549,8 @@ int check_refname_format(const char *refname, int flags); * reflogs are consistent, and non-zero otherwise. The errors will be * written to stderr. */ -int refs_fsck(struct ref_store *refs, struct fsck_options *o); +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt); /* * Apply the rules from check_refname_format, but mutate the result until it diff --git a/refs/debug.c b/refs/debug.c index 45e2e784a0..72e80ddd6d 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -420,10 +420,11 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname, } static int debug_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = drefs->refs->be->fsck(drefs->refs, o); + int res = drefs->refs->be->fsck(drefs->refs, o, wt); trace_printf_key(&trace_refs, "fsck: %d\n", res); return res; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 8edb700568..8bfdce64bc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -23,6 +23,7 @@ #include "../dir.h" #include "../chdir-notify.h" #include "../setup.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../revision.h" @@ -3539,6 +3540,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, static int files_fsck_refs_dir(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, + struct worktree *wt, files_fsck_refs_fn *fsck_refs_fn) { struct strbuf refname = STRBUF_INIT; @@ -3561,6 +3563,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } else if (S_ISREG(iter->st.st_mode) || S_ISLNK(iter->st.st_mode)) { strbuf_reset(&refname); + + if (!is_main_worktree(wt)) + strbuf_addf(&refname, "worktrees/%s/", wt->id); strbuf_addf(&refname, "%s/%s", refs_check_dir, iter->relative_path); @@ -3590,7 +3595,8 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } static int files_fsck_refs(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, @@ -3599,17 +3605,18 @@ static int files_fsck_refs(struct ref_store *ref_store, if (o->verbose) fprintf_ln(stderr, _("Checking references consistency")); - return files_fsck_refs_dir(ref_store, o, "refs", fsck_refs_fn); + return files_fsck_refs_dir(ref_store, o, "refs", wt, fsck_refs_fn); } static int files_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, "fsck"); - return files_fsck_refs(ref_store, o) | - refs->packed_ref_store->be->fsck(refs->packed_ref_store, o); + return files_fsck_refs(ref_store, o, wt) | + refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt); } struct ref_storage_be refs_be_files = { diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 07c57fd541..46dcaec654 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -13,6 +13,7 @@ #include "../lockfile.h" #include "../chdir-notify.h" #include "../statinfo.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../trace2.h" @@ -1754,8 +1755,13 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s } static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt) { + + if (!is_main_worktree(wt)) + return 0; + return 0; } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..037d7991cd 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -653,7 +653,8 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam struct strbuf *referent); typedef int fsck_fn(struct ref_store *ref_store, - struct fsck_options *o); + struct fsck_options *o, + struct worktree *wt); struct ref_storage_be { const char *name; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index f5f957e6de..b6a63c1015 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -2443,7 +2443,8 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, } static int reftable_be_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt UNUSED) { return 0; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 2a172c913d..1e17393a3d 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -107,4 +107,55 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' 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 $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_done From patchwork Sun Nov 10 12:09:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869795 Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B5CA01537C6 for ; Sun, 10 Nov 2024 12:09:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240596; cv=none; b=RUdDYzTj35hSIQXZM/edtiIpFO9qxBaNfMjkhrqXtSl2l71J4WVOkfhuBFOYM8X+EX98FwTp0XGIMTKmqesbdM+UN3zNruu/J8c7/FhCQa/cdU6qRK1CaGj74kDZP86l7oecQbePaBhIOORS9A6eqe2XMdRD1O6mOu9/yDF33eY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240596; c=relaxed/simple; bh=OsnWJNxiKyo/n2Z7hmkTDl/rWU2xj8B7kKDzEECYBkU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=p3005Qfjvmp2a6PEeiVZxkBAHjWpA+oZjrrqvwmVb3CJbQu5t5IDYi3zz3eq4Pfxz2NqkZEaekRr1lqF6O8rmH/CNLcYOPXnSAoixgnKnkH/9pQ9R31yysrlgWm2BkxCeTbPGnNZmj38UQcNDyj4fSgNUJa81B3cWyQtsqcg2PE= 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=UMsqKbtx; arc=none smtp.client-ip=209.85.215.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="UMsqKbtx" Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-7f3f1849849so2498569a12.1 for ; Sun, 10 Nov 2024 04:09:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240593; x=1731845393; 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=Axn/V3vvugd/C60cNtfS/CP3iY8fWxJMJxFlnUTCwz8=; b=UMsqKbtxpN81vf5VVzpZGCESEftjikOnWUxtOEMS8n1vJVsTOA0Z0OuNFvw3SWMbrd D9hrDVP92FMQ5LWNTJJBSz4B4TzVLMoxr0zzgjmA1QHXxfkKc8cHlFQ4gXJa5qtMbeNt VBiTywiKih2JzothA7a5Kzg35Vm6OJsR0TYuYYwqEzaR3hoUgqxTHzQQ9LYtR+Hc0MRG 5pe2AehrQBBdlakJfi/YIrxd2y8sPWKNQ68PbSTU3UergiCNXuBgqm3hlqnH0OjnZ1kR v5E1b/6P3UCP7uKplpb8GJZnG1FNuVSuu7Le7hYLfGHkdx0RxJrdXovcEI52W9KKZC94 cwiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240593; x=1731845393; 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=Axn/V3vvugd/C60cNtfS/CP3iY8fWxJMJxFlnUTCwz8=; b=PdJSMILh0AwS2/BwnPjBVfP99TKV+Koru6KzfwOUMBA8lj2P4E5ZQn/tqOfG2jweph msVlhgcSKDpJuxBK1VxZawh2yf3iCo5WvElsqATSGiM/H4LeApX+a6vhv20suk8LlX06 ye4LqIkq0vA/dO5XA0q2/cLVfhd4J6LPhyXyOQOjdozvBdOBwKrPvLmgvaIF/Fo7tsDz tEV2WrWhOjNYJD8SMUJ3GTsCJocOBktLUJh0rPjHqUawZ0SK69Y0dtxM09mxTgfuqXka kWFBeoB8RoG0gRDOwJxB479YgSgKMx9a72dWXm7ag69HAmnCHGRIwZfh29IlFN1F+1Vx HiCA== X-Gm-Message-State: AOJu0Yz+ANdU3x4PVU23tXIGoXwG8bFJI0J4LjaI7cuU8RqBP9WzMiC1 upN7MhKsSf0mInfOKLkBTgk8/f3jfev4oZxS2rRWWuuodMGV3YPqZtEo6N+9VmY= X-Google-Smtp-Source: AGHT+IF3UTgeUOtIHxsU0uYAU30wEtb8uIhR88SdcqcomLYyJuVwibobAG8+CCRwgCrDMMnqFaQ/AQ== X-Received: by 2002:a17:90b:4b91:b0:2e1:d5c9:1bc4 with SMTP id 98e67ed59e1d1-2e9b165595cmr12988739a91.7.1731240593202; Sun, 10 Nov 2024 04:09:53 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2e9a5fe87c2sm6695619a91.48.2024.11.10.04.09.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:09:52 -0800 (PST) Date: Sun, 10 Nov 2024 20:09:51 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 5/9] ref: port git-fsck(1) regular refs check for files backend 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: "git-fsck(1)" implicitly checks the ref content by passing the callback "fsck_handle_ref" to the "refs.c::refs_for_each_rawref". Then, it will check whether the ref content (eventually "oid") is valid. If not, it will report the following error to the user. error: refs/heads/main: invalid sha1 pointer 0000... And it will also report above errors when there are dangling symrefs in the repository wrongly. This does not align with the behavior of the "git symbolic-ref" command which allows users to create dangling symrefs. As we have already introduced the "git refs verify" command, we'd better check the ref content explicitly in the "git refs verify" command thus later we could remove these checks in "git-fsck(1)" and launch a subprocess to call "git refs verify" in "git-fsck(1)" to make the "git-fsck(1)" more clean. Following what "git-fsck(1)" does, add a similar check to "git refs verify". Then add a new fsck error message "badRefContent(ERROR)" to represent that a ref has an invalid content. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/files-backend.c | 43 ++++++++++++++ t/t0602-reffiles-fsck.sh | 105 ++++++++++++++++++++++++++++++++++ 4 files changed, 152 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 68a2801f15..22c385ea22 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -19,6 +19,9 @@ `badParentSha1`:: (ERROR) A commit object has a bad parent sha1. +`badRefContent`:: + (ERROR) A ref has bad content. + `badRefFiletype`:: (ERROR) A ref has a bad file type. diff --git a/fsck.h b/fsck.h index 500b4c04d2..0d99a87911 100644 --- a/fsck.h +++ b/fsck.h @@ -31,6 +31,7 @@ enum fsck_msg_type { FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ FUNC(BAD_PARENT_SHA1, ERROR) \ + FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index 8bfdce64bc..2d126ecbbe 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3505,6 +3505,48 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refname, struct dir_iterator *iter); +static int files_fsck_refs_content(struct ref_store *ref_store, + struct fsck_options *o, + const char *target_name, + struct dir_iterator *iter) +{ + struct strbuf ref_content = STRBUF_INIT; + struct strbuf referent = STRBUF_INIT; + struct fsck_ref_report report = { 0 }; + unsigned int type = 0; + int failure_errno = 0; + struct object_id oid; + int ret = 0; + + report.path = target_name; + + if (S_ISLNK(iter->st.st_mode)) + goto cleanup; + + if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "cannot read ref file '%s': %s", + iter->path.buf, strerror(errno)); + goto cleanup; + } + + if (parse_loose_ref_contents(ref_store->repo->hash_algo, + ref_content.buf, &oid, &referent, + &type, &failure_errno)) { + strbuf_rtrim(&ref_content); + ret = fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_CONTENT, + "%s", ref_content.buf); + goto cleanup; + } + +cleanup: + strbuf_release(&ref_content); + strbuf_release(&referent); + return ret; +} + static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, const char *refname, @@ -3600,6 +3642,7 @@ static int files_fsck_refs(struct ref_store *ref_store, { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, + files_fsck_refs_content, NULL, }; diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 1e17393a3d..162370077b 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -158,4 +158,109 @@ test_expect_success 'ref name check should work for multiple worktrees' ' 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" && + + 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/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 +' + +test_expect_success 'regular ref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + 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 && + + 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 + EOF + sort err >sorted_err && + test_cmp expect sorted_err +' + +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 + ) && + + 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 +' + test_done From patchwork Sun Nov 10 12:10:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869796 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.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 39E561537C6 for ; Sun, 10 Nov 2024 12:10:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240608; cv=none; b=TeovZtcazcSHdk3YjoWJwocRTf4AaFtwZfT6Q9NPOxiUed2DN1Kjf6PGiKORethVrNdm1ZHkzoB0XFKEMCIXpCBMyR6eadciAKz3hlGbczgRGXl3ZCz7o+L3trzK5TurALeP3xo9o2ehGLFidFjsERvnUDERzu8JPEaQNiZ5ZBY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240608; c=relaxed/simple; bh=afwqolSTzx4hNOmHAZU++IjpKj5i3Ao9MuTohyY3Pwk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rUmloO5BqeI5PMwgaLtOiaqVV5WIBwdgXWnYagnoii7EItDXURndL4dYeMQ+houUX0+IzCMJwMV64srJfWXYKPxTgxt2M7so8B5eKSqC2rMKF0cN0YRgcMybefi1rrvGQKsNnoZtkHm8hTGtEONDrzGRHwnvHHh1yr+Zt6mJfaY= 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=dRZc/WqO; arc=none smtp.client-ip=209.85.210.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="dRZc/WqO" Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-720be27db27so2915557b3a.2 for ; Sun, 10 Nov 2024 04:10:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240606; x=1731845406; 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=3oTzlqaK5G282+jHQ0kfI2JnrYlouXKCq9QhUvCXkLE=; b=dRZc/WqOzfw/DHMBKLw/cHEcG+86ZHo/6SSFL0vvPi44HyQQ1Rz9WvBinxUjJPaG6D sO/RYHkO7jZXp/9mufgEbbBZLeFiXDWy0RFDemAXAhn7mQvoP3r9LN/kWOebdGAQIq0U kTWun0M/LidN3xmVZbsypLIgQKpw5vJZncG4qN2c72jl2Kvube3rnSdASWra7aJYRdo5 0xGwXlMjZb2n0lVcy/YaEBGZEcy6KmDiPci2tMf2RK7AhvswQLHMl+bNkj7pbtjorSFV T+9iLEB6pQEOuFkK8fuGgmrsBcWlt/iJh3AhLyh/e+cmqM3F/6wtqjEO6iPNRolZTaPW OCBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240606; x=1731845406; 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=3oTzlqaK5G282+jHQ0kfI2JnrYlouXKCq9QhUvCXkLE=; b=DeTSVrORfBN2aUnGZRujP2fOeKeVhtLpLlVVmX+UM5Qjd8Fo+oJ3EPecKaqZEhyTOE F4v72O4F2iJg6xOi28X4MSdvFQsgrWkXSqQBcrCWP6uIdTAgXyuufLI+mONk3/2SK/FU JpTFm9NG2q/NZPeBOA1EoEasNoMoN4Oz2caqGXvfY+aO2HphSrKz75L9CCerVJLnUdQZ yP3+On4l70LsTyZs1iZG6EF037UF/vH79JlAUOJkvWv/BYm/BZVRHSQJnletncqFlqsv W1717q+OLdUQeQjyqBsps1CvcxinoEbDGF17bjm/sKMi/22VLDlNA2773YaKxPJuliEZ v3XQ== X-Gm-Message-State: AOJu0Yyu6JhQcxQbeZkqdvrcYmYcUoo/3/WjrfYrE2jLUtt6GFE+iIA0 exG7Pk1OMQUhug4+lEuE6/mCQGuN8yWcSRkGYGX1Q7njBONjjVHlsE15OcHnmRE= X-Google-Smtp-Source: AGHT+IH0eJA0bHgaBEhc3rtl1UQWVdwH5QxbvR6Ifl6pXgKypZGlWof4UZ9AEqwIsJRTm3Fgogoq+g== X-Received: by 2002:a05:6a00:21cc:b0:71e:587d:f268 with SMTP id d2e1a72fcca58-724132788e0mr12515213b3a.4.1731240605860; Sun, 10 Nov 2024 04:10:05 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-724078a7d75sm7244135b3a.68.2024.11.10.04.10.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:10:05 -0800 (PST) Date: Sun, 10 Nov 2024 20:10:03 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 6/9] ref: add more strict checks for regular refs Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have already used "parse_loose_ref_contents" function to check whether the ref content is valid in files backend. However, by using "parse_loose_ref_contents", we allow the ref's content to end with garbage or without a newline. Even though we never create such loose refs ourselves, we have accepted such loose refs. So, it is entirely possible that some third-party tools may rely on such loose refs being valid. We should not report an error fsck message at current. We should notify the users about such "curiously formatted" loose refs so that adequate care is taken before we decide to tighten the rules in the future. And it's not suitable either to report a warn fsck message to the user. We don't yet want the "--strict" flag that controls this bit to end up generating errors for such weirdly-formatted reference contents, as we first want to assess whether this retroactive tightening will cause issues for any tools out there. It may cause compatibility issues which may break the repository. So, we add the following two fsck infos to represent the situation where the ref content ends without newline or has trailing garbages: 1. refMissingNewline(INFO): A loose ref that does not end with newline(LF). 2. trailingRefContent(INFO): A loose ref has trailing content. It might appear that we can't provide the user with any warnings by using FSCK_INFO. However, in "fsck.c::fsck_vreport", we will convert FSCK_INFO to FSCK_WARN and we can still warn the user about these situations when using "git refs verify" without introducing compatibility issues. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 14 +++++++++ fsck.h | 2 ++ refs.c | 2 +- refs/files-backend.c | 26 ++++++++++++++-- refs/refs-internal.h | 2 +- t/t0602-reffiles-fsck.sh | 57 +++++++++++++++++++++++++++++++++-- 6 files changed, 96 insertions(+), 7 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 22c385ea22..6db0eaa84a 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -173,6 +173,20 @@ `nullSha1`:: (WARN) Tree contains entries pointing to a null sha1. +`refMissingNewline`:: + (INFO) A loose ref that does not end with newline(LF). As + valid implementations of Git never created such a loose ref + file, it may become an error in the future. Report to the + git@vger.kernel.org mailing list if you see this error, as + we need to know what tools created such a file. + +`trailingRefContent`:: + (INFO) A loose ref has trailing content. As valid implementations + of Git never created such a loose ref file, it may become an + error in the future. Report to the git@vger.kernel.org mailing + list if you see this error, as we need to know what tools + created such a file. + `treeNotSorted`:: (ERROR) A tree is not properly sorted. diff --git a/fsck.h b/fsck.h index 0d99a87911..b85072df57 100644 --- a/fsck.h +++ b/fsck.h @@ -85,6 +85,8 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(REF_MISSING_NEWLINE, INFO) \ + FUNC(TRAILING_REF_CONTENT, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/refs.c b/refs.c index 395a17273c..f88b32a633 100644 --- a/refs.c +++ b/refs.c @@ -1789,7 +1789,7 @@ static int refs_read_special_head(struct ref_store *ref_store, } result = parse_loose_ref_contents(ref_store->repo->hash_algo, content.buf, - oid, referent, type, failure_errno); + oid, referent, type, NULL, failure_errno); done: strbuf_release(&full_path); diff --git a/refs/files-backend.c b/refs/files-backend.c index 2d126ecbbe..871c8946f8 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -569,7 +569,7 @@ static int read_ref_internal(struct ref_store *ref_store, const char *refname, buf = sb_contents.buf; ret = parse_loose_ref_contents(ref_store->repo->hash_algo, buf, - oid, referent, type, &myerr); + oid, referent, type, NULL, &myerr); out: if (ret && !myerr) @@ -606,7 +606,7 @@ static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refn int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno) + const char **trailing, int *failure_errno) { const char *p; if (skip_prefix(buf, "ref:", &buf)) { @@ -628,6 +628,10 @@ int parse_loose_ref_contents(const struct git_hash_algo *algop, *failure_errno = EINVAL; return -1; } + + if (trailing) + *trailing = p; + return 0; } @@ -3513,6 +3517,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, struct strbuf ref_content = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct fsck_ref_report report = { 0 }; + const char *trailing = NULL; unsigned int type = 0; int failure_errno = 0; struct object_id oid; @@ -3533,7 +3538,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, if (parse_loose_ref_contents(ref_store->repo->hash_algo, ref_content.buf, &oid, &referent, - &type, &failure_errno)) { + &type, &trailing, &failure_errno)) { strbuf_rtrim(&ref_content); ret = fsck_report_ref(o, &report, FSCK_MSG_BAD_REF_CONTENT, @@ -3541,6 +3546,21 @@ static int files_fsck_refs_content(struct ref_store *ref_store, goto cleanup; } + if (!(type & REF_ISSYMREF)) { + if (!*trailing) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + goto cleanup; + } + if (*trailing != '\n' || *(trailing + 1)) { + ret = fsck_report_ref(o, &report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing garbage: '%s'", trailing); + goto cleanup; + } + } + cleanup: strbuf_release(&ref_content); strbuf_release(&referent); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 037d7991cd..125f1fe735 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -716,7 +716,7 @@ struct ref_store { int parse_loose_ref_contents(const struct git_hash_algo *algop, const char *buf, struct object_id *oid, struct strbuf *referent, unsigned int *type, - int *failure_errno); + const char **trailing, int *failure_errno); /* * Fill in the generic part of refs and add it to our collection of diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 162370077b..33e7a390ad 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -189,7 +189,48 @@ test_expect_success 'regular ref content should be checked (individual)' ' EOF rm $branch_dir_prefix/a/b/branch-bad && test_cmp expect err || return 1 - done + 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 && + 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: '\'' + + + garbage'\'' + EOF + rm $branch_dir_prefix/branch-garbage-special && + test_cmp expect err ' test_expect_success 'regular ref content should be checked (aggregate)' ' @@ -207,12 +248,16 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' 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 @@ -260,7 +305,15 @@ test_expect_success 'ref content checks should work with worktrees' ' EOF rm $worktree2_refdir_prefix/bad-branch-2 && test_cmp expect err || return 1 - done + 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 ' test_done From patchwork Sun Nov 10 12:10:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869797 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3B37F1537C6 for ; Sun, 10 Nov 2024 12:10:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240617; cv=none; b=MHYuuQK04BOptD55BUIpFowlrmG8tvRQLGH10jzzg7gPmt9eHFCV/x4faeTrgiQkPwrLOjAiQPhvJ94TYLafDwpoh/xcYkluw3+BLUR+HG+bslywS3PRWjlbe9GZgKTuilNknaxZKqW52ubJ3fHDIXox4Z+H3PYuDcrL5qhbYSo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240617; c=relaxed/simple; bh=v3l23AXmSqjgLLspfCxRfg2vze3g6DwMEbd39PLxxaM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lALOP1zAZ6IIz+OE8w4FmLE3RiySHUk7KMU1TtUOEY799Q7A7BrSTqi4y1xAXHyPxu8hURiajngeJH+odlvCD+z0vl99nqWyhuKw9nEjB16vme0UdHWL6oEtDxgd2xjSCY9372ylkre3N9u9JIDDM6cRTaY5OAb4po6I6bm/PBo= 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=BnEj6vOm; arc=none smtp.client-ip=209.85.214.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BnEj6vOm" Received: by mail-pl1-f176.google.com with SMTP id d9443c01a7336-20cb47387ceso38048125ad.1 for ; Sun, 10 Nov 2024 04:10:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240614; x=1731845414; 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=q6UStGRhZm0tFNAVoRqOD92u/ksbYDovxRbLdgrDs4M=; b=BnEj6vOmCUF3VYccJGm2HBmQg9wKeL+tY7AOB97yDkyeVit2p9DmjZzSqoFpbIB6KM MHz3nZwipT1A7GuLe+moLNb/nkkFhPVskIfYW40Mtpb3vVGM1CF3xWRjfHmsEBiukWUg A6OupjmwvfQUqoyHwtQ0OQCr9f4be/dXbsfOpuHgHOY7ZFnsH9J2HL883NHjzX0Xig8f J44knBBL65sWLmYOpaxX0c5GajbFpvt926CkJZj27eIhYJDxF/KZbONpJ+Tl+wYeOWmP c+AQvaq9dvWwyXVLBRwkuC6xuaFjWDHO0CPBxXm/2b64OQEKBAd+R9znJ4e3pM+onikl h70w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240614; x=1731845414; 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=q6UStGRhZm0tFNAVoRqOD92u/ksbYDovxRbLdgrDs4M=; b=ri4gIWR6xI70P1h0slqRGODLtb0AuLEJ4fSQp6M9O0xVFmIIo2lM5viFUaeHZLGOyX VSCEvoDtQG4Td4/oOE5CvvLAvhQNq2aOUKXZ5X2qcV67hVDHgzLJJZRLnSgJQjVO5nEx C8/ZajnSdlIwgm/HFXUo6XQOS51MANpF4QGUDEIVMclDgQVDxfr62yg5Tm85I60d03v7 dpi0z/Tg1MGoEPiK5ehyerzVBvC47AZ++DNstvQICzREEyUJbFyyXSfQUI1I8Ybkw42Q bwEYl/WLeEIPVzNzt/+ZlYIxkFMH5xq7Yz2RTrakNJejxed19pCFGAYPNVgVDwixiqlE f3JQ== X-Gm-Message-State: AOJu0YzKOrj4wv965CWvdrgSH9KvlUXcsajRscDLprVIiIMhU+iYz8V1 RrGJuiF7RXd0/i8tUu9lFVVCmDgbSrIQKyhXsJuz2ijVmDroKYVWf2upaAm6PhY= X-Google-Smtp-Source: AGHT+IG6HuIuMmjxfBQD4KUbcafDHK0C3pPqUvcwzu2sCNfp6OU4R17izRP2WVM7eujkmIHM4xEqnw== X-Received: by 2002:a17:902:db0d:b0:20c:6b11:deef with SMTP id d9443c01a7336-211835bb654mr115121255ad.48.1731240614057; Sun, 10 Nov 2024 04:10:14 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21177e59dfesm58765615ad.181.2024.11.10.04.10.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:10:13 -0800 (PST) Date: Sun, 10 Nov 2024 20:10:11 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 7/9] ref: add basic symref content check for files backend Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: We have code that checks regular ref contents, but we do not yet check the contents of symbolic refs. By using "parse_loose_ref_content" for symbolic refs, we will get the information of the "referent". We do not need to check the "referent" by opening the file. This is because if "referent" exists in the file system, we will eventually check its correctness by inspecting every file in the "refs" directory. If the "referent" does not exist in the filesystem, this is OK as it is seen as the dangling symref. So we just need to check the "referent" string content. A regular ref could be accepted as a textual symref if it begins with "ref:", followed by zero or more whitespaces, followed by the full refname, followed only by whitespace characters. However, we always write a single SP after "ref:" and a single LF after the refname. It may seem that we should report a fsck error message when the "referent" does not apply above rules and we should not be so aggressive because third-party reimplementations of Git may have taken advantage of the looser syntax. Put it more specific, we accept the following contents: 1. "ref: refs/heads/master " 2. "ref: refs/heads/master \n \n" 3. "ref: refs/heads/master\n\n" When introducing the regular ref content checks, we created two fsck infos "refMissingNewline" and "trailingRefContent" which exactly represents above situations. So we will reuse these two fsck messages to write checks to info the user about these situations. But we do not allow any other trailing garbage. The followings are bad symref contents which will be reported as fsck error by "git-fsck(1)". 1. "ref: refs/heads/master garbage\n" 2. "ref: refs/heads/master \n\n\n garbage " And we introduce a new "badReferentName(ERROR)" fsck message to report above errors by using "is_root_ref" and "check_refname_format" to check the "referent". Since both "is_root_ref" and "check_refname_format" don't work with whitespaces, we use the trimmed version of "referent" with these functions. In order to add checks, we will do the following things: 1. Record the untrimmed length "orig_len" and untrimmed last byte "orig_last_byte". 2. Use "strbuf_rtrim" to trim the whitespaces or newlines to make sure "is_root_ref" and "check_refname_format" won't be failed by them. 3. Use "orig_len" and "orig_last_byte" to check whether the "referent" misses '\n' at the end or it has trailing whitespaces or newlines. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 3 + fsck.h | 1 + refs/files-backend.c | 40 ++++++++++++ t/t0602-reffiles-fsck.sh | 111 ++++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index 6db0eaa84a..dcea05edfc 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -28,6 +28,9 @@ `badRefName`:: (ERROR) A ref has an invalid format. +`badReferentName`:: + (ERROR) The referent name of a symref is invalid. + `badTagName`:: (INFO) A tag has an invalid format. diff --git a/fsck.h b/fsck.h index b85072df57..5227dfdef2 100644 --- a/fsck.h +++ b/fsck.h @@ -34,6 +34,7 @@ enum fsck_msg_type { FUNC(BAD_REF_CONTENT, ERROR) \ FUNC(BAD_REF_FILETYPE, ERROR) \ FUNC(BAD_REF_NAME, ERROR) \ + FUNC(BAD_REFERENT_NAME, ERROR) \ FUNC(BAD_TIMEZONE, ERROR) \ FUNC(BAD_TREE, ERROR) \ FUNC(BAD_TREE_SHA1, ERROR) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index 871c8946f8..8bc7c6e0c2 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3509,6 +3509,43 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, const char *refname, struct dir_iterator *iter); +static int files_fsck_symref_target(struct fsck_options *o, + struct fsck_ref_report *report, + struct strbuf *referent) +{ + char orig_last_byte; + size_t orig_len; + int ret = 0; + + orig_len = referent->len; + orig_last_byte = referent->buf[orig_len - 1]; + strbuf_rtrim(referent); + + if (!is_root_ref(referent->buf) && + check_refname_format(referent->buf, 0)) { + ret = fsck_report_ref(o, report, + FSCK_MSG_BAD_REFERENT_NAME, + "points to invalid refname '%s'", referent->buf); + goto out; + } + + if (referent->len == orig_len || + (referent->len < orig_len && orig_last_byte != '\n')) { + ret = fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + } + + if (referent->len != orig_len && referent->len != orig_len - 1) { + ret = fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing whitespaces or newlines"); + } + +out: + return ret; +} + static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *target_name, @@ -3559,6 +3596,9 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "has trailing garbage: '%s'", trailing); goto cleanup; } + } else { + ret = files_fsck_symref_target(o, &report, &referent); + goto cleanup; } cleanup: diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 33e7a390ad..ee1e5f2864 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -263,6 +263,109 @@ test_expect_success 'regular ref content should be checked (aggregate)' ' 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" && + + 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 && + + 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 +' + +test_expect_success 'textual symref content should be checked (aggregate)' ' + test_when_finished "rm -rf repo" && + 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 +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && @@ -313,6 +416,14 @@ test_expect_success 'ref content checks should work with worktrees' ' 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 ' From patchwork Sun Nov 10 12:10:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869798 Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7ADFB1537C6 for ; Sun, 10 Nov 2024 12:10:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.175 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240625; cv=none; b=HTaJJ1LO9H9eJLIrDASTAXhrFfmDcrXZziFUGfXTkqlpbUVw+IPXQqk9lpkHpDTk65eDz2F+Gix16CPNqIhgduLrjSvEVeXsdICgnoScm4mcg0bpSAraTiry1JpuM0dacKZlJn5LNb6p2TxDQydpMHtfgP99WrU5aNaPC5cgIbY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240625; c=relaxed/simple; bh=ycach4/Zu/pXj5xhF7w8AKVzUPQtcrGbaxA51kPNzsQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eJV4np8IP7Ntan2vKURcvmvENfD+r2w6BvIVJkYSxF61/O35rsSTfx9Yf20qcaVlA0SvlVK2QpA9+AZtZjxeFHfIcN9BeqM8uYMWcDTjTVETq6vpXlNn+E8Nj8qio6H0OhH7p/HfzMo46c67VnAOmMUAe3GEICwNAiL+siakPx8= 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=FTMRrMW7; arc=none smtp.client-ip=209.85.210.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="FTMRrMW7" Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-7242f559a9fso566554b3a.1 for ; Sun, 10 Nov 2024 04:10:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240622; x=1731845422; 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=uJIm0PLKopA0pLigBMFzpW3ZN+UPJ4RZA8VqzbVNzfc=; b=FTMRrMW7rvOunmhHqeucigftxITIT156EHAqRRjTH2b8r7u4X9jBAYmHF/cf5U2UDf ek32SzyAYS8GqJdm4oQLzei6zSx9VbkQWR38SeVz8vPkIxFJ6AD4o6bAPDWpm/uMRuhF /BFC0K0zAQRvGE8kecGNebzsBDPeb/RrAaQvChVfjAQd2PKJKteU+cA6QklaTfTsGYGl TaQL1nHjEhEK5xi05GPdP4z9mzgFBBWMI+Ox3aV9BzemRkhxiRKwKBPcWkGoDdH7qSkj XUUOUF6TQOnyH6Wxov0m5G+gOvmq78XGms0p2f268fra5XTZ5hvzLcsmPKji6cruJhDl d6Qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240622; x=1731845422; 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=uJIm0PLKopA0pLigBMFzpW3ZN+UPJ4RZA8VqzbVNzfc=; b=XjYJJBrRqHeTPLVFyb6ayHMwmqie+/iDz79+MLX36mngYagSUgamDawwMlOJOG3RnE 6oT8b+DL9kCpzxbeSAoE2+jUD/0sz89BS/crwhAczJHX1z2uZ/8ePgNehnKvRuLF90xt Pcz9yQSq7D/bsqwf9swBwoOYmbc+5sEW37WZi+bwU04a7+S6O/wKwqrfPPI3fD+DnxgD uXhjdZry4jL0OlQvEYEwAQTTl6to+YCdmD9NfD3m1L9xjLes/ES7g3S7ba91Edgb0nqs JGwthkcff+EnouRH1TZWSBJ0QCI16AFWl2KApr1qRHpC3QBc+NEDIT9UhZmOnRjGMyiI Nq+g== X-Gm-Message-State: AOJu0YyD5/LDL/fSZXWV8PZCkQTRHtfnu9efHdqjaInC5dpUstq6Spn7 dQUJ68tjBxWmU8IrywmU88bM0T9d+Kh3tggsreJH20PSVX2uTDI4IFthDk3+leU= X-Google-Smtp-Source: AGHT+IFWMRtQX3gVDSz7fct91VlL9Ht916qr/jMm7iN0Z1oS3PtAw63AuvB5Y9urujqEDfFuBOXvJw== X-Received: by 2002:a05:6a00:1307:b0:71e:6728:72d5 with SMTP id d2e1a72fcca58-724132c1a44mr13489473b3a.15.1731240622348; Sun, 10 Nov 2024 04:10:22 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72407860a6bsm7000562b3a.6.2024.11.10.04.10.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:10:21 -0800 (PST) Date: Sun, 10 Nov 2024 20:10:20 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 8/9] ref: check whether the target of the symref is a ref 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: Ideally, we want to the users use "git symbolic-ref" to create symrefs instead of writing raw contents into the filesystem. However, "git symbolic-ref" is strict with the refname but not strict with the referent. For example, we can make the "referent" located at the "$(gitdir)/logs/aaa" and manually write the content into this where we can still successfully parse this symref by using "git rev-parse". $ git init repo && cd repo && git commit --allow-empty -mx $ git symbolic-ref refs/heads/test logs/aaa $ echo $(git rev-parse HEAD) > .git/logs/aaa $ git rev-parse test We may need to add some restrictions for "referent" parameter when using "git symbolic-ref" to create symrefs because ideally all the nonpseudo-refs should be located under the "refs" directory and we may tighten this in the future. In order to tell the user we may tighten the above situation, create a new fsck message "symrefTargetIsNotARef" to notify the user that this may become an error in the future. Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 9 +++++++++ fsck.h | 1 + refs/files-backend.c | 14 ++++++++++++-- t/t0602-reffiles-fsck.sh | 29 +++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index dcea05edfc..f82ebc58e8 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -183,6 +183,15 @@ git@vger.kernel.org mailing list if you see this error, as we need to know what tools created such a file. +`symrefTargetIsNotARef`:: + (INFO) The target of a symbolic reference points neither to + a root reference nor to a reference starting with "refs/". + Although we allow create a symref pointing to the referent which + is outside the "ref" by using `git symbolic-ref`, we may tighten + the rule in the future. Report to the git@vger.kernel.org + mailing list if you see this error, as we need to know what tools + created such a file. + `trailingRefContent`:: (INFO) A loose ref has trailing content. As valid implementations of Git never created such a loose ref file, it may become an diff --git a/fsck.h b/fsck.h index 5227dfdef2..53a47612e6 100644 --- a/fsck.h +++ b/fsck.h @@ -87,6 +87,7 @@ enum fsck_msg_type { FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ FUNC(REF_MISSING_NEWLINE, INFO) \ + FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/refs/files-backend.c b/refs/files-backend.c index 8bc7c6e0c2..b3ec409920 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3513,6 +3513,7 @@ static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, struct strbuf *referent) { + int is_referent_root; char orig_last_byte; size_t orig_len; int ret = 0; @@ -3521,8 +3522,17 @@ static int files_fsck_symref_target(struct fsck_options *o, orig_last_byte = referent->buf[orig_len - 1]; strbuf_rtrim(referent); - if (!is_root_ref(referent->buf) && - check_refname_format(referent->buf, 0)) { + is_referent_root = is_root_ref(referent->buf); + if (!is_referent_root && + !starts_with(referent->buf, "refs/") && + !starts_with(referent->buf, "worktrees/")) { + ret = fsck_report_ref(o, report, + FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF, + "points to non-ref target '%s'", referent->buf); + + } + + if (!is_referent_root && check_refname_format(referent->buf, 0)) { ret = fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME, "points to invalid refname '%s'", referent->buf); diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index ee1e5f2864..692b30727a 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -366,6 +366,35 @@ test_expect_success 'textual symref content should be checked (aggregate)' ' test_cmp expect sorted_err ' +test_expect_success 'the target of the textual symref should be checked' ' + test_when_finished "rm -rf repo" && + 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 +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo && From patchwork Sun Nov 10 12:10:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13869799 Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.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 4CD60155300 for ; Sun, 10 Nov 2024 12:10:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240633; cv=none; b=cDMQn5PxGMje7EtRVpSTxstbqBfI6eQTg492UXvogXLDF5tXMvoOPkLAY+KXLJVsxnUxFsNpwSVucI8LogOHlqQIvGsfdZrtLmE6bIMAq9mHVZg8NVbsHmig+fBLW85QIFCeEFSpVja8t0p9bRywI+0HAH5wK9ustvuJKkNdrzQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731240633; c=relaxed/simple; bh=mozBS4WCRR5ACm1EGYA93JDGzTk4WjAiIKg1vdGVcKA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rlFQMOzUB3UpRP/PBPvOypFzCukp+Oo1zRjeWn6G14B5uL0M5FR+TV6fq4tGT9tbeD/a9ZtzvPfMSNF8rvuYegalt1DUPpDAL1m78okRTC6Qoqgs+3WSeMtlHxJUuD70s9A+R4zugythtM0Jx5qWE7bBWmCdPH/JIO9mUs8C6aM= 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=hRS2hqdc; arc=none smtp.client-ip=209.85.210.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="hRS2hqdc" Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-720c286bcd6so3065109b3a.3 for ; Sun, 10 Nov 2024 04:10:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731240631; x=1731845431; 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=lzFoboD1+JUayKVMziqruojj8Znx34nL/4YO57+HUNo=; b=hRS2hqdczqgFlQmjFrO3r7+PL7WutccCY46YLo9oaBCqcivIrbUFi7BF6vHDQYVTyf UU7I2xC3knHxRIoeFxbDS+G+eneeOM4F05ZK7L7ySJ4U9C2dRO/BlC8r9ok4wcHQkihB nVx0ahKMEhEcsU7cSJeR1JeS0wSEt8S8q9KudDu+wygIPxcrP1diBmft/Dbpv4bKpnQ0 ChjpYunFeeDQPJW+lR7yOYhMYQR6mUCf3hpMOsLbvbh8EzXlvfSUaMgVnPaa05gDh86M XHukkzR2fO1zvBObMNI+0lruww8szCA0O3xeNXI1qfsdV/AiUbYYJmO5+I7tGeAD6Ezr PvFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731240631; x=1731845431; 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=lzFoboD1+JUayKVMziqruojj8Znx34nL/4YO57+HUNo=; b=WbEhGUgZ2IJjqRmMCrEZlmWuxrVQCcGGqpdUj1Ub/41EQ+O89eTm8eoapRKf4jE27G OIJdZRJDW1aIfBnPKJb1tl2hE7K67QNiJbfj3/eWM5+XeKSQYerbbPvsYCPxS2S2gPpx 1tmrJ8wiI535L+Z8CgHK+jkV837jvQxp8VCrmCRK/ywd18ChwiTYCkkiV9SuU0WGsqG4 fIbF2e7bHKAChszv2htEbEpEleq9zW+WG3rOCt+jSVkRanujHyZBMZZMjfnIEYNYB6Va Lg4Nm2zojHzD6GzasNC9c39zbTnPLiDt+bWXilBOkM7Gl7Y+h99tfu2nvoV/rdhFDbFt 7o3g== X-Gm-Message-State: AOJu0YxAG0ifexFr6k3xzNo8kw6fmL0nALZZIs0/JWEfoCqyvqYTBOEL KI+DBoksbzPbxM5qi8w0oHpYdg6ApiJup0hk+rEE65nP3VwoIachXHIeWoeVEtE= X-Google-Smtp-Source: AGHT+IHQUEi2AqJ38U+Jbzy3sZUEaECOsTl8cPxLkwzjUG20tVc1dxux0V/4tvl7ypkMoPfVtYN8Qg== X-Received: by 2002:aa7:888c:0:b0:71e:4e2a:38bf with SMTP id d2e1a72fcca58-724133632a1mr12811249b3a.18.1731240631143; Sun, 10 Nov 2024 04:10:31 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72407a58528sm7024571b3a.197.2024.11.10.04.10.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Nov 2024 04:10:30 -0800 (PST) Date: Sun, 10 Nov 2024 20:10:27 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v7 9/9] ref: add symlink ref content check for files backend 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: Besides the textual symref, we also allow symbolic links as the symref. So, we should also provide the consistency check as what we have done for textual symref. And also we consider deprecating writing the symbolic links. We first need to access whether symbolic links still be used. So, add a new fsck message "symlinkRef(INFO)" to tell the user be aware of this information. We have already introduced "files_fsck_symref_target". We should reuse this function to handle the symrefs which use legacy symbolic links. We should not check the trailing garbage for symbolic refs. Add a new parameter "symbolic_link" to disable some checks which should only be executed for textual symrefs. And we need to also generate the "referent" parameter for reusing "files_fsck_symref_target" by the following steps: 1. Use "strbuf_add_real_path" to resolve the symlink and get the absolute path "ref_content" which the symlink ref points to. 2. Generate the absolute path "abs_gitdir" of "gitdir" and combine "ref_content" and "abs_gitdir" to extract the relative path "relative_referent_path". 3. If "ref_content" is outside of "gitdir", we just set "referent" with "ref_content". Instead, we set "referent" with "relative_referent_path". Mentored-by: Patrick Steinhardt Mentored-by: Karthik Nayak Signed-off-by: shejialuo --- Documentation/fsck-msgids.txt | 6 +++++ fsck.h | 1 + refs/files-backend.c | 38 +++++++++++++++++++++++++---- t/t0602-reffiles-fsck.sh | 45 +++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index f82ebc58e8..b14bc44ca4 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -183,6 +183,12 @@ git@vger.kernel.org mailing list if you see this error, as we need to know what tools created such a file. +`symlinkRef`:: + (INFO) A symbolic link is used as a symref. Report to the + git@vger.kernel.org mailing list if you see this error, as we + are assessing the feasibility of dropping the support to drop + creating symbolic links as symrefs. + `symrefTargetIsNotARef`:: (INFO) The target of a symbolic reference points neither to a root reference nor to a reference starting with "refs/". diff --git a/fsck.h b/fsck.h index 53a47612e6..a44c231a5f 100644 --- a/fsck.h +++ b/fsck.h @@ -86,6 +86,7 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(SYMLINK_REF, INFO) \ FUNC(REF_MISSING_NEWLINE, INFO) \ FUNC(SYMREF_TARGET_IS_NOT_A_REF, INFO) \ FUNC(TRAILING_REF_CONTENT, INFO) \ diff --git a/refs/files-backend.c b/refs/files-backend.c index b3ec409920..37c669a30f 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1,6 +1,7 @@ #define USE_THE_REPOSITORY_VARIABLE #include "../git-compat-util.h" +#include "../abspath.h" #include "../config.h" #include "../copy.h" #include "../environment.h" @@ -3511,7 +3512,8 @@ typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, static int files_fsck_symref_target(struct fsck_options *o, struct fsck_ref_report *report, - struct strbuf *referent) + struct strbuf *referent, + unsigned int symbolic_link) { int is_referent_root; char orig_last_byte; @@ -3520,7 +3522,8 @@ static int files_fsck_symref_target(struct fsck_options *o, orig_len = referent->len; orig_last_byte = referent->buf[orig_len - 1]; - strbuf_rtrim(referent); + if (!symbolic_link) + strbuf_rtrim(referent); is_referent_root = is_root_ref(referent->buf); if (!is_referent_root && @@ -3539,6 +3542,9 @@ static int files_fsck_symref_target(struct fsck_options *o, goto out; } + if (symbolic_link) + goto out; + if (referent->len == orig_len || (referent->len < orig_len && orig_last_byte != '\n')) { ret = fsck_report_ref(o, report, @@ -3562,6 +3568,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, struct dir_iterator *iter) { struct strbuf ref_content = STRBUF_INIT; + struct strbuf abs_gitdir = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; struct fsck_ref_report report = { 0 }; const char *trailing = NULL; @@ -3572,8 +3579,30 @@ static int files_fsck_refs_content(struct ref_store *ref_store, report.path = target_name; - if (S_ISLNK(iter->st.st_mode)) + if (S_ISLNK(iter->st.st_mode)) { + const char* relative_referent_path = NULL; + + ret = fsck_report_ref(o, &report, + FSCK_MSG_SYMLINK_REF, + "use deprecated symbolic link for symref"); + + strbuf_add_absolute_path(&abs_gitdir, ref_store->gitdir); + strbuf_normalize_path(&abs_gitdir); + if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1])) + strbuf_addch(&abs_gitdir, '/'); + + strbuf_add_real_path(&ref_content, iter->path.buf); + skip_prefix(ref_content.buf, abs_gitdir.buf, + &relative_referent_path); + + if (relative_referent_path) + strbuf_addstr(&referent, relative_referent_path); + else + strbuf_addbuf(&referent, &ref_content); + + ret |= files_fsck_symref_target(o, &report, &referent, 1); goto cleanup; + } if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { ret = fsck_report_ref(o, &report, @@ -3607,13 +3636,14 @@ static int files_fsck_refs_content(struct ref_store *ref_store, goto cleanup; } } else { - ret = files_fsck_symref_target(o, &report, &referent); + ret = files_fsck_symref_target(o, &report, &referent, 0); goto cleanup; } cleanup: strbuf_release(&ref_content); strbuf_release(&referent); + strbuf_release(&abs_gitdir); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 692b30727a..0d5eda6d22 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -395,6 +395,51 @@ test_expect_success 'the target of the textual symref should be checked' ' done ' +test_expect_success SYMLINKS 'symlink symref content should be checked' ' + test_when_finished "rm -rf repo" && + 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 +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo &&