@matheusbsbs / matheus-bagliano / commits / c6da34a610

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

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

Junio C Hamano committed Apr 13, 2022 at 15:51 UTC c6da34a610e58f7e58042b5ed24a19bd2c18e928

8 files changed +19 -114

refs.c

+3 -8

@@ -794,7 +794,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,

794 struct ref_transaction *transaction;

795 struct strbuf err = STRBUF_INIT;

796

797 - transaction = ref_store_transaction_begin(refs, 0, &err);

797 + transaction = ref_store_transaction_begin(refs, &err);

798 if (!transaction ||

799 ref_transaction_delete(transaction, refname, old_oid,

800 flags, msg, &err) ||

@@ -999,7 +999,6 @@ int read_ref_at(struct ref_store *refs, const char *refname,

999 }

1000

1001 struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,

1002 - unsigned int flags,

1002 struct strbuf *err)

1003 {

1004 struct ref_transaction *tr;

@@ -1007,13 +1006,12 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,

1006

1007 CALLOC_ARRAY(tr, 1);

1008 tr->ref_store = refs;

1010 - tr->flags = flags;

1009 return tr;

1010 }

1011

1012 struct ref_transaction *ref_transaction_begin(struct strbuf *err)

1013 {

1016 - return ref_store_transaction_begin(get_main_ref_store(the_repository), 0, err);

1014 + return ref_store_transaction_begin(get_main_ref_store(the_repository), err);

1015 }

1016

1017 void ref_transaction_free(struct ref_transaction *transaction)

@@ -1152,7 +1150,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,

1150 struct strbuf err = STRBUF_INIT;

1151 int ret = 0;

1152

1155 - t = ref_store_transaction_begin(refs, 0, &err);

1153 + t = ref_store_transaction_begin(refs, &err);

1154 if (!t ||

1155 ref_transaction_update(t, refname, new_oid, old_oid, flags, msg,

1156 &err) ||

@@ -2074,9 +2072,6 @@ static int run_transaction_hook(struct ref_transaction *transaction,

2072 const char *hook;

2073 int ret = 0, i;

2074

2077 - if (transaction->flags & REF_TRANSACTION_SKIP_HOOK)

2078 - return 0;

2079 -

2075 hook = find_hook("reference-transaction");

2076 if (!hook)

2077 return ret;

refs.h

+1 -7

@@ -229,7 +229,7 @@ char *repo_default_branch_name(struct repository *r, int quiet);

229 * struct strbuf err = STRBUF_INIT;

230 * int ret = 0;

231 *

232 - * transaction = ref_store_transaction_begin(refs, 0, &err);

232 + * transaction = ref_store_transaction_begin(refs, &err);

233 * if (!transaction ||

234 * ref_transaction_update(...) ||

235 * ref_transaction_create(...) ||

@@ -566,17 +566,11 @@ enum action_on_err {

566 UPDATE_REFS_QUIET_ON_ERR

567 };

568

569 -/*

570 - * Skip executing the reference-transaction hook.

571 - */

572 -#define REF_TRANSACTION_SKIP_HOOK (1 << 0)

573 -

569 /*

570 * Begin a reference transaction. The reference transaction must

571 * be freed by calling ref_transaction_free().

572 */

573 struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,

579 - unsigned int flags,

574 struct strbuf *err);

575 struct ref_transaction *ref_transaction_begin(struct strbuf *err);

576

refs/files-backend.c

+7 -19

@@ -1136,8 +1136,7 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r)

1136 if (check_refname_format(r->name, 0))

1137 return;

1138

1139 - transaction = ref_store_transaction_begin(&refs->base,

1140 - REF_TRANSACTION_SKIP_HOOK, &err);

1139 + transaction = ref_store_transaction_begin(&refs->base, &err);

1140 if (!transaction)

1141 goto cleanup;

1142 ref_transaction_add_update(

@@ -1208,8 +1207,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)

1207 struct strbuf err = STRBUF_INIT;

1208 struct ref_transaction *transaction;

1209

1211 - transaction = ref_store_transaction_begin(refs->packed_ref_store,

1212 - REF_TRANSACTION_SKIP_HOOK, &err);

1210 + transaction = ref_store_transaction_begin(refs->packed_ref_store, &err);

1211 if (!transaction)

1212 return -1;

1213

@@ -1266,7 +1264,6 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,

1264 {

1265 struct files_ref_store *refs =

1266 files_downcast(ref_store, REF_STORE_WRITE, "delete_refs");

1269 - struct ref_transaction *transaction = NULL;

1267 struct strbuf err = STRBUF_INIT;

1268 int i, result = 0;

1269

@@ -1276,15 +1273,10 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,

1273 if (packed_refs_lock(refs->packed_ref_store, 0, &err))

1274 goto error;

1275

1279 - transaction = ref_store_transaction_begin(refs->packed_ref_store,

1280 - REF_TRANSACTION_SKIP_HOOK, &err);

1281 - if (!transaction)

1282 - goto error;

1283 -

1284 - result = packed_refs_delete_refs(refs->packed_ref_store,

1285 - transaction, msg, refnames, flags);

1286 - if (result)

1276 + if (refs_delete_refs(refs->packed_ref_store, msg, refnames, flags)) {

1277 + packed_refs_unlock(refs->packed_ref_store);

1278 goto error;

1279 + }

1280

1281 packed_refs_unlock(refs->packed_ref_store);

1282

@@ -1295,7 +1287,6 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,

1287 result |= error(_("could not remove reference %s"), refname);

1288 }

1289

1298 - ref_transaction_free(transaction);

1290 strbuf_release(&err);

1291 return result;

1292

@@ -1312,7 +1303,6 @@ error:

1303 else

1304 error(_("could not delete references: %s"), err.buf);

1305

1315 - ref_transaction_free(transaction);

1306 strbuf_release(&err);

1307 return -1;

1308 }

@@ -2784,8 +2774,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,

2774 */

2775 if (!packed_transaction) {

2776 packed_transaction = ref_store_transaction_begin(

2787 - refs->packed_ref_store,

2788 - REF_TRANSACTION_SKIP_HOOK, err);

2777 + refs->packed_ref_store, err);

2778 if (!packed_transaction) {

2779 ret = TRANSACTION_GENERIC_ERROR;

2780 goto cleanup;

@@ -3056,8 +3045,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,

3045 &affected_refnames))

3046 BUG("initial ref transaction called with existing refs");

3047

3059 - packed_transaction = ref_store_transaction_begin(refs->packed_ref_store,

3060 - REF_TRANSACTION_SKIP_HOOK, err);

3048 + packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, err);

3049 if (!packed_transaction) {

3050 ret = TRANSACTION_GENERIC_ERROR;

3051 goto cleanup;

refs/packed-backend.c

+7 -21

@@ -1522,10 +1522,15 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store,

1522 static int packed_delete_refs(struct ref_store *ref_store, const char *msg,

1523 struct string_list *refnames, unsigned int flags)

1524 {

1525 + struct packed_ref_store *refs =

1526 + packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");

1527 struct strbuf err = STRBUF_INIT;

1528 struct ref_transaction *transaction;

1529 + struct string_list_item *item;

1530 int ret;

1531

1532 + (void)refs; /* We need the check above, but don't use the variable */

1533 +

1534 if (!refnames->nr)

1535 return 0;

1536

@@ -1535,30 +1540,10 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,

1540 * updates into a single transaction.

1541 */

1542

1538 - transaction = ref_store_transaction_begin(ref_store, 0, &err);

1543 + transaction = ref_store_transaction_begin(ref_store, &err);

1544 if (!transaction)

1545 return -1;

1546

1542 - ret = packed_refs_delete_refs(ref_store, transaction,

1543 - msg, refnames, flags);

1544 -

1545 - ref_transaction_free(transaction);

1546 - return ret;

1547 -}

1548 -

1549 -int packed_refs_delete_refs(struct ref_store *ref_store,

1550 - struct ref_transaction *transaction,

1551 - const char *msg,

1552 - struct string_list *refnames,

1553 - unsigned int flags)

1554 -{

1555 - struct strbuf err = STRBUF_INIT;

1556 - struct string_list_item *item;

1557 - int ret;

1558 -

1559 - /* Assert that the ref store refers to a packed backend. */

1560 - packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");

1561 -

1547 for_each_string_list_item(item, refnames) {

1548 if (ref_transaction_delete(transaction, item->string, NULL,

1549 flags, msg, &err)) {

@@ -1578,6 +1563,7 @@ int packed_refs_delete_refs(struct ref_store *ref_store,

1563 error(_("could not delete references: %s"), err.buf);

1564 }

1565

1566 + ref_transaction_free(transaction);

1567 strbuf_release(&err);

1568 return ret;

1569 }

refs/packed-backend.h

-7

@@ -3,7 +3,6 @@

4 struct repository;

5 struct ref_transaction;

6 -struct string_list;

7 /*

8 * 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)

27 void packed_refs_unlock(struct ref_store *ref_store);

28 int packed_refs_is_locked(struct ref_store *ref_store);

31 -int packed_refs_delete_refs(struct ref_store *ref_store,

32 - struct ref_transaction *transaction,

33 - const char *msg,

34 - struct string_list *refnames,

35 - unsigned int flags);

36 -

30 /*

31 * Return true if `transaction` really needs to be carried out against

32 * the specified packed_ref_store, or false if it can be skipped

refs/refs-internal.h

-1

@@ -213,7 +213,6 @@ struct ref_transaction {

213 size_t nr;

214 enum ref_transaction_state state;

215 void *backend_data;

216 - unsigned int flags;

216 };

217

218 /*

sequencer.c

+1 -1

@@ -3584,7 +3584,7 @@ static int do_label(struct repository *r, const char *name, int len)

3584 strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);

3585 strbuf_addf(&msg, "rebase (label) '%.*s'", len, name);

3586

3587 - transaction = ref_store_transaction_begin(refs, 0, &err);

3587 + transaction = ref_store_transaction_begin(refs, &err);

3588 if (!transaction) {

3589 error("%s", err.buf);

3590 ret = -1;

t/t1416-ref-transaction-hooks.sh

-50

@@ -133,54 +133,4 @@ test_expect_success 'interleaving hook calls succeed' '

133 test_cmp expect target-repo.git/actual

134 '

135

136 -test_expect_success 'hook does not get called on packing refs' '

137 - # Pack references first such that we are in a known state.

138 - git pack-refs --all &&

139 -

140 - test_hook reference-transaction <<-\EOF &&

141 - echo "$@" >>actual

142 - cat >>actual

143 - EOF

144 - rm -f actual &&

145 -

146 - git update-ref refs/heads/unpacked-ref $POST_OID &&

147 - git pack-refs --all &&

148 -

149 - # We only expect a single hook invocation, which is the call to

150 - # git-update-ref(1).

151 - cat >expect <<-EOF &&

152 - prepared

153 - $ZERO_OID $POST_OID refs/heads/unpacked-ref

154 - committed

155 - $ZERO_OID $POST_OID refs/heads/unpacked-ref

156 - EOF

157 -

158 - test_cmp expect actual

159 -'

160 -

161 -test_expect_success 'deleting packed ref calls hook once' '

162 - # Create a reference and pack it.

163 - git update-ref refs/heads/to-be-deleted $POST_OID &&

164 - git pack-refs --all &&

165 -

166 - test_hook reference-transaction <<-\EOF &&

167 - echo "$@" >>actual

168 - cat >>actual

169 - EOF

170 - rm -f actual &&

171 -

172 - git update-ref -d refs/heads/to-be-deleted $POST_OID &&

173 -

174 - # We only expect a single hook invocation, which is the logical

175 - # deletion.

176 - cat >expect <<-EOF &&

177 - prepared

178 - $POST_OID $ZERO_OID refs/heads/to-be-deleted

179 - committed

180 - $POST_OID $ZERO_OID refs/heads/to-be-deleted

181 - EOF

182 -

183 - test_cmp expect actual

184 -'

185 -

136 test_done