diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc index 145642bf05..cafee0aada 100644 --- a/Documentation/githooks.adoc +++ b/Documentation/githooks.adoc @@ -509,14 +509,15 @@ receives on standard input a line of the format: SP SP LF where `` is the old object name passed into the reference -transaction, `` is the new object name to be stored in the -ref and `` is the full name of the ref. When force updating -the reference regardless of its current value or when the reference is -to be created anew, `` is the all-zeroes object name. To -distinguish these cases, you can inspect the current value of -`` via `git rev-parse`. During the "preparing" state, symbolic -references are not resolved: `` will reflect the symbolic reference -itself rather than the object it points to. +transaction, or the value observed while preparing the transaction if no +old object name was passed. `` is the new object name to be +stored in the ref and `` is the full name of the ref. When the +reference does not exist, `` is the all-zeroes object name. +Because references are not yet locked in the "preparing" state, its observed +old value may differ from the value reported in subsequent states if the +reference changes before it is locked. During the "preparing" state, +symbolic references are not resolved: `` will reflect the symbolic +reference itself rather than the object it points to. For symbolic reference updates the `` and `` fields could denote references instead of objects. A reference will be diff --git a/refs.c b/refs.c index 951db56113..3bd7cd94f6 100644 --- a/refs.c +++ b/refs.c @@ -1283,6 +1283,7 @@ void ref_transaction_free(struct ref_transaction *transaction) free(transaction->updates[i]->committer_info); free((char *)transaction->updates[i]->new_target); free((char *)transaction->updates[i]->old_target); + free(transaction->updates[i]->hook_old_target); free((char *)transaction->updates[i]->rejection_details); free(transaction->updates[i]); } @@ -2629,6 +2630,8 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_ struct transaction_feed_cb_data *feed_cb_data = pp_task_cb; struct strbuf *buf = &feed_cb_data->buf; struct ref_update *update; + const struct object_id *old_oid; + const char *old_target; size_t i = feed_cb_data->index++; int ret; @@ -2642,12 +2645,18 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_ strbuf_reset(buf); - if (!(update->flags & REF_HAVE_OLD)) - strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo))); - else if (update->old_target) - strbuf_addf(buf, "ref:%s ", update->old_target); + if (update->flags & REF_HAVE_OLD) { + old_oid = &update->old_oid; + old_target = update->old_target; + } else { + old_oid = &update->hook_old_oid; + old_target = update->hook_old_target; + } + + if (old_target) + strbuf_addf(buf, "ref:%s ", old_target); else - strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid)); + strbuf_addf(buf, "%s ", oid_to_hex(old_oid)); if (!(update->flags & REF_HAVE_NEW)) strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo))); @@ -2683,6 +2692,36 @@ static void transaction_feed_cb_data_free(void *data) free(d); } +static void resolve_transaction_hook_old_values(struct ref_transaction *transaction) +{ + struct ref_store *refs = transaction->ref_store; + struct strbuf referent = STRBUF_INIT; + + if (!hook_exists(refs->repo, "reference-transaction")) + return; + + for (size_t i = 0; i < transaction->nr; i++) { + struct ref_update *update = transaction->updates[i]; + unsigned int type = 0; + int failure_errno; + + if (update->flags & (REF_HAVE_OLD | REF_LOG_ONLY)) + continue; + + oidclr(&update->hook_old_oid, refs->repo->hash_algo); + FREE_AND_NULL(update->hook_old_target); + strbuf_reset(&referent); + + if (!refs_read_raw_ref(refs, update->refname, + &update->hook_old_oid, &referent, + &type, &failure_errno) && + (type & REF_ISSYMREF)) + update->hook_old_target = xstrdup(referent.buf); + } + + strbuf_release(&referent); +} + static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { @@ -2732,6 +2771,8 @@ int ref_transaction_prepare(struct ref_transaction *transaction, if (ref_update_reject_duplicates(&transaction->refnames, err)) return REF_TRANSACTION_ERROR_GENERIC; + resolve_transaction_hook_old_values(transaction); + /* Preparing checks before locking references */ ret = run_transaction_hook(transaction, "preparing"); if (ret) { @@ -2743,6 +2784,9 @@ int ref_transaction_prepare(struct ref_transaction *transaction, if (ret) return ret; + /* Refresh old values now that the references are locked. */ + resolve_transaction_hook_old_values(transaction); + ret = run_transaction_hook(transaction, "prepared"); if (ret) { ref_transaction_abort(transaction, err); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index c3ac7b556f..a7471b2481 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -99,6 +99,14 @@ struct ref_update { */ struct object_id old_oid; + /* + * The old value observed for the reference-transaction hook when the + * caller did not provide an expected old value. Unlike old_oid and + * old_target, these fields do not constrain the update. + */ + struct object_id hook_old_oid; + char *hook_old_target; + /* * If the new_oid points to a tag object, set this to the peeled * object ID for optimized retrieval without needed to hit the odb. diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 4fe9d9b234..fcc7404943 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -14,6 +14,66 @@ test_expect_success setup ' POST_OID=$(git rev-parse POST) ' +test_expect_success 'hook gets old values for batched unconditional deletion' ' + test_when_finished "rm -f actual" && + test_when_finished "git remote remove origin && rm -rf empty.git" && + git init --bare empty.git && + git remote add origin ./empty.git && + git branch delete-a PRE && + git branch delete-b POST && + git tag delete-tag POST && + git update-ref refs/remotes/origin/to-prune $PRE_OID && + test_hook reference-transaction <<-\EOF && + if test "$1" = committed + then + cat >>actual + fi + EOF + git branch -D delete-a delete-b && + git tag -d delete-tag && + git remote prune origin && + cat >expect <<-EOF && + $PRE_OID $ZERO_OID refs/heads/delete-a + $POST_OID $ZERO_OID refs/heads/delete-b + $POST_OID $ZERO_OID refs/tags/delete-tag + $PRE_OID $ZERO_OID refs/remotes/origin/to-prune + EOF + test_cmp expect actual +' + +test_expect_success 'unconditional deletion remains unconditional' ' + test_when_finished "rm -f actual" && + test_when_finished "rm -f \"$(git rev-parse --git-path delete-race-once)\"" && + git branch delete-race PRE && + test_hook reference-transaction <<-\EOF && + state=$1 + while read -r old new ref + do + if test "$state" != aborted + then + case "$new" in + *[!0]*) ;; + *) echo "$state $old $new $ref" >>actual ;; + esac + fi + done + marker=$(git rev-parse --git-path delete-race-once) + if test "$state" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/heads/delete-race POST + fi + EOF + git branch -D delete-race && + cat >expect <<-EOF && + preparing $PRE_OID $ZERO_OID refs/heads/delete-race + prepared $POST_OID $ZERO_OID refs/heads/delete-race + committed $POST_OID $ZERO_OID refs/heads/delete-race + EOF + test_cmp expect actual && + test_must_fail git show-ref --verify refs/heads/delete-race +' + test_expect_success 'hook allows updating ref if successful' ' git reset --hard PRE && test_hook reference-transaction <<-\EOF && @@ -65,7 +125,7 @@ test_expect_success 'hook gets all queued updates in prepared state' ' fi EOF cat >expect <<-EOF && - $ZERO_OID $POST_OID refs/heads/main + $PRE_OID $POST_OID refs/heads/main EOF git update-ref HEAD POST <<-EOF && update HEAD $ZERO_OID $POST_OID @@ -87,7 +147,7 @@ test_expect_success 'hook gets all queued updates in committed state' ' fi EOF cat >expect <<-EOF && - $ZERO_OID $POST_OID refs/heads/main + $PRE_OID $POST_OID refs/heads/main EOF git update-ref HEAD POST && test_cmp expect actual