Re: [PATCH v6 6/6] refs: add GIT_REFERENCE_BACKEND to specify reference backend

[Toon Claes <toon@iotcl.com>]

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

> Git allows setting a different object directory via
> 'GIT_OBJECT_DIRECTORY', but provides no equivalent for references. In
> the previous commit we extended the 'extensions.refStorage' config to
> also support an URI input for reference backend with location.
>
> Let's also add a new environment variable 'GIT_REFERENCE_BACKEND' that
> takes in the same input as the config variable. Having an environment
> variable allows us to modify the reference backend and location on the
> fly for individual Git commands.
>
> The environment variable also allows usage of alternate reference
> directories during 'git-clone(1)' and 'git-init(1)'. Add the config to
> the repository when created with the environment variable set.
>
> When initializing the repository with an alternate reference folder,
> create the required stubs in the repositories $GIT_DIR. The inverse,
> i.e. removal of the ref store doesn't clean up the stubs in the $GIT_DIR
> since that would render it unusable. Removal of ref store is only used
> when migrating between ref formats and cleanup of the $GIT_DIR doesn't
> make sense in such a situation.
>
> Helped-by: Jean-Noël Avila <jn.avila@free.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git.adoc |   5 ++
>  environment.h          |   1 +
>  refs.c                 |  23 +++++---
>  setup.c                |  55 ++++++++++++++++-
>  t/t1423-ref-backend.sh | 157 ++++++++++++++++++++++++++++++++++++++-----------
>  5 files changed, 198 insertions(+), 43 deletions(-)
>
> [snip]
>
> diff --git a/refs.c b/refs.c
> index 87ef54abd4..6b3883a325 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2192,16 +2192,21 @@ int ref_store_create_on_disk(struct ref_store *refs, int flags, struct strbuf *e
>  {
>  	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);
> +	if (!ret) {
> +		/* Creation of stubs for linked worktrees are handled in the worktree code. */
> +		if (!(flags & REF_STORE_CREATE_ON_DISK_IS_WORKTREE) && refs->repo->ref_storage_payload) {
> +			refs_create_refdir_stubs(refs->repo, refs->repo->gitdir,
> +						 "repository uses alternate refs storage");
> +		} else if (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);
> +		}
>  	}
>  
>  	return ret;
> +
>  }
>  
>  int ref_store_remove_on_disk(struct ref_store *refs, struct strbuf *err)
> @@ -2216,6 +2221,10 @@ int ref_store_remove_on_disk(struct ref_store *refs, struct strbuf *err)
>  		if (format == REF_STORAGE_FORMAT_FILES)
>  			return ret;
>  
> +		/* Alternate refs backend require stubs in the gitdir. */

I find this comment rather confusing, you say "require stubs" and you do
an early return. I had to read it more than once to understand. What do
you think about:

 +		/* No stubs required in the alternate refs backend,
 +		 * stubs only should be created in the gitdir. */

> +		if (refs->repo->ref_storage_payload)
> +			return ret;
> +

-- 
Cheers,
Toon