mirror of
https://github.com/git/git.git
synced 2024-05-11 15:56:08 +02:00
Merge branch 'jc/revert-ref-transaction-hook-changes'
Revert the "deletion of a ref should not trigger transaction events for loose and packed ref backends separately" that regresses the behaviour when a ref is not modified since it was packed. * jc/revert-ref-transaction-hook-changes: RelNotes: revert the description on the reverted topics Revert "fetch: increase test coverage of fetches" Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"
This commit is contained in:
Junio C Hamano
commit
4027e30c53
|
|
@ -298,12 +298,6 @@ Fixes since v2.35
|
|||
future "gc" needs to clean up.
|
||||
(merge 5407764069 cb/clear-quarantine-early-on-all-ref-update-errors later to maint).
|
||||
|
||||
* Because a deletion of ref would need to remove it from both the
|
||||
loose ref store and the packed ref store, a delete-ref operation
|
||||
that logically removes one ref may end up invoking ref-transaction
|
||||
hook twice, which has been corrected.
|
||||
(merge 2ed1b64ebd ps/avoid-unnecessary-hook-invocation-with-packed-refs later to maint).
|
||||
|
||||
* When there is no object to write .bitmap file for, "git
|
||||
multi-pack-index" triggered an error, instead of just skipping,
|
||||
which has been corrected.
|
||||
|
|
@ -351,10 +345,6 @@ Fixes since v2.35
|
|||
recorded the last level component of the branch name, which has
|
||||
been corrected.
|
||||
|
||||
* "git fetch" can make two separate fetches, but ref updates coming
|
||||
from them were in two separate ref transactions under "--atomic",
|
||||
which has been corrected.
|
||||
|
||||
* Check the return value from parse_tree_indirect() to turn segfaults
|
||||
into calls to die().
|
||||
(merge 8d2eaf649a gc/parse-tree-indirect-errors later to maint).
|
||||
|
|
|
|||
11
refs.c
11
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;
|
||||
|
|
|
|||
8
refs.h
8
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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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 @@ 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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -213,7 +213,6 @@ struct ref_transaction {
|
|||
size_t nr;
|
||||
enum ref_transaction_state state;
|
||||
void *backend_data;
|
||||
unsigned int flags;
|
||||
};
|
||||
|
||||
/*
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -160,68 +160,4 @@ 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.
|
||||
test_hook -C clone3 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 &&
|
||||
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' '
|
||||
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
|
||||
$ZERO_OID $T refs/tags/tag1
|
||||
committed
|
||||
$ZERO_OID $B refs/heads/something
|
||||
$ZERO_OID $S refs/tags/tag2
|
||||
$ZERO_OID $T refs/tags/tag1
|
||||
EOF
|
||||
|
||||
test_hook -C clone4 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 &&
|
||||
|
||||
# Create a tag that is nested below the tag we are about to fetch via
|
||||
# the backfill mechanism. This causes a D/F conflict when backfilling
|
||||
# and should thus cause the command to fail.
|
||||
empty_blob=$(git -C clone5 hash-object -w --stdin </dev/null) &&
|
||||
git -C clone5 update-ref refs/tags/tag1/nested $empty_blob &&
|
||||
|
||||
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
|
||||
'
|
||||
|
||||
test_done
|
||||
|
|
|
|||
|
|
@ -343,35 +343,6 @@ 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.
|
||||
cat >expected <<-EOF &&
|
||||
prepared
|
||||
$ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
|
||||
$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
|
||||
|
||||
test_hook -C atomic 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 &&
|
||||
|
|
|
|||
Loading…
Reference in New Issue