Re: [PATCH v7 3/6] refs: move out stub modification to generic layer

[Toon Claes <toon@iotcl.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> When creating the reftable reference backend on disk, we create stubs to
> ensure that the directory can be recognized as a Git repository. This is
> done by calling `refs_create_refdir_stubs()`. Move this to the generic
> layer as this is needed for all backends excluding from the files
> backends. In an upcoming commit where we introduce alternate reference
> backend locations, we'll have to also create stubs in the $GIT_DIR
> irrespective of the backend being used. This commit builds the base to
> add that logic.
>
> Similarly, move the logic for deletion of stubs to the generic layer.
> The files backend recursively calls the remove function of the
> 'packed-backend', here skip calling the generic function since that
> would try to delete stubs.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  refs.c                  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>  refs/files-backend.c    |  6 +++++-
>  refs/reftable-backend.c | 27 ---------------------------
>  3 files changed, 50 insertions(+), 30 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 77b93d655b..c83af63dc5 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2189,12 +2189,55 @@ void refs_create_refdir_stubs(struct repository *repo, const char *refdir,
>  /* backend functions */
>  int ref_store_create_on_disk(struct ref_store *refs, int flags, struct strbuf *err)
>  {
> -	return refs->be->create_on_disk(refs, flags, err);
> +	int ret = refs->be->create_on_disk(refs, flags, err);
> +
> +	if (!ret &&
> +	    ref_storage_format_by_name(refs->be->name) != REF_STORAGE_FORMAT_FILES) {
> +		struct strbuf msg = STRBUF_INIT;
> +
> +		strbuf_addf(&msg, "this repository uses the %s format", refs->be->name);
> +		refs_create_refdir_stubs(refs->repo, refs->gitdir, msg.buf);
> +		strbuf_release(&msg);

To me it would feel nicer to have an early return instead of wrapping
this code inside the if block. But because we only need the strbuf
instead this block, having a block to scope it makes more sense.

Looks good to me.

-- 
Cheers,
Toon