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

On Wed, Sep 09, 2026 at 01:30:55PM +0530, Kaartic Sivaraam wrote:

On 9/7/26 16:48, Patrick Steinhardt wrote:

quoted

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