diff --git a/bisect.c b/bisect.c index 9cbb3dc677..931a80098f 100644 --- a/bisect.c +++ b/bisect.c @@ -1206,7 +1206,7 @@ int bisect_clean_state(void) string_list_append(&refs_for_removal, "BISECT_EXPECTED_REV"); result = refs_delete_refs(get_main_ref_store(the_repository), "bisect: remove", &refs_for_removal, - REF_NO_DEREF); + NULL, REF_NO_DEREF); string_list_clear(&refs_for_removal, 0); unlink_or_warn(git_path_bisect_ancestors_ok()); unlink_or_warn(git_path_bisect_log()); diff --git a/builtin/branch.c b/builtin/branch.c index a613148fc7..f222a26449 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -16,6 +16,7 @@ #include "commit.h" #include "gettext.h" #include "object-name.h" +#include "oid-array.h" #include "remote.h" #include "parse-options.h" #include "branch.h" @@ -248,6 +249,7 @@ static int delete_branches(int argc, const char **argv, int kinds, struct strbuf bname = STRBUF_INIT; enum interpret_branch_kind allowed_interpret; struct string_list refs_to_delete = STRING_LIST_INIT_DUP; + struct oid_array old_oids = OID_ARRAY_INIT; struct string_list_item *item; int branch_name_pos; const char *fmt_remotes = "refs/remotes/%s"; @@ -342,6 +344,7 @@ static int delete_branches(int argc, const char **argv, int kinds, } item = string_list_append(&refs_to_delete, name); + oid_array_append(&old_oids, &oid); item->util = xstrdup((ref_flags & REF_ISBROKEN) ? "broken" : (ref_flags & REF_ISSYMREF) ? target : repo_find_unique_abbrev(the_repository, &oid, DEFAULT_ABBREV)); @@ -351,7 +354,8 @@ static int delete_branches(int argc, const char **argv, int kinds, } if (!(flags & DELETE_BRANCH_DRY_RUN) && - refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF)) + refs_delete_refs(get_main_ref_store(the_repository), NULL, + &refs_to_delete, &old_oids, REF_NO_DEREF)) ret = 1; for_each_string_list_item(item, &refs_to_delete) { @@ -376,6 +380,7 @@ static int delete_branches(int argc, const char **argv, int kinds, free(describe_ref); } string_list_clear(&refs_to_delete, 0); + oid_array_clear(&old_oids); free(name); strbuf_release(&bname); diff --git a/builtin/fetch.c b/builtin/fetch.c index 981dd44742..29fdc30e9e 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1506,28 +1506,36 @@ static int prune_refs(struct display_state *display_state, struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; struct string_list refnames = STRING_LIST_INIT_NODUP; - - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); + struct oid_array old_oids = OID_ARRAY_INIT; if (!dry_run) { if (transaction) { for (ref = stale_refs; ref; ref = ref->next) { - result = ref_transaction_delete(transaction, ref->name, NULL, - NULL, 0, "fetch: prune", &err); + result = ref_transaction_delete(transaction, ref->name, + &ref->new_oid, NULL, 0, + "fetch: prune", &err); if (result) goto cleanup; } } else { + for (ref = stale_refs; ref; ref = ref->next) { + string_list_append(&refnames, ref->name); + oid_array_append(&old_oids, &ref->new_oid); + } result = refs_delete_refs(get_main_ref_store(the_repository), "fetch: prune", &refnames, - 0); + &old_oids, 0); } + if (result) + goto cleanup; } if (verbosity >= 0) { int summary_width = transport_summary_width(stale_refs); + if (!refnames.nr) + for (ref = stale_refs; ref; ref = ref->next) + string_list_append(&refnames, ref->name); for (ref = stale_refs; ref; ref = ref->next) { display_ref_update(display_state, '-', _("[deleted]"), NULL, _("(none)"), ref->name, @@ -1541,6 +1549,7 @@ static int prune_refs(struct display_state *display_state, cleanup: string_list_clear(&refnames, 0); + oid_array_clear(&old_oids); strbuf_release(&err); free_refs(stale_refs); return result; diff --git a/builtin/remote.c b/builtin/remote.c index de989ea3ba..a99d188326 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -17,6 +17,7 @@ #include "refs.h" #include "refspec.h" #include "odb.h" +#include "oid-array.h" #include "strvec.h" #include "commit-reach.h" #include "progress.h" @@ -380,6 +381,11 @@ struct ref_states { int queried; }; +struct stale_ref { + struct object_id oid; + char name[FLEX_ARRAY]; +}; + #define REF_STATES_INIT { \ .new_refs = STRING_LIST_INIT_DUP, \ .skipped = STRING_LIST_INIT_DUP, \ @@ -410,9 +416,13 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat } stale_refs = get_stale_heads(&states->remote->fetch, fetch_map); for (ref = stale_refs; ref; ref = ref->next) { + struct stale_ref *stale_ref; struct string_list_item *item = string_list_append(&states->stale, abbrev_branch(ref->name)); - item->util = xstrdup(ref->name); + + FLEX_ALLOC_STR(stale_ref, name, ref->name); + oidcpy(&stale_ref->oid, &ref->new_oid); + item->util = stale_ref; } free_refs(stale_refs); free_refs(fetch_map); @@ -1073,7 +1083,7 @@ static int rm(int argc, const char **argv, const char *prefix, if (!result) result = refs_delete_refs(get_main_ref_store(the_repository), "remote: remove", &branches, - REF_NO_DEREF); + NULL, REF_NO_DEREF); string_list_clear(&branches, 0); if (skipped.nr) { @@ -1627,6 +1637,7 @@ static int prune_remote(const char *remote, int dry_run) int result = 0; struct ref_states states = REF_STATES_INIT; struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; + struct oid_array old_oids = OID_ARRAY_INIT; struct string_list_item *item; get_remote_ref_states(remote, &states, GET_REF_STATES); @@ -1639,16 +1650,24 @@ static int prune_remote(const char *remote, int dry_run) printf_ln(_("Pruning %s"), remote); printf_ln(_("URL: %s"), states.remote->url.v[0]); - for_each_string_list_item(item, &states.stale) - string_list_append(&refs_to_prune, item->util); - string_list_sort(&refs_to_prune); + for_each_string_list_item(item, &states.stale) { + struct stale_ref *stale_ref = item->util; - if (!dry_run) + string_list_append(&refs_to_prune, stale_ref->name); + oid_array_append(&old_oids, &stale_ref->oid); + } + + if (!dry_run) { result |= refs_delete_refs(get_main_ref_store(the_repository), - "remote: prune", &refs_to_prune, 0); + "remote: prune", &refs_to_prune, + &old_oids, 0); + if (result) + goto cleanup; + } for_each_string_list_item(item, &states.stale) { - const char *refname = item->util; + struct stale_ref *stale_ref = item->util; + const char *refname = stale_ref->name; if (dry_run) printf_ln(_(" * [would prune] %s"), @@ -1661,7 +1680,9 @@ static int prune_remote(const char *remote, int dry_run) refs_warn_dangling_symrefs(get_main_ref_store(the_repository), stdout, " ", dry_run, &refs_to_prune); +cleanup: string_list_clear(&refs_to_prune, 0); + oid_array_clear(&old_oids); free_remote_ref_states(&states); return result; } diff --git a/builtin/tag.c b/builtin/tag.c index 06c125b53c..32b70c3694 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -105,27 +105,38 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn, return had_error; } +struct tags_to_delete { + struct string_list refs; + struct oid_array old_oids; +}; + static int collect_tags(const char *name UNUSED, const char *ref, const struct object_id *oid, void *cb_data) { - struct string_list *ref_list = cb_data; + struct tags_to_delete *data = cb_data; + struct string_list_item *item; - string_list_append(ref_list, ref); - ref_list->items[ref_list->nr - 1].util = oiddup(oid); + item = string_list_append(&data->refs, ref); + item->util = oiddup(oid); + oid_array_append(&data->old_oids, oid); return 0; } static int delete_tags(const char **argv) { int result; - struct string_list refs_to_delete = STRING_LIST_INIT_DUP; + struct tags_to_delete data = { + .refs = STRING_LIST_INIT_DUP, + .old_oids = OID_ARRAY_INIT, + }; struct string_list_item *item; - result = for_each_tag_name(argv, collect_tags, (void *)&refs_to_delete); - if (refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF)) + result = for_each_tag_name(argv, collect_tags, &data); + if (refs_delete_refs(get_main_ref_store(the_repository), NULL, + &data.refs, &data.old_oids, REF_NO_DEREF)) result = 1; - for_each_string_list_item(item, &refs_to_delete) { + for_each_string_list_item(item, &data.refs) { const char *name = item->string; struct object_id *oid = item->util; if (!refs_ref_exists(get_main_ref_store(the_repository), name)) @@ -135,7 +146,8 @@ static int delete_tags(const char **argv) free(oid); } - string_list_clear(&refs_to_delete, 0); + string_list_clear(&data.refs, 0); + oid_array_clear(&data.old_oids); return result; } diff --git a/refs.c b/refs.c index 951db56113..b91f2a6b05 100644 --- a/refs.c +++ b/refs.c @@ -16,6 +16,7 @@ #include "refs/refs-internal.h" #include "hook.h" #include "object-name.h" +#include "oid-array.h" #include "odb.h" #include "object.h" #include "path.h" @@ -3092,34 +3093,60 @@ void ref_transaction_for_each_rejected_update(struct ref_transaction *transactio } } -int refs_delete_refs(struct ref_store *refs, const char *logmsg, - struct string_list *refnames, unsigned int flags) +struct delete_refs_rejection_data { + int failures; +}; + +static void delete_refs_rejection_handler(const char *refname, + const struct object_id *old_oid UNUSED, + const struct object_id *new_oid UNUSED, + const char *old_target UNUSED, + const char *new_target UNUSED, + enum ref_transaction_error err, + const char *details, + void *cb_data) { + struct delete_refs_rejection_data *data = cb_data; + + warning(_("could not delete reference %s: %s"), refname, + details ? details : ref_transaction_error_msg(err)); + data->failures = 1; +} + +int refs_delete_refs(struct ref_store *refs, const char *logmsg, + struct string_list *refnames, + const struct oid_array *old_oids, + unsigned int flags) +{ + struct delete_refs_rejection_data rejection_data = { 0 }; struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - struct string_list_item *item; + size_t i; int ret = 0, failures = 0; char *msg; if (!refnames->nr) return 0; + if (old_oids && old_oids->nr != refnames->nr) + BUG("refname and old OID counts do not match"); msg = normalize_reflog_message(logmsg); - /* - * Since we don't check the references' old_oids, the - * individual updates can't fail, so we can pack all of the - * updates into a single transaction. - */ - transaction = ref_store_transaction_begin(refs, 0, &err); + transaction = ref_store_transaction_begin(refs, + old_oids ? REF_TRANSACTION_ALLOW_FAILURE : 0, &err); if (!transaction) { ret = error("%s", err.buf); goto out; } - for_each_string_list_item(item, refnames) { + for (i = 0; i < refnames->nr; i++) { + struct string_list_item *item = &refnames->items[i]; + const struct object_id *old_oid = old_oids ? &old_oids->oid[i] : NULL; + + if (old_oid && is_null_oid(old_oid)) + old_oid = NULL; ret = ref_transaction_delete(transaction, item->string, - NULL, NULL, flags, msg, &err); + old_oid, NULL, flags, msg, &err); if (ret) { warning(_("could not delete reference %s: %s"), item->string, err.buf); @@ -3135,9 +3162,14 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg, refnames->items[0].string, err.buf); else error(_("could not delete references: %s"), err.buf); - } + } else if (old_oids) + ref_transaction_for_each_rejected_update(transaction, + delete_refs_rejection_handler, + &rejection_data); out: + if (rejection_data.failures) + failures = 1; if (!ret && failures) ret = -1; ref_transaction_free(transaction); diff --git a/refs.h b/refs.h index ee3b8a62ef..02421589d6 100644 --- a/refs.h +++ b/refs.h @@ -9,6 +9,7 @@ struct fsck_options; struct object_id; struct ref_store; +struct oid_array; struct strbuf; struct string_list; struct string_list_item; @@ -627,13 +628,21 @@ int refs_delete_ref(struct ref_store *refs, const char *msg, unsigned int flags); /* - * Delete the specified references. If there are any problems, emit + * Delete the specified references. If old_oids is non-NULL, it must contain + * an entry for each refname, in the same order. Each non-null OID is used to + * verify the current value of the corresponding reference before deleting + * it. A null OID requests an unconditional deletion, which allows callers to + * include broken refs whose old value cannot be resolved. + * + * If there are any problems, emit * errors but attempt to keep going (i.e., the deletes are not done in * an all-or-nothing transaction). msg and flags are passed through to * ref_transaction_delete(). */ int refs_delete_refs(struct ref_store *refs, const char *msg, - struct string_list *refnames, unsigned int flags); + struct string_list *refnames, + const struct oid_array *old_oids, + unsigned int flags); /** Delete a reflog */ int refs_delete_reflog(struct ref_store *refs, const char *refname); diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index db58f00589..6d6857cbd3 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -132,7 +132,7 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv) while (*argv) string_list_append(&refnames, *argv++); - result = refs_delete_refs(refs, msg, &refnames, flags); + result = refs_delete_refs(refs, msg, &refnames, NULL, flags); string_list_clear(&refnames, 0); return result; } diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 4fe9d9b234..8b52f23662 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -14,6 +14,130 @@ test_expect_success setup ' POST_OID=$(git rev-parse POST) ' +test_expect_success 'hook gets old values for batched branch/tag deletion' ' + test_when_finished "rm -f actual" && + git branch to-delete PRE && + git tag delete-tag POST && + git pack-refs --all && + test_hook reference-transaction <<-\EOF && + if test "$1" = committed + then + # Ignore backend-internal zero-to-zero records. + while read -r old new ref + do + case "$old" in + *[!0]*) + echo "$old $new $ref" + ;; + esac + done >>actual + fi + EOF + cat >expect <<-EOF && + $PRE_OID $ZERO_OID refs/heads/to-delete + $POST_OID $ZERO_OID refs/tags/delete-tag + EOF + git branch -D to-delete && + git tag -d delete-tag && + test_cmp expect actual +' + +test_expect_success 'branch deletion rejects a concurrent update' ' + git branch delete-race PRE && + test_hook reference-transaction <<-\EOF && + marker=$(git rev-parse --git-path delete-race-once) + if test "$1" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/heads/delete-race POST + fi + exit 0 + EOF + test_must_fail git branch -D delete-race 2>err && + test_grep "is at $POST_OID but expected $PRE_OID" err && + test_cmp_rev POST refs/heads/delete-race +' + +test_expect_success 'hook gets old values when pruning remote refs' ' + test_when_finished "rm -rf empty.git prune" && + git init --bare empty.git && + git init prune && + ( + cd prune && + git remote add origin ../empty.git && + git commit --allow-empty -m one && + one=$(git rev-parse HEAD) && + git commit --allow-empty -m two && + two=$(git rev-parse HEAD) && + git update-ref refs/remotes/origin/remote-prune-z "$one" && + git update-ref refs/remotes/origin/remote-prune-a "$two" + ) && + test_hook -C prune reference-transaction <<-\EOF && + if test "$1" = committed + then + # Ignore backend-internal zero-to-zero records. + while read -r old new ref + do + case "$old" in + *[!0]*) + echo "$old $new $ref" + ;; + esac + done >>actual + fi + EOF + ( + cd prune && + one=$(git rev-parse HEAD^) && + two=$(git rev-parse HEAD) && + git remote prune origin && + git update-ref refs/remotes/origin/fetch-prune "$one" && + git fetch --prune origin && + git update-ref refs/remotes/origin/atomic-prune "$one" && + git fetch --atomic --prune origin && + cat >expect <<-EOF && + $two $ZERO_OID refs/remotes/origin/remote-prune-a + $one $ZERO_OID refs/remotes/origin/remote-prune-z + $one $ZERO_OID refs/remotes/origin/fetch-prune + $one $ZERO_OID refs/remotes/origin/atomic-prune + EOF + test_cmp expect actual + ) +' + +test_expect_success 'remote prune rejects a concurrent update' ' + test_when_finished "rm -rf race-empty.git race-prune" && + git init --bare race-empty.git && + git init race-prune && + ( + cd race-prune && + git commit --allow-empty -m one && + one=$(git rev-parse HEAD) && + git commit --allow-empty -m two && + two=$(git rev-parse HEAD) && + git remote add origin ../race-empty.git && + git update-ref refs/remotes/origin/race "$one" && + git update-ref refs/remotes/origin/other "$one" + ) && + test_hook -C race-prune reference-transaction <<-\EOF && + marker=$(git rev-parse --git-path prune-race-once) + if test "$1" = preparing && test ! -e "$marker" + then + >"$marker" + git update-ref refs/remotes/origin/race HEAD + fi + exit 0 + EOF + ( + cd race-prune && + two=$(git rev-parse HEAD) && + test_must_fail git remote prune origin >out 2>err && + test_cmp_rev "$two" refs/remotes/origin/race && + test_must_fail git rev-parse --verify refs/remotes/origin/other && + test_grep ! "\[pruned\]" out + ) +' + test_expect_success 'hook allows updating ref if successful' ' git reset --hard PRE && test_hook reference-transaction <<-\EOF &&