Re: [PATCH 04/16] refs: add do_for_each_per_worktree_ref

[Junio C Hamano <gitster@pobox.com>]

David Turner <dturner@twopensource.com> writes:

> Alternate refs backends might still use files to store per-worktree
> refs.  So the files backend's ref-loading infrastructure should be
> available to those backends, just for use on per-worktree refs.  Add
> do_for_each_per_worktree_ref, which iterates over per-worktree refs.

Is this "might still use"?  I am instead getting an impression that,
we are declaring that the recommended way to store per-worktree refs
is with filesystem backend.

Not complaining against either such a design decision (if that is
what is made here) or the above description in the log message--just
want to understand the intention better.

I also wonder if it is cleaner to have a single interface that lets
anybody ask a pointer to the backend struct by name.  With such an
interface, a new backend that wants to delegate per-worktree refs to
files backend could

	static int DB_for_each_ref(...)
        {
		struct ref_be *files = locate_ref_backend("files");
		files->for_each_ref(... | WORKTREE_ONLY, ...);
		... do its own enumeration of non-per-worktree refs ...
	}

and such a delegation does not need to be limited to per-worktree
iteration.

Or is that a road to expose too much implementation details of
a backend to other backends?

> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  refs/files-backend.c | 15 ++++++++++++---
>  refs/refs-internal.h | 10 ++++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d4bd6cf..bde4892 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -518,9 +518,6 @@ static void sort_ref_dir(struct ref_dir *dir)
>  	dir->sorted = dir->nr = i;
>  }
>  
> -/* Include broken references in a do_for_each_ref*() iteration: */
> -#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
> -
>  /*
>   * Return true iff the reference described by entry can be resolved to
>   * an object in the database.  Emit a warning if the referred-to
> @@ -568,6 +565,10 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
>  	struct ref_entry *old_current_ref;
>  	int retval;
>  
> +	if (data->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&
> +	    ref_type(entry->name) != REF_TYPE_PER_WORKTREE)
> +		return 0;
> +
>  	if (!starts_with(entry->name, data->base))
>  		return 0;
>  
> @@ -1738,6 +1739,14 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
>  	return do_for_each_entry(refs, base, do_one_ref, &data);
>  }
>  
> +int do_for_each_per_worktree_ref(const char *submodule, const char *base,
> +				 each_ref_fn fn, int trim, int flags,
> +				 void *cb_data)
> +{
> +	return do_for_each_ref(get_ref_cache(submodule), base, fn, trim,
> +			       flags | DO_FOR_EACH_PER_WORKTREE_ONLY, cb_data);
> +}
> +
>  static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
>  {
>  	struct object_id oid;
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index ad683df..433d0fe 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -42,6 +42,16 @@
>   * value to ref_update::flags
>   */
>  
> +/* Include broken references in a do_for_each_ref*() iteration */
> +#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
> +
> +/* Only include per-worktree refs in a do_for_each_ref*() iteration */
> +#define DO_FOR_EACH_PER_WORKTREE_ONLY 0x02
> +
> +int do_for_each_per_worktree_ref(const char *submodule, const char *base,
> +				 each_ref_fn fn, int trim, int flags,
> +				 void *cb_data);
> +
>  /*
>   * Return true iff refname is minimally safe. "Safe" here means that
>   * deleting a loose reference by this name will not do any damage, for