Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"

This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing
changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.

diff --git a/refs.c b/refs.c
index 1a964505f92ad64da049439439f6da2656abe058..9db66e995518c9f85acd0de0c5fa44658fdcf523 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -794,7 +794,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, 0, &err);
+       transaction = ref_store_transaction_begin(refs, &err);
        if (!transaction ||
            ref_transaction_delete(transaction, refname, old_oid,
                                   flags, msg, &err) ||
@@ -999,7 +999,6 @@ 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;
@@ -1007,13 +1006,12 @@ 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), 0, err);
+       return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
 }
 
 void ref_transaction_free(struct ref_transaction *transaction)
@@ -1152,7 +1150,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, 0, &err);
+       t = ref_store_transaction_begin(refs, &err);
        if (!t ||
            ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,
                                   &err) ||
@@ -2074,9 +2072,6 @@ 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 23479c7ee09b9c5b73d8360a2107d71d03257bae..47cb9edbaa8913c3af721d347744beefacb1e754 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -229,7 +229,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, 0, &err);
+ *         transaction = ref_store_transaction_begin(refs, &err);
  *         if (!transaction ||
  *             ref_transaction_update(...) ||
  *             ref_transaction_create(...) ||
@@ -566,17 +566,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().
  */
 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 95acab78eef34c6f7a8323220b46485a47d45336..8db7882aacb533ebf6635a0457c8b2ac7855c392 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1136,8 +1136,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,
-                                                 REF_TRANSACTION_SKIP_HOOK, &err);
+       transaction = ref_store_transaction_begin(&refs->base, &err);
        if (!transaction)
                goto cleanup;
        ref_transaction_add_update(
@@ -1208,8 +1207,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,
-                                                 REF_TRANSACTION_SKIP_HOOK, &err);
+       transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);
        if (!transaction)
                return -1;
 
@@ -1266,7 +1264,6 @@ 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;
 
@@ -1276,15 +1273,10 @@ 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,
-                                                 REF_TRANSACTION_SKIP_HOOK, &err);
-       if (!transaction)
-               goto error;
-
-       result = packed_refs_delete_refs(refs->packed_ref_store,
-                                        transaction, msg, refnames, flags);
-       if (result)
+       if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {
+               packed_refs_unlock(refs->packed_ref_store);
                goto error;
+       }
 
        packed_refs_unlock(refs->packed_ref_store);
 
@@ -1295,7 +1287,6 @@ 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;
 
@@ -1312,7 +1303,6 @@ error:
        else
                error(_("could not delete references: %s"), err.buf);
 
-       ref_transaction_free(transaction);
        strbuf_release(&err);
        return -1;
 }
@@ -2784,8 +2774,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
                         */
                        if (!packed_transaction) {
                                packed_transaction = ref_store_transaction_begin(
-                                               refs->packed_ref_store,
-                                               REF_TRANSACTION_SKIP_HOOK, err);
+                                               refs->packed_ref_store, err);
                                if (!packed_transaction) {
                                        ret = TRANSACTION_GENERIC_ERROR;
                                        goto cleanup;
@@ -3056,8 +3045,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,
-                                                        REF_TRANSACTION_SKIP_HOOK, err);
+       packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, err);
        if (!packed_transaction) {
                ret = TRANSACTION_GENERIC_ERROR;
                goto cleanup;

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 66c4574c99d7fcc346943224b94cfef06e2e0902..97b68377673067d3a28d60e01f2b4532433750f2 100644 (file)
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1522,10 +1522,15 @@ 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;
 
@@ -1535,30 +1540,10 @@ 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, 0, &err);
+       transaction = ref_store_transaction_begin(ref_store, &err);
        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)) {
@@ -1578,6 +1563,7 @@ int packed_refs_delete_refs(struct ref_store *ref_store,
                        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 52e0490753c3b15e165ea297a4a3ddabfbbf6e97..9dd8a344c34dd7ae078f8226abf044219d5f3a3c 100644 (file)
--- a/refs/packed-backend.h
+++ b/refs/packed-backend.h
@@ -3,7 +3,6 @@
 
 struct repository;
 struct ref_transaction;
-struct string_list;
 
 /*
  * Support for storing references in a `packed-refs` file.
@@ -28,12 +27,6 @@ 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

diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 001ef1583540b002241dfd1d351f72791d39cc9c..69f93b0e2ac9fa9ec2778b783fa98792030c327e 100644 (file)
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -213,7 +213,6 @@ 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 a1bb39383db9ff3319c66faa357463e0c32c3257..a5f678f4529b1953af8dd7b6e168a4a03333e2e9 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -3584,7 +3584,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, 0, &err);
+       transaction = ref_store_transaction_begin(refs, &err);
        if (!transaction) {
                error("%s", err.buf);
                ret = -1;

diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
index 085a7a46f21eec03b2c757e5140b397dc34898e6..27731722a5b6b26bad0b1929c274414239cc449e 100755 (executable)
--- a/t/t1416-ref-transaction-hooks.sh
+++ b/t/t1416-ref-transaction-hooks.sh
@@ -133,54 +133,4 @@ 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 &&
-
-       test_hook 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).
-       cat >expect <<-EOF &&
-               prepared
-               $ZERO_OID $POST_OID refs/heads/unpacked-ref
-               committed
-               $ZERO_OID $POST_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 &&
-
-       test_hook 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.
-       cat >expect <<-EOF &&
-               prepared
-               $POST_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