Re: [PATCH 34/38] refs: add method for delete_refs
From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:47
Michael Haggerty [off-list ref] writes:
From: David Turner <redacted> In the file-based backend, delete_refs has some special optimization to deal with packed refs. In other backends, we might be able to make ref deletion faster by putting all deletions into a single transaction. So we need a special backend function for this. Signed-off-by: David Turner <redacted> Signed-off-by: Junio C Hamano <redacted> Signed-off-by: Michael Haggerty <redacted> --- I think that we could get away without this method if we make ref_transactions a bit smarter (for example, by supporting best-effort updates that can fail without causing the entire transaction to be aborted). But that would be a significant detour, so let's leave it here for now.
Hmm, I actually was wondering why 'pack without' was there while reading 24/38; IIUC, that is one of the "special optimization" that is very much tied to the files backend, and it may make sense to hide it behind delete_refs() as its implementation detail. Which is exactly what this step is about, so I am happy ;-) Unlike other changes like the ones that did read_raw_ref(), verify_refname_available(), etc., the title does not follow the pattern "refs: make X() virtual", even though as far as I can see the intent is the same as others. Perhaps a minor retitle is in order?
quoted hunk
refs.c | 7 +++++++ refs/files-backend.c | 6 ++++-- refs/refs-internal.h | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-)diff --git a/refs.c b/refs.c index 6c1e899..8ab9862 100644 --- a/refs.c +++ b/refs.c@@ -1529,3 +1529,10 @@ int initial_ref_transaction_commit(struct ref_transaction *transaction, return refs->be->initial_transaction_commit(refs, transaction, err); } + +int delete_refs(struct string_list *refnames, unsigned int flags) +{ + struct ref_store *refs = get_ref_store(NULL); + + return refs->be->delete_refs(refs, refnames, flags); +}diff --git a/refs/files-backend.c b/refs/files-backend.c index 253899f..5681141 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c@@ -2455,10 +2455,11 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) return 0; } -int delete_refs(struct string_list *refnames, unsigned int flags) +static int files_delete_refs(struct ref_store *ref_store, + struct string_list *refnames, unsigned int flags) { struct files_ref_store *refs = - get_files_ref_store(NULL, "delete_refs"); + files_downcast(ref_store, 0, "delete_refs"); struct strbuf err = STRBUF_INIT; int i, result = 0;@@ -4070,6 +4071,7 @@ struct ref_storage_be refs_be_files = { files_pack_refs, files_peel_ref, files_create_symref, + files_delete_refs, files_ref_iterator_begin, files_read_raw_ref,diff --git a/refs/refs-internal.h b/refs/refs-internal.h index e462b54..f944b7a 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h@@ -485,6 +485,8 @@ 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, + struct string_list *refnames, unsigned int flags); /* * Iterate over the references in the specified ref_store that are@@ -582,6 +584,7 @@ struct ref_storage_be { pack_refs_fn *pack_refs; peel_ref_fn *peel_ref; create_symref_fn *create_symref; + delete_refs_fn *delete_refs; ref_iterator_begin_fn *iterator_begin; read_raw_ref_fn *read_raw_ref;