fetch: allow passing a transaction to `s_update_ref()`

The handling of ref updates is completely handled by `s_update_ref()`,
which will manage the complete lifecycle of the reference transaction.
This is fine right now given that git-fetch(1) does not support atomic
fetches, so each reference gets its own transaction. It is quite
inflexible though, as `s_update_ref()` only knows about a single
reference update at a time, so it doesn't allow us to alter the
strategy.

This commit prepares `s_update_ref()` and its only caller
`update_local_ref()` to allow passing an external transaction. If none
is given, then the existing behaviour is triggered which creates a new
transaction and directly commits it. Otherwise, if the caller provides a
transaction, then we only queue the update but don't commit it. This
optionally allows the caller to manage when a transaction will be
committed.

Given that `update_local_ref()` is always called with a `NULL`
transaction for now, 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>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b24a9e09a46f075fad963f56ee0addce3c73b70e..cada732325a0a657cec4bf5ec8674fd4cefaa337 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -583,11 +583,12 @@ static struct ref *get_ref_map(struct remote *remote,
 
 static int s_update_ref(const char *action,
                        struct ref *ref,
+                       struct ref_transaction *transaction,
                        int check_old)
 {
        char *msg;
        char *rla = getenv("GIT_REFLOG_ACTION");
-       struct ref_transaction *transaction;
+       struct ref_transaction *our_transaction = NULL;
        struct strbuf err = STRBUF_INIT;
        int ret;
 
@@ -597,10 +598,17 @@ static int s_update_ref(const char *action,
                rla = default_rla.buf;
        msg = xstrfmt("%s: %s", rla, action);
 
-       transaction = ref_transaction_begin(&err);
+       /*
+        * If no transaction was passed to us, we manage the transaction
+        * ourselves. Otherwise, we trust the caller to handle the transaction
+        * lifecycle.
+        */
        if (!transaction) {
-               ret = STORE_REF_ERROR_OTHER;
-               goto out;
+               transaction = our_transaction = ref_transaction_begin(&err);
+               if (!transaction) {
+                       ret = STORE_REF_ERROR_OTHER;
+                       goto out;
+               }
        }
 
        ret = ref_transaction_update(transaction, ref->name, &ref->new_oid,
@@ -611,19 +619,21 @@ static int s_update_ref(const char *action,
                goto out;
        }
 
-       switch (ref_transaction_commit(transaction, &err)) {
-       case 0:
-               break;
-       case TRANSACTION_NAME_CONFLICT:
-               ret = STORE_REF_ERROR_DF_CONFLICT;
-               goto out;
-       default:
-               ret = STORE_REF_ERROR_OTHER;
-               goto out;
+       if (our_transaction) {
+               switch (ref_transaction_commit(our_transaction, &err)) {
+               case 0:
+                       break;
+               case TRANSACTION_NAME_CONFLICT:
+                       ret = STORE_REF_ERROR_DF_CONFLICT;
+                       goto out;
+               default:
+                       ret = STORE_REF_ERROR_OTHER;
+                       goto out;
+               }
        }
 
 out:
-       ref_transaction_free(transaction);
+       ref_transaction_free(our_transaction);
        if (ret)
                error("%s", err.buf);
        strbuf_release(&err);
@@ -766,6 +776,7 @@ static void format_display(struct strbuf *display, char code,
 }
 
 static int update_local_ref(struct ref *ref,
+                           struct ref_transaction *transaction,
                            const char *remote,
                            const struct ref *remote_ref,
                            struct strbuf *display,
@@ -806,7 +817,7 @@ static int update_local_ref(struct ref *ref,
            starts_with(ref->name, "refs/tags/")) {
                if (force || ref->force) {
                        int r;
-                       r = s_update_ref("updating tag", ref, 0);
+                       r = s_update_ref("updating tag", ref, transaction, 0);
                        format_display(display, r ? '!' : 't', _("[tag update]"),
                                       r ? _("unable to update local ref") : NULL,
                                       remote, pretty_ref, summary_width);
@@ -843,7 +854,7 @@ static int update_local_ref(struct ref *ref,
                        what = _("[new ref]");
                }
 
-               r = s_update_ref(msg, ref, 0);
+               r = s_update_ref(msg, ref, transaction, 0);
                format_display(display, r ? '!' : '*', what,
                               r ? _("unable to update local ref") : NULL,
                               remote, pretty_ref, summary_width);
@@ -865,7 +876,7 @@ static int update_local_ref(struct ref *ref,
                strbuf_add_unique_abbrev(&quickref, &current->object.oid, DEFAULT_ABBREV);
                strbuf_addstr(&quickref, "..");
                strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
-               r = s_update_ref("fast-forward", ref, 1);
+               r = s_update_ref("fast-forward", ref, transaction, 1);
                format_display(display, r ? '!' : ' ', quickref.buf,
                               r ? _("unable to update local ref") : NULL,
                               remote, pretty_ref, summary_width);
@@ -877,7 +888,7 @@ static int update_local_ref(struct ref *ref,
                strbuf_add_unique_abbrev(&quickref, &current->object.oid, DEFAULT_ABBREV);
                strbuf_addstr(&quickref, "...");
                strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
-               r = s_update_ref("forced-update", ref, 1);
+               r = s_update_ref("forced-update", ref, transaction, 1);
                format_display(display, r ? '!' : '+', quickref.buf,
                               r ? _("unable to update local ref") : _("forced update"),
                               remote, pretty_ref, summary_width);
@@ -1094,8 +1105,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 
                        strbuf_reset(&note);
                        if (ref) {
-                               rc |= update_local_ref(ref, what, rm, &note,
-                                                      summary_width);
+                               rc |= update_local_ref(ref, NULL, what,
+                                                      rm, &note, summary_width);
                                free(ref);
                        } else if (write_fetch_head || dry_run) {
                                /*