From patchwork Thu Nov 14 16:53:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875482 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) (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 CD6B95674E for ; Thu, 14 Nov 2024 16:53:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603237; cv=none; b=bs/TSz7fPfSpXtYBj5jj9llHMTe6ep+yCnLJkdE/M3LvMXDOgRoI6CpdOUFvbGLUiJc73SB8juzHxB/kP8xHOSjLL+4lNohmQlKLGgyCIUj7hJvAYLce5qecutDXhHTE0EXTOo8jb4yF/cmkh2rENc8PeZ8K687XRAT7dm7oj+E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603237; c=relaxed/simple; bh=Iq240slf6F8USCuNlFVvJmIT6fjkny4AKF4dW34UQj8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=o+031ehSA+AvJn2yfvHMxLELOlhgMHfWSUEYhYTthzUw8l1LcJ1s4B9WfJF8f4abuIIM9EzsUPTK2hyBfjeJnOIxgVP1/jK94YwD3h3ohOweSSKYrbebmMB5836mpi7Q18IXKvK65/PIaJo0YA3mT6SCo/GWhzTxczVIxX6UptY= 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=TvciWNdz; arc=none smtp.client-ip=209.85.214.171 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="TvciWNdz" Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-2114214c63eso8056915ad.3 for ; Thu, 14 Nov 2024 08:53:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603235; x=1732208035; 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=TvciWNdzMI93G9NEp7K850X42wKpXLnC/UoNxP/s4LP4a1wsfvzqY/rejJ5t/PqO0M yIL0NtPIzk7wOW59gW9pSJTibW5ely3IH5lLh7fdQM8sb9uRWeP1ubt09NyPexW5WuTq cMiUCV4yEKK3H4PmWUg68Vp/6w5xwboZ3F0STqaZF5tyY3uAZnrTdizeOEgpdfH9KN93 bawQyepn6btyyyWLYEQjr8lGRBhngsOT2o38QusaylDgnLT8kk8jkTFCvgu83JtD2SZn j5JEG874i5incUEDBA6jPFYLng2nDFnlrfhXH9EZn2TU37Nl+0+QPlT6OGF+CTo/s1zZ GQ0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603235; x=1732208035; 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=j2HicQEzYCAG7P8lHG/rRhnJBzhB7vRLLTP7Ol6mhLiHnJm6x3mPMPMw+tDQyhFVBi jYcuy5HViSA2uFn7mBoPncngvksPEV/ltMqTtznIh+Wc19OX9N4VvWo39fsuzd3IsE31 NoB/ygXLm3lpqEyuAHaUidt7yUBerLuSrLgIDmGoEaCnmfy8ezoJ6of54DxBj30ekNOK Ycrr2G2eXdAJPvfrwwyOcxSkjg485eJLLngBEyakwUARNmV430Bn0BdraYqumqxtNK8I hdjtY+MWyQTjQdhLVn5fXrKNyfAbWWQ3PwyFLVEnwwxutTpGvOukTMsbfdtvHGvAD7s7 CLeQ== X-Gm-Message-State: AOJu0Yyq41Cld8LUmiYuJNyay85IgjMYEG31UPPsqwNHQwwuHKHfCTtW sXwJowHlEvk5VEhIsgMIXzehoZK0pSqd0nxNjtcb/9W0C94hB9oD8cFq6Q== X-Google-Smtp-Source: AGHT+IEwUfqeaAh78+RrcTqjauTvkOhEPLpr+T91XDeKBO8t3SQ5f8haQB4fWdm1yculQYfBUwoscw== X-Received: by 2002:a17:902:fc85:b0:203:a0ea:63c5 with SMTP id d9443c01a7336-211833296ffmr352317945ad.0.1731603234611; Thu, 14 Nov 2024 08:53:54 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211c7c28ee3sm12975555ad.47.2024.11.14.08.53.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:53:54 -0800 (PST) Date: Fri, 15 Nov 2024 00:53:57 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 Thu Nov 14 16:54:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875483 Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com [209.85.210.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9CAF5183088 for ; Thu, 14 Nov 2024 16:54:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.170 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603246; cv=none; b=dZu0sfbNVaLb1hgznAN9Iux10LmcYXMSL0Nc5q6e3iXikdbcvXgNiPCQsMc0skUAv9USxueGHN5EeCjuqJKjQbz9uaWfgHWxIU00sX1yKuLn0I3jZaPZYjesiu+j1iWs3jwgYFrula9+2gPTtVtkgbRGrhSZmmWURZidR6qyTCg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603246; 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=gH00RNHAa/l2h/yhGoXZsQKgG51AXqK8b4Ug6lFZ9LH9HeUvQ5ua28l59nRfmbddwfkPx9DKeSSsGB2YQOOQurj11ctgzTlLiTJ43m8MeXMBbd1XZ2E8VIee5zzWBE+ETy2286I+1/M3pxis5/TLQLoXlV0A9g6bh+sUsFuNe/4= 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=MyOLXA+B; arc=none smtp.client-ip=209.85.210.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MyOLXA+B" Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-723db2798caso798987b3a.0 for ; Thu, 14 Nov 2024 08:54:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603242; x=1732208042; 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=MyOLXA+BPk/DGbiH6hkmz3WvQy+DtaKBaod36nwPcZCwzuPNht18NZDzvy2I43vDv3 wPYPOaqpax6znxPjmHkqKKrtB54Icku8S+XOjmwGGPWFJn247wx1NODJy4cEWYi7ma0N zwuU6/qRcOaE1U6rtr4B3aSu30zaIrQgXTvz5H6PVnh7VsPqsKzG51Gy4f8+Ce+uRc7P Eikh5Fh4EsoFM6PaspAxkZIPy0Ygzz+oUKppqxbMH2uUMU+mB/fgKdzDOI41K+2BrjSR qlrbyB3jdWAbkbciMxBAz+GWtqf5ISL4boBHL/SPzppzNJU9PXKe549DhkV+804q888F ZA5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603242; x=1732208042; 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=eANHOBwWjIOP2WgB06yj+7xZU0sNAbdvkAXacmp54Pe9PgwMTmq+RZdhz3yvr3upUA x6eBLNESTWzVGGRSZXNaydLNOWZC/Qz2uXi7VoQoV+5axXIMb2CvEYVkUp1Z6L4PiCxp teHGSOgjvLGZPzKtQe1TXfMWxxH39I3GJNNvVKoTdlsVp0gMBg7MoR2nzV80RxlPTbBT wgmmX9WGzH7PgmWWkt7MsSLMXkXPUZRaUbINrgx88lC5hSBYHu+4zGByzbQ+d5UcfoR3 zCxSQYrJhf82dkmlE+R8DLignzDcqi/uc8oKjjufiZTwjIWGFAqZv+okd8zo9LXuBfPi jMFA== X-Gm-Message-State: AOJu0YwWvcv+80wyDkRYYGNzJbpt/m94HIJSYmagoVUnxI7aUXLzOCi2 vy3WPc90P4etVnsWKmPZ008hIJ6QPU+dzGQg5npG6xSEWOZ/he/sxrUsdA== X-Google-Smtp-Source: AGHT+IG2pHMyzriRYKF6KOQXfwVjjbZvU5v22bCRl0gSZ9OQfUgPCUsR+ZsuPhXI49i3Ypg7JY1pUg== X-Received: by 2002:a05:6a00:99c:b0:71d:f510:b791 with SMTP id d2e1a72fcca58-7244a5423f2mr12954884b3a.12.1731603242162; Thu, 14 Nov 2024 08:54:02 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7246a5cfa1bsm1484140b3a.8.2024.11.14.08.54.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:01 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:04 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 Thu Nov 14 16:54:12 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875484 Received: from mail-pg1-f173.google.com (mail-pg1-f173.google.com [209.85.215.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 88A835674E for ; Thu, 14 Nov 2024 16:54:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.173 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603254; cv=none; b=onF0vzI5rc3iIEnQsrtZ80oWqXPIA5kHHTNMbnBfyQNBiA4ZomV/eCcd8QgWG9Eoj+U0ZVf71herObbhrONgg0g6gl5AGvS40CavFd0CKK1ekZJEUSzUQAIId3pqLpboWEG9LUSxSXpeaJc2UfWQd/Y389MXGdsQ2g4bWbWhK3o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603254; c=relaxed/simple; bh=5V0XiAbP8HWSYG2I6t6wyCWdiAvyF9V1n8no7T7g7Rw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bmzdDP99MEww/s6YyNyEl0Cwrd9/lqPwSN5HEQG2JZbRmkVcVc75BWDX+DFg1skEqD51YHVdrU8EtuaqgM2eWIvZJfWxF/NkVKtNWCWu480Cl2Ec6ywYL08NIbkqdFjh4PwpDxtLJ9bUfrPNfcCWBQJIRYqnKEz7ptXtUBL3OWE= 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=M2Rvk3rC; arc=none smtp.client-ip=209.85.215.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="M2Rvk3rC" Received: by mail-pg1-f173.google.com with SMTP id 41be03b00d2f7-7ed9c16f687so664047a12.0 for ; Thu, 14 Nov 2024 08:54:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603251; x=1732208051; 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=M2Rvk3rCnf01xgUkRcA3/nBhMkHfbe+CCatXTF8Nz+QL1WLuEXN1wk16Ca4UduwHKp dddq0o/TCZoN+dnKYN2kKD0eshVsuO4fzF4F1jGbNyOKzLiRfVES1TYu2/wsrar9TpzC EbHKcO+B0YcG3ymx2vCNORlQZJeB2N4b85OCRT8nAhsBLH61Lkdn/Cn08g/spYQkK2wA iZFyEJACFu/vxGuFu7HhMvbvK5U5Vl/STs5svfhwxspJuFNtQ9b6RbpNq3Jasvjpu6+L nX3X4vzXKDJDfWXQU8k1tJixjc4Cvq7PxsN5Lbq4XQbsBxj79MNSW1IQYEDzaeZSMWI4 SRdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603251; x=1732208051; 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=fZLhmDBXorhC2LbSfG6nFczwGuAAx/PUQdcz9gqUAZQD59iyau3QrctcAPWMfhRgUQ cgKMKZ4rwxB4rTA02/jJa/HG+cNIBuYvYHzHUCLTOHRrvTU+wyEYsQxjCkedy3qOmck5 jpJmOn9A76wAg5w1nwvwTWFLI+pOo0Axjrh4MK+e+jTZnNkdRhBtF6RWpJdDONqBvajv aCZJZhQBCTVjDSH18PwLG84RY77T8mIThrlId4L6PYYjHIis4ahWUnnx1elVeoB19rU3 NcOGrnPlo0+vcX/OBsAG78A5GUf5s7eruUiKMpHJtxIP5Sa2BwnxNGbHd0xVBGmYjkDm liUA== X-Gm-Message-State: AOJu0YwQWIry5t+Sk5A+yEQgMsg5gNGVIUlpm8udJNrzEzwp57J4ZfmA ieEk0w+FJH6LwIblk/twqBt8xz6gv+RQYTuCmXIMfi1ds7mBgV8IdrJTkg== X-Google-Smtp-Source: AGHT+IGuA+Mkrsy4Ejm5+WH6GiE/wBTqBQcsVLJ9y/vXAW3Znsa4YJPU5/KM56ZRHvwG/2zghL57GQ== X-Received: by 2002:a17:90b:3e8c:b0:2e2:e4d3:3401 with SMTP id 98e67ed59e1d1-2e9f2c906cbmr9253109a91.20.1731603249789; Thu, 14 Nov 2024 08:54:09 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2ea024949a4sm1508486a91.11.2024.11.14.08.54.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:09 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:12 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 Thu Nov 14 16:54:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875485 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 20A1B186284 for ; Thu, 14 Nov 2024 16:54:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.182 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603260; cv=none; b=ElGx4rQJWiZznVJiNQwC7+I1BtnWyMcxpM37M50yEvJgHrdtn+iCZHMCQgzOQIRMouDS7ltIDtvxeBnEcwl92CnxHjtpDSSealGnUH0luQ0H3bBuxLUz+yHRXPlJrOWe7EnKE+FbjYxIZxPbH1ZJtOR16mX4sH46W2kdbzn/k68= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603260; 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=P0eE0Q5yMc0a+qfzuRcGJMOI/meSg2krdsklQtdZrnmyG0M7HOuwMkT10jIKItJlO0kmJYAtq/W35FbHIoJILB/MYiYjspPsHYrlqzG5NPz3b4nsuvDr7GXVfYUBs4HiaoU3ZIWm7yDlAnil83bmXNzShC2vFM49gcYdf8lusXw= 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=TkH+7RQa; arc=none smtp.client-ip=209.85.214.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="TkH+7RQa" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-2110a622d76so7812195ad.3 for ; Thu, 14 Nov 2024 08:54:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603258; x=1732208058; 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=TkH+7RQaYEX9xw7Yjw3RwyZ1IqlG7RwTIwJEJkiHcjtlg6dtl7wHHbR2Zv5t9wIS0s i55JIOaLUIHKYibVeBvHZd9HJf3DG7hGF8YKR4EXG8MPUkMao0lnIiv68jfIsS9/19dF Dpj5OeFH2go9XqGRsj4vc/u3YmVu/CkJh0eYa7/OIaxNRezqH4VSigBUyX1VcLzBaTUa 4emWaycCmLX7Q6EF4F6Ox/mrqMRSjukN317teoua2LS8vNfOG43fVfdAE5hr3YDsUBj5 v85EnJHPXNyXVpIpHVqeqsoigSlzKgVjAbKqGVdW3UrB5VeLZSSeSTlYCNN8MILMCzgf 5teQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603258; x=1732208058; 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=r7Mq15xHDW7fWOzi44zQ4GUV/NZA3wAl9REge/ikTscWCez1pWDVKpjkjw+W0a1SmG 34q/eiKCDLTcuSGd92IrlLPEqwRA8SS8NU5yygZAeqI0gWC7Us3QyZ3vBvzGIgvmCBup dtCENUD2OdM51bXSrAQxP6nyuB9cr9JaRIQWEF7AP49iq+DSaRgeToV3+83RPnwhsyUD PqNqlX078OJ1Dhs9QhRgG8uH5qLTlFOYyRj6X6scD8Q84l2/aq/GJT291LOmw778oU6n zYwJZAhkJfRKBRZKFIFHQVcGt+oWVHe183sAFBMDRiQae3L9dI27ZGC/N51s21OxGOQe YhzA== X-Gm-Message-State: AOJu0YwAFS/JDnTyldBjTuor8yD5zEw/osp7QH6CzanO5WmxnqVG88W5 vMmPDwuKJ0J+z3cn7bWK/bSCIgPYs1eOElBKidqEBgG2pFQCxiTrrXaC4w== X-Google-Smtp-Source: AGHT+IFdskRxKXwtwf2TFD9A8SaWHkQi7yeXutZTonS76UnzqO+YpyK2MnZ6hfS47v7qczQYKnVpKQ== X-Received: by 2002:a17:902:e549:b0:20f:c225:f28c with SMTP id d9443c01a7336-21183da3eeemr353772235ad.52.1731603257684; Thu, 14 Nov 2024 08:54:17 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211c7ce9461sm12888635ad.117.2024.11.14.08.54.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:17 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:19 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 Thu Nov 14 16:54:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875486 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) (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 C906918A6DD for ; Thu, 14 Nov 2024 16:54:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603269; cv=none; b=HsKPZnAo9LEatRp0IT2CyFru5AleGs1IF/f/ecnx8GzppWK+5y4DLtuSuvYdvUl4FAHJ97eX0McBxreawKznKx4ZS9iNMXKNgkDyrHHBk5EWbh9wlq55MrXd8Zdcdrl03BfZpJD0JMYUO5hsuoXksqUqS1wbt/ftQozyhfrZtco= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603269; c=relaxed/simple; bh=Nhit5ew+y8vnV0i5riHPMA9e1Teeug4OKRIW/Bbny+M=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Fie9OJb+V79ba7h59ccpZV3VZEzb/tGK3Xb9wERh9LdcZPwy5GfLvv72M05mjwL4FIHrqV7zgnF/iDb/171sRkjVktrh2OGvoO4NIXeWhrgT/pCE8oX0R7HdCNoi1jF0GEKysT5QkmGbE8nCgjuA2R3A2jwRe+/dFK0usUUb8H4= 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=Jh3kEC4n; arc=none smtp.client-ip=209.85.216.41 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="Jh3kEC4n" Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-2e2ed2230d8so758782a91.0 for ; Thu, 14 Nov 2024 08:54:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603267; x=1732208067; 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=KV9iu4x0mbx3aXLm0O2C465d7V2HVq19faMLX5ow8hc=; b=Jh3kEC4nBUzKXKENbWHH1R5lMNTeMv2WkAqqAOWqNECq7caovivTNKx9cHBF0xDYVS FzvN/syXAFn0a4V1QLZqNPP7STxi/CTO8P8OXNZvO6HWtuyCiAfXT6pdGZ3kCUeUQqYJ 2lVLG/Da3xulvsGGR/kfy5hD4KZEEARKdOCY7S/gXT5mKv/Pyb/InttoOYYoA8600ior u9yCPJgyLdsYLTVdW5u/vc0xFw9L1UKmMAT711oPzV3ubdHOQivDVFyS+cZSQfpXH69z 0ogtI6tgg/OMBavcGRVYny637XzYGplhLPGWwqJ35h+t+enpw5UgSN/EB1CevvoCZAhc x5nA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603267; x=1732208067; 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=KV9iu4x0mbx3aXLm0O2C465d7V2HVq19faMLX5ow8hc=; b=VIYHk/BEIzPrlo3PvLSRQjJ3rjfiwilE+S4ZeR13iUW0hx2CTF49NxU3T9324p6vBg klt+vE/SB5j/jGxRSi3Jwi4csAoggh5ApGaXv2HzhBLaebMRO9wJT4zi8Gk2vk96L8nE QsR2+WUxUwgVTqWnqjV3wu2SXPSLI2shjtVpiuHPaZJsC7adAiwMGJviSu+qQeDLtW9L CgK2QRiqVRJ8OdPDmjuYGjmr6VHtTB4sYj9wlMfF931zw0jpumkLHG6F/5SdWxUYJ0xP W/Y/hnOzZ0m2z1APPDUvXd+4uRU8IBNPByK0o0+lUfJcvHYfd3+lqXf8yCfrra9Rrf+6 iRQA== X-Gm-Message-State: AOJu0YzXXKe0c9YOvUdSQFHD+21hPiS+UyuV2KwRF1VdBk9dHAHSos+Q 3fu/uN4emlvsYGhvWY1/GhdyVU9nKzrPQuSPUQYSrHTSMCCSHSsrW1BUTg== X-Google-Smtp-Source: AGHT+IGyN88YYMUDqlCE0ayjmDIV9elj5GLKNHAOqx0T0n3wfc3EVUTL8GhEfa/3oWzRHYKxBO87dQ== X-Received: by 2002:a17:90b:4d11:b0:2da:88b3:d001 with SMTP id 98e67ed59e1d1-2ea06388504mr3019899a91.18.1731603266699; Thu, 14 Nov 2024 08:54:26 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2ea06f9c513sm1420224a91.35.2024.11.14.08.54.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:26 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:28 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 | 48 ++++++++++++++++ t/t0602-reffiles-fsck.sh | 105 ++++++++++++++++++++++++++++++++++ 4 files changed, 157 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..f81b4c8dd5 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3505,6 +3505,53 @@ 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 ) { + /* + * Ref file could be removed by another concurrent process. We should + * ignore this error and continue to the next ref. + */ + if (errno == ENOENT) + goto cleanup; + + ret = error_errno(_("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 +3647,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 Thu Nov 14 16:54:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875487 Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 75DD640849 for ; Thu, 14 Nov 2024 16:54:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603278; cv=none; b=FSspuG+fj6JhiEhX88ZI1L+zpcUTdG8LEpujsYazx0b8AdcUXwUOtHL0pJZlnPviwKsOPRLzBYPQXwaWjmpEMOnG5yQCEePr1+UaTyNAcUSGE8z8fPDQgVs7HkZsDstMm2j9/2MFUIfv+BcNmbnwncikaaAGLEG8cp+BMOy7JwA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603278; c=relaxed/simple; bh=6R0cwGmOtyKQjkBRiEBxHCOUVeS+LLzfbHBEKrSx+B0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=a8aqVDa7ufBGBW16QylEVxMft5PgBtV9X4O3gZpqNfap4UMvmMuFl6FvE8bPFp4olU/FPl0cM54PRITBLLqYOsHl1t/Tb/cSY1lq4imqggZGntNqx42oksfCzZ4bLEuz5pKNrS4JxkL9Cj7e2UhCkgir+QG/QcFe4peWUye7JoE= 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=RGuLGiLd; arc=none smtp.client-ip=209.85.210.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RGuLGiLd" Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-71e5a62031aso667732b3a.1 for ; Thu, 14 Nov 2024 08:54:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603275; x=1732208075; 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=ngmKOS4mug1ZxW0Vwx+eY85VxZEonYqgfpKPy7uyMcE=; b=RGuLGiLdsd6HdZjrS9XCCuriXnttnPh3cKnhYyAC74BH7QFM2816DXYIEPeqN9KvnP lc8r87q1PfhPMZeqq+PHOMwt7U8gF/znJqcQvM/8yhgQMt7Qr6bO6iYYW9lUtEyvP5eG v8rg6o9ky03jmKr58z/1d5PLFxIoNygAwwWgOqE7utm2XTe0MDDSFCBW87HdM4vojQGh 4nDbISBKDwssHD9KQztd82d/h9dYoPLhyYVpQHjba5UbZ0c7DmM5j/867HErWusDmXZU /be/2/b8krjTBxuKMEs4Map1SMdCO3C/f8mghEmPC7YAIm/DvZcRYAkLsuwHxCuc2jDA Hc6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603275; x=1732208075; 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=ngmKOS4mug1ZxW0Vwx+eY85VxZEonYqgfpKPy7uyMcE=; b=Jz0pNVPL5m1TB+P9d5Z/5DXH3aF5z6BlUiwQCu403Lbi+td2aBIAo3h6hQ+aZKtMeu FZCQHdNavwpwRHAtOKY/njJA0BUshQDaQKo7ravsTdDKR85oY5XvzVDzPb1bQlM+oomK ePi9xrCmBNGAzWRTWamwy/TsXhC3vIKDEjk+2qv4qfC8RSn5dW6CtfsjebWNVP/gCMg2 Zoh2hhvpsQpPhWAj/Hh9eDdd3qoi+QYREvS6S+bzBpKLjUZmFogD4fW8Dfb2bxDQjv40 tn81dZkL0oUw3M+wNQxEQ2dGdiJ8xGae69zkSdz9JRqTPsO6u+E9r3Gak4vgPct8fHvi UdUg== X-Gm-Message-State: AOJu0YyU55K3gm+KgAhuH/WzyaFtM06paD+Vf4AXAUhGaVZRFWIiXord pzeE5bqisiv6j0V326Zn5e/OAb/uFsHs1CHRy9JfFxtqKckCtuaLkMNeAQ== X-Google-Smtp-Source: AGHT+IHK7zUCdffkTyDDxAMB8fLdZSos5iqc9mT1xZfku1lZiZnv0033JmP2Lne6d5YwxtTsCEdUFg== X-Received: by 2002:a05:6a00:10d1:b0:718:ea3c:35c3 with SMTP id d2e1a72fcca58-72469d1f5b9mr3183231b3a.15.1731603274623; Thu, 14 Nov 2024 08:54:34 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7246a6e6fe8sm1522983b3a.62.2024.11.14.08.54.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:34 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:36 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 f81b4c8dd5..a325b102b8 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; @@ -3538,7 +3543,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, @@ -3546,6 +3551,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 Thu Nov 14 16:54:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875488 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 8A5A818C014 for ; Thu, 14 Nov 2024 16:54:43 +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=1731603285; cv=none; b=dakS+ChnxpLvw/RZdx8mBi+tCrHgv3cKTBdEcE2xiiD7KQxZ9NvGURgstJi94yr3XGFfgfIbvUcFTCpNFXBUvI2gRAbegOPB9dyrkAlO0lGHxPLuyJSBlw/WVGXZ8bgVVrvvos8MlJnMrN9EzbermTDQ3glcaUGj7CdQ3W6nga0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603285; c=relaxed/simple; bh=KP7zLmvL4WCb0FwFHXnLksrHy46xrQU50fd8UpICRKE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Vy917vLEJ/4ldtXkrMEvKomoG+17kmMx28LuozKuPeWC45q3aMecI7UvO8eWsiscHP8Vi4pxplten2kYS5WCPmjeUJNFmMPSs8jOxjBOYH7bKgOb/YvcDsQVJfIK6MFwpqskaEdLf2V2/0InPLKycV/IrtYP9lWHaU9OQeI15ME= 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=KNqE/owp; 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="KNqE/owp" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-211a4682fcaso6642015ad.2 for ; Thu, 14 Nov 2024 08:54:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603282; x=1732208082; 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=HEzocaeG9RkX415G8X6vWBCudzN4Zft684u9n/co4Gc=; b=KNqE/owplC5EZRzCeLJw4YB9Q6VPl2NPOk4b1s3KvJjpOUvNb41Dfi+Xu10sjCtlYs IuX9oNioZ3lGS+1A4f8KGVyga51vZqa4LrZv62Yj2g5PQ9s4OpK/EzWsipx8QnPmIF8X Kiw6s7visJ0Bam/roauJ5tuo2RcX/ylFMAsXN/VGN5Q1mpnU5d3CnnvmvqKjjJeAZbrl qixbBMRosS6g0ay4QZ/lgd+EdDjzOqSbBOvgkXjnaQGltxSabq1RpvN0oOH+6laTCbUb gqEj2dbnwlT2yVMdd0u5OVl4fnmGVEHGGknQ73INHq2hv6jFjFCghO35X0EJuQ3btyio mlIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603282; x=1732208082; 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=HEzocaeG9RkX415G8X6vWBCudzN4Zft684u9n/co4Gc=; b=XWYPK1aFA0pyPrJ8CcTwlKl0Uk1yP1Iti8541tJ/+wtJ7yu5blAEvKLOZbDKNo/wSX uBytYW4e0LsF1W2Y3FOPXwnADEFEeK96nBWOOzOLUokwnrhXHHeuTPpfi3IBRLxaSTtk /JPzMbOwN3WEx9uzd+xYDCn5S3sIKVbJ5xHOhMIwpJc10XesZgyydVYKHBTWcoA/XDIV LGQyUpZXgdH2fipIo5Rn/hufjV/cth/lckStusyhzaGpNIDU+5WW4qaCDAHqva8a2Pr1 nCi1fzLXgFx/qL9SRZgODvft45936Zb0G85uMlm5pmQcsglojOmvqM6U0jEMnoofb6BF N/Vg== X-Gm-Message-State: AOJu0YyJLeXDBgvCno11IZq5BQEqY0JlchhHr+WHOwQzVJ5gswusdPAI ghIWrOMSmfIOUQFptNXdxvQQDrf1CYt6+Mnbh1P9m5lOgtl0OJgCo+Y0Sg== X-Google-Smtp-Source: AGHT+IEH4fZEnEBC+ft1pnnyog+1G0NMXJPojgjjBqgQ7kN4YddATs9bH0sDu6BCo0s+5dfO8jqtwA== X-Received: by 2002:a17:903:41c4:b0:20b:b39d:9735 with SMTP id d9443c01a7336-211835cf279mr325678255ad.54.1731603282008; Thu, 14 Nov 2024 08:54:42 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211c7c52711sm12862825ad.103.2024.11.14.08.54.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:41 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:44 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 a325b102b8..c496006db1 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, @@ -3564,6 +3601,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 Thu Nov 14 16:54:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875489 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) (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 B10EF2AE77 for ; Thu, 14 Nov 2024 16:54:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.48 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603294; cv=none; b=TfhPP82CC+KJBqydq7gVL6oG9JNQRRU4wJGiI/0Lp6fDAoktlVbzTfEqm6gLC9Pwur+g9yZXqHXP/po7faSVZBYEl9OOwAAnCWnTrwX1j7hUEFOFyFwRv55Hx8BzjRUvNswmvgdEpu87d5A8KeoQqNdz3aSSYLClG2zs/q84ZRg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603294; c=relaxed/simple; bh=JLq2Y3OBs0WZNcgMwgxzr29NojqSbZpGEf011cXBD7U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fnkL0uk/o9Vs/kXZ5LIrIJrGdTyJX3+N0JgB/KxvIFA73Zup2OOI/w8SG0sn8FEuFQZdtE1FkUM3yoB2mnqZpfx5kVi0zMEmFpHzHmwqTFRfJ8tggYCv7z0YNjflWREvitnqEWLo9/rZTjLNiTMY8/0Xc4gjSNO4fouYD/u5wzk= 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=Byh/rJ+G; arc=none smtp.client-ip=209.85.216.48 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="Byh/rJ+G" Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-2e2fb304e7dso776824a91.1 for ; Thu, 14 Nov 2024 08:54:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603292; x=1732208092; 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=QaujNNIFynb0YO+A+DH6JkVyJyGzo0pXrn+Uc6d5Yqs=; b=Byh/rJ+GRG8hmMA1319FpaxMXFcrfMPQMsjnKItV/vxVx5OW3aV6hma39uBhXrukUn sucmWXUCp8ELGp0BzeSfoU9pQD7DTU4zXGdoUpByK+r0OmRVJ0evPcJmDOqBCoN0ilRq WaZZkolYss5TbUSusHgZXsW3md+iiGPJRz5GWIbLJpdh6tOGrKOEt0gAEVsMtMKFI9Ub TsKrV8Mncs9IvKrVc5KzXrS2wqHYbNg3XEJXZNyCaBf+1d9Js/TL3KReJqK0zpvXjBRn 6obFMNhTXzjRkrS+N2EhSmdggSEFmdA5ZmfD/xutvSvPLsxR5z25f8IxicYZ1F1Vc6KJ RG/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603292; x=1732208092; 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=QaujNNIFynb0YO+A+DH6JkVyJyGzo0pXrn+Uc6d5Yqs=; b=sGfITjQf4fUb/qCxAx/aI3SNz4JMpcIYHhcoD+iwSxaqUPgARcbvvR2TtMmSacpXTP nGtOlQmB6DbDGzFsneu1Fhj5raOv9zkkvr4fHyxBA8lgEOgW/gZoNyCcqeHwKxkxv3sP bFDzmZSE0P54YCh++xdvk1qY6XTC9QzAwohA87zjTH/W/ZzahAniX9ui71SaeqgUjujY tQ5RKE/r/At0OLXQmVZHzsXhYSM1zhV0qgvH3ygDR6Qn/LBVk9Eo5VByXV5lzqrcect8 OyT2EHBa9PeExkDL8xLx9iZJWxWXxakgceOskoT6rD7XAnhkZeUnWeOy3kCHbnzx2yMJ 3tMA== X-Gm-Message-State: AOJu0YxNHuxxjrXCCZkZkoRZHtaI9+HZPYtj2CRAAfsxBLO0Fgw0G1L5 Xfwtzqir1iuuhJZZAQVtcG9LuGs+qU2tlMo/v7MEBpGEJqVHjsfyH2cURQ== X-Google-Smtp-Source: AGHT+IEY5TE1QYIPUmpLEJa6u4h9t9j/9ozf4ktDpIIy/C54sxJe3TEMHgtET969Bg9tei+clw+6bQ== X-Received: by 2002:a17:90b:1c8d:b0:2e2:cd65:de55 with SMTP id 98e67ed59e1d1-2e9b17413b9mr32407116a91.20.1731603291545; Thu, 14 Nov 2024 08:54:51 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2e9f1e24ce4sm3388934a91.0.2024.11.14.08.54.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:54:51 -0800 (PST) Date: Fri, 15 Nov 2024 00:54:53 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 c496006db1..edf73d6cce 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 Thu Nov 14 16:55:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: shejialuo X-Patchwork-Id: 13875490 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D628818D655 for ; Thu, 14 Nov 2024 16:55:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603307; cv=none; b=ulxgInbdhhiOBM5A5BFNzVdEnPenc5ecSzMm2ACD+eKLMMeO25na64axr25ObKP4xDhRhoBVJpHzJEnVQ3tjFlGw1Ze9CJ30h4SXe11qEDIly1INwS0LL9UBXDJ7x9xU/aargnPMm9sbGbfyPWT7p+kN5GB0tJmglJ/5OZJUfDk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731603307; c=relaxed/simple; bh=13D5jG/o+uPsRNzxO3dtap6QCVThXdWrc+LaXYdOjGc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JcSzDrfkkxn4SmfkwidiT5E9KksnM//pmHyaDKXV56SHKn049ksN38Ij8FL0MjvkskSXl1pTM7vAFJkwiPVDt3NG5Grvu6fkgrn+lrxAdM127OrrRN3biWyHBdSaINukjQF7pEnR0iS53WexR2CndLH/XxrpXMCZI5P3TlMjXSM= 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=UOb2SOAC; arc=none smtp.client-ip=209.85.214.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="UOb2SOAC" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-2113da91b53so6884575ad.3 for ; Thu, 14 Nov 2024 08:55:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1731603302; x=1732208102; 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=NkLeIpLTWn3acvMtN0I/VIzfCtB9KqRuRIm4NEBIh38=; b=UOb2SOACUIylJBUaT7sBAQqR82kylBpb8ykncw+wPGmpcwawNbfDJ6IgevNzOTzqpp EzGKqcNhrUvfIEeDLONYJNEZsbsnx8r10k6ctN0OKIwav0bgEarQPekzapUx29Pg9tbp lxjPfNdexDSTQm0Vg+qD9N8dTbJQgvH5yOdPnqG5ZdZt2PZGLUgHIDDUkzdgibgbNE0N WUC8uAGIqTCrenmnqejQ5lNGWNZ5wsGqh6/jtTvchmtfY5VIWCENg9qg91O1WadUDD9a Gyw2RB+h0yW0jw1y2wiGJhBGY5YwtGIG4tM/ay0wpNHivdIaIHLwz1ekEu6SW68u8dSl S0QQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731603302; x=1732208102; 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=NkLeIpLTWn3acvMtN0I/VIzfCtB9KqRuRIm4NEBIh38=; b=EZfhVXtEAWIDlKR8U5Rcd1nE9qZ0URkIBOcFPTKhZOuCKOLcWYpNHWyWMheDjPnccS ug0EAguOI9J+GAj81OIMalMKRnckBBOXD0nyCMw1GcqFBdJFZvWduEHUfEfZOlTTLDNP 4DHu81D3xx1fdc4LDJrjcSmN6T2s/Xc2SlSDSZafbEDiuO3N/bR5IBy/BankHPmmG6UC qmS1sH+P6piBbzoDtsqKAyvFx1X7XJ/FGFupLGaDTEjEEJTRBF7XeY5oPouIY25af+Gl q3CAXjeBYdofVgF9TQPxZDkrP/66Onwkpt2OiKMNLR40d0J6kl14x7vNClOeO9cz4k0l 6zdA== X-Gm-Message-State: AOJu0YwDuw3rA7VPmseyfKMwSL6t/SBp6vjXpeIqounJjuO58kugWe5I PQJenNNWTlu2egyUfIYOYz1aHFiXbMSMewenhvIDqYbQu4i7KF6+N6zv0w== X-Google-Smtp-Source: AGHT+IE7zENokNuvAkJjENpCtN20xxJSnHIFEc6neiIqXLIxUHwGoxcAD4PvhHgWS4H7RgZjJXiOTQ== X-Received: by 2002:a17:902:cf10:b0:211:9316:da12 with SMTP id d9443c01a7336-2119316da85mr296198975ad.22.1731603301903; Thu, 14 Nov 2024 08:55:01 -0800 (PST) Received: from localhost ([2605:52c0:1:4cf:6c5a:92ff:fe25:ceff]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-211c7d2a001sm12951195ad.249.2024.11.14.08.55.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Nov 2024 08:55:01 -0800 (PST) Date: Fri, 15 Nov 2024 00:55:04 +0800 From: shejialuo To: git@vger.kernel.org Cc: Patrick Steinhardt , Karthik Nayak , Junio C Hamano Subject: [PATCH v8 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 | 141 ++++++++++++++++++++++++++++++++++ 4 files changed, 182 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 edf73d6cce..c715e411f3 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->repo->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 ) { /* @@ -3612,13 +3641,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..f8f27cfc6c 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -395,6 +395,147 @@ 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 SYMLINKS 'symlink symref content should be checked (worktree)' ' + test_when_finished "rm -rf repo" && + git init repo && + cd repo && + test_commit default && + git branch branch-1 && + git branch branch-2 && + git branch branch-3 && + git worktree add ./worktree-1 branch-2 && + git worktree add ./worktree-2 branch-3 && + main_worktree_refdir_prefix=.git/refs/heads && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-1 + ) && + + ln -sf ../../../../refs/heads/good-branch $worktree1_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $worktree1_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../../../worktrees/worktree-1/good-branch $worktree2_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $worktree2_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../worktrees/worktree-2/good-branch $main_worktree_refdir_prefix/branch-symbolic-good && + git refs verify 2>err && + cat >expect <<-EOF && + warning: refs/heads/branch-symbolic-good: symlinkRef: use deprecated symbolic link for symref + EOF + rm $main_worktree_refdir_prefix/branch-symbolic-good && + test_cmp expect err && + + ln -sf ../../../../logs/branch-escape $worktree1_refdir_prefix/branch-symbolic && + git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symlinkRef: use deprecated symbolic link for symref + warning: worktrees/worktree-1/refs/worktree/branch-symbolic: symrefTargetIsNotARef: points to non-ref target '\''logs/branch-escape'\'' + EOF + rm $worktree1_refdir_prefix/branch-symbolic && + test_cmp expect err && + + for bad_referent_name in ".tag" "branch " + do + ln -sf ./"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-1/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree1_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-1/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-1/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree1_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ./"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''worktrees/worktree-2/refs/worktree/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err && + + ln -sf ../../../../refs/heads/"$bad_referent_name" $worktree2_refdir_prefix/bad-symbolic && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + warning: worktrees/worktree-2/refs/worktree/bad-symbolic: symlinkRef: use deprecated symbolic link for symref + error: worktrees/worktree-2/refs/worktree/bad-symbolic: badReferentName: points to invalid refname '\''refs/heads/$bad_referent_name'\'' + EOF + rm $worktree2_refdir_prefix/bad-symbolic && + test_cmp expect err || return 1 + done +' + test_expect_success 'ref content checks should work with worktrees' ' test_when_finished "rm -rf repo" && git init repo &&