From patchwork Wed Mar 12 15:56:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14013659 Received: from fout-b4-smtp.messagingengine.com (fout-b4-smtp.messagingengine.com [202.12.124.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 930B92571C4 for ; Wed, 12 Mar 2025 15:56:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; cv=none; b=Kc55+BJqll2HFzqyNUPVNxI197FCaz7FJ/s1sidqhFiTdkE9KWrISd7fiUHCZ+IqvzGglJ9v/LrwN06SbzE+/fg9L+4oXRZSHT6N/gAAfjdunOIjVeHcE9/G8/jrYyGL/Wr9642VVtazPAXeoH2afYSh8iWKKnEptG0/CmG4oT4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741794980; c=relaxed/simple; bh=3kdGZPL2ENDTBmC7UevJ12/4eqDbZsY1F0jafitTZl8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ZWMrlhqXvg/KwvU2i8tzZN26eWIvJdIhht3dLrV0olNcmDd7zKWoF8H3p2GnVUu8sdZ8rp9Sp5rDwBEG3LkqpysSMMdJCP8VQyIKV2PQ87mRNHCP2ywdLP4onyKdQUkdIqS9HNh7wVai4SHIWtMJHbxdiQpOzTXa9o9DG56Fr90= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject 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=eeVx3YdF; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=TqslUMxi; arc=none smtp.client-ip=202.12.124.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject 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="eeVx3YdF"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="TqslUMxi" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfout.stl.internal (Postfix) with ESMTP id 8B32A11401E3; Wed, 12 Mar 2025 11:56:17 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-07.internal (MEProxy); Wed, 12 Mar 2025 11:56:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding: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=1741794977; x=1741881377; bh=KM0W6Noa1jrB8Rui3gkcxBbCxVhqgDyEONEyXs+tsPs=; b= eeVx3YdFo5PjEnoLueBLSQKzPuaYEuGNM9ifQYAaMnbS1QOcovJqrkMKeKXsSNFA NRqT3mIfbO4vXOULzOYxXEsGJGo+7anbQN+c7tHuGVO02Jo4pzqixjNT+zfDv8hI OxNr6m25MZAurZX+M4P+Z/RUey7ZrEh+sssxiQO+k8t1B9pMXWFNdEOV0x0S7LXL RLX2KDyiFgCtn6yItVHtWpI3p0RaFoQtOPgJJjnfMoWk5mqj3C7Qy/M9WFP8qPoE hmKYWsM48Yqqn66oyNQ5CYCLx2PfQCXQwD3a07ijNyC+waMmOxdf421fO+Qopmp5 s1iX+TVbNi2lA6oRYBnArw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :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-sender:x-me-sender:x-sasl-enc; s=fm1; t=1741794977; x= 1741881377; bh=KM0W6Noa1jrB8Rui3gkcxBbCxVhqgDyEONEyXs+tsPs=; b=T qslUMxiG/NfxfXf8ER3YJn6W1d7xylVC0MqxrNRypveYXxItH65bokpOJvnYqL9O yRKUMCOblf0DmT3oHrB8r4YPP+pDKCkyjqyGhdt7KVbuUk9jcdppKVuTxSUrXbTE sI9LZfCRb+jy0tn09Qn44Cc71fVMCbkbiIw/R+Y6aG67FJQBHB+pw8H1iEXHAK5P ISZY3NGIyqZ+hBQDaH12Kecr8h0me/Rgqi5/5QQNGLWmXd26rD+FkMQZaP1sA5QO KOTNU6OUWRVMBw/qBUplpxqkbzbdA79XKZ7GvyGyMl+noQwZt1/FnPu+0DBzNcit jjsOSoJgeABTxFRJnUKIw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehgeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeejpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpth htohepshhhvghjihgrlhhuohesghhmrghilhdrtghomhdprhgtphhtthhopehgihhtsehv ghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoheptghhrhhishgtohholhesthhugi hfrghmihhlhidrohhrghdprhgtphhtthhopehkrghrthhhihhkrddukeeksehgmhgrihhl rdgtohhmpdhrtghpthhtohepghhithhsthgvrhesphhosghogidrtghomhdprhgtphhtth hopehsrghnuggrlhhssegtrhhushhthihtohhothhhphgrshhtvgdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Mar 2025 11:56:15 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 79240a2b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 12 Mar 2025 15:56:13 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 12 Mar 2025 16:56:10 +0100 Subject: [PATCH v6 04/16] refs: introduce function to batch refname availability checks Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250312-pks-update-ref-optimization-v6-4-f778e0414f55@pks.im> References: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> In-Reply-To: <20250312-pks-update-ref-optimization-v6-0-f778e0414f55@pks.im> To: git@vger.kernel.org Cc: Karthik Nayak , "brian m. carlson" , Jeff King , Junio C Hamano , shejialuo , Christian Couder X-Mailer: b4 0.14.2 The `refs_verify_refname_available()` functions checks whether a reference update can be committed or whether it would conflict with either a prefix or suffix thereof. This function needs to be called once per reference that one wants to check, which requires us to redo a couple of checks every time the function is called. Introduce a new function `refs_verify_refnames_available()` that does the same, but for a list of references. For now, the new function uses the exact same implementation, except that we loop through all refnames provided by the caller. This will be tuned in subsequent commits. The existing `refs_verify_refname_available()` function is reimplemented on top of the new function. As such, the diff is best viewed with the `--ignore-space-change option`. Signed-off-by: Patrick Steinhardt --- refs.c | 170 +++++++++++++++++++++++++++++++++++++---------------------------- refs.h | 12 +++++ 2 files changed, 110 insertions(+), 72 deletions(-) diff --git a/refs.c b/refs.c index f4094a326a9..d91a2184e06 100644 --- a/refs.c +++ b/refs.c @@ -2467,19 +2467,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, return ret; } -int refs_verify_refname_available(struct ref_store *refs, - const char *refname, - const struct string_list *extras, - const struct string_list *skip, - unsigned int initial_transaction, - struct strbuf *err) +int refs_verify_refnames_available(struct ref_store *refs, + const struct string_list *refnames, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err) { - const char *slash; - const char *extra_refname; struct strbuf dirname = STRBUF_INIT; struct strbuf referent = STRBUF_INIT; - struct object_id oid; - unsigned int type; + struct string_list_item *item; int ret = -1; /* @@ -2489,79 +2486,91 @@ int refs_verify_refname_available(struct ref_store *refs, assert(err); - strbuf_grow(&dirname, strlen(refname) + 1); - for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) { - /* - * Just saying "Is a directory" when we e.g. can't - * lock some multi-level ref isn't very informative, - * the user won't be told *what* is a directory, so - * let's not use strerror() below. - */ - int ignore_errno; - /* Expand dirname to the new prefix, not including the trailing slash: */ - strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len); + for_each_string_list_item(item, refnames) { + const char *refname = item->string; + const char *extra_refname; + struct object_id oid; + unsigned int type; + const char *slash; + + strbuf_reset(&dirname); + + for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) { + /* + * Just saying "Is a directory" when we e.g. can't + * lock some multi-level ref isn't very informative, + * the user won't be told *what* is a directory, so + * let's not use strerror() below. + */ + int ignore_errno; + + /* Expand dirname to the new prefix, not including the trailing slash: */ + strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len); + + /* + * We are still at a leading dir of the refname (e.g., + * "refs/foo"; if there is a reference with that name, + * it is a conflict, *unless* it is in skip. + */ + if (skip && string_list_has_string(skip, dirname.buf)) + continue; + + if (!initial_transaction && + !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, + &type, &ignore_errno)) { + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + dirname.buf, refname); + goto cleanup; + } + + if (extras && string_list_has_string(extras, dirname.buf)) { + strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), + refname, dirname.buf); + goto cleanup; + } + } /* - * We are still at a leading dir of the refname (e.g., - * "refs/foo"; if there is a reference with that name, - * it is a conflict, *unless* it is in skip. + * We are at the leaf of our refname (e.g., "refs/foo/bar"). + * There is no point in searching for a reference with that + * name, because a refname isn't considered to conflict with + * itself. But we still need to check for references whose + * names are in the "refs/foo/bar/" namespace, because they + * *do* conflict. */ - if (skip && string_list_has_string(skip, dirname.buf)) - continue; + strbuf_addstr(&dirname, refname + dirname.len); + strbuf_addch(&dirname, '/'); + + if (!initial_transaction) { + struct ref_iterator *iter; + int ok; + + iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, + DO_FOR_EACH_INCLUDE_BROKEN); + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { + if (skip && + string_list_has_string(skip, iter->refname)) + continue; + + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->refname, refname); + ref_iterator_abort(iter); + goto cleanup; + } - if (!initial_transaction && - !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, - &type, &ignore_errno)) { - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - dirname.buf, refname); - goto cleanup; + if (ok != ITER_DONE) + BUG("error while iterating over references"); } - if (extras && string_list_has_string(extras, dirname.buf)) { + extra_refname = find_descendant_ref(dirname.buf, extras, skip); + if (extra_refname) { strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, dirname.buf); + refname, extra_refname); goto cleanup; } } - /* - * We are at the leaf of our refname (e.g., "refs/foo/bar"). - * There is no point in searching for a reference with that - * name, because a refname isn't considered to conflict with - * itself. But we still need to check for references whose - * names are in the "refs/foo/bar/" namespace, because they - * *do* conflict. - */ - strbuf_addstr(&dirname, refname + dirname.len); - strbuf_addch(&dirname, '/'); - - if (!initial_transaction) { - struct ref_iterator *iter; - int ok; - - iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, - DO_FOR_EACH_INCLUDE_BROKEN); - while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - if (skip && - string_list_has_string(skip, iter->refname)) - continue; - - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); - ref_iterator_abort(iter); - goto cleanup; - } - - if (ok != ITER_DONE) - BUG("error while iterating over references"); - } - - extra_refname = find_descendant_ref(dirname.buf, extras, skip); - if (extra_refname) - strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), - refname, extra_refname); - else - ret = 0; + ret = 0; cleanup: strbuf_release(&referent); @@ -2569,6 +2578,23 @@ int refs_verify_refname_available(struct ref_store *refs, return ret; } +int refs_verify_refname_available(struct ref_store *refs, + const char *refname, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err) +{ + struct string_list_item item = { .string = (char *) refname }; + struct string_list refnames = { + .items = &item, + .nr = 1, + }; + + return refs_verify_refnames_available(refs, &refnames, extras, skip, + initial_transaction, err); +} + struct do_for_each_reflog_help { each_reflog_fn *fn; void *cb_data; diff --git a/refs.h b/refs.h index a0cdd99250e..185aed5a461 100644 --- a/refs.h +++ b/refs.h @@ -124,6 +124,18 @@ int refs_verify_refname_available(struct ref_store *refs, unsigned int initial_transaction, struct strbuf *err); +/* + * Same as `refs_verify_refname_available()`, but checking for a list of + * refnames instead of only a single item. This is more efficient in the case + * where one needs to check multiple refnames. + */ +int refs_verify_refnames_available(struct ref_store *refs, + const struct string_list *refnames, + const struct string_list *extras, + const struct string_list *skip, + unsigned int initial_transaction, + struct strbuf *err); + int refs_ref_exists(struct ref_store *refs, const char *refname); int should_autocreate_reflog(enum log_refs_config log_all_ref_updates,