Re: [PATCH 34/38] refs: add method for delete_refs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 7, 2016, 17:43 UTC
- Message-ID
- <xmqqporsrj21.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <17da5760ac671c98811cc5a3ec46a61f8261d8d9.1464983301.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 16 quoted lines
> From: David Turner <dturner@twopensource.com> > > 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 <dturner@twopensource.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> > --- > 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?
Show 67 quoted lines
> 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;