From patchwork Tue Apr 30 12:26:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13648912 Received: from wfout3-smtp.messagingengine.com (wfout3-smtp.messagingengine.com [64.147.123.146]) (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 32A0B14037E for ; Tue, 30 Apr 2024 12:26:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.146 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714480018; cv=none; b=qik9gVqeQoh1Oken4ENUGqnU1GrH+/aeiKBisZa01GI2H9zEAJfmCnh0sU3RHXLg8d2EIHiemUv1q4yk/kCE2p+sAiWk5eSj3eRRMpI0g88LME6s6j4IVwx4l83qm/v15IBo7bmd9T2rJjOrUO54fRPhxaKCUuyiGZ9VtjkFosU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714480018; c=relaxed/simple; bh=RTUdWAgX8wMZunTj7yaB0xjF2ZQrt/EBTNY0ZdpGEAU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rPmqUuL/BdWhQDYJaC2hVVE3dtEttDW3CntjwyqgzHoNALWZDBDM1w4SGB5YeX9TUR9EcVjzH01GztOElKQRhBhV8q7GXj7iq9OLx6PmahFss0sMI2DjZXLobVOshAENv/tfm3Faz1OaCkn02kRCkjc8n6odVu+6Cl/+KCaclgQ= 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=No8rxVku; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=TNl1XR+c; arc=none smtp.client-ip=64.147.123.146 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="No8rxVku"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="TNl1XR+c" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailfout.west.internal (Postfix) with ESMTP id 22F2F1C000EF; Tue, 30 Apr 2024 08:26:56 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 30 Apr 2024 08:26:56 -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=fm3; t=1714480015; x=1714566415; bh=QY47j0rS+3 a2TzzSkTXgGW/a4ow0u6ZQKvc0kQibxpQ=; b=No8rxVkuhB2atj2abX5jgfrSSh zgepRC5J5gnmqCQYgFMhjAEL8JECy/YwXEDjVpyA9l7b+ZeXJn1dngc0X0draDD5 jXHX0nYoPaq3t+vcMXBQLhmb2WIxuvfviUCxoPc3B0raoHca35IxOKr8Cc91dgM4 HFRed2VcXdsMksgpfnLBFKZp+Gk3ozCr2FZU/1y5axF7Df/LYzm8t9LK91XxXKzE NAcx6+H/ps9F257f19WtWM8QGFOpe4Bq0QyztccOUvBLG8cwJbwSxaxe7kKyp6i9 Wf3iG3zQx2KxaHz5koAeH5nL4KiKBEKbiJcSiSKdyBJmrawZkDsXlGk9A7gw== 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= fm3; t=1714480015; x=1714566415; bh=QY47j0rS+3a2TzzSkTXgGW/a4ow0 u6ZQKvc0kQibxpQ=; b=TNl1XR+cBJMxyh20G7iRhnYuWR2y5jf/8j/iMUkBVWu8 Qr/WxFM9HTtYv7dQSwkvUuDRxBhgoxNHKFqvRGt+mORo0wBX6kcsyJd3PtqmGuxz Jf12utF9trxStKGP8nk9YED7mWp0s951i4C3jSj5XkpvOm2P/fEXfNtTEJVCp99/ GTMssguQgF1mAnSyf+Uepxow0GKoX15D3JVgPK1Ldl9a27bHmI+iYBD+aOLgI+ds I1BL5hIP8ZUhPut2HbSjlo6LMBc0a3sr+Xz33+nIadIlPdupXuGVw2BnnpdMeo+X bghUTKVxqbTIeQP8icx4wxNCAuYrvymorqZLOiHz9A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvddufedghedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgepvdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 30 Apr 2024 08:26:54 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 09210981 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 30 Apr 2024 12:26:32 +0000 (UTC) Date: Tue, 30 Apr 2024 14:26:52 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Jeff King , Karthik Nayak , Phillip Wood , Junio C Hamano , Justin Tobler Subject: [PATCH v2 07/10] refs: root refs can be symbolic 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: Before this patch series, root refs except for "HEAD" and our special refs were classified as pseudorefs. Furthermore, our terminology clarified that pseudorefs must not be symbolic refs. This restriction is enforced in `is_root_ref()`, which explicitly checks that a supposed root ref resolves to an object ID without recursing. This has been extremely confusing right from the start because (in old terminology) a ref name may sometimes be a pseudoref and sometimes not depending on whether it is a symbolic or regular ref. This behaviour does not seem reasonable at all and I very much doubt that it results in anything sane. Furthermore, the behaviour is different to `is_headref()`, which only checks for the ref to exist. While that is in line with our glossary, this inconsistency only adds to the confusion. Last but not least, the current behaviour can actually lead to a segfault when calling `is_root_ref()` with a reference that either does not exist or that is a symbolic ref because we never intialized `oid`. Let's loosen the restrictions in accordance to the new definition of root refs, which are simply plain refs that may as well be a symbolic ref. Consequently, we can just check for the ref to exist instead of requiring it to be a regular ref. Add a test that verifies that this does not change user-visible behaviour. Namely, we still don't want to show broken refs to the user by default in git-for-each-ref(1). What this does allow though is for internal callers to surface dangling root refs when they pass in the `DO_FOR_EACH_INCLUDE_BROKEN` flag. Signed-off-by: Patrick Steinhardt --- refs.c | 50 ++++++++++++++++++++++++---------- t/t6302-for-each-ref-filter.sh | 17 ++++++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index 5b89e83ad7..ca9844bc3e 100644 --- a/refs.c +++ b/refs.c @@ -869,7 +869,10 @@ int is_root_ref(struct ref_store *refs, const char *refname) "NOTES_MERGE_REF", "MERGE_AUTOSTASH", }; - struct object_id oid; + struct strbuf referent = STRBUF_INIT; + struct object_id oid = { 0 }; + int failure_errno, ret = 0; + unsigned int flags; size_t i; if (!is_root_ref_syntax(refname)) @@ -877,30 +880,49 @@ int is_root_ref(struct ref_store *refs, const char *refname) if (is_headref(refs, refname)) return 1; + /* + * Note that we cannot use `refs_ref_exists()` here because that also + * checks whether its target ref exists in case refname is a symbolic + * ref. + */ if (ends_with(refname, "_HEAD")) { - refs_resolve_ref_unsafe(refs, refname, - RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); - return !is_null_oid(&oid); + ret = !refs_read_raw_ref(refs, refname, &oid, &referent, + &flags, &failure_errno); + goto done; } - for (i = 0; i < ARRAY_SIZE(irregular_root_refs); i++) + for (i = 0; i < ARRAY_SIZE(irregular_root_refs); i++) { if (!strcmp(refname, irregular_root_refs[i])) { - refs_resolve_ref_unsafe(refs, refname, - RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); - return !is_null_oid(&oid); + ret = !refs_read_raw_ref(refs, refname, &oid, &referent, + &flags, &failure_errno); + goto done; } + } - return 0; +done: + strbuf_release(&referent); + return ret; } int is_headref(struct ref_store *refs, const char *refname) { - if (!strcmp(refname, "HEAD")) - return refs_ref_exists(refs, refname); + struct strbuf referent = STRBUF_INIT; + struct object_id oid = { 0 }; + int failure_errno, ret = 0; + unsigned int flags; - return 0; + /* + * Note that we cannot use `refs_ref_exists()` here because that also + * checks whether its target ref exists in case refname is a symbolic + * ref. + */ + if (!strcmp(refname, "HEAD")) { + ret = !refs_read_raw_ref(refs, refname, &oid, &referent, + &flags, &failure_errno); + } + + strbuf_release(&referent); + return ret; } static int is_current_worktree_ref(const char *ref) { diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index 948f1bb5f4..92ed8957c8 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -62,6 +62,23 @@ test_expect_success '--include-root-refs with other patterns' ' test_cmp expect actual ' +test_expect_success '--include-root-refs omits dangling symrefs' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + git symbolic-ref DANGLING_HEAD refs/heads/missing && + cat >expect <<-EOF && + HEAD + $(git symbolic-ref HEAD) + refs/tags/initial + EOF + git for-each-ref --format="%(refname)" --include-root-refs >actual && + test_cmp expect actual + ) +' + test_expect_success 'filtering with --points-at' ' cat >expect <<-\EOF && refs/heads/main