diff options
| author | Karthik Nayak <knayak@gitlab.com> | 2022-09-19 17:51:32 +0300 |
|---|---|---|
| committer | Karthik Nayak <knayak@gitlab.com> | 2022-10-21 11:55:05 +0300 |
| commit | 248e71d0921d44a6fc3dfbdbb7402b02962bebf7 (patch) | |
| tree | fd64a89f25a87f4d86f7788923adb46a3d0b1eb8 /_support | |
| parent | 9092f5d9aac102a9d9cc18ac5c4c34b6ed2301a7 (diff) | |
Makefile: Remove bundled git v2.35.0
In the commit (cc04215eb) we removed the flag to enable git v2.37.0.
Making it now the default git version. Now we can remove the older git
version v2.35.0. In this commit remove it from the Makefile. This means
it will no longer be bundled with Gitaly.
Also remove the patches added for git v2.35.0, which are no longer
required.
Diffstat (limited to '_support')
21 files changed, 0 insertions, 2643 deletions
diff --git a/_support/git-patches/v2.35.1.gl1/0019-fetch-pack-use-commit-graph-when-computing-cutoff.patch b/_support/git-patches/v2.35.1.gl1/0019-fetch-pack-use-commit-graph-when-computing-cutoff.patch deleted file mode 100644 index 855bcc193..000000000 --- a/_support/git-patches/v2.35.1.gl1/0019-fetch-pack-use-commit-graph-when-computing-cutoff.patch +++ /dev/null @@ -1,92 +0,0 @@ -From 6fd1cc8f985ccd8b014e945a819482b267dae21f Mon Sep 17 00:00:00 2001 -Message-Id: <6fd1cc8f985ccd8b014e945a819482b267dae21f.1645001444.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Thu, 10 Feb 2022 13:28:09 +0100 -Subject: [PATCH 1/2] fetch-pack: use commit-graph when computing cutoff -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -During packfile negotiation we iterate over all refs announced by the -remote side to check whether their IDs refer to commits already known to -us. If a commit is known to us already, then its date is a potential -cutoff point for commits we have in common with the remote side. - -There is potentially a lot of commits announced by the remote depending -on how many refs there are in the remote repository, and for every one -of them we need to search for it in our object database and, if found, -parse the corresponding object to find out whether it is a candidate for -the cutoff date. This can be sped up by trying to look up commits via -the commit-graph first, which is a lot more efficient. - -Benchmarks in a repository with about 2,1 million refs and an up-to-date -commit-graph show an almost 20% speedup when mirror-fetching: - - Benchmark 1: git fetch +refs/*:refs/* (v2.35.0) - Time (mean ± σ): 115.587 s ± 2.009 s [User: 109.874 s, System: 11.305 s] - Range (min … max): 113.584 s … 118.820 s 5 runs - - Benchmark 2: git fetch +refs/*:refs/* (HEAD) - Time (mean ± σ): 96.859 s ± 0.624 s [User: 91.948 s, System: 10.980 s] - Range (min … max): 96.180 s … 97.875 s 5 runs - - Summary - 'git fetch +refs/*:refs/* (HEAD)' ran - 1.19 ± 0.02 times faster than 'git fetch +refs/*:refs/* (v2.35.0)' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - fetch-pack.c | 28 ++++++++++++++++------------ - 1 file changed, 16 insertions(+), 12 deletions(-) - -diff --git a/fetch-pack.c b/fetch-pack.c -index dd6ec449f2..c5967e228e 100644 ---- a/fetch-pack.c -+++ b/fetch-pack.c -@@ -696,26 +696,30 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, - - trace2_region_enter("fetch-pack", "parse_remote_refs_and_find_cutoff", NULL); - for (ref = *refs; ref; ref = ref->next) { -- struct object *o; -+ struct commit *commit; - -- if (!has_object_file_with_flags(&ref->old_oid, -+ commit = lookup_commit_in_graph(the_repository, &ref->old_oid); -+ if (!commit) { -+ struct object *o; -+ -+ if (!has_object_file_with_flags(&ref->old_oid, - OBJECT_INFO_QUICK | -- OBJECT_INFO_SKIP_FETCH_OBJECT)) -- continue; -- o = parse_object(the_repository, &ref->old_oid); -- if (!o) -- continue; -+ OBJECT_INFO_SKIP_FETCH_OBJECT)) -+ continue; -+ o = parse_object(the_repository, &ref->old_oid); -+ if (!o || o->type != OBJ_COMMIT) -+ continue; -+ -+ commit = (struct commit *)o; -+ } - - /* - * We already have it -- which may mean that we were - * in sync with the other side at some time after - * that (it is OK if we guess wrong here). - */ -- if (o->type == OBJ_COMMIT) { -- struct commit *commit = (struct commit *)o; -- if (!cutoff || cutoff < commit->date) -- cutoff = commit->date; -- } -+ if (!cutoff || cutoff < commit->date) -+ cutoff = commit->date; - } - trace2_region_leave("fetch-pack", "parse_remote_refs_and_find_cutoff", NULL); - --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0020-fetch-skip-computing-output-width-when-not-printing-.patch b/_support/git-patches/v2.35.1.gl1/0020-fetch-skip-computing-output-width-when-not-printing-.patch deleted file mode 100644 index 2ef3c109a..000000000 --- a/_support/git-patches/v2.35.1.gl1/0020-fetch-skip-computing-output-width-when-not-printing-.patch +++ /dev/null @@ -1,84 +0,0 @@ -From b18aaaa5e931d79d057f68ac0d7c3dd0377e5f03 Mon Sep 17 00:00:00 2001 -Message-Id: <b18aaaa5e931d79d057f68ac0d7c3dd0377e5f03.1645001444.git.ps@pks.im> -In-Reply-To: <6fd1cc8f985ccd8b014e945a819482b267dae21f.1645001444.git.ps@pks.im> -References: <6fd1cc8f985ccd8b014e945a819482b267dae21f.1645001444.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Thu, 10 Feb 2022 13:28:16 +0100 -Subject: [PATCH 2/2] fetch: skip computing output width when not printing - anything -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -When updating references via git-fetch(1), then by default we report to -the user which references have been changed. This output is formatted in -a nice table such that the different columns are aligned. Because the -first column contains abbreviated object IDs we thus need to iterate -over all refs which have changed and compute the minimum length for -their respective abbreviated hashes. While this effort makes sense in -most cases, it is wasteful when the user passes the `--quiet` flag: we -don't print the summary, but still compute the length. - -Skip computing the summary width when the user asked for us to be quiet. -This gives us a speedup of nearly 10% when doing a mirror-fetch in a -repository with thousands of references being updated: - - Benchmark 1: git fetch --quiet +refs/*:refs/* (HEAD~) - Time (mean ± σ): 96.078 s ± 0.508 s [User: 91.378 s, System: 10.870 s] - Range (min … max): 95.449 s … 96.760 s 5 runs - - Benchmark 2: git fetch --quiet +refs/*:refs/* (HEAD) - Time (mean ± σ): 88.214 s ± 0.192 s [User: 83.274 s, System: 10.978 s] - Range (min … max): 87.998 s … 88.446 s 5 runs - - Summary - 'git fetch --quiet +refs/*:refs/* (HEAD)' ran - 1.09 ± 0.01 times faster than 'git fetch --quiet +refs/*:refs/* (HEAD~)' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - builtin/fetch.c | 8 ++++++-- - 1 file changed, 6 insertions(+), 2 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index 5b3b18a72f..7ef305c66d 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1094,12 +1094,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - struct ref *rm; - char *url; - int want_status; -- int summary_width = transport_summary_width(ref_map); -+ int summary_width = 0; - - rc = open_fetch_head(&fetch_head); - if (rc) - return -1; - -+ if (verbosity >= 0) -+ summary_width = transport_summary_width(ref_map); -+ - if (raw_url) - url = transport_anonymize_url(raw_url); - else -@@ -1345,7 +1348,6 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, - int url_len, i, result = 0; - struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); - char *url; -- int summary_width = transport_summary_width(stale_refs); - const char *dangling_msg = dry_run - ? _(" (%s will become dangling)") - : _(" (%s has become dangling)"); -@@ -1374,6 +1376,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, - } - - if (verbosity >= 0) { -+ int summary_width = transport_summary_width(stale_refs); -+ - for (ref = stale_refs; ref; ref = ref->next) { - struct strbuf sb = STRBUF_INIT; - if (!shown_url) { --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0021-refs-extract-packed_refs_delete_refs-to-allow-contro.patch b/_support/git-patches/v2.35.1.gl1/0021-refs-extract-packed_refs_delete_refs-to-allow-contro.patch deleted file mode 100644 index 47dd3e41c..000000000 --- a/_support/git-patches/v2.35.1.gl1/0021-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.35.1.gl1/0022-refs-allow-passing-flags-when-beginning-transactions.patch b/_support/git-patches/v2.35.1.gl1/0022-refs-allow-passing-flags-when-beginning-transactions.patch deleted file mode 100644 index 8038daca2..000000000 --- a/_support/git-patches/v2.35.1.gl1/0022-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.35.1.gl1/0023-refs-allow-skipping-the-reference-transaction-hook.patch b/_support/git-patches/v2.35.1.gl1/0023-refs-allow-skipping-the-reference-transaction-hook.patch deleted file mode 100644 index cd194f8aa..000000000 --- a/_support/git-patches/v2.35.1.gl1/0023-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.35.1.gl1/0024-refs-demonstrate-excessive-execution-of-the-referenc.patch b/_support/git-patches/v2.35.1.gl1/0024-refs-demonstrate-excessive-execution-of-the-referenc.patch deleted file mode 100644 index aa6d96a3e..000000000 --- a/_support/git-patches/v2.35.1.gl1/0024-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.35.1.gl1/0025-refs-do-not-execute-reference-transaction-hook-on-pa.patch b/_support/git-patches/v2.35.1.gl1/0025-refs-do-not-execute-reference-transaction-hook-on-pa.patch deleted file mode 100644 index f2e7c06ed..000000000 --- a/_support/git-patches/v2.35.1.gl1/0025-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.35.1.gl1/0026-refs-skip-hooks-when-deleting-uncovered-packed-refs.patch b/_support/git-patches/v2.35.1.gl1/0026-refs-skip-hooks-when-deleting-uncovered-packed-refs.patch deleted file mode 100644 index 3b21bf489..000000000 --- a/_support/git-patches/v2.35.1.gl1/0026-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 - diff --git a/_support/git-patches/v2.35.1.gl1/0027-fetch-prune-exit-with-error-if-pruning-fails.patch b/_support/git-patches/v2.35.1.gl1/0027-fetch-prune-exit-with-error-if-pruning-fails.patch deleted file mode 100644 index a6a75c054..000000000 --- a/_support/git-patches/v2.35.1.gl1/0027-fetch-prune-exit-with-error-if-pruning-fails.patch +++ /dev/null @@ -1,74 +0,0 @@ -From 05bb5009a6bded848b30bb82cc02e6bcb90a8fc3 Mon Sep 17 00:00:00 2001 -Message-Id: <05bb5009a6bded848b30bb82cc02e6bcb90a8fc3.1646206542.git.ps@pks.im> -In-Reply-To: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im> -References: <c74f385fb46855ac0db222b6845ddb95e6a36264.1646206541.git.ps@pks.im> -From: Thomas Gummerer <t.gummerer@gmail.com> -Date: Mon, 31 Jan 2022 13:30:47 +0000 -Subject: [PATCH 27/34] fetch --prune: exit with error if pruning fails - -When pruning refs fails, we print an error to stderr, but still -exit 0 from 'git fetch'. Since this is a genuine error, fetch -should be exiting with some non-zero exit code. Make it so. - -The --prune option was introduced in f360d844de ("builtin-fetch: add ---prune option", 2009-11-10). Unfortunately it's unclear from that -commit whether ignoring the exit code was an oversight or -intentional, but it feels like an oversight. - -Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> -Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit c9e04d905edb5487c43b03304704e8d1248f9ac0) ---- - builtin/fetch.c | 10 ++++++---- - t/t5510-fetch.sh | 11 +++++++++++ - 2 files changed, 17 insertions(+), 4 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index ebbde5d56d..f5a64c7351 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1613,12 +1613,14 @@ static int do_fetch(struct transport *transport, - * don't care whether --tags was specified. - */ - if (rs->nr) { -- prune_refs(rs, ref_map, transport->url); -+ retcode = prune_refs(rs, ref_map, transport->url); - } else { -- prune_refs(&transport->remote->fetch, -- ref_map, -- transport->url); -+ retcode = prune_refs(&transport->remote->fetch, -+ ref_map, -+ transport->url); - } -+ if (retcode != 0) -+ retcode = 1; - } - if (fetch_and_consume_refs(transport, ref_map, worktrees)) { - free_refs(ref_map); -diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh -index 20f7110ec1..ef0da0a63b 100755 ---- a/t/t5510-fetch.sh -+++ b/t/t5510-fetch.sh -@@ -164,6 +164,17 @@ test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' - git rev-parse sometag - ' - -+test_expect_success REFFILES 'fetch --prune fails to delete branches' ' -+ cd "$D" && -+ git clone . prune-fail && -+ cd prune-fail && -+ git update-ref refs/remotes/origin/extrabranch main && -+ : this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds && -+ >.git/packed-refs.new && -+ -+ test_must_fail git fetch --prune origin -+' -+ - test_expect_success 'fetch --atomic works with a single branch' ' - test_when_finished "rm -rf \"$D\"/atomic" && - --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0028-fetch-increase-test-coverage-of-fetches.patch b/_support/git-patches/v2.35.1.gl1/0028-fetch-increase-test-coverage-of-fetches.patch deleted file mode 100644 index 412a908ef..000000000 --- a/_support/git-patches/v2.35.1.gl1/0028-fetch-increase-test-coverage-of-fetches.patch +++ /dev/null @@ -1,169 +0,0 @@ -From e6b86c1dcd01137afe71fe2c303efcf587b01abf Mon Sep 17 00:00:00 2001 -Message-Id: <e6b86c1dcd01137afe71fe2c303efcf587b01abf.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: Thu, 17 Feb 2022 14:04:16 +0100 -Subject: [PATCH 28/34] fetch: increase test coverage of fetches - -When using git-fetch(1) with the `--atomic` flag the expectation is that -either all of the references are updated, or alternatively none are in -case the fetch fails. While we already have tests for this, we do not -have any tests which exercise atomicity either when pruning deleted refs -or when backfilling tags. This gap in test coverage hides that we indeed -don't handle atomicity correctly for both of these cases. - -Add test cases which cover these testing gaps to demonstrate the broken -behaviour. Note that tests are not marked as `test_expect_failure`: this -is done to explicitly demonstrate the current known-wrong behaviour, and -they will be fixed up as soon as we fix the underlying bugs. - -While at it this commit also adds another test case which demonstrates -that backfilling of tags does not return an error code in case the -backfill fails. This bug will also be fixed by a subsequent commit. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit 2a0cafd464709cfa22fe7249290c644a2a26c520) ---- - t/t5503-tagfollow.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++ - t/t5510-fetch.sh | 33 ++++++++++++++++++ - 2 files changed, 114 insertions(+) - -diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh -index 195fc64dd4..6ffe2a5719 100755 ---- a/t/t5503-tagfollow.sh -+++ b/t/t5503-tagfollow.sh -@@ -160,4 +160,85 @@ test_expect_success 'new clone fetch main and tags' ' - test_cmp expect actual - ' - -+test_expect_success 'atomic fetch with failing backfill' ' -+ git init clone3 && -+ -+ # We want to test whether a failure when backfilling tags correctly -+ # aborts the complete transaction when `--atomic` is passed: we should -+ # neither create the branch nor should we create the tag when either -+ # one of both fails to update correctly. -+ # -+ # To trigger failure we simply abort when backfilling a tag. -+ write_script clone3/.git/hooks/reference-transaction <<-\EOF && -+ while read oldrev newrev reference -+ do -+ if test "$reference" = refs/tags/tag1 -+ then -+ exit 1 -+ fi -+ done -+ EOF -+ -+ test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something && -+ -+ # Creation of the tag has failed, so ideally refs/heads/something -+ # should not exist. The fact that it does demonstrates that there is -+ # a bug in the `--atomic` flag. -+ test $B = "$(git -C clone3 rev-parse --verify refs/heads/something)" -+' -+ -+test_expect_success 'atomic fetch with backfill should use single transaction' ' -+ git init clone4 && -+ -+ # Fetching with the `--atomic` flag should update all references in a -+ # single transaction, including backfilled tags. We thus expect to see -+ # a single reference transaction for the created branch and tags. -+ cat >expected <<-EOF && -+ prepared -+ $ZERO_OID $B refs/heads/something -+ $ZERO_OID $S refs/tags/tag2 -+ committed -+ $ZERO_OID $B refs/heads/something -+ $ZERO_OID $S refs/tags/tag2 -+ prepared -+ $ZERO_OID $T refs/tags/tag1 -+ committed -+ $ZERO_OID $T refs/tags/tag1 -+ EOF -+ -+ write_script clone4/.git/hooks/reference-transaction <<-\EOF && -+ ( echo "$*" && cat ) >>actual -+ EOF -+ -+ git -C clone4 fetch --atomic .. $B:refs/heads/something && -+ test_cmp expected clone4/actual -+' -+ -+test_expect_success 'backfill failure causes command to fail' ' -+ git init clone5 && -+ -+ write_script clone5/.git/hooks/reference-transaction <<-EOF && -+ while read oldrev newrev reference -+ do -+ if test "\$reference" = refs/tags/tag1 -+ then -+ # Create a nested tag below the actual tag we -+ # wanted to write, which causes a D/F conflict -+ # later when we want to commit refs/tags/tag1. -+ # We cannot just `exit 1` here given that this -+ # would cause us to die immediately. -+ git update-ref refs/tags/tag1/nested $B -+ exit \$! -+ fi -+ done -+ EOF -+ -+ # Even though we fail to create refs/tags/tag1 the below command -+ # unexpectedly succeeds. -+ git -C clone5 fetch .. $B:refs/heads/something && -+ test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && -+ test $S = $(git -C clone5 rev-parse --verify tag2) && -+ test_must_fail git -C clone5 rev-parse --verify tag1 -+' -+ - test_done -diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh -index ef0da0a63b..70d51f343b 100755 ---- a/t/t5510-fetch.sh -+++ b/t/t5510-fetch.sh -@@ -343,6 +343,39 @@ test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' ' - test_cmp expected atomic/.git/FETCH_HEAD - ' - -+test_expect_success 'fetch --atomic --prune executes a single reference transaction only' ' -+ test_when_finished "rm -rf \"$D\"/atomic" && -+ -+ cd "$D" && -+ git branch scheduled-for-deletion && -+ git clone . atomic && -+ git branch -D scheduled-for-deletion && -+ git branch new-branch && -+ head_oid=$(git rev-parse HEAD) && -+ -+ # Fetching with the `--atomic` flag should update all references in a -+ # single transaction. It is currently missing coverage of pruned -+ # references though, and as a result those may be committed to disk -+ # even if updating references fails later. -+ cat >expected <<-EOF && -+ prepared -+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion -+ committed -+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion -+ prepared -+ $ZERO_OID $head_oid refs/remotes/origin/new-branch -+ committed -+ $ZERO_OID $head_oid refs/remotes/origin/new-branch -+ EOF -+ -+ write_script atomic/.git/hooks/reference-transaction <<-\EOF && -+ ( echo "$*" && cat ) >>actual -+ EOF -+ -+ git -C atomic fetch --atomic --prune origin && -+ test_cmp expected atomic/actual -+' -+ - test_expect_success '--refmap="" ignores configured refspec' ' - cd "$TRASH_DIRECTORY" && - git clone "$D" remote-refs && --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0029-fetch-backfill-tags-before-setting-upstream.patch b/_support/git-patches/v2.35.1.gl1/0029-fetch-backfill-tags-before-setting-upstream.patch deleted file mode 100644 index 360bd10c2..000000000 --- a/_support/git-patches/v2.35.1.gl1/0029-fetch-backfill-tags-before-setting-upstream.patch +++ /dev/null @@ -1,116 +0,0 @@ -From 6bb95bf1ec7cb06eca5161fe7dcf02cfabd8de83 Mon Sep 17 00:00:00 2001 -Message-Id: <6bb95bf1ec7cb06eca5161fe7dcf02cfabd8de83.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: Thu, 17 Feb 2022 14:04:20 +0100 -Subject: [PATCH 29/34] fetch: backfill tags before setting upstream - -The fetch code flow is a bit hard to understand right now: - - 1. We optionally prune all references which have vanished on the - remote side. - 2. We fetch and update all other references locally. - 3. We update the upstream branch in the gitconfig. - 4. We backfill tags pointing into the history we have just fetched. - -It is quite confusing that we fetch objects and update references in -both (2) and (4), which is further stressed by the point that we use a -`skip` goto label to jump from (3) to (4) in case we fail to update the -gitconfig as expected. - -Reorder the code to first update all local references, and only after we -have done so update the upstream branch information. This improves the -code flow and furthermore makes it easier to refactor the way we update -references together. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit efbade066083eb0a8ccee5a8290cd3fc834705f3) ---- - builtin/fetch.c | 35 ++++++++++++++++++----------------- - 1 file changed, 18 insertions(+), 17 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index f5a64c7351..4ae1afb918 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1539,7 +1539,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map, - static int do_fetch(struct transport *transport, - struct refspec *rs) - { -- struct ref *ref_map; -+ struct ref *ref_map = NULL; - int autotags = (transport->remote->fetch_tags == 1); - int retcode = 0; - const struct ref *remote_refs; -@@ -1623,11 +1623,24 @@ static int do_fetch(struct transport *transport, - retcode = 1; - } - if (fetch_and_consume_refs(transport, ref_map, worktrees)) { -- free_refs(ref_map); - retcode = 1; - goto cleanup; - } - -+ /* -+ * If neither --no-tags nor --tags was specified, do automated tag -+ * following. -+ */ -+ if (tags == TAGS_DEFAULT && autotags) { -+ struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; -+ -+ find_non_local_tags(remote_refs, &tags_ref_map, &tail); -+ if (tags_ref_map) -+ backfill_tags(transport, tags_ref_map, worktrees); -+ -+ free_refs(tags_ref_map); -+ } -+ - if (set_upstream) { - struct branch *branch = branch_get("HEAD"); - struct ref *rm; -@@ -1647,7 +1660,7 @@ static int do_fetch(struct transport *transport, - if (!rm->peer_ref) { - if (source_ref) { - warning(_("multiple branches detected, incompatible with --set-upstream")); -- goto skip; -+ goto cleanup; - } else { - source_ref = rm; - } -@@ -1661,7 +1674,7 @@ static int do_fetch(struct transport *transport, - warning(_("could not set upstream of HEAD to '%s' from '%s' when " - "it does not point to any branch."), - shortname, transport->remote->name); -- goto skip; -+ goto cleanup; - } - - if (!strcmp(source_ref->name, "HEAD") || -@@ -1681,21 +1694,9 @@ static int do_fetch(struct transport *transport, - "you need to specify exactly one branch with the --set-upstream option")); - } - } --skip: -- free_refs(ref_map); -- -- /* if neither --no-tags nor --tags was specified, do automated tag -- * following ... */ -- if (tags == TAGS_DEFAULT && autotags) { -- struct ref **tail = &ref_map; -- ref_map = NULL; -- find_non_local_tags(remote_refs, &ref_map, &tail); -- if (ref_map) -- backfill_tags(transport, ref_map, worktrees); -- free_refs(ref_map); -- } - - cleanup: -+ free_refs(ref_map); - free_worktrees(worktrees); - return retcode; - } --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0030-fetch-control-lifecycle-of-FETCH_HEAD-in-a-single-pl.patch b/_support/git-patches/v2.35.1.gl1/0030-fetch-control-lifecycle-of-FETCH_HEAD-in-a-single-pl.patch deleted file mode 100644 index dc9d1364a..000000000 --- a/_support/git-patches/v2.35.1.gl1/0030-fetch-control-lifecycle-of-FETCH_HEAD-in-a-single-pl.patch +++ /dev/null @@ -1,170 +0,0 @@ -From ded9df2b091da730b28e0aa20b667f3ee2ccaffa Mon Sep 17 00:00:00 2001 -Message-Id: <ded9df2b091da730b28e0aa20b667f3ee2ccaffa.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: Thu, 17 Feb 2022 14:04:24 +0100 -Subject: [PATCH 30/34] fetch: control lifecycle of FETCH_HEAD in a single - place - -There are two different locations where we're appending to FETCH_HEAD: -first when storing updated references, and second when backfilling tags. -Both times we open the file, append to it and then commit it into place, -which is essentially duplicate work. - -Improve the lifecycle of updating FETCH_HEAD by opening and committing -it once in `do_fetch()`, where we pass the structure down to the code -which wants to append to it. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit 2983cec0f26b7409ccc2dd5710b40ff4809cd4b1) ---- - builtin/fetch.c | 35 +++++++++++++++++++---------------- - 1 file changed, 19 insertions(+), 16 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index 4ae1afb918..897fa247d9 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1083,9 +1083,8 @@ N_("it took %.2f seconds to check forced updates; you can use\n" - - static int store_updated_refs(const char *raw_url, const char *remote_name, - int connectivity_checked, struct ref *ref_map, -- struct worktree **worktrees) -+ struct fetch_head *fetch_head, struct worktree **worktrees) - { -- struct fetch_head fetch_head; - int url_len, i, rc = 0; - struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; - struct ref_transaction *transaction = NULL; -@@ -1095,10 +1094,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - int want_status; - int summary_width = 0; - -- rc = open_fetch_head(&fetch_head); -- if (rc) -- return -1; -- - if (verbosity >= 0) - summary_width = transport_summary_width(ref_map); - -@@ -1208,7 +1203,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - strbuf_addf(¬e, "'%s' of ", what); - } - -- append_fetch_head(&fetch_head, &rm->old_oid, -+ append_fetch_head(fetch_head, &rm->old_oid, - rm->fetch_head_status, - note.buf, url, url_len); - -@@ -1248,9 +1243,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - } - } - -- if (!rc) -- commit_fetch_head(&fetch_head); -- - if (rc & STORE_REF_ERROR_DF_CONFLICT) - error(_("some local refs could not be updated; try running\n" - " 'git remote prune %s' to remove any old, conflicting " -@@ -1270,7 +1262,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - strbuf_release(&err); - ref_transaction_free(transaction); - free(url); -- close_fetch_head(&fetch_head); - return rc; - } - -@@ -1311,6 +1302,7 @@ static int check_exist_and_connected(struct ref *ref_map) - - static int fetch_and_consume_refs(struct transport *transport, - struct ref *ref_map, -+ struct fetch_head *fetch_head, - struct worktree **worktrees) - { - int connectivity_checked = 1; -@@ -1333,7 +1325,7 @@ static int fetch_and_consume_refs(struct transport *transport, - - trace2_region_enter("fetch", "consume_refs", the_repository); - ret = store_updated_refs(transport->url, transport->remote->name, -- connectivity_checked, ref_map, worktrees); -+ connectivity_checked, ref_map, fetch_head, worktrees); - trace2_region_leave("fetch", "consume_refs", the_repository); - - out: -@@ -1506,7 +1498,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) - return transport; - } - --static void backfill_tags(struct transport *transport, struct ref *ref_map, -+static void backfill_tags(struct transport *transport, -+ struct ref *ref_map, -+ struct fetch_head *fetch_head, - struct worktree **worktrees) - { - int cannot_reuse; -@@ -1528,7 +1522,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map, - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); - transport_set_option(transport, TRANS_OPT_DEPTH, "0"); - transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); -- fetch_and_consume_refs(transport, ref_map, worktrees); -+ fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); - - if (gsecondary) { - transport_disconnect(gsecondary); -@@ -1547,6 +1541,7 @@ static int do_fetch(struct transport *transport, - TRANSPORT_LS_REFS_OPTIONS_INIT; - int must_list_refs = 1; - struct worktree **worktrees = get_worktrees(); -+ struct fetch_head fetch_head = { 0 }; - - if (tags == TAGS_DEFAULT) { - if (transport->remote->fetch_tags == 2) -@@ -1604,6 +1599,10 @@ static int do_fetch(struct transport *transport, - if (!update_head_ok) - check_not_current_branch(ref_map, worktrees); - -+ retcode = open_fetch_head(&fetch_head); -+ if (retcode) -+ goto cleanup; -+ - if (tags == TAGS_DEFAULT && autotags) - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); - if (prune) { -@@ -1622,7 +1621,8 @@ static int do_fetch(struct transport *transport, - if (retcode != 0) - retcode = 1; - } -- if (fetch_and_consume_refs(transport, ref_map, worktrees)) { -+ -+ if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) { - retcode = 1; - goto cleanup; - } -@@ -1636,11 +1636,13 @@ static int do_fetch(struct transport *transport, - - find_non_local_tags(remote_refs, &tags_ref_map, &tail); - if (tags_ref_map) -- backfill_tags(transport, tags_ref_map, worktrees); -+ backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); - - free_refs(tags_ref_map); - } - -+ commit_fetch_head(&fetch_head); -+ - if (set_upstream) { - struct branch *branch = branch_get("HEAD"); - struct ref *rm; -@@ -1696,6 +1698,7 @@ static int do_fetch(struct transport *transport, - } - - cleanup: -+ close_fetch_head(&fetch_head); - free_refs(ref_map); - free_worktrees(worktrees); - return retcode; --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0031-fetch-report-errors-when-backfilling-tags-fails.patch b/_support/git-patches/v2.35.1.gl1/0031-fetch-report-errors-when-backfilling-tags-fails.patch deleted file mode 100644 index a2cfd7b3e..000000000 --- a/_support/git-patches/v2.35.1.gl1/0031-fetch-report-errors-when-backfilling-tags-fails.patch +++ /dev/null @@ -1,100 +0,0 @@ -From ab0fa78af6741c409979889d7596042eb03192b5 Mon Sep 17 00:00:00 2001 -Message-Id: <ab0fa78af6741c409979889d7596042eb03192b5.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: Thu, 17 Feb 2022 14:04:28 +0100 -Subject: [PATCH 31/34] fetch: report errors when backfilling tags fails - -When the backfilling of tags fails we do not report this error to the -caller, but only report it implicitly at a later point when reporting -updated references. This leaves callers unable to act upon the -information of whether the backfilling succeeded or not. - -Refactor the function to return an error code and pass it up the -callstack. This causes us to correctly propagate the error back to the -user of git-fetch(1). - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit 62091b4c87a199c172556f15c5662c6c3679e9cd) ---- - builtin/fetch.c | 26 ++++++++++++++++++-------- - t/t5503-tagfollow.sh | 4 +--- - 2 files changed, 19 insertions(+), 11 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index 897fa247d9..2d0ffb9859 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1498,12 +1498,12 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) - return transport; - } - --static void backfill_tags(struct transport *transport, -- struct ref *ref_map, -- struct fetch_head *fetch_head, -- struct worktree **worktrees) -+static int backfill_tags(struct transport *transport, -+ struct ref *ref_map, -+ struct fetch_head *fetch_head, -+ struct worktree **worktrees) - { -- int cannot_reuse; -+ int retcode, cannot_reuse; - - /* - * Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it -@@ -1522,12 +1522,14 @@ static void backfill_tags(struct transport *transport, - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); - transport_set_option(transport, TRANS_OPT_DEPTH, "0"); - transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); -- fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); -+ retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); - - if (gsecondary) { - transport_disconnect(gsecondary); - gsecondary = NULL; - } -+ -+ return retcode; - } - - static int do_fetch(struct transport *transport, -@@ -1635,8 +1637,16 @@ static int do_fetch(struct transport *transport, - struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; - - find_non_local_tags(remote_refs, &tags_ref_map, &tail); -- if (tags_ref_map) -- backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); -+ if (tags_ref_map) { -+ /* -+ * If backfilling of tags fails then we want to tell -+ * the user so, but we have to continue regardless to -+ * populate upstream information of the references we -+ * have already fetched above. -+ */ -+ if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) -+ retcode = 1; -+ } - - free_refs(tags_ref_map); - } -diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh -index 6ffe2a5719..c057c49e80 100755 ---- a/t/t5503-tagfollow.sh -+++ b/t/t5503-tagfollow.sh -@@ -233,9 +233,7 @@ test_expect_success 'backfill failure causes command to fail' ' - done - EOF - -- # Even though we fail to create refs/tags/tag1 the below command -- # unexpectedly succeeds. -- git -C clone5 fetch .. $B:refs/heads/something && -+ test_must_fail git -C clone5 fetch .. $B:refs/heads/something && - test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && - test $S = $(git -C clone5 rev-parse --verify tag2) && - test_must_fail git -C clone5 rev-parse --verify tag1 --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0032-refs-add-interface-to-iterate-over-queued-transactio.patch b/_support/git-patches/v2.35.1.gl1/0032-refs-add-interface-to-iterate-over-queued-transactio.patch deleted file mode 100644 index cc6a3e320..000000000 --- a/_support/git-patches/v2.35.1.gl1/0032-refs-add-interface-to-iterate-over-queued-transactio.patch +++ /dev/null @@ -1,98 +0,0 @@ -From 7c90ce62cd5ab5f769ec789eff038b43e8f5d910 Mon Sep 17 00:00:00 2001 -Message-Id: <7c90ce62cd5ab5f769ec789eff038b43e8f5d910.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: Thu, 17 Feb 2022 14:04:32 +0100 -Subject: [PATCH 32/34] refs: add interface to iterate over queued - transactional updates - -There is no way for a caller to see whether a reference update has -already been queued up for a given reference transaction. There are -multiple alternatives to provide this functionality: - - - We may add a function that simply tells us whether a specific - reference has already been queued. If implemented naively then - this would potentially be quadratic in runtime behaviour if this - question is asked repeatedly because we have to iterate over all - references every time. The alternative would be to add a hashmap - of all queued reference updates to speed up the lookup, but this - adds overhead to all callers. - - - We may add a flag to `ref_transaction_add_update()` that causes it - to skip duplicates, but this has the same runtime concerns as the - first alternative. - - - We may add an interface which lets callers collect all updates - which have already been queued such that he can avoid re-adding - them. This is the most flexible approach and puts the burden on - the caller, but also allows us to not impact any of the existing - callsites which don't need this information. - -This commit implements the last approach: it allows us to compute the -map of already-queued updates once up front such that we can then skip -all subsequent references which are already part of this map. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit 4f2ba2d06a7dd7e84e105a2779a7f07549d04231) ---- - refs.c | 16 ++++++++++++++++ - refs.h | 14 ++++++++++++++ - 2 files changed, 30 insertions(+) - -diff --git a/refs.c b/refs.c -index 435b90b1ec..5d9cb306ad 100644 ---- a/refs.c -+++ b/refs.c -@@ -2434,6 +2434,22 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction, - return refs->be->initial_transaction_commit(refs, transaction, err); - } - -+void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, -+ ref_transaction_for_each_queued_update_fn cb, -+ void *cb_data) -+{ -+ int i; -+ -+ for (i = 0; i < transaction->nr; i++) { -+ struct ref_update *update = transaction->updates[i]; -+ -+ cb(update->refname, -+ (update->flags & REF_HAVE_OLD) ? &update->old_oid : NULL, -+ (update->flags & REF_HAVE_NEW) ? &update->new_oid : NULL, -+ cb_data); -+ } -+} -+ - int refs_delete_refs(struct ref_store *refs, const char *logmsg, - struct string_list *refnames, unsigned int flags) - { -diff --git a/refs.h b/refs.h -index a015354fd6..be5947f111 100644 ---- a/refs.h -+++ b/refs.h -@@ -781,6 +781,20 @@ int ref_transaction_abort(struct ref_transaction *transaction, - int initial_ref_transaction_commit(struct ref_transaction *transaction, - struct strbuf *err); - -+/* -+ * Execute the given callback function for each of the reference updates which -+ * have been queued in the given transaction. `old_oid` and `new_oid` may be -+ * `NULL` pointers depending on whether the update has these object IDs set or -+ * not. -+ */ -+typedef void ref_transaction_for_each_queued_update_fn(const char *refname, -+ const struct object_id *old_oid, -+ const struct object_id *new_oid, -+ void *cb_data); -+void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, -+ ref_transaction_for_each_queued_update_fn cb, -+ void *cb_data); -+ - /* - * Free `*transaction` and all associated data. - */ --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0033-fetch-make-atomic-flag-cover-backfilling-of-tags.patch b/_support/git-patches/v2.35.1.gl1/0033-fetch-make-atomic-flag-cover-backfilling-of-tags.patch deleted file mode 100644 index 9e136f838..000000000 --- a/_support/git-patches/v2.35.1.gl1/0033-fetch-make-atomic-flag-cover-backfilling-of-tags.patch +++ /dev/null @@ -1,310 +0,0 @@ -From 3a62f32960d2df2451003f2561a4f14d459991ec Mon Sep 17 00:00:00 2001 -Message-Id: <3a62f32960d2df2451003f2561a4f14d459991ec.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: Thu, 17 Feb 2022 14:04:36 +0100 -Subject: [PATCH 33/34] fetch: make `--atomic` flag cover backfilling of tags - -When fetching references from a remote we by default also fetch all tags -which point into the history we have fetched. This is a separate step -performed after updating local references because it requires us to walk -over the history on the client-side to determine whether the remote has -announced any tags which point to one of the fetched commits. - -This backfilling of tags isn't covered by the `--atomic` flag: right -now, it only applies to the step where we update our local references. -This is an oversight at the time the flag was introduced: its purpose is -to either update all references or none, but right now we happily update -local references even in the case where backfilling failed. - -Fix this by pulling up creation of the reference transaction such that -we can pass the same transaction to both the code which updates local -references and to the code which backfills tags. This allows us to only -commit the transaction in case both actions succeed. - -Note that we also have to start passing the transaction into -`find_non_local_tags()`: this function is responsible for finding all -tags which we need to backfill. Right now, it will happily return tags -which have already been updated with our local references. But when we -use a single transaction for both local references and backfilling then -it may happen that we try to queue the same reference update twice to -the transaction, which consequently triggers a bug. We thus have to skip -over any tags which have already been queued. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit b3a804663c4682f6df55dd6703f8f8af9a7c6ab5) ---- - builtin/fetch.c | 92 +++++++++++++++++++++++++++++++------------- - t/t5503-tagfollow.sh | 11 ++---- - 2 files changed, 69 insertions(+), 34 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index 2d0ffb9859..e996a74862 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -348,7 +348,19 @@ static void clear_item(struct refname_hash_entry *item) - item->ignore = 1; - } - -+ -+static void add_already_queued_tags(const char *refname, -+ const struct object_id *old_oid, -+ const struct object_id *new_oid, -+ void *cb_data) -+{ -+ struct hashmap *queued_tags = cb_data; -+ if (starts_with(refname, "refs/tags/") && new_oid) -+ (void) refname_hash_add(queued_tags, refname, new_oid); -+} -+ - static void find_non_local_tags(const struct ref *refs, -+ struct ref_transaction *transaction, - struct ref **head, - struct ref ***tail) - { -@@ -366,6 +378,16 @@ static void find_non_local_tags(const struct ref *refs, - create_fetch_oidset(head, &fetch_oids); - - for_each_ref(add_one_refname, &existing_refs); -+ -+ /* -+ * If we already have a transaction, then we need to filter out all -+ * tags which have already been queued up. -+ */ -+ if (transaction) -+ ref_transaction_for_each_queued_update(transaction, -+ add_already_queued_tags, -+ &existing_refs); -+ - for (ref = refs; ref; ref = ref->next) { - if (!starts_with(ref->name, "refs/tags/")) - continue; -@@ -599,7 +621,7 @@ static struct ref *get_ref_map(struct remote *remote, - /* also fetch all tags */ - get_fetch_map(remote_refs, tag_refspec, &tail, 0); - else if (tags == TAGS_DEFAULT && *autotags) -- find_non_local_tags(remote_refs, &ref_map, &tail); -+ find_non_local_tags(remote_refs, NULL, &ref_map, &tail); - - /* Now append any refs to be updated opportunistically: */ - *tail = orefs; -@@ -1082,12 +1104,12 @@ N_("it took %.2f seconds to check forced updates; you can use\n" - "to avoid this check\n"); - - static int store_updated_refs(const char *raw_url, const char *remote_name, -- int connectivity_checked, struct ref *ref_map, -+ int connectivity_checked, -+ struct ref_transaction *transaction, struct ref *ref_map, - struct fetch_head *fetch_head, struct worktree **worktrees) - { - int url_len, i, rc = 0; - struct strbuf note = STRBUF_INIT, err = STRBUF_INIT; -- struct ref_transaction *transaction = NULL; - const char *what, *kind; - struct ref *rm; - char *url; -@@ -1112,14 +1134,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - } - } - -- if (atomic_fetch) { -- transaction = ref_transaction_begin(&err); -- if (!transaction) { -- error("%s", err.buf); -- goto abort; -- } -- } -- - prepare_format_display(ref_map); - - /* -@@ -1235,14 +1249,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - } - } - -- if (!rc && transaction) { -- rc = ref_transaction_commit(transaction, &err); -- if (rc) { -- error("%s", err.buf); -- goto abort; -- } -- } -- - if (rc & STORE_REF_ERROR_DF_CONFLICT) - error(_("some local refs could not be updated; try running\n" - " 'git remote prune %s' to remove any old, conflicting " -@@ -1260,7 +1266,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - abort: - strbuf_release(¬e); - strbuf_release(&err); -- ref_transaction_free(transaction); - free(url); - return rc; - } -@@ -1301,6 +1306,7 @@ static int check_exist_and_connected(struct ref *ref_map) - } - - static int fetch_and_consume_refs(struct transport *transport, -+ struct ref_transaction *transaction, - struct ref *ref_map, - struct fetch_head *fetch_head, - struct worktree **worktrees) -@@ -1325,7 +1331,8 @@ static int fetch_and_consume_refs(struct transport *transport, - - trace2_region_enter("fetch", "consume_refs", the_repository); - ret = store_updated_refs(transport->url, transport->remote->name, -- connectivity_checked, ref_map, fetch_head, worktrees); -+ connectivity_checked, transaction, ref_map, -+ fetch_head, worktrees); - trace2_region_leave("fetch", "consume_refs", the_repository); - - out: -@@ -1499,6 +1506,7 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) - } - - static int backfill_tags(struct transport *transport, -+ struct ref_transaction *transaction, - struct ref *ref_map, - struct fetch_head *fetch_head, - struct worktree **worktrees) -@@ -1522,7 +1530,7 @@ static int backfill_tags(struct transport *transport, - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); - transport_set_option(transport, TRANS_OPT_DEPTH, "0"); - transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); -- retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); -+ retcode = fetch_and_consume_refs(transport, transaction, ref_map, fetch_head, worktrees); - - if (gsecondary) { - transport_disconnect(gsecondary); -@@ -1535,6 +1543,7 @@ static int backfill_tags(struct transport *transport, - static int do_fetch(struct transport *transport, - struct refspec *rs) - { -+ struct ref_transaction *transaction = NULL; - struct ref *ref_map = NULL; - int autotags = (transport->remote->fetch_tags == 1); - int retcode = 0; -@@ -1544,6 +1553,7 @@ static int do_fetch(struct transport *transport, - int must_list_refs = 1; - struct worktree **worktrees = get_worktrees(); - struct fetch_head fetch_head = { 0 }; -+ struct strbuf err = STRBUF_INIT; - - if (tags == TAGS_DEFAULT) { - if (transport->remote->fetch_tags == 2) -@@ -1605,6 +1615,14 @@ static int do_fetch(struct transport *transport, - if (retcode) - goto cleanup; - -+ if (atomic_fetch) { -+ transaction = ref_transaction_begin(&err); -+ if (!transaction) { -+ retcode = error("%s", err.buf); -+ goto cleanup; -+ } -+ } -+ - if (tags == TAGS_DEFAULT && autotags) - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1"); - if (prune) { -@@ -1624,7 +1642,7 @@ static int do_fetch(struct transport *transport, - retcode = 1; - } - -- if (fetch_and_consume_refs(transport, ref_map, &fetch_head, worktrees)) { -+ if (fetch_and_consume_refs(transport, transaction, ref_map, &fetch_head, worktrees)) { - retcode = 1; - goto cleanup; - } -@@ -1636,21 +1654,37 @@ static int do_fetch(struct transport *transport, - if (tags == TAGS_DEFAULT && autotags) { - struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; - -- find_non_local_tags(remote_refs, &tags_ref_map, &tail); -+ find_non_local_tags(remote_refs, transaction, &tags_ref_map, &tail); - if (tags_ref_map) { - /* - * If backfilling of tags fails then we want to tell - * the user so, but we have to continue regardless to - * populate upstream information of the references we -- * have already fetched above. -+ * have already fetched above. The exception though is -+ * when `--atomic` is passed: in that case we'll abort -+ * the transaction and don't commit anything. - */ -- if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) -+ if (backfill_tags(transport, transaction, tags_ref_map, -+ &fetch_head, worktrees)) - retcode = 1; - } - - free_refs(tags_ref_map); - } - -+ if (transaction) { -+ if (retcode) -+ goto cleanup; -+ -+ retcode = ref_transaction_commit(transaction, &err); -+ if (retcode) { -+ error("%s", err.buf); -+ ref_transaction_free(transaction); -+ transaction = NULL; -+ goto cleanup; -+ } -+ } -+ - commit_fetch_head(&fetch_head); - - if (set_upstream) { -@@ -1708,7 +1742,13 @@ static int do_fetch(struct transport *transport, - } - - cleanup: -+ if (retcode && transaction) { -+ ref_transaction_abort(transaction, &err); -+ error("%s", err.buf); -+ } -+ - close_fetch_head(&fetch_head); -+ strbuf_release(&err); - free_refs(ref_map); - free_worktrees(worktrees); - return retcode; -diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh -index c057c49e80..e72fdc2534 100755 ---- a/t/t5503-tagfollow.sh -+++ b/t/t5503-tagfollow.sh -@@ -180,11 +180,8 @@ test_expect_success 'atomic fetch with failing backfill' ' - EOF - - test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something && -- -- # Creation of the tag has failed, so ideally refs/heads/something -- # should not exist. The fact that it does demonstrates that there is -- # a bug in the `--atomic` flag. -- test $B = "$(git -C clone3 rev-parse --verify refs/heads/something)" -+ test_must_fail git -C clone3 rev-parse --verify refs/heads/something && -+ test_must_fail git -C clone3 rev-parse --verify refs/tags/tag2 - ' - - test_expect_success 'atomic fetch with backfill should use single transaction' ' -@@ -197,12 +194,10 @@ test_expect_success 'atomic fetch with backfill should use single transaction' ' - prepared - $ZERO_OID $B refs/heads/something - $ZERO_OID $S refs/tags/tag2 -+ $ZERO_OID $T refs/tags/tag1 - committed - $ZERO_OID $B refs/heads/something - $ZERO_OID $S refs/tags/tag2 -- prepared -- $ZERO_OID $T refs/tags/tag1 -- committed - $ZERO_OID $T refs/tags/tag1 - EOF - --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0034-fetch-make-atomic-flag-cover-pruning-of-refs.patch b/_support/git-patches/v2.35.1.gl1/0034-fetch-make-atomic-flag-cover-pruning-of-refs.patch deleted file mode 100644 index e0f50a7e1..000000000 --- a/_support/git-patches/v2.35.1.gl1/0034-fetch-make-atomic-flag-cover-pruning-of-refs.patch +++ /dev/null @@ -1,149 +0,0 @@ -From c1334339eccde428151dc9e544ab2ec4d0178cad Mon Sep 17 00:00:00 2001 -Message-Id: <c1334339eccde428151dc9e544ab2ec4d0178cad.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: Thu, 17 Feb 2022 14:04:41 +0100 -Subject: [PATCH 34/34] fetch: make `--atomic` flag cover pruning of refs -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -When fetching with the `--prune` flag we will delete any local -references matching the fetch refspec which have disappeared on the -remote. This step is not currently covered by the `--atomic` flag: we -delete branches even though updating of local references has failed, -which means that the fetch is not an all-or-nothing operation. - -Fix this bug by passing in the global transaction into `prune_refs()`: -if one is given, then we'll only queue up deletions and not commit them -right away. - -This change also improves performance when pruning many branches in a -repository with a big packed-refs file: every references is pruned in -its own transaction, which means that we potentially have to rewrite -the packed-refs files for every single reference we're about to prune. - -The following benchmark demonstrates this: it performs a pruning fetch -from a repository with a single reference into a repository with 100k -references, which causes us to prune all but one reference. This is of -course a very artificial setup, but serves to demonstrate the impact of -only having to write the packed-refs file once: - - Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~) - Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s] - Range (min … max): 2.328 s … 2.407 s 10 runs - - Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD) - Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s] - Range (min … max): 1.346 s … 1.400 s 10 runs - - Summary - 'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran - 1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> -(cherry picked from commit 583bc419235cedc6a2ba12593f058a9f812b9594) ---- - builtin/fetch.c | 30 ++++++++++++++++++++++-------- - t/t5510-fetch.sh | 8 ++------ - 2 files changed, 24 insertions(+), 14 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index e996a74862..25cc0b906e 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1340,11 +1340,14 @@ static int fetch_and_consume_refs(struct transport *transport, - return ret; - } - --static int prune_refs(struct refspec *rs, struct ref *ref_map, -+static int prune_refs(struct refspec *rs, -+ struct ref_transaction *transaction, -+ struct ref *ref_map, - const char *raw_url) - { - int url_len, i, result = 0; - struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); -+ struct strbuf err = STRBUF_INIT; - char *url; - const char *dangling_msg = dry_run - ? _(" (%s will become dangling)") -@@ -1364,13 +1367,22 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, - url_len = i - 3; - - if (!dry_run) { -- struct string_list refnames = STRING_LIST_INIT_NODUP; -+ if (transaction) { -+ for (ref = stale_refs; ref; ref = ref->next) { -+ result = ref_transaction_delete(transaction, ref->name, NULL, 0, -+ "fetch: prune", &err); -+ if (result) -+ goto cleanup; -+ } -+ } else { -+ struct string_list refnames = STRING_LIST_INIT_NODUP; - -- for (ref = stale_refs; ref; ref = ref->next) -- string_list_append(&refnames, ref->name); -+ for (ref = stale_refs; ref; ref = ref->next) -+ string_list_append(&refnames, ref->name); - -- result = delete_refs("fetch: prune", &refnames, 0); -- string_list_clear(&refnames, 0); -+ result = delete_refs("fetch: prune", &refnames, 0); -+ string_list_clear(&refnames, 0); -+ } - } - - if (verbosity >= 0) { -@@ -1391,6 +1403,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, - } - } - -+cleanup: -+ strbuf_release(&err); - free(url); - free_refs(stale_refs); - return result; -@@ -1632,10 +1646,10 @@ static int do_fetch(struct transport *transport, - * don't care whether --tags was specified. - */ - if (rs->nr) { -- retcode = prune_refs(rs, ref_map, transport->url); -+ retcode = prune_refs(rs, transaction, ref_map, transport->url); - } else { - retcode = prune_refs(&transport->remote->fetch, -- ref_map, -+ transaction, ref_map, - transport->url); - } - if (retcode != 0) -diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh -index 70d51f343b..48e14e2dab 100755 ---- a/t/t5510-fetch.sh -+++ b/t/t5510-fetch.sh -@@ -354,17 +354,13 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact - head_oid=$(git rev-parse HEAD) && - - # Fetching with the `--atomic` flag should update all references in a -- # single transaction. It is currently missing coverage of pruned -- # references though, and as a result those may be committed to disk -- # even if updating references fails later. -+ # single transaction. - cat >expected <<-EOF && - prepared - $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion -- committed -- $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion -- prepared - $ZERO_OID $head_oid refs/remotes/origin/new-branch - committed -+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion - $ZERO_OID $head_oid refs/remotes/origin/new-branch - EOF - --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0035-upload-pack-look-up-want-lines-via-commit-graph.patch b/_support/git-patches/v2.35.1.gl1/0035-upload-pack-look-up-want-lines-via-commit-graph.patch deleted file mode 100644 index c8ada5b44..000000000 --- a/_support/git-patches/v2.35.1.gl1/0035-upload-pack-look-up-want-lines-via-commit-graph.patch +++ /dev/null @@ -1,97 +0,0 @@ -From 4de656263aa195080495fc0a103351b9eaac8160 Mon Sep 17 00:00:00 2001 -Message-Id: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Tue, 1 Mar 2022 10:33:37 +0100 -Subject: [PATCH 35/39] upload-pack: look up "want" lines via commit-graph -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -During packfile negotiation the client will send "want" and "want-ref" -lines to the server to tell it which objects it is interested in. The -server-side parses each of those and looks them up to see whether it -actually has requested objects. This lookup is performed by calling -`parse_object()` directly, which thus hits the object database. In the -general case though most of the objects the client requests will be -commits. We can thus try to look up the object via the commit-graph -opportunistically, which is much faster than doing the same via the -object database. - -Refactor parsing of both "want" and "want-ref" lines to do so. - -The following benchmark is executed in a repository with a huge number -of references. It uses cached request from git-fetch(1) as input to -git-upload-pack(1) that contains about 876,000 "want" lines: - - Benchmark 1: HEAD~ - Time (mean ± σ): 7.113 s ± 0.028 s [User: 6.900 s, System: 0.662 s] - Range (min … max): 7.072 s … 7.168 s 10 runs - - Benchmark 2: HEAD - Time (mean ± σ): 6.622 s ± 0.061 s [User: 6.452 s, System: 0.650 s] - Range (min … max): 6.535 s … 6.727 s 10 runs - - Summary - 'HEAD' ran - 1.07 ± 0.01 times faster than 'HEAD~' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - upload-pack.c | 20 +++++++++++++++++--- - 1 file changed, 17 insertions(+), 3 deletions(-) - -diff --git a/upload-pack.c b/upload-pack.c -index 8acc98741b..3a851b3606 100644 ---- a/upload-pack.c -+++ b/upload-pack.c -@@ -1400,13 +1400,19 @@ static int parse_want(struct packet_writer *writer, const char *line, - const char *arg; - if (skip_prefix(line, "want ", &arg)) { - struct object_id oid; -+ struct commit *commit; - struct object *o; - - if (get_oid_hex(arg, &oid)) - die("git upload-pack: protocol error, " - "expected to get oid, not '%s'", line); - -- o = parse_object(the_repository, &oid); -+ commit = lookup_commit_in_graph(the_repository, &oid); -+ if (commit) -+ o = &commit->object; -+ else -+ o = parse_object(the_repository, &oid); -+ - if (!o) { - packet_writer_error(writer, - "upload-pack: not our ref %s", -@@ -1434,7 +1440,7 @@ static int parse_want_ref(struct packet_writer *writer, const char *line, - if (skip_prefix(line, "want-ref ", &refname_nons)) { - struct object_id oid; - struct string_list_item *item; -- struct object *o; -+ struct object *o = NULL; - struct strbuf refname = STRBUF_INIT; - - strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons); -@@ -1448,7 +1454,15 @@ static int parse_want_ref(struct packet_writer *writer, const char *line, - item = string_list_append(wanted_refs, refname_nons); - item->util = oiddup(&oid); - -- o = parse_object_or_die(&oid, refname_nons); -+ if (!starts_with(refname_nons, "refs/tags/")) { -+ struct commit *commit = lookup_commit_in_graph(the_repository, &oid); -+ if (commit) -+ o = &commit->object; -+ } -+ -+ if (!o) -+ o = parse_object_or_die(&oid, refname_nons); -+ - if (!(o->flags & WANTED)) { - o->flags |= WANTED; - add_object_array(o, NULL, want_obj); --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0036-fetch-avoid-lookup-of-commits-when-not-appending-to-.patch b/_support/git-patches/v2.35.1.gl1/0036-fetch-avoid-lookup-of-commits-when-not-appending-to-.patch deleted file mode 100644 index 686dfde8e..000000000 --- a/_support/git-patches/v2.35.1.gl1/0036-fetch-avoid-lookup-of-commits-when-not-appending-to-.patch +++ /dev/null @@ -1,119 +0,0 @@ -From 8e55634b47858f036ca773f3e1d754e5dbd4bb59 Mon Sep 17 00:00:00 2001 -Message-Id: <8e55634b47858f036ca773f3e1d754e5dbd4bb59.1647334702.git.ps@pks.im> -In-Reply-To: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -References: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Tue, 1 Mar 2022 10:33:41 +0100 -Subject: [PATCH 36/39] fetch: avoid lookup of commits when not appending to - FETCH_HEAD -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -When fetching from a remote repository we will by default write what has -been fetched into the special FETCH_HEAD reference. The order in which -references are written depends on whether the reference is for merge or -not, which, despite some other conditions, is also determined based on -whether the old object ID the reference is being updated from actually -exists in the repository. - -To write FETCH_HEAD we thus loop through all references thrice: once for -the references that are about to be merged, once for the references that -are not for merge, and finally for all references that are ignored. For -every iteration, we then look up the old object ID to determine whether -the referenced object exists so that we can label it as "not-for-merge" -if it doesn't exist. It goes without saying that this can be expensive -in case where we are fetching a lot of references. - -While this is hard to avoid in the case where we're writing FETCH_HEAD, -users can in fact ask us to skip this work via `--no-write-fetch-head`. -In that case, we do not care for the result of those lookups at all -because we don't have to order writes to FETCH_HEAD in the first place. - -Skip this busywork in case we're not writing to FETCH_HEAD. The -following benchmark performs a mirror-fetch in a repository with about -two million references via `git fetch --prune --no-write-fetch-head -+refs/*:refs/*`: - - Benchmark 1: HEAD~ - Time (mean ± σ): 75.388 s ± 1.942 s [User: 71.103 s, System: 8.953 s] - Range (min … max): 73.184 s … 76.845 s 3 runs - - Benchmark 2: HEAD - Time (mean ± σ): 69.486 s ± 1.016 s [User: 65.941 s, System: 8.806 s] - Range (min … max): 68.864 s … 70.659 s 3 runs - - Summary - 'HEAD' ran - 1.08 ± 0.03 times faster than 'HEAD~' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - builtin/fetch.c | 42 +++++++++++++++++++++++++++--------------- - 1 file changed, 27 insertions(+), 15 deletions(-) - -diff --git a/builtin/fetch.c b/builtin/fetch.c -index ec1ec91da2..3d4581d3c6 100644 ---- a/builtin/fetch.c -+++ b/builtin/fetch.c -@@ -1143,7 +1143,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - want_status <= FETCH_HEAD_IGNORE; - want_status++) { - for (rm = ref_map; rm; rm = rm->next) { -- struct commit *commit = NULL; - struct ref *ref = NULL; - - if (rm->status == REF_STATUS_REJECT_SHALLOW) { -@@ -1154,21 +1153,34 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, - } - - /* -- * References in "refs/tags/" are often going to point -- * to annotated tags, which are not part of the -- * commit-graph. We thus only try to look up refs in -- * the graph which are not in that namespace to not -- * regress performance in repositories with many -- * annotated tags. -+ * When writing FETCH_HEAD we need to determine whether -+ * we already have the commit or not. If not, then the -+ * reference is not for merge and needs to be written -+ * to the reflog after other commits which we already -+ * have. We're not interested in this property though -+ * in case FETCH_HEAD is not to be updated, so we can -+ * skip the classification in that case. - */ -- if (!starts_with(rm->name, "refs/tags/")) -- commit = lookup_commit_in_graph(the_repository, &rm->old_oid); -- if (!commit) { -- commit = lookup_commit_reference_gently(the_repository, -- &rm->old_oid, -- 1); -- if (!commit) -- rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; -+ if (fetch_head->fp) { -+ struct commit *commit = NULL; -+ -+ /* -+ * References in "refs/tags/" are often going to point -+ * to annotated tags, which are not part of the -+ * commit-graph. We thus only try to look up refs in -+ * the graph which are not in that namespace to not -+ * regress performance in repositories with many -+ * annotated tags. -+ */ -+ if (!starts_with(rm->name, "refs/tags/")) -+ commit = lookup_commit_in_graph(the_repository, &rm->old_oid); -+ if (!commit) { -+ commit = lookup_commit_reference_gently(the_repository, -+ &rm->old_oid, -+ 1); -+ if (!commit) -+ rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; -+ } - } - - if (rm->fetch_head_status != want_status) --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0037-refs-add-ability-for-backends-to-special-case-readin.patch b/_support/git-patches/v2.35.1.gl1/0037-refs-add-ability-for-backends-to-special-case-readin.patch deleted file mode 100644 index 695f04a1b..000000000 --- a/_support/git-patches/v2.35.1.gl1/0037-refs-add-ability-for-backends-to-special-case-readin.patch +++ /dev/null @@ -1,166 +0,0 @@ -From cd475b3b03809b1b1c664e0dca9f16f815456719 Mon Sep 17 00:00:00 2001 -Message-Id: <cd475b3b03809b1b1c664e0dca9f16f815456719.1647334702.git.ps@pks.im> -In-Reply-To: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -References: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Tue, 1 Mar 2022 10:33:46 +0100 -Subject: [PATCH 37/39] refs: add ability for backends to special-case reading - of symbolic refs - -Reading of symbolic and non-symbolic references is currently treated the -same in reference backends: we always call `refs_read_raw_ref()` and -then decide based on the returned flags what type it is. This has one -downside though: symbolic references may be treated different from -normal references in a backend from normal references. The packed-refs -backend for example doesn't even know about symbolic references, and as -a result it is pointless to even ask it for one. - -There are cases where we really only care about whether a reference is -symbolic or not, but don't care about whether it exists at all or may be -a non-symbolic reference. But it is not possible to optimize for this -case right now, and as a consequence we will always first check for a -loose reference to exist, and if it doesn't, we'll query the packed-refs -backend for a known-to-not-be-symbolic reference. This is inefficient -and requires us to search all packed references even though we know to -not care for the result at all. - -Introduce a new function `refs_read_symbolic_ref()` which allows us to -fix this case. This function will only ever return symbolic references -and can thus optimize for the scenario layed out above. By default, if -the backend doesn't provide an implementation for it, we just use the -old code path and fall back to `read_raw_ref()`. But in case the backend -provides its own, more efficient implementation, we will use that one -instead. - -Note that this function is explicitly designed to not distinguish -between missing references and non-symbolic references. If it did, we'd -be forced to always search the packed-refs backend to see whether the -symbolic reference the user asked for really doesn't exist, or if it -exists as a non-symbolic reference. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - refs.c | 17 +++++++++++++++++ - refs.h | 3 +++ - refs/debug.c | 1 + - refs/files-backend.c | 1 + - refs/packed-backend.c | 1 + - refs/refs-internal.h | 16 ++++++++++++++++ - 6 files changed, 39 insertions(+) - -diff --git a/refs.c b/refs.c -index 35d4e69687..d75a5dd53d 100644 ---- a/refs.c -+++ b/refs.c -@@ -1672,6 +1672,23 @@ int refs_read_raw_ref(struct ref_store *ref_store, const char *refname, - type, failure_errno); - } - -+int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname, -+ struct strbuf *referent) -+{ -+ struct object_id oid; -+ int ret, failure_errno = 0; -+ unsigned int type = 0; -+ -+ if (ref_store->be->read_symbolic_ref) -+ return ref_store->be->read_symbolic_ref(ref_store, refname, referent); -+ -+ ret = refs_read_raw_ref(ref_store, refname, &oid, referent, &type, &failure_errno); -+ if (ret || !(type & REF_ISSYMREF)) -+ return -1; -+ -+ return 0; -+} -+ - const char *refs_resolve_ref_unsafe(struct ref_store *refs, - const char *refname, - int resolve_flags, -diff --git a/refs.h b/refs.h -index 1ae12c410a..23479c7ee0 100644 ---- a/refs.h -+++ b/refs.h -@@ -82,6 +82,9 @@ int read_ref_full(const char *refname, int resolve_flags, - struct object_id *oid, int *flags); - int read_ref(const char *refname, struct object_id *oid); - -+int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname, -+ struct strbuf *referent); -+ - /* - * Return 0 if a reference named refname could be created without - * conflicting with the name of an existing reference. Otherwise, -diff --git a/refs/debug.c b/refs/debug.c -index 2b0771ca53..c590d37720 100644 ---- a/refs/debug.c -+++ b/refs/debug.c -@@ -435,6 +435,7 @@ struct ref_storage_be refs_be_debug = { - - debug_ref_iterator_begin, - debug_read_raw_ref, -+ NULL, - - debug_reflog_iterator_begin, - debug_for_each_reflog_ent, -diff --git a/refs/files-backend.c b/refs/files-backend.c -index f59589d6cc..f3428a9f12 100644 ---- a/refs/files-backend.c -+++ b/refs/files-backend.c -@@ -3286,6 +3286,7 @@ struct ref_storage_be refs_be_files = { - - files_ref_iterator_begin, - files_read_raw_ref, -+ NULL, - - files_reflog_iterator_begin, - files_for_each_reflog_ent, -diff --git a/refs/packed-backend.c b/refs/packed-backend.c -index 27dd8c3922..f56e2cc635 100644 ---- a/refs/packed-backend.c -+++ b/refs/packed-backend.c -@@ -1684,6 +1684,7 @@ struct ref_storage_be refs_be_packed = { - - packed_ref_iterator_begin, - packed_read_raw_ref, -+ NULL, - - packed_reflog_iterator_begin, - packed_for_each_reflog_ent, -diff --git a/refs/refs-internal.h b/refs/refs-internal.h -index 6e15db3ca4..001ef15835 100644 ---- a/refs/refs-internal.h -+++ b/refs/refs-internal.h -@@ -649,6 +649,21 @@ typedef int read_raw_ref_fn(struct ref_store *ref_store, const char *refname, - struct object_id *oid, struct strbuf *referent, - unsigned int *type, int *failure_errno); - -+/* -+ * Read a symbolic reference from the specified reference store. This function -+ * is optional: if not implemented by a backend, then `read_raw_ref_fn` is used -+ * to read the symbolcic reference instead. It is intended to be implemented -+ * only in case the backend can optimize the reading of symbolic references. -+ * -+ * Return 0 on success, or -1 on failure. `referent` will be set to the target -+ * of the symbolic reference on success. This function explicitly does not -+ * distinguish between error cases and the reference not being a symbolic -+ * reference to allow backends to optimize this operation in case symbolic and -+ * non-symbolic references are treated differently. -+ */ -+typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refname, -+ struct strbuf *referent); -+ - struct ref_storage_be { - struct ref_storage_be *next; - const char *name; -@@ -668,6 +683,7 @@ struct ref_storage_be { - - ref_iterator_begin_fn *iterator_begin; - read_raw_ref_fn *read_raw_ref; -+ read_symbolic_ref_fn *read_symbolic_ref; - - reflog_iterator_begin_fn *reflog_iterator_begin; - for_each_reflog_ent_fn *for_each_reflog_ent; --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0038-remote-read-symbolic-refs-via-refs_read_symbolic_ref.patch b/_support/git-patches/v2.35.1.gl1/0038-remote-read-symbolic-refs-via-refs_read_symbolic_ref.patch deleted file mode 100644 index 0ea1df7b0..000000000 --- a/_support/git-patches/v2.35.1.gl1/0038-remote-read-symbolic-refs-via-refs_read_symbolic_ref.patch +++ /dev/null @@ -1,101 +0,0 @@ -From 1553f5e76c72243f61b454412b651706a869388f Mon Sep 17 00:00:00 2001 -Message-Id: <1553f5e76c72243f61b454412b651706a869388f.1647334702.git.ps@pks.im> -In-Reply-To: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -References: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Tue, 1 Mar 2022 10:33:50 +0100 -Subject: [PATCH 38/39] remote: read symbolic refs via - `refs_read_symbolic_ref()` - -We have two cases in the remote code where we check whether a reference -is symbolic or not, but don't mind in case it doesn't exist or in case -it exists but is a non-symbolic reference. Convert these two callsites -to use the new `refs_read_symbolic_ref()` function, whose intent is to -implement exactly that usecase. - -No change in behaviour is expected from this change. - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - builtin/remote.c | 8 +++++--- - remote.c | 14 +++++++------- - 2 files changed, 12 insertions(+), 10 deletions(-) - -diff --git a/builtin/remote.c b/builtin/remote.c -index 299c466116..805a517192 100644 ---- a/builtin/remote.c -+++ b/builtin/remote.c -@@ -766,13 +766,15 @@ static int mv(int argc, const char **argv) - for_each_ref(read_remote_branches, &rename); - for (i = 0; i < remote_branches.nr; i++) { - struct string_list_item *item = remote_branches.items + i; -- int flag = 0; -+ struct strbuf referent = STRBUF_INIT; - -- read_ref_full(item->string, RESOLVE_REF_READING, NULL, &flag); -- if (!(flag & REF_ISSYMREF)) -+ if (refs_read_symbolic_ref(get_main_ref_store(the_repository), item->string, -+ &referent)) - continue; - if (delete_ref(NULL, item->string, NULL, REF_NO_DEREF)) - die(_("deleting '%s' failed"), item->string); -+ -+ strbuf_release(&referent); - } - for (i = 0; i < remote_branches.nr; i++) { - struct string_list_item *item = remote_branches.items + i; -diff --git a/remote.c b/remote.c -index c97c626eaa..42a4e7106e 100644 ---- a/remote.c -+++ b/remote.c -@@ -1945,13 +1945,9 @@ const char *branch_get_push(struct branch *branch, struct strbuf *err) - return branch->push_tracking_ref; - } - --static int ignore_symref_update(const char *refname) -+static int ignore_symref_update(const char *refname, struct strbuf *scratch) - { -- int flag; -- -- if (!resolve_ref_unsafe(refname, 0, NULL, &flag)) -- return 0; /* non-existing refs are OK */ -- return (flag & REF_ISSYMREF); -+ return !refs_read_symbolic_ref(get_main_ref_store(the_repository), refname, scratch); - } - - /* -@@ -1964,6 +1960,7 @@ static int ignore_symref_update(const char *refname) - static struct ref *get_expanded_map(const struct ref *remote_refs, - const struct refspec_item *refspec) - { -+ struct strbuf scratch = STRBUF_INIT; - const struct ref *ref; - struct ref *ret = NULL; - struct ref **tail = &ret; -@@ -1971,11 +1968,13 @@ static struct ref *get_expanded_map(const struct ref *remote_refs, - for (ref = remote_refs; ref; ref = ref->next) { - char *expn_name = NULL; - -+ strbuf_reset(&scratch); -+ - if (strchr(ref->name, '^')) - continue; /* a dereference item */ - if (match_name_with_pattern(refspec->src, ref->name, - refspec->dst, &expn_name) && -- !ignore_symref_update(expn_name)) { -+ !ignore_symref_update(expn_name, &scratch)) { - struct ref *cpy = copy_ref(ref); - - cpy->peer_ref = alloc_ref(expn_name); -@@ -1987,6 +1986,7 @@ static struct ref *get_expanded_map(const struct ref *remote_refs, - free(expn_name); - } - -+ strbuf_release(&scratch); - return ret; - } - --- -2.35.1 - diff --git a/_support/git-patches/v2.35.1.gl1/0039-refs-files-backend-optimize-reading-of-symbolic-refs.patch b/_support/git-patches/v2.35.1.gl1/0039-refs-files-backend-optimize-reading-of-symbolic-refs.patch deleted file mode 100644 index 465257950..000000000 --- a/_support/git-patches/v2.35.1.gl1/0039-refs-files-backend-optimize-reading-of-symbolic-refs.patch +++ /dev/null @@ -1,122 +0,0 @@ -From 0a7b38707d5d5c8a7baeb88a85d6259cee4f4e4d Mon Sep 17 00:00:00 2001 -Message-Id: <0a7b38707d5d5c8a7baeb88a85d6259cee4f4e4d.1647334703.git.ps@pks.im> -In-Reply-To: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -References: <4de656263aa195080495fc0a103351b9eaac8160.1647334702.git.ps@pks.im> -From: Patrick Steinhardt <ps@pks.im> -Date: Tue, 1 Mar 2022 10:33:54 +0100 -Subject: [PATCH 39/39] refs/files-backend: optimize reading of symbolic refs -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -When reading references via `files_read_raw_ref()` we always consult -both the loose reference, and if that wasn't found, we also consult the -packed-refs file. While this makes sense to read a generic reference, it -is wasteful in the case where we only care about symbolic references: -the packed-refs backend does not support them, and thus it cannot ever -return one for us. - -Special-case reading of symbolic references for the files backend such -that we always skip asking the packed-refs backend. - -We use `refs_read_symbolic_ref()` extensively to determine whether we -need to skip updating local symbolic references during a fetch, which is -why the change results in a significant speedup when doing fetches in -repositories with huge numbers of references. The following benchmark -executes a mirror-fetch in a repository with about 2 million references -via `git fetch --prune --no-write-fetch-head +refs/*:refs/*`: - - Benchmark 1: HEAD~ - Time (mean ± σ): 68.372 s ± 2.344 s [User: 65.629 s, System: 8.786 s] - Range (min … max): 65.745 s … 70.246 s 3 runs - - Benchmark 2: HEAD - Time (mean ± σ): 60.259 s ± 0.343 s [User: 61.019 s, System: 7.245 s] - Range (min … max): 60.003 s … 60.649 s 3 runs - - Summary - 'HEAD' ran - 1.13 ± 0.04 times faster than 'HEAD~' - -Signed-off-by: Patrick Steinhardt <ps@pks.im> -Signed-off-by: Junio C Hamano <gitster@pobox.com> ---- - refs/files-backend.c | 34 ++++++++++++++++++++++++++++------ - 1 file changed, 28 insertions(+), 6 deletions(-) - -diff --git a/refs/files-backend.c b/refs/files-backend.c -index f3428a9f12..0457ecdb42 100644 ---- a/refs/files-backend.c -+++ b/refs/files-backend.c -@@ -338,9 +338,9 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs) - return refs->loose; - } - --static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, -- struct object_id *oid, struct strbuf *referent, -- unsigned int *type, int *failure_errno) -+static int read_ref_internal(struct ref_store *ref_store, const char *refname, -+ struct object_id *oid, struct strbuf *referent, -+ unsigned int *type, int *failure_errno, int skip_packed_refs) - { - struct files_ref_store *refs = - files_downcast(ref_store, REF_STORE_READ, "read_raw_ref"); -@@ -381,7 +381,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, - if (lstat(path, &st) < 0) { - int ignore_errno; - myerr = errno; -- if (myerr != ENOENT) -+ if (myerr != ENOENT || skip_packed_refs) - goto out; - if (refs_read_raw_ref(refs->packed_ref_store, refname, oid, - referent, type, &ignore_errno)) { -@@ -425,7 +425,8 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, - * ref is supposed to be, there could still be a - * packed ref: - */ -- if (refs_read_raw_ref(refs->packed_ref_store, refname, oid, -+ if (skip_packed_refs || -+ refs_read_raw_ref(refs->packed_ref_store, refname, oid, - referent, type, &ignore_errno)) { - myerr = EISDIR; - goto out; -@@ -470,6 +471,27 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, - return ret; - } - -+static int files_read_raw_ref(struct ref_store *ref_store, const char *refname, -+ struct object_id *oid, struct strbuf *referent, -+ unsigned int *type, int *failure_errno) -+{ -+ return read_ref_internal(ref_store, refname, oid, referent, type, failure_errno, 0); -+} -+ -+static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refname, -+ struct strbuf *referent) -+{ -+ struct object_id oid; -+ int failure_errno, ret; -+ unsigned int type; -+ -+ ret = read_ref_internal(ref_store, refname, &oid, referent, &type, &failure_errno, 1); -+ if (ret) -+ return ret; -+ -+ return !(type & REF_ISSYMREF); -+} -+ - int parse_loose_ref_contents(const char *buf, struct object_id *oid, - struct strbuf *referent, unsigned int *type, - int *failure_errno) -@@ -3286,7 +3308,7 @@ struct ref_storage_be refs_be_files = { - - files_ref_iterator_begin, - files_read_raw_ref, -- NULL, -+ files_read_symbolic_ref, - - files_reflog_iterator_begin, - files_for_each_reflog_ent, --- -2.35.1 - |