Makefile: Use Gitaly's tagged Git versions instead of ad-hoc patching

Right now, the way we apply Git patches is by adding them to the Gitaly
project and using git-apply(1) to apply them ad-hoc. This is starting to
show its limits though:

    - It is hard for us to provide a simple pointer to the Git sources
      that we distribute to the customer.

    - It is hard to execute tests for the patched Git version in an
      automated fashion.

    - It is hard to work on top of the already-patched Git distribution
      to for example apply more patches.

These limitations are getting more noticeable now that we have split up
the Gitaly team into two teams, where we potentially want to backport
patches more aggressively.

With the split we have now made the repository at [1] the canonical Git
repository for all our efforts. This indeed opens up a much better way
to use custom Git versions: instead of hosting the patches in the Gitaly
repository, we start to tag Gitaly-specific releases in that repository.
These releases then carry all the additional patches on top.

This makes it trivial to use normal workflows:

    - We can point customers to the Gitaly-specific tags which hold our
      patches on top.

    - We can automatically trigger CI pipelines on top of patched
      Gitaly-specific releases by just pushing branches or tags.

    - You can just clone the repository and checkout out the specific
      tags.

Drop the infrastructure to patch releases in-place in favor of this new
architecture.

[1]: https://gitlab.com/gitlab-org/git.git

7 files changed, 9 insertions, 698 deletions

diff --git a/Makefile b/Makefile
index 4bb2b811a..a084cbec9 100644
--- a/Makefile
+++ b/Makefile
@@ -132,24 +132,15 @@ GIT_EXECUTABLES += git-http-backend
 ## WITH_BUNDLED_GIT=YesPlease. Can be set to an arbitrary Git revision with
 ## tags, branches, and commit ids.
 GIT_VERSION ?=
+## The Git version used for bundled Git v2.37.
+GIT_VERSION_2_37_1 ?= v2.37.1.gl1
+## The Git version used for bundled Git v2.38.
+GIT_VERSION_2_38 ?= v2.38.1.gl0
 
 # The default version is used in case the caller does not set the variable or
 # if it is either set to the empty string or "default".
 ifeq (${GIT_VERSION:default=},)
-    override GIT_VERSION := v2.37.1
-
-    # This extra version has two intentions: first, it allows us to detect
-    # capabilities of the command at runtime. Second, it helps admins to
-    # discover which version is currently in use. As such, this version must be
-    # incremented whenever a new patch is added above. When no patches exist,
-    # then this should be undefined. Otherwise, it must be set to at least
-    # `gl1` given that `0` is the "default" GitLab patch level.
-    GIT_EXTRA_VERSION := gl1
-
-    # Before adding custom patches, please read doc/PROCESS.md#Patching-git
-    # first to make sure your patches meet our acceptance criteria. Patches
-    # must be put into `_support/git-patches`.
-    GIT_PATCHES := $(sort $(wildcard ${SOURCE_DIR}/_support/git-patches/v2.37.1.gl1/*))
+    override GIT_VERSION := v2.37.1.gl1
 else
     # Support both vX.Y.Z and X.Y.Z version patterns, since callers across GitLab
     # use both.
@@ -582,14 +573,11 @@ ${DEPENDENCY_DIR}/git-distribution/git: ${DEPENDENCY_DIR}/git-distribution/Makef
 	${Q}env -u PROFILE -u MAKEFLAGS -u GIT_VERSION ${MAKE} -C "$(<D)" -j$(shell nproc) prefix=${GIT_PREFIX} ${GIT_BUILD_OPTIONS}
 	${Q}touch $@
 
-${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: override GIT_PATCHES := $(sort $(wildcard ${SOURCE_DIR}/_support/git-patches/v2.37.1.gl1/*))
-${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: override GIT_VERSION = v2.37.1
-${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: override GIT_EXTRA_VERSION = gl1
+${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: override GIT_VERSION = ${GIT_VERSION_2_37_1}
 ${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: ${DEPENDENCY_DIR}/git-v2.37.1.gl1/% | ${BUILD_DIR}/bin
 	${Q}install $< $@
 
-${BUILD_DIR}/bin/gitaly-%-v2.38: override GIT_VERSION = v2.38.1
-${BUILD_DIR}/bin/gitaly-%-v2.38: override GIT_EXTRA_VERSION = gl0
+${BUILD_DIR}/bin/gitaly-%-v2.38: override GIT_VERSION = ${GIT_VERSION_2_38}
 ${BUILD_DIR}/bin/gitaly-%-v2.38: ${DEPENDENCY_DIR}/git-v2.38/% | ${BUILD_DIR}/bin
 	${Q}install $< $@
 
@@ -643,7 +631,7 @@ ${BUILD_DIR}/intermediate/%:                 clear-go-build-cache-if-needed .FOR
 ${DEPENDENCY_DIR}/libgit2.version: dependency-version | ${DEPENDENCY_DIR}
 	${Q}[ x"$$(cat "$@" 2>/dev/null)" = x"${LIBGIT2_VERSION} ${LIBGIT2_BUILD_OPTIONS}" ] || >$@ echo -n "${LIBGIT2_VERSION} ${LIBGIT2_BUILD_OPTIONS}"
 ${DEPENDENCY_DIR}/git-%.version: dependency-version | ${DEPENDENCY_DIR}
-	${Q}[ x"$$(cat "$@" 2>/dev/null)" = x"${GIT_VERSION}.${GIT_EXTRA_VERSION} ${GIT_BUILD_OPTIONS} ${GIT_PATCHES}" ] || >$@ echo -n "${GIT_VERSION}.${GIT_EXTRA_VERSION} ${GIT_BUILD_OPTIONS} ${GIT_PATCHES}"
+	${Q}[ x"$$(cat "$@" 2>/dev/null)" = x"${GIT_VERSION} ${GIT_BUILD_OPTIONS}" ] || >$@ echo -n "${GIT_VERSION} ${GIT_BUILD_OPTIONS}"
 ${DEPENDENCY_DIR}/protoc.version: dependency-version | ${DEPENDENCY_DIR}
 	${Q}[ x"$$(cat "$@" 2>/dev/null)" = x"${PROTOC_VERSION} ${PROTOC_BUILD_OPTIONS}" ] || >$@ echo -n "${PROTOC_VERSION} ${PROTOC_BUILD_OPTIONS}"
 
@@ -672,11 +660,10 @@ ${DEPENDENCY_DIR}/git-%/Makefile: ${DEPENDENCY_DIR}/git-%.version
 	${Q}${GIT} -C "${@D}" fetch --depth 1 ${GIT_QUIET} origin ${GIT_VERSION}
 	${Q}${GIT} -C "${@D}" reset --hard
 	${Q}${GIT} -C "${@D}" checkout ${GIT_QUIET} --detach FETCH_HEAD
-	${Q}if test -n "${GIT_PATCHES}"; then ${GIT} -C "${@D}" apply ${GIT_PATCHES}; fi
 	@ # We're writing the version into the "version" file in Git's own source
 	@ # directory. If it exists, Git's Makefile will pick it up and use it as
 	@ # the version instead of auto-detecting via git-describe(1).
-	${Q}if test -n "${GIT_EXTRA_VERSION}"; then echo ${GIT_VERSION}.${GIT_EXTRA_VERSION} >"${@D}"/version; else rm -f "${@D}"/version; fi
+	${Q}echo ${GIT_VERSION} >"${@D}"/version
 	${Q}touch $@
 
 $(patsubst %,${DEPENDENCY_DIR}/git-\%/%,${GIT_EXECUTABLES}): ${DEPENDENCY_DIR}/git-%/Makefile
diff --git a/_support/git-patches/v2.37.1.gl1/0001-refs-extract-packed_refs_delete_refs-to-allow-contro.patch b/_support/git-patches/v2.37.1.gl1/0001-refs-extract-packed_refs_delete_refs-to-allow-contro.patch
deleted file mode 100644
index 47dd3e41c..000000000
--- a/_support/git-patches/v2.37.1.gl1/0001-refs-extract-packed_refs_delete_refs-to-allow-contro.patch
+++ /dev/null
@@ -1,156 +0,0 @@
-From c74f385fb46855ac0db222b6845ddb95e6a36264 Mon Sep 17 00:00:00 2001
-Message-Id: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:31 +0100
-Subject: [PATCH 21/34] refs: extract packed_refs_delete_refs() to allow
- control of transaction
-
-When deleting loose refs, then we also have to delete the refs in the
-packed backend. This is done by calling `refs_delete_refs()`, which
-then uses the packed-backend's logic to delete refs. This doesn't allow
-us to exercise any control over the reference transaction which is being
-created in the packed backend, which is required in a subsequent commit.
-
-Extract a new function `packed_refs_delete_refs()`, which hosts most of
-the logic to delete refs except for creating the transaction itself.
-Like this, we can easily create the transaction in the files backend
-and thus exert more control over it.
-
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit 69840cc0f7b4f3352903bd2b8f3de7077916c26b)
----
- refs/files-backend.c  | 13 ++++++++++---
- refs/packed-backend.c | 26 ++++++++++++++++++++------
- refs/packed-backend.h |  7 +++++++
- 3 files changed, 37 insertions(+), 9 deletions(-)
-
-diff --git a/refs/files-backend.c b/refs/files-backend.c
-index 43a3b882d7..18baea4c6a 100644
---- a/refs/files-backend.c
-+++ b/refs/files-backend.c
-@@ -1244,6 +1244,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- {
- 	struct files_ref_store *refs =
- 		files_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
-+	struct ref_transaction *transaction = NULL;
- 	struct strbuf err = STRBUF_INIT;
- 	int i, result = 0;
- 
-@@ -1253,10 +1254,14 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- 	if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- 		goto error;
- 
--	if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {
--		packed_refs_unlock(refs->packed_ref_store);
-+	transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
-+	if (!transaction)
-+		goto error;
-+
-+	result = packed_refs_delete_refs(refs->packed_ref_store,
-+					 transaction, msg, refnames, flags);
-+	if (result)
- 		goto error;
--	}
- 
- 	packed_refs_unlock(refs->packed_ref_store);
- 
-@@ -1267,6 +1272,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- 			result |= error(_("could not remove reference %s"), refname);
- 	}
- 
-+	ref_transaction_free(transaction);
- 	strbuf_release(&err);
- 	return result;
- 
-@@ -1283,6 +1289,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- 	else
- 		error(_("could not delete references: %s"), err.buf);
- 
-+	ref_transaction_free(transaction);
- 	strbuf_release(&err);
- 	return -1;
- }
-diff --git a/refs/packed-backend.c b/refs/packed-backend.c
-index d91a2018f6..960b43ff76 100644
---- a/refs/packed-backend.c
-+++ b/refs/packed-backend.c
-@@ -1521,15 +1521,10 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store,
- static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- 			     struct string_list *refnames, unsigned int flags)
- {
--	struct packed_ref_store *refs =
--		packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
- 	struct strbuf err = STRBUF_INIT;
- 	struct ref_transaction *transaction;
--	struct string_list_item *item;
- 	int ret;
- 
--	(void)refs; /* We need the check above, but don't use the variable */
--
- 	if (!refnames->nr)
- 		return 0;
- 
-@@ -1543,6 +1538,26 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- 	if (!transaction)
- 		return -1;
- 
-+	ret = packed_refs_delete_refs(ref_store, transaction,
-+				      msg, refnames, flags);
-+
-+	ref_transaction_free(transaction);
-+	return ret;
-+}
-+
-+int packed_refs_delete_refs(struct ref_store *ref_store,
-+			    struct ref_transaction *transaction,
-+			    const char *msg,
-+			    struct string_list *refnames,
-+			    unsigned int flags)
-+{
-+	struct strbuf err = STRBUF_INIT;
-+	struct string_list_item *item;
-+	int ret;
-+
-+	/* Assert that the ref store refers to a packed backend. */
-+	packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
-+
- 	for_each_string_list_item(item, refnames) {
- 		if (ref_transaction_delete(transaction, item->string, NULL,
- 					   flags, msg, &err)) {
-@@ -1562,7 +1577,6 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- 			error(_("could not delete references: %s"), err.buf);
- 	}
- 
--	ref_transaction_free(transaction);
- 	strbuf_release(&err);
- 	return ret;
- }
-diff --git a/refs/packed-backend.h b/refs/packed-backend.h
-index 9dd8a344c3..52e0490753 100644
---- a/refs/packed-backend.h
-+++ b/refs/packed-backend.h
-@@ -3,6 +3,7 @@
- 
- struct repository;
- struct ref_transaction;
-+struct string_list;
- 
- /*
-  * Support for storing references in a `packed-refs` file.
-@@ -27,6 +28,12 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
- void packed_refs_unlock(struct ref_store *ref_store);
- int packed_refs_is_locked(struct ref_store *ref_store);
- 
-+int packed_refs_delete_refs(struct ref_store *ref_store,
-+			    struct ref_transaction *transaction,
-+			    const char *msg,
-+			    struct string_list *refnames,
-+			    unsigned int flags);
-+
- /*
-  * Return true if `transaction` really needs to be carried out against
-  * the specified packed_ref_store, or false if it can be skipped
--- 
-2.35.1
-
diff --git a/_support/git-patches/v2.37.1.gl1/0002-refs-allow-passing-flags-when-beginning-transactions.patch b/_support/git-patches/v2.37.1.gl1/0002-refs-allow-passing-flags-when-beginning-transactions.patch
deleted file mode 100644
index 8038daca2..000000000
--- a/_support/git-patches/v2.37.1.gl1/0002-refs-allow-passing-flags-when-beginning-transactions.patch
+++ /dev/null
@@ -1,183 +0,0 @@
-From 60d0682e02db5c197c09bc4cd5a643dc2269cca2 Mon Sep 17 00:00:00 2001
-Message-Id: <60d0682e02db5c197c09bc4cd5a643dc2269cca2.1646206541.git.ps@pks.im>
-In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:35 +0100
-Subject: [PATCH 22/34] refs: allow passing flags when beginning transactions
-
-We do not currently have any flags when creating reference transactions,
-but we'll add one to disable execution of the reference transaction hook
-in some cases.
-
-Allow passing flags to `ref_store_transaction_begin()` to prepare for
-this change.
-
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit fbe73f61cbc29f6c4a85478cf792c37dbe5aa26c)
----
- refs.c                |  8 +++++---
- refs.h                |  3 ++-
- refs/files-backend.c  | 10 +++++-----
- refs/packed-backend.c |  2 +-
- refs/refs-internal.h  |  1 +
- sequencer.c           |  2 +-
- 6 files changed, 15 insertions(+), 11 deletions(-)
-
-diff --git a/refs.c b/refs.c
-index addb26293b..f498d232e5 100644
---- a/refs.c
-+++ b/refs.c
-@@ -800,7 +800,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
- 	struct ref_transaction *transaction;
- 	struct strbuf err = STRBUF_INIT;
- 
--	transaction = ref_store_transaction_begin(refs, &err);
-+	transaction = ref_store_transaction_begin(refs, 0, &err);
- 	if (!transaction ||
- 	    ref_transaction_delete(transaction, refname, old_oid,
- 				   flags, msg, &err) ||
-@@ -1005,6 +1005,7 @@ int read_ref_at(struct ref_store *refs, const char *refname,
- }
- 
- struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
-+						    unsigned int flags,
- 						    struct strbuf *err)
- {
- 	struct ref_transaction *tr;
-@@ -1012,12 +1013,13 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
- 
- 	CALLOC_ARRAY(tr, 1);
- 	tr->ref_store = refs;
-+	tr->flags = flags;
- 	return tr;
- }
- 
- struct ref_transaction *ref_transaction_begin(struct strbuf *err)
- {
--	return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
-+	return ref_store_transaction_begin(get_main_ref_store(the_repository), 0, err);
- }
- 
- void ref_transaction_free(struct ref_transaction *transaction)
-@@ -1156,7 +1158,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
- 	struct strbuf err = STRBUF_INIT;
- 	int ret = 0;
- 
--	t = ref_store_transaction_begin(refs, &err);
-+	t = ref_store_transaction_begin(refs, 0, &err);
- 	if (!t ||
- 	    ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,
- 				   &err) ||
-diff --git a/refs.h b/refs.h
-index 8f91a7f9ff..3a141d7066 100644
---- a/refs.h
-+++ b/refs.h
-@@ -231,7 +231,7 @@ char *repo_default_branch_name(struct repository *r, int quiet);
-  *         struct strbuf err = STRBUF_INIT;
-  *         int ret = 0;
-  *
-- *         transaction = ref_store_transaction_begin(refs, &err);
-+ *         transaction = ref_store_transaction_begin(refs, 0, &err);
-  *         if (!transaction ||
-  *             ref_transaction_update(...) ||
-  *             ref_transaction_create(...) ||
-@@ -573,6 +573,7 @@ enum action_on_err {
-  * be freed by calling ref_transaction_free().
-  */
- struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
-+						    unsigned int flags,
- 						    struct strbuf *err);
- struct ref_transaction *ref_transaction_begin(struct strbuf *err);
- 
-diff --git a/refs/files-backend.c b/refs/files-backend.c
-index 18baea4c6a..758d12a0fa 100644
---- a/refs/files-backend.c
-+++ b/refs/files-backend.c
-@@ -1116,7 +1116,7 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)
- 	if (check_refname_format(r->name, 0))
- 		return;
- 
--	transaction = ref_store_transaction_begin(&refs->base, &err);
-+	transaction = ref_store_transaction_begin(&refs->base, 0, &err);
- 	if (!transaction)
- 		goto cleanup;
- 	ref_transaction_add_update(
-@@ -1187,7 +1187,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
- 	struct strbuf err = STRBUF_INIT;
- 	struct ref_transaction *transaction;
- 
--	transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
-+	transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err);
- 	if (!transaction)
- 		return -1;
- 
-@@ -1254,7 +1254,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- 	if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- 		goto error;
- 
--	transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
-+	transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err);
- 	if (!transaction)
- 		goto error;
- 
-@@ -2769,7 +2769,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
- 			 */
- 			if (!packed_transaction) {
- 				packed_transaction = ref_store_transaction_begin(
--						refs->packed_ref_store, err);
-+						refs->packed_ref_store, 0, err);
- 				if (!packed_transaction) {
- 					ret = TRANSACTION_GENERIC_ERROR;
- 					goto cleanup;
-@@ -3040,7 +3040,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
- 				 &affected_refnames))
- 		BUG("initial ref transaction called with existing refs");
- 
--	packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, err);
-+	packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, err);
- 	if (!packed_transaction) {
- 		ret = TRANSACTION_GENERIC_ERROR;
- 		goto cleanup;
-diff --git a/refs/packed-backend.c b/refs/packed-backend.c
-index 960b43ff76..27dd8c3922 100644
---- a/refs/packed-backend.c
-+++ b/refs/packed-backend.c
-@@ -1534,7 +1534,7 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
- 	 * updates into a single transaction.
- 	 */
- 
--	transaction = ref_store_transaction_begin(ref_store, &err);
-+	transaction = ref_store_transaction_begin(ref_store, 0, &err);
- 	if (!transaction)
- 		return -1;
- 
-diff --git a/refs/refs-internal.h b/refs/refs-internal.h
-index 7ff6fba4f0..6e15db3ca4 100644
---- a/refs/refs-internal.h
-+++ b/refs/refs-internal.h
-@@ -213,6 +213,7 @@ struct ref_transaction {
- 	size_t nr;
- 	enum ref_transaction_state state;
- 	void *backend_data;
-+	unsigned int flags;
- };
- 
- /*
-diff --git a/sequencer.c b/sequencer.c
-index 5213d16e97..5a2b609557 100644
---- a/sequencer.c
-+++ b/sequencer.c
-@@ -3588,7 +3588,7 @@ static int do_label(struct repository *r, const char *name, int len)
- 	strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
- 	strbuf_addf(&msg, "rebase (label) '%.*s'", len, name);
- 
--	transaction = ref_store_transaction_begin(refs, &err);
-+	transaction = ref_store_transaction_begin(refs, 0, &err);
- 	if (!transaction) {
- 		error("%s", err.buf);
- 		ret = -1;
--- 
-2.35.1
-
diff --git a/_support/git-patches/v2.37.1.gl1/0003-refs-allow-skipping-the-reference-transaction-hook.patch b/_support/git-patches/v2.37.1.gl1/0003-refs-allow-skipping-the-reference-transaction-hook.patch
deleted file mode 100644
index cd194f8aa..000000000
--- a/_support/git-patches/v2.37.1.gl1/0003-refs-allow-skipping-the-reference-transaction-hook.patch
+++ /dev/null
@@ -1,60 +0,0 @@
-From 4eccd16b45516df5ab02288d0c50c16e03cc44e4 Mon Sep 17 00:00:00 2001
-Message-Id: <4eccd16b45516df5ab02288d0c50c16e03cc44e4.1646206541.git.ps@pks.im>
-In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:39 +0100
-Subject: [PATCH 23/34] refs: allow skipping the reference-transaction hook
-
-The reference-transaction hook is executing whenever we prepare, commit
-or abort a reference transaction. While this is mostly intentional, in
-case of the files backend we're leaking the implementation detail that
-the store is in fact a composite store with one loose and one packed
-backend to the caller. So while we want to execute the hook for all
-logical updates, executing it for such implementation details is
-unexpected.
-
-Prepare for a fix by adding a new flag which allows to skip execution of
-the hook.
-
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit 958fbc74e3d0fcc88b2065190e23db556a963644)
----
- refs.c | 3 +++
- refs.h | 5 +++++
- 2 files changed, 8 insertions(+)
-
-diff --git a/refs.c b/refs.c
-index f498d232e5..435b90b1ec 100644
---- a/refs.c
-+++ b/refs.c
-@@ -2084,6 +2084,9 @@ static int run_transaction_hook(struct ref_transaction *transaction,
- 	const char *hook;
- 	int ret = 0, i;
- 
-+	if (transaction->flags & REF_TRANSACTION_SKIP_HOOK)
-+		return 0;
-+
- 	hook = find_hook("reference-transaction");
- 	if (!hook)
- 		return ret;
-diff --git a/refs.h b/refs.h
-index 3a141d7066..a015354fd6 100644
---- a/refs.h
-+++ b/refs.h
-@@ -568,6 +568,11 @@ enum action_on_err {
- 	UPDATE_REFS_QUIET_ON_ERR
- };
- 
-+/*
-+ * Skip executing the reference-transaction hook.
-+ */
-+#define REF_TRANSACTION_SKIP_HOOK (1 << 0)
-+
- /*
-  * Begin a reference transaction.  The reference transaction must
-  * be freed by calling ref_transaction_free().
--- 
-2.35.1
-
diff --git a/_support/git-patches/v2.37.1.gl1/0004-refs-demonstrate-excessive-execution-of-the-referenc.patch b/_support/git-patches/v2.37.1.gl1/0004-refs-demonstrate-excessive-execution-of-the-referenc.patch
deleted file mode 100644
index aa6d96a3e..000000000
--- a/_support/git-patches/v2.37.1.gl1/0004-refs-demonstrate-excessive-execution-of-the-referenc.patch
+++ /dev/null
@@ -1,97 +0,0 @@
-From 079f96ed16bb70f99fbe810b1646b1709bb82871 Mon Sep 17 00:00:00 2001
-Message-Id: <079f96ed16bb70f99fbe810b1646b1709bb82871.1646206541.git.ps@pks.im>
-In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:44 +0100
-Subject: [PATCH 24/34] refs: demonstrate excessive execution of the
- reference-transaction hook
-
-Add tests which demonstate that we're executing the
-reference-transaction hook too often in some cases, which thus leaks
-implementation details about the reference store's implementation
-itself. Behaviour will be fixed in follow-up commits.
-
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit 2ce825436268d6409bee8ebb5f5500b7ff172b1e)
----
- t/t1416-ref-transaction-hooks.sh | 64 ++++++++++++++++++++++++++++++++
- 1 file changed, 64 insertions(+)
-
-diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
-index 6c941027a8..0567fbdf0b 100755
---- a/t/t1416-ref-transaction-hooks.sh
-+++ b/t/t1416-ref-transaction-hooks.sh
-@@ -136,4 +136,68 @@ test_expect_success 'interleaving hook calls succeed' '
- 	test_cmp expect target-repo.git/actual
- '
- 
-+test_expect_success 'hook does not get called on packing refs' '
-+	# Pack references first such that we are in a known state.
-+	git pack-refs --all &&
-+
-+	write_script .git/hooks/reference-transaction <<-\EOF &&
-+		echo "$@" >>actual
-+		cat >>actual
-+	EOF
-+	rm -f actual &&
-+
-+	git update-ref refs/heads/unpacked-ref $POST_OID &&
-+	git pack-refs --all &&
-+
-+	# We only expect a single hook invocation, which is the call to
-+	# git-update-ref(1). But currently, packing refs will also trigger the
-+	# hook.
-+	cat >expect <<-EOF &&
-+		prepared
-+		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-+		committed
-+		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-+		prepared
-+		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-+		committed
-+		$ZERO_OID $POST_OID refs/heads/unpacked-ref
-+		prepared
-+		$POST_OID $ZERO_OID refs/heads/unpacked-ref
-+		committed
-+		$POST_OID $ZERO_OID refs/heads/unpacked-ref
-+	EOF
-+
-+	test_cmp expect actual
-+'
-+
-+test_expect_success 'deleting packed ref calls hook once' '
-+	# Create a reference and pack it.
-+	git update-ref refs/heads/to-be-deleted $POST_OID &&
-+	git pack-refs --all &&
-+
-+	write_script .git/hooks/reference-transaction <<-\EOF &&
-+		echo "$@" >>actual
-+		cat >>actual
-+	EOF
-+	rm -f actual &&
-+
-+	git update-ref -d refs/heads/to-be-deleted $POST_OID &&
-+
-+	# We only expect a single hook invocation, which is the logical
-+	# deletion. But currently, we see two interleaving transactions, once
-+	# for deleting the loose refs and once for deleting the packed ref.
-+	cat >expect <<-EOF &&
-+		prepared
-+		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
-+		prepared
-+		$POST_OID $ZERO_OID refs/heads/to-be-deleted
-+		committed
-+		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
-+		committed
-+		$POST_OID $ZERO_OID refs/heads/to-be-deleted
-+	EOF
-+
-+	test_cmp expect actual
-+'
-+
- test_done
--- 
-2.35.1
-
diff --git a/_support/git-patches/v2.37.1.gl1/0005-refs-do-not-execute-reference-transaction-hook-on-pa.patch b/_support/git-patches/v2.37.1.gl1/0005-refs-do-not-execute-reference-transaction-hook-on-pa.patch
deleted file mode 100644
index f2e7c06ed..000000000
--- a/_support/git-patches/v2.37.1.gl1/0005-refs-do-not-execute-reference-transaction-hook-on-pa.patch
+++ /dev/null
@@ -1,81 +0,0 @@
-From ef35ab926309bf406d3871679c576c718708a93b Mon Sep 17 00:00:00 2001
-Message-Id: <ef35ab926309bf406d3871679c576c718708a93b.1646206541.git.ps@pks.im>
-In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:48 +0100
-Subject: [PATCH 25/34] refs: do not execute reference-transaction hook on
- packing refs
-
-The reference-transaction hook is supposed to track logical changes to
-references, but it currently also gets executed when packing refs in a
-repository. This is unexpected and ultimately not all that useful:
-packing refs is not supposed to result in any user-visible change to the
-refs' state, and it ultimately is an implementation detail of how refs
-stores work.
-
-Fix this excessive execution of the hook when packing refs.
-
-Reported-by: Waleed Khan <me@waleedkhan.name>
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit ffad9941383465553bf26d88050f3243726f30df)
----
- refs/files-backend.c             |  6 ++++--
- t/t1416-ref-transaction-hooks.sh | 11 +----------
- 2 files changed, 5 insertions(+), 12 deletions(-)
-
-diff --git a/refs/files-backend.c b/refs/files-backend.c
-index 758d12a0fa..19f43e8d29 100644
---- a/refs/files-backend.c
-+++ b/refs/files-backend.c
-@@ -1116,7 +1116,8 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)
- 	if (check_refname_format(r->name, 0))
- 		return;
- 
--	transaction = ref_store_transaction_begin(&refs->base, 0, &err);
-+	transaction = ref_store_transaction_begin(&refs->base,
-+						  REF_TRANSACTION_SKIP_HOOK, &err);
- 	if (!transaction)
- 		goto cleanup;
- 	ref_transaction_add_update(
-@@ -1187,7 +1188,8 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
- 	struct strbuf err = STRBUF_INIT;
- 	struct ref_transaction *transaction;
- 
--	transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err);
-+	transaction = ref_store_transaction_begin(refs->packed_ref_store,
-+						  REF_TRANSACTION_SKIP_HOOK, &err);
- 	if (!transaction)
- 		return -1;
- 
-diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
-index 0567fbdf0b..f9d3d5213f 100755
---- a/t/t1416-ref-transaction-hooks.sh
-+++ b/t/t1416-ref-transaction-hooks.sh
-@@ -150,21 +150,12 @@ test_expect_success 'hook does not get called on packing refs' '
- 	git pack-refs --all &&
- 
- 	# We only expect a single hook invocation, which is the call to
--	# git-update-ref(1). But currently, packing refs will also trigger the
--	# hook.
-+	# git-update-ref(1).
- 	cat >expect <<-EOF &&
- 		prepared
- 		$ZERO_OID $POST_OID refs/heads/unpacked-ref
- 		committed
- 		$ZERO_OID $POST_OID refs/heads/unpacked-ref
--		prepared
--		$ZERO_OID $POST_OID refs/heads/unpacked-ref
--		committed
--		$ZERO_OID $POST_OID refs/heads/unpacked-ref
--		prepared
--		$POST_OID $ZERO_OID refs/heads/unpacked-ref
--		committed
--		$POST_OID $ZERO_OID refs/heads/unpacked-ref
- 	EOF
- 
- 	test_cmp expect actual
--- 
-2.35.1
-
diff --git a/_support/git-patches/v2.37.1.gl1/0006-refs-skip-hooks-when-deleting-uncovered-packed-refs.patch b/_support/git-patches/v2.37.1.gl1/0006-refs-skip-hooks-when-deleting-uncovered-packed-refs.patch
deleted file mode 100644
index 3b21bf489..000000000
--- a/_support/git-patches/v2.37.1.gl1/0006-refs-skip-hooks-when-deleting-uncovered-packed-refs.patch
+++ /dev/null
@@ -1,99 +0,0 @@
-From d56f2a0e1eed4d0a39b99638117e0c8259e5078d Mon Sep 17 00:00:00 2001
-Message-Id: <d56f2a0e1eed4d0a39b99638117e0c8259e5078d.1646206542.git.ps@pks.im>
-In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im>
-From: Patrick Steinhardt <ps@pks.im>
-Date: Mon, 17 Jan 2022 09:12:53 +0100
-Subject: [PATCH 26/34] refs: skip hooks when deleting uncovered packed refs
-
-When deleting refs from the loose-files refs backend, then we need to be
-careful to also delete the same ref from the packed refs backend, if it
-exists. If we don't, then deleting the loose ref would "uncover" the
-packed ref. We thus always have to queue up deletions of refs for both
-the loose and the packed refs backend. This is done in two separate
-transactions, where the end result is that the reference-transaction
-hook is executed twice for the deleted refs.
-
-This behaviour is quite misleading: it's exposing implementation details
-of how the files backend works to the user, in contrast to the logical
-updates that we'd really want to expose via the hook. Worse yet, whether
-the hook gets executed once or twice depends on how well-packed the
-repository is: if the ref only exists as a loose ref, then we execute it
-once, otherwise if it is also packed then we execute it twice.
-
-Fix this behaviour and don't execute the reference-transaction hook at
-all when refs in the packed-refs backend if it's driven by the files
-backend. This works as expected even in case the refs to be deleted only
-exist in the packed-refs backend because the loose-backend always queues
-refs in its own transaction even if they don't exist such that they can
-be locked for concurrent creation. And it also does the right thing in
-case neither of the backends has the ref because that would cause the
-transaction to fail completely.
-
-Signed-off-by: Patrick Steinhardt <ps@pks.im>
-Signed-off-by: Junio C Hamano <gitster@pobox.com>
-(cherry picked from commit 2ed1b64ebdeefc7f9473ae159fb45ff0c6cf121a)
----
- refs/files-backend.c             | 9 ++++++---
- t/t1416-ref-transaction-hooks.sh | 7 +------
- 2 files changed, 7 insertions(+), 9 deletions(-)
-
-diff --git a/refs/files-backend.c b/refs/files-backend.c
-index 19f43e8d29..c931dd479c 100644
---- a/refs/files-backend.c
-+++ b/refs/files-backend.c
-@@ -1256,7 +1256,8 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
- 	if (packed_refs_lock(refs->packed_ref_store, 0, &err))
- 		goto error;
- 
--	transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, &err);
-+	transaction = ref_store_transaction_begin(refs->packed_ref_store,
-+						  REF_TRANSACTION_SKIP_HOOK, &err);
- 	if (!transaction)
- 		goto error;
- 
-@@ -2771,7 +2772,8 @@ static int files_transaction_prepare(struct ref_store *ref_store,
- 			 */
- 			if (!packed_transaction) {
- 				packed_transaction = ref_store_transaction_begin(
--						refs->packed_ref_store, 0, err);
-+						refs->packed_ref_store,
-+						REF_TRANSACTION_SKIP_HOOK, err);
- 				if (!packed_transaction) {
- 					ret = TRANSACTION_GENERIC_ERROR;
- 					goto cleanup;
-@@ -3042,7 +3044,8 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
- 				 &affected_refnames))
- 		BUG("initial ref transaction called with existing refs");
- 
--	packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, 0, err);
-+	packed_transaction = ref_store_transaction_begin(refs->packed_ref_store,
-+							 REF_TRANSACTION_SKIP_HOOK, err);
- 	if (!packed_transaction) {
- 		ret = TRANSACTION_GENERIC_ERROR;
- 		goto cleanup;
-diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
-index f9d3d5213f..4e1e84a91f 100755
---- a/t/t1416-ref-transaction-hooks.sh
-+++ b/t/t1416-ref-transaction-hooks.sh
-@@ -175,16 +175,11 @@ test_expect_success 'deleting packed ref calls hook once' '
- 	git update-ref -d refs/heads/to-be-deleted $POST_OID &&
- 
- 	# We only expect a single hook invocation, which is the logical
--	# deletion. But currently, we see two interleaving transactions, once
--	# for deleting the loose refs and once for deleting the packed ref.
-+	# deletion.
- 	cat >expect <<-EOF &&
--		prepared
--		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
- 		prepared
- 		$POST_OID $ZERO_OID refs/heads/to-be-deleted
- 		committed
--		$ZERO_OID $ZERO_OID refs/heads/to-be-deleted
--		committed
- 		$POST_OID $ZERO_OID refs/heads/to-be-deleted
- 	EOF
- 
--- 
-2.35.1
-