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

> 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