From patchwork Thu Apr 4 05:48:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13617302 Received: from fout4-smtp.messagingengine.com (fout4-smtp.messagingengine.com [103.168.172.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 60D0643AB7 for ; Thu, 4 Apr 2024 05:48:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712209695; cv=none; b=MPhupEG5nIVgJ3pGSxaw6XyCsu/v7/Lo0yk+CWfLMSmpxdGuU7G0CvYA/1pw0Foe1w+qDFijtkzqC2b3/K0rb4Rom82+QBGYW8kzzsXB2wFhlsDPyvepOIgEL2GlcsPsLmHmn4vXSg+4AUF7WZrpboYJP6f00fDiTXvCg+CrhxM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712209695; c=relaxed/simple; bh=k7e5i0JT6/KWWUDfybaszt4H9xKevUfBEgnJVAeJ6rQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qTUHEt9/cABUAnV+w7srHzqCMhbXdnIL1lcVDguPUGMJNlUmdMt+RHnRBP3QB2lhgQlgG4Ddus0/2BsCJx8nUShfai5aw4O+3WzFbRuSUCffvy19ZPuMdUFFWgeHujVUr8lFktkn1w25JQ0XGRcDMYo92U6MKUgquYP/oBaxSUk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=S95o2l9l; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=rb+Ps2a1; arc=none smtp.client-ip=103.168.172.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="S95o2l9l"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="rb+Ps2a1" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfout.nyi.internal (Postfix) with ESMTP id 6B2321380128; Thu, 4 Apr 2024 01:48:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Thu, 04 Apr 2024 01:48:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1712209692; x=1712296092; bh=UbKzqy01O0 4ASpFyt4CpmGkXBb+qA6qOZuH9X806jc0=; b=S95o2l9l6kRjcwjmDsxK0vk6MZ Yy/a4XIZMNErTtpSXFCEWQNQM4z8vQ+qdOnAh6Xv4Tj3m/tSX/r1KOOXadQCFTQp iy5Vqlasg9XJPYYsaPLoV4EjaSDJ7KMtcN191ezQWFguu02j2MPEmUT4j4KrVS8t ZRfPRbH7ESig08K7PvIi929wAgioUa4mBKiNoPoyZwvEHV8z6bw1ClSiql5RU0uz okHXGJBfxc/vp89zP/TLec1ForopNKzdNMlLNhYgqX0ORO4xgTdXwbXhUfEqmb3t IHY0ed8nSF2ZLc4WTcOsqM2an4h4fh7rpPSKDJx/ogSDY3hBezbKyquTB6WA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712209692; x=1712296092; bh=UbKzqy01O04ASpFyt4CpmGkXBb+q A6qOZuH9X806jc0=; b=rb+Ps2a1szGp6CJiVbkG83c3/zV4DKAorXiHj+kQGnqG 0yZ+cg2TLSHNzLuflRDFFdHo63Ouui8LuOtxy35K2Opuip5BuEDqAxYGJmYqegWy vf65FNiw8d865o4Cm7E5iikj4TrreiMiwWUvGHIgVbzWSZVqSPSyyfl8OxOtuuse YyOSKWW55gShySPHZSkNXwZAGNdZ1p2OgDvEMNX25b6W1oaynQEGajREV2dlnx3e 4fRwdYVGbg1HpDxr6I4oyunFOY7GrgD8ytAK8wsv1k5vdpL9XySjnJTIo1qqsxRY wM0jlgp0jU7LQeSuaz/MDYVE2cgJUaPT0g5XAGumYA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudefjedguddttdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurhepfffhvfevuffkfhggtggujgesgh dtreertddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhs sehpkhhsrdhimheqnecuggftrfgrthhtvghrnhepueektdevtdffveeljeetgfehheeige ekleduvdeffeeghefgledttdehjeelffetnecuvehluhhsthgvrhfuihiivgeptdenucfr rghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 4 Apr 2024 01:48:11 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id b2bbcf38 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 4 Apr 2024 05:48:08 +0000 (UTC) Date: Thu, 4 Apr 2024 07:48:09 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Han-Wen Nienhuys Subject: [PATCH v2 01/11] refs/reftable: fix D/F conflict error message on ref copy Message-ID: <926e80239580b601cff752fa5f2086b2ff9298f6.1712209149.git.ps@pks.im> 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: The `write_copy_table()` function is shared between the reftable implementations for renaming and copying refs. The only difference between those two cases is that the rename will also delete the old reference, whereas copying won't. This has resulted in a bug though where we don't properly verify refname availability. When calling `refs_verify_refname_available()`, we always add the old ref name to the list of refs to be skipped when computing availability, which indicates that the name would be available even if it already exists at the current point in time. This is only the right thing to do for renames though, not for copies. The consequence of this bug is quite harmless because the reftable backend has its own checks for D/F conflicts further down in the call stack, and thus we refuse the update regardless of the bug. But all the user gets in this case is an uninformative message that copying the ref has failed, without any further details. Fix the bug and only add the old name to the skip-list in case we rename the ref. Consequently, this error case will now be handled by `refs_verify_refname_available()`, which knows to provide a proper error message. Signed-off-by: Patrick Steinhardt --- refs/reftable-backend.c | 3 ++- t/t0610-reftable-basics.sh | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e206d5a073..0358da14db 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1351,7 +1351,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) /* * Verify that the new refname is available. */ - string_list_insert(&skip, arg->oldname); + if (arg->delete_old) + string_list_insert(&skip, arg->oldname); ret = refs_verify_refname_available(&arg->refs->base, arg->newname, NULL, &skip, &errbuf); if (ret < 0) { diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 686781192e..055231a707 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -730,6 +730,39 @@ test_expect_success 'reflog: updates via HEAD update HEAD reflog' ' ) ' +test_expect_success 'branch: copying branch with D/F conflict' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + git branch branch && + cat >expect <<-EOF && + error: ${SQ}refs/heads/branch${SQ} exists; cannot create ${SQ}refs/heads/branch/moved${SQ} + fatal: branch copy failed + EOF + test_must_fail git branch -c branch branch/moved 2>err && + test_cmp expect err + ) +' + +test_expect_success 'branch: moving branch with D/F conflict' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + git branch branch && + git branch conflict && + cat >expect <<-EOF && + error: ${SQ}refs/heads/conflict${SQ} exists; cannot create ${SQ}refs/heads/conflict/moved${SQ} + fatal: branch rename failed + EOF + test_must_fail git branch -m branch conflict/moved 2>err && + test_cmp expect err + ) +' + test_expect_success 'worktree: adding worktree creates separate stack' ' test_when_finished "rm -rf repo worktree" && git init repo &&