Re: [PATCH v5 2/4] refs: forward and use the reference storage payload

> On Mon, Feb 09, 2026 at 04:58:19PM +0100, Karthik Nayak wrote:
>> An upcoming commit will add support for providing an URI via the
>> 'extensions.refStorage' config. The URI will contain the reference
>> backend and a corresponding payload. The payload can be then used for
>> providing an alternate locations for the reference backend.
>>
>> To prepare for this, modify the existing backends to accept such an
>> argument when initializing via the 'init()' function. Both the files
>> and reftable backends will parse the information to be filesystem paths
>> to store references.
>
> Maybe add: "to store references. Given that no callers pass any payload
> yet this is essentially a no-op change for now."
>

>> diff --git a/refs.c b/refs.c
>> index 36f3441632..d9df25d7c0 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3425,3 +3426,33 @@ void refs_create_refdir_stubs(struct repository *repo, const char *refdir,
>>
>>  	strbuf_release(&path);
>>  }
>> +
>> +void refs_compute_filesystem_location(const char *gitdir, const char *payload,
>> +				      bool *is_worktree, struct strbuf *refdir,
>> +				      struct strbuf *ref_common_dir)
>> +{
>> +	struct strbuf sb = STRBUF_INIT;
>> +
>> +	strbuf_addstr(refdir, gitdir);
>> +	*is_worktree = get_common_dir_noenv(ref_common_dir, gitdir);
>> +
>> +	if (!payload)
>> +		return;
>
> I think you should add a comment here that explains why it's not
> necessary to modify the `refdir` in case `*is_worktree`. I'd arguably
> even move that code into `if (!payload)`, as we otherwise only set it to
> reset it later. So:
>
>         if (!payload) {
>                 /*
>                  * We can use `gitdir` as `refdir` without appending the
>                  * worktree path because...
>                  /
>                  strbuf_addstr(refdir, gitdir);
>         }
>
>> +	if (!is_absolute_path(payload)) {
>> +		strbuf_addf(&sb, "%s/%s", ref_common_dir->buf, payload);
>> +		strbuf_realpath(ref_common_dir, sb.buf, 1);
>> +	} else {
>> +		strbuf_realpath(ref_common_dir, payload, 1);
>> +	}
>> +
>> +	strbuf_reset(refdir);
>
> And then you can drop this call to `strbuf_reset()`.
>

>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index 240d3c3b26..b192ce606d 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -106,19 +106,24 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
>>   * set of caches.
>>   */
>>  static struct ref_store *files_ref_store_init(struct repository *repo,
>> +					      const char *payload,
>>  					      const char *gitdir,
>>  					      unsigned int flags)
>>  {
>>  	struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
>>  	struct ref_store *ref_store = (struct ref_store *)refs;
>> -	struct strbuf sb = STRBUF_INIT;
>> +	struct strbuf ref_common_dir = STRBUF_INIT;
>> +	struct strbuf refdir = STRBUF_INIT;
>> +	bool is_worktree;
>> +
>> +	refs_compute_filesystem_location(gitdir, payload, &is_worktree, &refdir,
>> +					 &ref_common_dir);
>>
>> -	base_ref_store_init(ref_store, repo, gitdir, &refs_be_files);
>> +	base_ref_store_init(ref_store, repo, refdir.buf, &refs_be_files);
>>  	refs->store_flags = flags;
>> -	get_common_dir_noenv(&sb, gitdir);
>> -	refs->gitcommondir = strbuf_detach(&sb, NULL);
>> +	refs->gitcommondir = strbuf_detach(&ref_common_dir, NULL);
>>  	refs->packed_ref_store =
>> -		packed_ref_store_init(repo, refs->gitcommondir, flags);
>> +		packed_ref_store_init(repo, payload, refs->gitcommondir, flags);
>
> It's a bit weird that we end up passing the payload even though we
> unconditionally ignore it in `packed_ref_store_init()`. I'd argue that
> we should either pass a `NULL` pointer as payload, or let the packed
> backend call `refs_compute_filesystem_location()` itsefl.
>

>> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
>> index 4ea0c12299..028fbc0585 100644
>> --- a/refs/packed-backend.c
>> +++ b/refs/packed-backend.c
>> @@ -212,6 +212,7 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot)
>>  }
>>
>>  struct ref_store *packed_ref_store_init(struct repository *repo,
>> +					const char *payload UNUSED,
>>  					const char *gitdir,
>>  					unsigned int store_flags)
>>  {
>
> And here we should probably explain why we don't have to respect the
> payload.
>

>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
>> index c7d2a6e50b..bd09b1280c 100644
>> --- a/refs/refs-internal.h
>> +++ b/refs/refs-internal.h
>> @@ -666,4 +667,18 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs
>>  					  unsigned int initial_transaction,
>>  					  struct strbuf *err);
>>
>> +/*
>> + * Given a gitdir and the reference storage payload provided, retrieve the
>> + * 'refdir' and 'ref_common_dir'. The former is where references should be
>> + * stored for the current worktree, the latter is the common reference
>> + * directory if working with a linked worktree. If working with the main
>> + * worktree, both values will be the same.
>> + *
>> + * This is used by backends such as {files, reftable} which store references in
>> + * dedicated filesystem paths.
>> + */
>
> I guess we can say "This is used by backends that store store files in
> the repository directly."
>
> Patrick