From patchwork Thu Dec 14 13:36:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13493084 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="QZAC1Usb"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="xBi+k8F1" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3373011B for ; Thu, 14 Dec 2023 05:37:02 -0800 (PST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 7E7F05C01B3; Thu, 14 Dec 2023 08:37:01 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 14 Dec 2023 08:37:01 -0500 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=fm1; t=1702561021; x=1702647421; bh=nCymLJ6XS0 Y4DAiOX+nRDTJ+BbXnaC6dlXTXMlNNkIw=; b=QZAC1Usb9GvzIhFVKtNz0WBUy7 LrTsAlbf4tSaK72yQEvI8eiq7SUVLVxUTHQHwp5DFF01Xnp5so0N4H4/YoltRlnM R2z+/pICcTCRFBDyONGHf96XkeQyxgb+Yk4hdnfyiDUTdj7ASt2MOl2PXQo3c7/k uOA+pNe/ZkAv56DbwjNfAe5eFcCu7qh/9IjLbLWhP8hc47Ai+8OMpPQH7gWjglUU VI052G0kjwoclzlVwSj+Mg9Z8RojwsNTeQBPKOTnxkFE00BkZGyI7rZ+X4W93V2j AEqoxc6kvwN4PmnbmTLWJ3R5++u742F8maQZG3oaBa1qiKAz7wSnahfDIDyQ== 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= fm1; t=1702561021; x=1702647421; bh=nCymLJ6XS0Y4DAiOX+nRDTJ+BbXn aC6dlXTXMlNNkIw=; b=xBi+k8F1dwTUl2iZ3fb12HaHY5Uu1QF1UynOowduht4X jgCPBTAC5BU0fRcBV1tK1vUWqXQKpK80lD7FGH4Jp08M9a1ZgTC77xyX6Yp5Iqm5 loEUESqpN1L/hq26RBIUFw5RReIJFQxdoT3imUToNn67Jt7Nj5UbV/iwabUojzDj J+xToPYyRO6oiHT2Q1DcJP9o0mJTqQKVqwT6Ii0M5RWgcfAqYRjK83nHtFzo97eo b1dZHRZ9erH+cQPeB4SdYiTVTT6S1nw60HkE1lKNFQXrrxRG+7LyOHv0q+eIczAi 4JHP/ku/4de5n8VE1+WzPhWEnr3KiG22WI7xqT2GoA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelledgheefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 14 Dec 2023 08:37:00 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id f4dc766a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 14 Dec 2023 13:35:17 +0000 (UTC) Date: Thu, 14 Dec 2023 14:36:58 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Taylor Blau , Phillip Wood , Ramsay Jones Subject: [PATCH v3 1/4] wt-status: read HEAD and ORIG_HEAD via the refdb 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 read both the HEAD and ORIG_HEAD references directly from the filesystem in order to figure out whether we're currently splitting a commit. If both of the following are true: - HEAD points to the same object as "rebase-merge/amend". - ORIG_HEAD points to the same object as "rebase-merge/orig-head". Then we are currently splitting commits. The current code only works by chance because we only have a single reference backend implementation. Refactor it to instead read both refs via the refdb layer so that we'll also be compatible with alternate reference backends. There are some subtleties involved here: - We pass `RESOLVE_REF_READING` so that a missing ref will cause `read_ref_full()` to return an error. - We pass `RESOLVE_REF_NO_RECURSE` so that we do not try to resolve symrefs. The old code didn't resolve symrefs either, and we only ever write object IDs into the refs in "rebase-merge/". - In the same spirit we verify that successfully-read refs are not symbolic refs. Signed-off-by: Patrick Steinhardt --- wt-status.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/wt-status.c b/wt-status.c index 9f45bf6949..da19923981 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1295,26 +1295,32 @@ static char *read_line_from_git_path(const char *filename) static int split_commit_in_progress(struct wt_status *s) { int split_in_progress = 0; - char *head, *orig_head, *rebase_amend, *rebase_orig_head; + struct object_id head_oid, orig_head_oid; + char *rebase_amend, *rebase_orig_head; + int head_flags, orig_head_flags; if ((!s->amend && !s->nowarn && !s->workdir_dirty) || !s->branch || strcmp(s->branch, "HEAD")) return 0; - head = read_line_from_git_path("HEAD"); - orig_head = read_line_from_git_path("ORIG_HEAD"); + if (read_ref_full("HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &head_oid, &head_flags) || + read_ref_full("ORIG_HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &orig_head_oid, &orig_head_flags)) + return 0; + if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF) + return 0; + rebase_amend = read_line_from_git_path("rebase-merge/amend"); rebase_orig_head = read_line_from_git_path("rebase-merge/orig-head"); - if (!head || !orig_head || !rebase_amend || !rebase_orig_head) + if (!rebase_amend || !rebase_orig_head) ; /* fall through, no split in progress */ else if (!strcmp(rebase_amend, rebase_orig_head)) - split_in_progress = !!strcmp(head, rebase_amend); - else if (strcmp(orig_head, rebase_orig_head)) + split_in_progress = !!strcmp(oid_to_hex(&head_oid), rebase_amend); + else if (strcmp(oid_to_hex(&orig_head_oid), rebase_orig_head)) split_in_progress = 1; - free(head); - free(orig_head); free(rebase_amend); free(rebase_orig_head); From patchwork Thu Dec 14 13:37:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13493085 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="Zm+NYHBC"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Eiz4Fblj" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE638111 for ; Thu, 14 Dec 2023 05:37:06 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 22F205C0163; Thu, 14 Dec 2023 08:37:06 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Thu, 14 Dec 2023 08:37:06 -0500 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=fm1; t=1702561026; x=1702647426; bh=IlRA5wR2O/ /F2fNfr00UfjVZHFHDvVtJMlc/L+cUd8M=; b=Zm+NYHBCGuSsNqXArvn9y+4pjD 4ovrYIqY6SRieJAnApWjnLFotg2dTv0PGKylxhIWko7DKW1BRj7UjupfvOIho9x7 LgKPLgogS4MkyBRCzBv/FKk4XEx/13oQDR35qlBpL5+P5N8Ee5Y9iyHGtfVqCvej kK4A+sJwl5htAEpmOc/gB0CQCKOwbGLUYSXRxzt+70L/yUOI2dGtKd3LLPig+VWc V9Fgbw9yC7uGSgAlv0fTtsymDJGQ+9rZccsfBke7zI1QudAB4AafzC4Ph5cg1K4/ LNWlKqwZkdxEe8wPgPtOv1WMYW/SCR8ozSUfkDmX89mpY/6GheN0FzTgRKQQ== 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= fm1; t=1702561026; x=1702647426; bh=IlRA5wR2O//F2fNfr00UfjVZHFHD vVtJMlc/L+cUd8M=; b=Eiz4Fblj1TTv2AKZcIz8q9N5lwBxYBQbZxvQRLSiWkQ8 lsC6+HCfQqiiUr9dQoEt6AwiyYspgEDSXRg0bPhIwCDsldF+sfZpPY/hBbllJXS6 7Gay8eXte+5A4pe54m92oL7l+njkvhQ1qIkmDq1Ww66o96K4uu4v8A4se5obt+eM A4PlIdn+Awaw77NB1qciu6dU1B/tqR052KOQ+u6odh9oug5K1uZxNi/EaB/4n6PZ 69AuIwPrEQUIqrCpgyQwckKkwAUi08m8SXS4k0yTMDUiEodtiOPIac8T4frO4PAn nCpHNvEut2rjitywl1XuomTAnY8TOUDiLix1++OHVw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelledgheefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 14 Dec 2023 08:37:04 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id f2fc1b14 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 14 Dec 2023 13:35:21 +0000 (UTC) Date: Thu, 14 Dec 2023 14:37:02 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Taylor Blau , Phillip Wood , Ramsay Jones Subject: [PATCH v3 2/4] refs: propagate errno when reading special refs fails Message-ID: <455ab69177586370b809b4a790ae99ce5e97dafa.1702560829.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: Some refs in Git are more special than others due to reasons explained in the next commit. These refs are read via `refs_read_special_head()`, but this function doesn't behave the same as when we try to read a normal ref. Most importantly, we do not propagate `failure_errno` in the case where the reference does not exist, which is behaviour that we rely on in many parts of Git. Fix this bug by propagating errno when `strbuf_read_file()` fails. Signed-off-by: Patrick Steinhardt --- refs.c | 4 +++- t/t1403-show-ref.sh | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index fcae5dddc6..00e72a2abf 100644 --- a/refs.c +++ b/refs.c @@ -1806,8 +1806,10 @@ static int refs_read_special_head(struct ref_store *ref_store, int result = -1; strbuf_addf(&full_path, "%s/%s", ref_store->gitdir, refname); - if (strbuf_read_file(&content, full_path.buf, 0) < 0) + if (strbuf_read_file(&content, full_path.buf, 0) < 0) { + *failure_errno = errno; goto done; + } result = parse_loose_ref_contents(content.buf, oid, referent, type, failure_errno); diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index b50ae6fcf1..66e6e77fa9 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -266,4 +266,14 @@ test_expect_success '--exists with directory fails with generic error' ' test_cmp expect err ' +test_expect_success '--exists with non-existent special ref' ' + test_expect_code 2 git show-ref --exists FETCH_HEAD +' + +test_expect_success '--exists with existing special ref' ' + test_when_finished "rm .git/FETCH_HEAD" && + git rev-parse HEAD >.git/FETCH_HEAD && + git show-ref --exists FETCH_HEAD +' + test_done From patchwork Thu Dec 14 13:37:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13493086 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="ZLB8vzbW"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Cz7IsPhT" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 133B0123 for ; Thu, 14 Dec 2023 05:37:11 -0800 (PST) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 7A3CF5C0163; Thu, 14 Dec 2023 08:37:10 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Thu, 14 Dec 2023 08:37:10 -0500 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=fm1; t=1702561030; x=1702647430; bh=+BmBebyCFe s0DD7JN6R1Fyf0FfAoHBkfs1MLjOfHFrM=; b=ZLB8vzbWDDf7Vwcp47Eo7KjAA5 Z/omUYdPW144Dp/uaD6Xadbk3szlIz/LZm0QCWT1vQyAO6+A/dwg7eyCwxF1JylB klPGRsiscKlxm1KuuFBJ1zPKdreeWfCWK6TPN7F0f0+oxjtxG6t9Zy6dchYt2VXD zlG1rlKN+ATGfE3M2WbaRfYzWUpTcWLToSQD0GymlVf2NydLzg7qwNG8Dsj9T0eM 4qXDStSF3UsioBVln2u9A3hFrpclDc73RWpn/F/m/CF4HLmI0+osIO37iu9Mg/da 6FROHPrgmcFEebMfpT3SZPtxOkEmDEfvETJy/ejxylt1Lmym5s66LzHtyOJg== 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= fm1; t=1702561030; x=1702647430; bh=+BmBebyCFes0DD7JN6R1Fyf0FfAo HBkfs1MLjOfHFrM=; b=Cz7IsPhTXM2fbGVT+lRrz3U3SPeqKDLkXOo7N3eqMfWk x/sQ6KFCy4gv6UL49JIWr9hIBf0M0y2IIkUlN9tYNKVTGd3YtPeVIsbP3uv+XKOR yTwn2H9WC+LdaqP7JfAWirz1aX1sQwNTghwjIV8+zJRcr7vqDeORxOW22jJr80qE SysYpm0FW3rLgAOl6eJlSHfKa6dFpgYHHZEkVoh8fJ+bklgRa7vpI2Buc78Ibtty O67buY05ZSSsOy+bgkCaWgzpEJJdQouxsEdVXTYD5uUEEJyOe8STqWepou3/iF1j R5lq8/6v3ZjI0oJLRXtMg1Ylv9MEVA/l0RhfOxoFiQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelledgheefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 14 Dec 2023 08:37:09 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 03dd695a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 14 Dec 2023 13:35:26 +0000 (UTC) Date: Thu, 14 Dec 2023 14:37:06 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Taylor Blau , Phillip Wood , Ramsay Jones Subject: [PATCH v3 3/4] refs: complete list of special refs Message-ID: <81ac092281d9ebfc0f231faef6adcdce05cdb1ec.1702560829.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: We have some references that are more special than others. The reason for them being special is that they either do not follow the usual format of references, or that they are written to the filesystem directly by the respective owning subsystem and thus circumvent the reference backend. This works perfectly fine right now because the reffiles backend will know how to read those refs just fine. But with the prospect of gaining a new reference backend implementation we need to be a lot more careful here: - We need to make sure that we are consistent about how those refs are written. They must either always be written via the filesystem, or they must always be written via the reference backend. Any mixture will lead to inconsistent state. - We need to make sure that such special refs are always handled specially when reading them. We're already mostly good with regard to the first item, except for `BISECT_EXPECTED_REV` which will be addressed in a subsequent commit. But the current list of special refs is missing some refs that really should be treated specially. Right now, we only treat `FETCH_HEAD` and `MERGE_HEAD` specially here. Introduce a new function `is_special_ref()` that contains all current instances of special refs to fix the reading path. Note that this is only a temporary measure where we record and rectify the current state. Ideally, the list of special refs should in the end only contain `FETCH_HEAD` and `MERGE_HEAD` again because they both may reference multiple objects and can contain annotations, so they indeed are special. Based-on-patch-by: Han-Wen Nienhuys Signed-off-by: Patrick Steinhardt --- refs.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 00e72a2abf..8fe34d51e4 100644 --- a/refs.c +++ b/refs.c @@ -1820,15 +1820,65 @@ static int refs_read_special_head(struct ref_store *ref_store, return result; } +static int is_special_ref(const char *refname) +{ + /* + * Special references get written and read directly via the filesystem + * by the subsystems that create them. Thus, they must not go through + * the reference backend but must instead be read directly. It is + * arguable whether this behaviour is sensible, or whether it's simply + * a leaky abstraction enabled by us only having a single reference + * backend implementation. But at least for a subset of references it + * indeed does make sense to treat them specially: + * + * - FETCH_HEAD may contain multiple object IDs, and each one of them + * carries additional metadata like where it came from. + * + * - MERGE_HEAD may contain multiple object IDs when merging multiple + * heads. + * + * There are some exceptions that you might expect to see on this list + * but which are handled exclusively via the reference backend: + * + * - CHERRY_PICK_HEAD + * + * - HEAD + * + * - ORIG_HEAD + * + * - "rebase-apply/" and "rebase-merge/" contain all of the state for + * rebases, including some reference-like files. These are + * exclusively read and written via the filesystem and never go + * through the refdb. + * + * Writing or deleting references must consistently go either through + * the filesystem (special refs) or through the reference backend + * (normal ones). + */ + static const char * const special_refs[] = { + "AUTO_MERGE", + "BISECT_EXPECTED_REV", + "FETCH_HEAD", + "MERGE_AUTOSTASH", + "MERGE_HEAD", + }; + size_t i; + + for (i = 0; i < ARRAY_SIZE(special_refs); i++) + if (!strcmp(refname, special_refs[i])) + return 1; + + return 0; +} + int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, struct object_id *oid, struct strbuf *referent, unsigned int *type, int *failure_errno) { assert(failure_errno); - if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) { + if (is_special_ref(refname)) return refs_read_special_head(ref_store, refname, oid, referent, type, failure_errno); - } return ref_store->be->read_raw_ref(ref_store, refname, oid, referent, type, failure_errno); From patchwork Thu Dec 14 13:37:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13493087 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="GdvpayY9"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="KBcAqtDi" Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A51FD118 for ; Thu, 14 Dec 2023 05:37:18 -0800 (PST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 191C15C01B7; Thu, 14 Dec 2023 08:37:18 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Thu, 14 Dec 2023 08:37:18 -0500 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=fm1; t=1702561038; x=1702647438; bh=izuZYshyx+ gHoMxy+8YwIOYTB9TabCJeQTAjhEfklnk=; b=GdvpayY9NbLGkYRX7SwyMnnbB+ X7hBEpoHiVDO40eq4sssIqyS+VNCuEjtPnAr1OB2hTpiUsGHLLENahToShxI3apq 0Rr3sPBXeYwhlNU4bhXNHgidXG6gl+sMOzIw5g1Ibmfb1dEzXgurQvhqYaaNo8q7 WS8GdydAwsMK9H5HSerTu9potOA+mqiTV+5k4bRj8AgbHGQq80kcEmQuAhu4qsyb HUbVGSOqj+raxirIoHiE3H89vjEeixy1XDkvkQ31wqh2qUWaZ1rS4VrhYU35/XdA YrkPTvH/i7r24BOekpw1K2fqNYEoqU89AOaHjzt7YMwWDZmq9pz5V2dnJmdw== 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= fm1; t=1702561038; x=1702647438; bh=izuZYshyx+gHoMxy+8YwIOYTB9Ta bCJeQTAjhEfklnk=; b=KBcAqtDiP/ad3V5JZeK2Et2SKe13iHsIsgi8jc0ZkE30 42FG5B8dMhpYb2EK+e9thEEdKDTuDd6A+vaHPD743whvwF+bztkJAraxcGrh5xeY nw5+ZEd8YNaL4E6Xu+F0mjzt7TqkwMwwINIz6YCjs461JgxVgRUL54pPpmMABsTm nKWQht6THoWZbKzJmtJw7IhqiD6zYe6HbuiMr7WA66/hsHOChfhJoOswCTDhkq0d 1e3/G2V6VsSwsGDkUqVpA1RArnV4SCRWVbXpNe7wKy3/nN6OP0sMbSH0Y+ayd/uv Te/coCIMdLwaiyww7KtzdTIcRp3gAqWKbsnSCiSi2A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudelledgheefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepudenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 14 Dec 2023 08:37:16 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 95bca8e9 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Thu, 14 Dec 2023 13:35:33 +0000 (UTC) Date: Thu, 14 Dec 2023 14:37:13 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Taylor Blau , Phillip Wood , Ramsay Jones Subject: [PATCH v3 4/4] bisect: consistently write BISECT_EXPECTED_REV via the refdb Message-ID: <324467816187a8d29407f03006c936b5938f811c.1702560829.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: We're inconsistently writing BISECT_EXPECTED_REV both via the filesystem and via the refdb, which violates the newly established rules for how special refs must be treated. This works alright in practice with the reffiles reference backend, but will cause bugs once we gain additional backends. Fix this issue and consistently write BISECT_EXPECTED_REV via the refdb so that it is no longer a special ref. Signed-off-by: Patrick Steinhardt --- bisect.c | 25 ++++--------------------- builtin/bisect.c | 8 ++------ refs.c | 3 ++- t/t6030-bisect-porcelain.sh | 2 +- 4 files changed, 9 insertions(+), 29 deletions(-) diff --git a/bisect.c b/bisect.c index 1be8e0a271..985b96ed13 100644 --- a/bisect.c +++ b/bisect.c @@ -471,7 +471,6 @@ static int read_bisect_refs(void) } static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") -static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK") static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN") static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START") @@ -707,26 +706,10 @@ static enum bisect_error error_if_skipped_commits(struct commit_list *tried, static int is_expected_rev(const struct object_id *oid) { - const char *filename = git_path_bisect_expected_rev(); - struct stat st; - struct strbuf str = STRBUF_INIT; - FILE *fp; - int res = 0; - - if (stat(filename, &st) || !S_ISREG(st.st_mode)) + struct object_id expected_oid; + if (read_ref("BISECT_EXPECTED_REV", &expected_oid)) return 0; - - fp = fopen_or_warn(filename, "r"); - if (!fp) - return 0; - - if (strbuf_getline_lf(&str, fp) != EOF) - res = !strcmp(str.buf, oid_to_hex(oid)); - - strbuf_release(&str); - fclose(fp); - - return res; + return oideq(oid, &expected_oid); } enum bisect_error bisect_checkout(const struct object_id *bisect_rev, @@ -1185,10 +1168,10 @@ int bisect_clean_state(void) struct string_list refs_for_removal = STRING_LIST_INIT_NODUP; for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal); string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD")); + string_list_append(&refs_for_removal, xstrdup("BISECT_EXPECTED_REV")); result = delete_refs("bisect: remove", &refs_for_removal, REF_NO_DEREF); refs_for_removal.strdup_strings = 1; string_list_clear(&refs_for_removal, 0); - unlink_or_warn(git_path_bisect_expected_rev()); unlink_or_warn(git_path_bisect_ancestors_ok()); unlink_or_warn(git_path_bisect_log()); unlink_or_warn(git_path_bisect_names()); diff --git a/builtin/bisect.c b/builtin/bisect.c index 35938b05fd..4e2c43daf5 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -17,7 +17,6 @@ #include "revision.h" static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS") -static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK") static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START") static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG") @@ -921,7 +920,6 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc, const char *state; int i, verify_expected = 1; struct object_id oid, expected; - struct strbuf buf = STRBUF_INIT; struct oid_array revs = OID_ARRAY_INIT; if (!argc) @@ -976,10 +974,8 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc, oid_array_append(&revs, &commit->object.oid); } - if (strbuf_read_file(&buf, git_path_bisect_expected_rev(), 0) < the_hash_algo->hexsz || - get_oid_hex(buf.buf, &expected) < 0) + if (read_ref("BISECT_EXPECTED_REV", &expected)) verify_expected = 0; /* Ignore invalid file contents */ - strbuf_release(&buf); for (i = 0; i < revs.nr; i++) { if (bisect_write(state, oid_to_hex(&revs.oid[i]), terms, 0)) { @@ -988,7 +984,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc, } if (verify_expected && !oideq(&revs.oid[i], &expected)) { unlink_or_warn(git_path_bisect_ancestors_ok()); - unlink_or_warn(git_path_bisect_expected_rev()); + delete_ref(NULL, "BISECT_EXPECTED_REV", NULL, REF_NO_DEREF); verify_expected = 0; } } diff --git a/refs.c b/refs.c index 8fe34d51e4..c76ce86bef 100644 --- a/refs.c +++ b/refs.c @@ -1840,6 +1840,8 @@ static int is_special_ref(const char *refname) * There are some exceptions that you might expect to see on this list * but which are handled exclusively via the reference backend: * + * - BISECT_EXPECTED_REV + * * - CHERRY_PICK_HEAD * * - HEAD @@ -1857,7 +1859,6 @@ static int is_special_ref(const char *refname) */ static const char * const special_refs[] = { "AUTO_MERGE", - "BISECT_EXPECTED_REV", "FETCH_HEAD", "MERGE_AUTOSTASH", "MERGE_HEAD", diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 2a5b7d8379..792c1504bc 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -1176,7 +1176,7 @@ test_expect_success 'git bisect reset cleans bisection state properly' ' git bisect bad $HASH4 && git bisect reset && test -z "$(git for-each-ref "refs/bisect/*")" && - test_path_is_missing ".git/BISECT_EXPECTED_REV" && + test_ref_missing BISECT_EXPECTED_REV && test_path_is_missing ".git/BISECT_ANCESTORS_OK" && test_path_is_missing ".git/BISECT_LOG" && test_path_is_missing ".git/BISECT_RUN" &&