Merge branch 'ps/ref-deletion-updates' into jch

Simplify API implementation to delete references by eliminating
duplication.

* ps/ref-deletion-updates:
  refs: remove `delete_refs` callback from backends
  refs: deduplicate code to delete references
  refs/files: use transactions to delete references
  t5510: ensure that the packed-refs file needs locking
This commit is contained in:
Junio C Hamano
2023-11-17 13:43:06 +09:00
6 changed files with 46 additions and 120 deletions

48
refs.c
View File

@ -2599,13 +2599,55 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,
int refs_delete_refs(struct ref_store *refs, const char *logmsg,
struct string_list *refnames, unsigned int flags)
{
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
struct string_list_item *item;
int ret = 0, failures = 0;
char *msg;
int retval;
if (!refnames->nr)
return 0;
msg = normalize_reflog_message(logmsg);
retval = refs->be->delete_refs(refs, msg, refnames, flags);
/*
* 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, &err);
if (!transaction) {
ret = error("%s", err.buf);
goto out;
}
for_each_string_list_item(item, refnames) {
ret = ref_transaction_delete(transaction, item->string,
NULL, flags, msg, &err);
if (ret) {
warning(_("could not delete reference %s: %s"),
item->string, err.buf);
strbuf_reset(&err);
failures = 1;
}
}
ret = ref_transaction_commit(transaction, &err);
if (ret) {
if (refnames->nr == 1)
error(_("could not delete reference %s: %s"),
refnames->items[0].string, err.buf);
else
error(_("could not delete references: %s"), err.buf);
}
out:
if (!ret && failures)
ret = -1;
ref_transaction_free(transaction);
strbuf_release(&err);
free(msg);
return retval;
return ret;
}
int delete_refs(const char *msg, struct string_list *refnames,

View File

@ -143,20 +143,6 @@ static int debug_create_symref(struct ref_store *ref_store,
return res;
}
static int debug_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
int res =
drefs->refs->be->delete_refs(drefs->refs, msg, refnames, flags);
int i;
trace_printf_key(&trace_refs, "delete_refs {\n");
for (i = 0; i < refnames->nr; i++)
trace_printf_key(&trace_refs, "%s\n", refnames->items[i].string);
trace_printf_key(&trace_refs, "}: %d\n", res);
return res;
}
static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
const char *newref, const char *logmsg)
{
@ -458,7 +444,6 @@ struct ref_storage_be refs_be_debug = {
.pack_refs = debug_pack_refs,
.create_symref = debug_create_symref,
.delete_refs = debug_delete_refs,
.rename_ref = debug_rename_ref,
.copy_ref = debug_copy_ref,

View File

@ -1265,54 +1265,6 @@ static int files_pack_refs(struct ref_store *ref_store,
return 0;
}
static int files_delete_refs(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
struct strbuf err = STRBUF_INIT;
int i, result = 0;
if (!refnames->nr)
return 0;
if (packed_refs_lock(refs->packed_ref_store, 0, &err))
goto error;
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);
for (i = 0; i < refnames->nr; i++) {
const char *refname = refnames->items[i].string;
if (refs_delete_ref(&refs->base, msg, refname, NULL, flags))
result |= error(_("could not remove reference %s"), refname);
}
strbuf_release(&err);
return result;
error:
/*
* If we failed to rewrite the packed-refs file, then it is
* unsafe to try to remove loose refs, because doing so might
* expose an obsolete packed value for a reference that might
* even point at an object that has been garbage collected.
*/
if (refnames->nr == 1)
error(_("could not delete reference %s: %s"),
refnames->items[0].string, err.buf);
else
error(_("could not delete references: %s"), err.buf);
strbuf_release(&err);
return -1;
}
/*
* People using contrib's git-new-workdir have .git/logs/refs ->
* /some/other/path/.git/logs/refs, and that may live on another device.
@ -3300,7 +3252,6 @@ struct ref_storage_be refs_be_files = {
.pack_refs = files_pack_refs,
.create_symref = files_create_symref,
.delete_refs = files_delete_refs,
.rename_ref = files_rename_ref,
.copy_ref = files_copy_ref,

View File

@ -1688,55 +1688,6 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED,
return ref_transaction_commit(transaction, err);
}
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;
/*
* 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(ref_store, &err);
if (!transaction)
return -1;
for_each_string_list_item(item, refnames) {
if (ref_transaction_delete(transaction, item->string, NULL,
flags, msg, &err)) {
warning(_("could not delete reference %s: %s"),
item->string, err.buf);
strbuf_reset(&err);
}
}
ret = ref_transaction_commit(transaction, &err);
if (ret) {
if (refnames->nr == 1)
error(_("could not delete reference %s: %s"),
refnames->items[0].string, err.buf);
else
error(_("could not delete references: %s"), err.buf);
}
ref_transaction_free(transaction);
strbuf_release(&err);
return ret;
}
static int packed_pack_refs(struct ref_store *ref_store UNUSED,
struct pack_refs_opts *pack_opts UNUSED)
{
@ -1765,7 +1716,6 @@ struct ref_storage_be refs_be_packed = {
.pack_refs = packed_pack_refs,
.create_symref = NULL,
.delete_refs = packed_delete_refs,
.rename_ref = NULL,
.copy_ref = NULL,

View File

@ -553,8 +553,6 @@ typedef int create_symref_fn(struct ref_store *ref_store,
const char *ref_target,
const char *refs_heads_master,
const char *logmsg);
typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags);
typedef int rename_ref_fn(struct ref_store *ref_store,
const char *oldref, const char *newref,
const char *logmsg);
@ -677,7 +675,6 @@ struct ref_storage_be {
pack_refs_fn *pack_refs;
create_symref_fn *create_symref;
delete_refs_fn *delete_refs;
rename_ref_fn *rename_ref;
copy_ref_fn *copy_ref;

View File

@ -169,6 +169,7 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' '
git clone . prune-fail &&
cd prune-fail &&
git update-ref refs/remotes/origin/extrabranch main &&
git pack-refs --all &&
: this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds &&
>.git/packed-refs.new &&