Re: [PATCH v2 08/11] setup: refactor how we configure the ref storage format

[Patrick Steinhardt <ps@pks.im>]

On Wed, Sep 09, 2026 at 01:30:55PM +0530, Kaartic Sivaraam wrote:
> On 9/7/26 16:48, Patrick Steinhardt wrote:
> > diff --git a/setup.c b/setup.c
> > index 3be7dac452..38fa5e854c 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -2761,40 +2761,65 @@ static void repository_format_configure(struct repository_format *repo_fmt,
> > 
> > ... snip ...
> > -
> > -	ref_backend_uri = getenv(GIT_REFERENCE_BACKEND_ENVIRONMENT);
> > -	if (ref_backend_uri) {
> > -		enum ref_storage_format format;
> > -		char *payload;
> > -
> > -		format = ref_storage_format_by_uri(ref_backend_uri, &payload);
> > -		if (format == REF_STORAGE_FORMAT_UNKNOWN)
> > -			die(_("unknown ref storage format: '%s'"), ref_backend_uri);
> > -
> > -		repo_fmt->ref_storage_format = format;
> > -		free(repo_fmt->ref_storage_payload);
> > -		repo_fmt->ref_storage_payload = payload;
> > -	}
> > +	/*
> > +	 * If we have a preexisting repository we need to verify that its
> > +	 * current ref storage format does not change.
> > +	 */
> > +	if (repo_fmt->version >= 0) {
> > +		if (ref_storage_format != repo_fmt->ref_storage_format)
> > +			die(_("attempt to reinitialize repository with different reference storage format"));
> > +		if ((ref_storage_payload || repo_fmt->ref_storage_payload) &&
> > +		    strcmp(ref_storage_payload ? ref_storage_payload : "",
> > +			   repo_fmt->ref_storage_payload ? repo_fmt->ref_storage_payload : ""))
> 
> Rather than a strcmp, wouldn't it be ideal to do the path comparison using
> strbuf_realpath to make sure we avoid similar paths like the following from
> failing during reinitialization?
> 
>   $ mkdir store
>   $ git init --ref-storage-format="files://$PWD/store" r1
>   Initialized empty Git repository in .../r1/.git/
>   $ git init --ref-storage-format="files://$PWD/store" r1
>   Reinitialized existing Git repository in .../r1/.git/
>   $ git init --ref-storage-format="files://$PWD/./store" r1
>   fatal: attempt to reinitialize repository with different reference storage
> payload
> 
> This matches with how we actually make use of the path at runtime in the
> refs_compute_filesystem_location function in refs.c.

No, because that'd assume that the payload even is a path. But in
theory, it could be anything, like for example a database host to
connect to. In `refs_compute_filesystem_location()` it's a different
thing though as that call happens inside the backends themselves, and
they know what kind of payload to expect.

Patrick