[PATCH 0/6] worktree: initialize refdb via ref backends

[PATCH 0/6] worktree: initialize refdb via ref backends

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4262 bytes --]

Hi,

when initializing worktrees we manually create the on-disk data
structures required for the ref backend in "worktree.c". This works just
fine right now where we only have a single user-exposed ref backend, but
it will become unwieldy once we have multiple ref backends. This patch
series thus refactors how we initialize worktrees so that we can use
`refs_init_db()` to initialize required files for us.

This patch series conflicts with ps/refstorage-extension. The conflict
can be solved as shown below. I'm happy to defer this patch series
though until the topic has landed on `master` in case this causes
issues.

Patrick

diff --cc setup.c
index f2d55994e2,4712bba6f8..0000000000
--- a/setup.c
+++ b/setup.c
@@@ -1904,7 -1926,23 +1926,8 @@@ void create_reference_database(int ref_
  	struct strbuf err = STRBUF_INIT;
  	int reinit = is_reinit();
  
 -	/*
 -	 * We need to create a "refs" dir in any case so that older versions of
 -	 * Git can tell that this is a repository. This serves two main purposes:
 -	 *
 -	 * - Clients will know to stop walking the parent-directory chain when
 -	 *   detecting the Git repository. Otherwise they may end up detecting
 -	 *   a Git repository in a parent directory instead.
 -	 *
 -	 * - Instead of failing to detect a repository with unknown reference
 -	 *   format altogether, old clients will print an error saying that
 -	 *   they do not understand the reference format extension.
 -	 */
 -	safe_create_dir(git_path("refs"), 1);
 -	adjust_shared_perm(git_path("refs"));
 -
+ 	repo_set_ref_storage_format(the_repository, ref_storage_format);
 -	if (refs_init_db(&err))
 +	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
  		die("failed to set up refs db: %s", err.buf);
  
  	/*
diff --cc worktree.c
index 085f2cc41a,9702ed0308..0000000000
--- a/worktree.c
+++ b/worktree.c
@@@ -79,7 -75,8 +80,8 @@@ static struct worktree *get_main_worktr
  	return worktree;
  }
  
- struct worktree *get_linked_worktree(const char *id)
 -static struct worktree *get_linked_worktree(const char *id,
 -					    int skip_reading_head)
++struct worktree *get_linked_worktree(const char *id,
++				     int skip_reading_head)
  {
  	struct worktree *worktree = NULL;
  	struct strbuf path = STRBUF_INIT;
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9d935bee84..558c5537f5 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -502,7 +502,7 @@ static int add_worktree(const char *path, const char *refname,
 	/*
 	 * Set up the ref store of the worktree and create the HEAD reference.
 	 */
-	wt = get_linked_worktree(name);
+	wt = get_linked_worktree(name, 1);
 	if (!wt) {
 		ret = error(_("could not find created worktree '%s'"), name);
 		goto done;
diff --git a/worktree.h b/worktree.h
index 8a75691eac..f14784a2ff 100644
--- a/worktree.h
+++ b/worktree.h
@@ -61,7 +61,8 @@ struct worktree *find_worktree(struct worktree **list,
  * Look up the worktree corresponding to `id`, or NULL of no such worktree
  * exists.
  */
-struct worktree *get_linked_worktree(const char *id);
+struct worktree *get_linked_worktree(const char *id,
+				     int skip_reading_head);
 
 /*
  * Return the worktree corresponding to `path`, or NULL if no such worktree

Patrick Steinhardt (6):
  refs: prepare `refs_init_db()` for initializing worktree refs
  setup: move creation of "refs/" into the files backend
  refs/files: skip creation of "refs/{heads,tags}" for worktrees
  builtin/worktree: move setup of commondir file earlier
  worktree: expose interface to look up worktree by name
  builtin/worktree: create refdb via ref backend

 builtin/worktree.c    | 53 ++++++++++++++++++++-----------------------
 refs.c                |  6 ++---
 refs.h                |  4 +++-
 refs/debug.c          |  4 ++--
 refs/files-backend.c  | 37 +++++++++++++++++++++++++-----
 refs/packed-backend.c |  1 +
 refs/refs-internal.h  |  4 +++-
 setup.c               | 17 +-------------
 worktree.c            | 25 ++++++++++++--------
 worktree.h            | 11 +++++++++
 10 files changed, 94 insertions(+), 68 deletions(-)


base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/6] refs: prepare `refs_init_db()` for initializing worktree refs

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4448 bytes --]

The purpose of `refs_init_db()` is to initialize the on-disk files of a
new ref database. The function is quite inflexible right now though, as
callers can neither specify the `struct ref_store` nor can they pass any
flags.

Refactor the interface to accept both of these. This will be required so
that we can start initializing per-worktree ref databases via the ref
backend instead of open-coding the initialization in "worktree.c".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.c                | 6 ++----
 refs.h                | 2 +-
 refs/debug.c          | 4 ++--
 refs/files-backend.c  | 4 +++-
 refs/packed-backend.c | 1 +
 refs/refs-internal.h  | 4 +++-
 setup.c               | 2 +-
 7 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/refs.c b/refs.c
index 16bfa21df7..87e2659db7 100644
--- a/refs.c
+++ b/refs.c
@@ -1928,11 +1928,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 }
 
 /* backend functions */
-int refs_init_db(struct strbuf *err)
+int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err)
 {
-	struct ref_store *refs = get_main_ref_store(the_repository);
-
-	return refs->be->init_db(refs, err);
+	return refs->be->init_db(refs, flags, err);
 }
 
 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
diff --git a/refs.h b/refs.h
index ff113bb12a..6090e92578 100644
--- a/refs.h
+++ b/refs.h
@@ -123,7 +123,7 @@ int should_autocreate_reflog(const char *refname);
 
 int is_branch(const char *refname);
 
-int refs_init_db(struct strbuf *err);
+int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err);
 
 /*
  * Return the peeled value of the oid currently being iterated via
diff --git a/refs/debug.c b/refs/debug.c
index 83b7a0ba65..c061def8b5 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -33,10 +33,10 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
 	return (struct ref_store *)res;
 }
 
-static int debug_init_db(struct ref_store *refs, struct strbuf *err)
+static int debug_init_db(struct ref_store *refs, int flags, struct strbuf *err)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
-	int res = drefs->refs->be->init_db(drefs->refs, err);
+	int res = drefs->refs->be->init_db(drefs->refs, flags, err);
 	trace_printf_key(&trace_refs, "init_db: %d\n", res);
 	return res;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index ad8b1d143f..387eeb5037 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3220,7 +3220,9 @@ static int files_reflog_expire(struct ref_store *ref_store,
 	return -1;
 }
 
-static int files_init_db(struct ref_store *ref_store, struct strbuf *err UNUSED)
+static int files_init_db(struct ref_store *ref_store,
+			 int flags UNUSED,
+			 struct strbuf *err UNUSED)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "init_db");
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index b9fa097a29..3f10bccf44 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1246,6 +1246,7 @@ static const char PACKED_REFS_HEADER[] =
 	"# pack-refs with: peeled fully-peeled sorted \n";
 
 static int packed_init_db(struct ref_store *ref_store UNUSED,
+			  int flags UNUSED,
 			  struct strbuf *err UNUSED)
 {
 	/* Nothing to do. */
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4af83bf9a5..a33b28c19d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -529,7 +529,9 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,
 					    const char *gitdir,
 					    unsigned int flags);
 
-typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
+typedef int ref_init_db_fn(struct ref_store *refs,
+			   int flags,
+			   struct strbuf *err);
 
 typedef int ref_transaction_prepare_fn(struct ref_store *refs,
 				       struct ref_transaction *transaction,
diff --git a/setup.c b/setup.c
index bc90bbd033..a4eb2a38ac 100644
--- a/setup.c
+++ b/setup.c
@@ -1919,7 +1919,7 @@ void create_reference_database(const char *initial_branch, int quiet)
 	safe_create_dir(git_path("refs"), 1);
 	adjust_shared_perm(git_path("refs"));
 
-	if (refs_init_db(&err))
+	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
 		die("failed to set up refs db: %s", err.buf);
 
 	/*
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/6] setup: move creation of "refs/" into the files backend

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]

When creating the ref database we unconditionally create the "refs/"
directory in "setup.c". This is a mandatory prerequisite for all Git
repositories regardless of the ref backend in use, because Git will be
unable to detect the directory as a repository if "refs/" doesn't exist.

We are about to add another new caller that will want to create a ref
database when creating worktrees. We would require the same logic to
create the "refs/" directory even though the caller really should not
care about such low-level details. Ideally, the ref database should be
fully initialized after calling `refs_init_db()`.

Move the code to create the directory into the files backend itself to
make it so. This means that future ref backends will also need to have
equivalent logic around to ensure that the directory exists, but it
seems a lot more sensible to have it this way round than to require
callers to create the directory themselves.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c | 17 +++++++++++++++++
 setup.c              | 15 ---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 387eeb5037..ed47c5dc08 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3228,9 +3228,26 @@ static int files_init_db(struct ref_store *ref_store,
 		files_downcast(ref_store, REF_STORE_WRITE, "init_db");
 	struct strbuf sb = STRBUF_INIT;
 
+	/*
+	 * We need to create a "refs" dir in any case so that older versions of
+	 * Git can tell that this is a repository. This serves two main purposes:
+	 *
+	 * - Clients will know to stop walking the parent-directory chain when
+	 *   detecting the Git repository. Otherwise they may end up detecting
+	 *   a Git repository in a parent directory instead.
+	 *
+	 * - Instead of failing to detect a repository with unknown reference
+	 *   format altogether, old clients will print an error saying that
+	 *   they do not understand the reference format extension.
+	 */
+	strbuf_addf(&sb, "%s/refs", ref_store->gitdir);
+	safe_create_dir(sb.buf, 1);
+	adjust_shared_perm(sb.buf);
+
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
+	strbuf_reset(&sb);
 	files_ref_path(refs, &sb, "refs/heads");
 	safe_create_dir(sb.buf, 1);
 
diff --git a/setup.c b/setup.c
index a4eb2a38ac..f2d55994e2 100644
--- a/setup.c
+++ b/setup.c
@@ -1904,21 +1904,6 @@ void create_reference_database(const char *initial_branch, int quiet)
 	struct strbuf err = STRBUF_INIT;
 	int reinit = is_reinit();
 
-	/*
-	 * We need to create a "refs" dir in any case so that older versions of
-	 * Git can tell that this is a repository. This serves two main purposes:
-	 *
-	 * - Clients will know to stop walking the parent-directory chain when
-	 *   detecting the Git repository. Otherwise they may end up detecting
-	 *   a Git repository in a parent directory instead.
-	 *
-	 * - Instead of failing to detect a repository with unknown reference
-	 *   format altogether, old clients will print an error saying that
-	 *   they do not understand the reference format extension.
-	 */
-	safe_create_dir(git_path("refs"), 1);
-	adjust_shared_perm(git_path("refs"));
-
 	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
 		die("failed to set up refs db: %s", err.buf);
 
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/6] refs/files: skip creation of "refs/{heads,tags}" for worktrees

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

The files ref backend will create both "refs/heads" and "refs/tags" in
the Git directory. While this logic makes sense for normal repositories,
it does not fo worktrees because those refs are "common" refs that would
always be contained in the main repository's ref database.

Introduce a new flag telling the backend that it is expected to create a
per-worktree ref database and skip creation of these dirs in the files
backend when the flag is set. No other backends (currently) need
worktree-specific logic, so this is the only required change to start
creating per-worktree ref databases via `refs_init_db()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.h               |  2 ++
 refs/files-backend.c | 22 ++++++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/refs.h b/refs.h
index 6090e92578..8ed890841b 100644
--- a/refs.h
+++ b/refs.h
@@ -123,6 +123,8 @@ int should_autocreate_reflog(const char *refname);
 
 int is_branch(const char *refname);
 
+#define REFS_INIT_DB_IS_WORKTREE (1 << 0)
+
 int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err);
 
 /*
diff --git a/refs/files-backend.c b/refs/files-backend.c
index ed47c5dc08..a82d6c453f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3221,7 +3221,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 }
 
 static int files_init_db(struct ref_store *ref_store,
-			 int flags UNUSED,
+			 int flags,
 			 struct strbuf *err UNUSED)
 {
 	struct files_ref_store *refs =
@@ -3245,15 +3245,21 @@ static int files_init_db(struct ref_store *ref_store,
 	adjust_shared_perm(sb.buf);
 
 	/*
-	 * Create .git/refs/{heads,tags}
+	 * There is no need to create directories for common refs when creating
+	 * a worktree ref store.
 	 */
-	strbuf_reset(&sb);
-	files_ref_path(refs, &sb, "refs/heads");
-	safe_create_dir(sb.buf, 1);
+	if (!(flags & REFS_INIT_DB_IS_WORKTREE)) {
+		/*
+		 * Create .git/refs/{heads,tags}
+		 */
+		strbuf_reset(&sb);
+		files_ref_path(refs, &sb, "refs/heads");
+		safe_create_dir(sb.buf, 1);
 
-	strbuf_reset(&sb);
-	files_ref_path(refs, &sb, "refs/tags");
-	safe_create_dir(sb.buf, 1);
+		strbuf_reset(&sb);
+		files_ref_path(refs, &sb, "refs/tags");
+		safe_create_dir(sb.buf, 1);
+	}
 
 	strbuf_release(&sb);
 	return 0;
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/6] builtin/worktree: move setup of commondir file earlier

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

Shuffle around how we create supporting worktree files so that we first
ensure that the worktree has all link files ("gitdir", "commondir")
before we try to initialize the ref database by writing "HEAD". This
will be required by a subsequent commit where we start to initialize the
ref database via `refs_init_db()`, which will require an initialized
`struct worktree *`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/worktree.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4ac1621541..58937a2a68 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -495,6 +495,10 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_realpath(&realpath, get_git_common_dir(), 1);
 	write_file(sb_git.buf, "gitdir: %s/worktrees/%s",
 		   realpath.buf, name);
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
+	write_file(sb.buf, "../..");
+
 	/*
 	 * This is to keep resolve_ref() happy. We need a valid HEAD
 	 * or is_git_directory() will reject the directory. Any value which
@@ -505,9 +509,6 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
 	write_file(sb.buf, "%s", oid_to_hex(null_oid()));
-	strbuf_reset(&sb);
-	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
-	write_file(sb.buf, "../..");
 
 	/*
 	 * If the current worktree has sparse-checkout enabled, then copy
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 5/6] worktree: expose interface to look up worktree by name

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]

Our worktree interfaces do not provide a way to look up a worktree by
its name. Expose `get_linked_worktree()` to allow for this usecase. As
callers are responsible for freeing this worktree, introduce a new
function `free_worktree()` that does so.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 worktree.c | 25 +++++++++++++++----------
 worktree.h | 11 +++++++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/worktree.c b/worktree.c
index a56a6c2a3d..085f2cc41a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -12,18 +12,23 @@
 #include "wt-status.h"
 #include "config.h"
 
+void free_worktree(struct worktree *worktree)
+{
+	if (!worktree)
+		return;
+	free(worktree->path);
+	free(worktree->id);
+	free(worktree->head_ref);
+	free(worktree->lock_reason);
+	free(worktree->prune_reason);
+	free(worktree);
+}
+
 void free_worktrees(struct worktree **worktrees)
 {
 	int i = 0;
-
-	for (i = 0; worktrees[i]; i++) {
-		free(worktrees[i]->path);
-		free(worktrees[i]->id);
-		free(worktrees[i]->head_ref);
-		free(worktrees[i]->lock_reason);
-		free(worktrees[i]->prune_reason);
-		free(worktrees[i]);
-	}
+	for (i = 0; worktrees[i]; i++)
+		free_worktree(worktrees[i]);
 	free (worktrees);
 }
 
@@ -74,7 +79,7 @@ static struct worktree *get_main_worktree(void)
 	return worktree;
 }
 
-static struct worktree *get_linked_worktree(const char *id)
+struct worktree *get_linked_worktree(const char *id)
 {
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
diff --git a/worktree.h b/worktree.h
index ce45b66de9..8a75691eac 100644
--- a/worktree.h
+++ b/worktree.h
@@ -57,6 +57,12 @@ struct worktree *find_worktree(struct worktree **list,
 			       const char *prefix,
 			       const char *arg);
 
+/*
+ * Look up the worktree corresponding to `id`, or NULL of no such worktree
+ * exists.
+ */
+struct worktree *get_linked_worktree(const char *id);
+
 /*
  * Return the worktree corresponding to `path`, or NULL if no such worktree
  * exists.
@@ -134,6 +140,11 @@ void repair_worktrees(worktree_repair_fn, void *cb_data);
  */
 void repair_worktree_at_path(const char *, worktree_repair_fn, void *cb_data);
 
+/*
+ * Free up the memory for a worktree.
+ */
+void free_worktree(struct worktree *);
+
 /*
  * Free up the memory for worktree(s)
  */
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 6/6] builtin/worktree: create refdb via ref backend

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

When creating a worktree we create the worktree's ref database manually
by first writing a "HEAD" file so that the directory is recognized as a
Git repository by other commands, and then running git-update-ref(1) or
git-symbolic-ref(1) to write the actual value. But while this is fine
for the files backend, this logic simply assumes too much about how the
ref backend works and will leave behind an invalid ref database once any
other ref backend lands.

Refactor the code to instead use `refs_init_db()` to initialize the ref
database so that git-worktree(1) itself does not need to know about how
to initialize it. This will allow future ref backends to customize how
the per-worktree ref database is set up. Furthermore, as we now already
have a worktree ref store around, we can also avoid spawning external
commands to write the HEAD reference and instead use the refs API to do
so.

Note that we do not have an equivalent to passing the `--quiet` flag to
git-symbolic-ref(1) as we did before. This flag does not have an effect
anyway though, as git-symbolic-ref(1) only honors it when reading a
symref, but never when writing one.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/worktree.c | 48 +++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 58937a2a68..9d935bee84 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -416,7 +416,6 @@ static int add_worktree(const char *path, const char *refname,
 	struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT;
 	const char *name;
-	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strvec child_env = STRVEC_INIT;
 	unsigned int counter = 0;
 	int len, ret;
@@ -424,7 +423,8 @@ static int add_worktree(const char *path, const char *refname,
 	struct commit *commit = NULL;
 	int is_branch = 0;
 	struct strbuf sb_name = STRBUF_INIT;
-	struct worktree **worktrees;
+	struct worktree **worktrees, *wt = NULL;
+	struct ref_store *wt_refs;
 
 	worktrees = get_worktrees();
 	check_candidate_path(path, opts->force, worktrees, "add");
@@ -500,15 +500,26 @@ static int add_worktree(const char *path, const char *refname,
 	write_file(sb.buf, "../..");
 
 	/*
-	 * This is to keep resolve_ref() happy. We need a valid HEAD
-	 * or is_git_directory() will reject the directory. Any value which
-	 * looks like an object ID will do since it will be immediately
-	 * replaced by the symbolic-ref or update-ref invocation in the new
-	 * worktree.
+	 * Set up the ref store of the worktree and create the HEAD reference.
 	 */
-	strbuf_reset(&sb);
-	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
-	write_file(sb.buf, "%s", oid_to_hex(null_oid()));
+	wt = get_linked_worktree(name);
+	if (!wt) {
+		ret = error(_("could not find created worktree '%s'"), name);
+		goto done;
+	}
+	wt_refs = get_worktree_ref_store(wt);
+
+	ret = refs_init_db(wt_refs, REFS_INIT_DB_IS_WORKTREE, &sb);
+	if (ret)
+		goto done;
+
+	if (!is_branch && commit)
+		ret = refs_update_ref(wt_refs, NULL, "HEAD", &commit->object.oid,
+				      NULL, 0, UPDATE_REFS_MSG_ON_ERR);
+	else
+		ret = refs_create_symref(wt_refs, "HEAD", symref.buf, NULL);
+	if (ret)
+		goto done;
 
 	/*
 	 * If the current worktree has sparse-checkout enabled, then copy
@@ -527,22 +538,6 @@ static int add_worktree(const char *path, const char *refname,
 
 	strvec_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
-	cp.git_cmd = 1;
-
-	if (!is_branch && commit) {
-		strvec_pushl(&cp.args, "update-ref", "HEAD",
-			     oid_to_hex(&commit->object.oid), NULL);
-	} else {
-		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
-			     symref.buf, NULL);
-		if (opts->quiet)
-			strvec_push(&cp.args, "--quiet");
-	}
-
-	strvec_pushv(&cp.env, child_env.v);
-	ret = run_command(&cp);
-	if (ret)
-		goto done;
 
 	if (opts->orphan &&
 	    (ret = make_worktree_orphan(refname, opts, &child_env)))
@@ -588,6 +583,7 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_release(&sb_git);
 	strbuf_release(&sb_name);
 	strbuf_release(&realpath);
+	free_worktree(wt);
 	return ret;
 }
 
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/6] worktree: initialize refdb via ref backends

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> when initializing worktrees we manually create the on-disk data
> structures required for the ref backend in "worktree.c". This works just
> fine right now where we only have a single user-exposed ref backend, but
> it will become unwieldy once we have multiple ref backends. This patch
> series thus refactors how we initialize worktrees so that we can use
> `refs_init_db()` to initialize required files for us.
>
> This patch series conflicts with ps/refstorage-extension. The conflict
> can be solved as shown below. I'm happy to defer this patch series
> though until the topic has landed on `master` in case this causes
> issues.

Resolution is not all that bad, but the change in function signature
means comments/explanations near both the caller and the callee of
the get_linked_worktree() function may need updating, I would think.
For example, ...

> diff --git a/worktree.h b/worktree.h
> index 8a75691eac..f14784a2ff 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -61,7 +61,8 @@ struct worktree *find_worktree(struct worktree **list,
>   * Look up the worktree corresponding to `id`, or NULL of no such worktree
>   * exists.
>   */
> -struct worktree *get_linked_worktree(const char *id);
> +struct worktree *get_linked_worktree(const char *id,
> +				     int skip_reading_head);

... this now needs to help developers who may want to add new
callers what to pass in "skip_reading_head" and why.

We may indeed want to build this on top of the refstorage-extansion
thing, as it seems to be relatively close to completion.

Thanks (and a happy new year).

Re: [PATCH 0/6] worktree: initialize refdb via ref backends

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2121 bytes --]

On Thu, Dec 28, 2023 at 10:11:31AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > when initializing worktrees we manually create the on-disk data
> > structures required for the ref backend in "worktree.c". This works just
> > fine right now where we only have a single user-exposed ref backend, but
> > it will become unwieldy once we have multiple ref backends. This patch
> > series thus refactors how we initialize worktrees so that we can use
> > `refs_init_db()` to initialize required files for us.
> >
> > This patch series conflicts with ps/refstorage-extension. The conflict
> > can be solved as shown below. I'm happy to defer this patch series
> > though until the topic has landed on `master` in case this causes
> > issues.
> 
> Resolution is not all that bad, but the change in function signature
> means comments/explanations near both the caller and the callee of
> the get_linked_worktree() function may need updating, I would think.
> For example, ...
> 
> > diff --git a/worktree.h b/worktree.h
> > index 8a75691eac..f14784a2ff 100644
> > --- a/worktree.h
> > +++ b/worktree.h
> > @@ -61,7 +61,8 @@ struct worktree *find_worktree(struct worktree **list,
> >   * Look up the worktree corresponding to `id`, or NULL of no such worktree
> >   * exists.
> >   */
> > -struct worktree *get_linked_worktree(const char *id);
> > +struct worktree *get_linked_worktree(const char *id,
> > +				     int skip_reading_head);
> 
> ... this now needs to help developers who may want to add new
> callers what to pass in "skip_reading_head" and why.
> 
> We may indeed want to build this on top of the refstorage-extansion
> thing, as it seems to be relatively close to completion.

Fair enough. I'll wait for the refstorage extension topic to hit `next`
or `master` first so as to not build deep dependency chains when things
may still move around. I don't mind waiting another one or two weeks,
especially during holidays where things are moving slower anyway.

> Thanks (and a happy new year).

Thanks, the same to you, too.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/6] refs/files: skip creation of "refs/{heads,tags}" for worktrees

[Eric Sunshine]

On Fri, Dec 29, 2023 at 5:16 AM Patrick Steinhardt <ps@pks.im> wrote:
> The files ref backend will create both "refs/heads" and "refs/tags" in
> the Git directory. While this logic makes sense for normal repositories,
> it does not fo worktrees because those refs are "common" refs that would
> always be contained in the main repository's ref database.

s/fo/for/

(not worth a reroll)

> Introduce a new flag telling the backend that it is expected to create a
> per-worktree ref database and skip creation of these dirs in the files
> backend when the flag is set. No other backends (currently) need
> worktree-specific logic, so this is the only required change to start
> creating per-worktree ref databases via `refs_init_db()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

Re: [PATCH 2/6] setup: move creation of "refs/" into the files backend

[Karthik Nayak]

Patrick Steinhardt <ps@pks.im> writes:

> Move the code to create the directory into the files backend itself to
> make it so. This means that future ref backends will also need to have
> equivalent logic around to ensure that the directory exists, but it
> seems a lot more sensible to have it this way round than to require
> callers to create the directory themselves.
>

Why not move it to refs.c:refs_init_db() instead? this way each
implementation doesn't have to do it?

@@ -2020,14 +2024,30 @@ const char *refs_resolve_ref_unsafe(struct
ref_store *refs,
 /* backend functions */
 int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err)
 {
+	/*
+	 * We need to create a "refs" dir in any case so that older versions of
+	 * Git can tell that this is a repository. This serves two main
+	 * purposes:
+	 *
+	 * - Clients will know to stop walking the parent-directory chain when
+	 *   detecting the Git repository. Otherwise they may end up detecting
+	 *   a Git repository in a parent directory instead.
+	 *
+	 * - Instead of failing to detect a repository with unknown reference
+	 *   format altogether, old clients will print an error saying that
+	 *   they do not understand the reference format extension.
+	 */
+	safe_create_dir(git_path("refs"), 1);
+	adjust_shared_perm(git_path("refs"));
+
 	return refs->be->init_db(refs, flags, err);
 }

Re: [PATCH 2/6] setup: move creation of "refs/" into the files backend

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

On Tue, Jan 02, 2024 at 05:23:18AM -0800, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Move the code to create the directory into the files backend itself to
> > make it so. This means that future ref backends will also need to have
> > equivalent logic around to ensure that the directory exists, but it
> > seems a lot more sensible to have it this way round than to require
> > callers to create the directory themselves.
> >
> 
> Why not move it to refs.c:refs_init_db() instead? this way each
> implementation doesn't have to do it?

True, that would be another possibility. But I think it is conceptually
unclean to split up creation of the refdb into multiple locations. The
"files" backend already has to create "refs/heads/" and "refs/tags/",
and the "reftable" backend will set up "refs/heads" as a file so that
it's impossible to create branches as loose files by accident. So both
do have specific knowledge around how exactly this directory hierarchy
should look like, and thus I think it is sensible to make the code self
contained inside of the backends.

My opinion on this would be different if we expected a proliferation of
backends to happen, but that's quite unlikely. Furthermore, we may at
some point decide that repos don't need "refs/" and/or "HEAD" at all
anymore, at which point it is easier to drop the creation of those files
and dirs from the reftable backend.

I'll update the commit message to include these considerations.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 0/6] worktree: initialize refdb via ref backends

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 6346 bytes --]

Hi,

this is the second version of my patch series that refactors the
initialization of worktree refdbs to use the refs API.

Changes compared to v1:

  - Improved commit messages.

  - This series is now based on `ps/refstorage-extension`, 1b2234079b
    (t9500: write "extensions.refstorage" into config, 2023-12-29).
    While there is no functional dependency between those series,
    merging both topics causes a merge conflict.

Patrick

Patrick Steinhardt (6):
  refs: prepare `refs_init_db()` for initializing worktree refs
  setup: move creation of "refs/" into the files backend
  refs/files: skip creation of "refs/{heads,tags}" for worktrees
  builtin/worktree: move setup of commondir file earlier
  worktree: expose interface to look up worktree by name
  builtin/worktree: create refdb via ref backend

 builtin/worktree.c    | 53 ++++++++++++++++++++-----------------------
 refs.c                |  6 ++---
 refs.h                |  4 +++-
 refs/debug.c          |  4 ++--
 refs/files-backend.c  | 37 +++++++++++++++++++++++++-----
 refs/packed-backend.c |  1 +
 refs/refs-internal.h  |  4 +++-
 setup.c               | 17 +-------------
 worktree.c            | 27 +++++++++++++---------
 worktree.h            | 12 ++++++++++
 10 files changed, 96 insertions(+), 69 deletions(-)

Range-diff against v1:
1:  6cb4e0a99f ! 1:  a4894b3e15 refs: prepare `refs_init_db()` for initializing worktree refs
    @@ refs/refs-internal.h: typedef struct ref_store *ref_store_init_fn(struct reposit
      				       struct ref_transaction *transaction,
     
      ## setup.c ##
    -@@ setup.c: void create_reference_database(const char *initial_branch, int quiet)
    - 	safe_create_dir(git_path("refs"), 1);
    +@@ setup.c: void create_reference_database(unsigned int ref_storage_format,
      	adjust_shared_perm(git_path("refs"));
      
    + 	repo_set_ref_storage_format(the_repository, ref_storage_format);
     -	if (refs_init_db(&err))
     +	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
      		die("failed to set up refs db: %s", err.buf);
2:  ae013eaa4a ! 2:  f500db51c2 setup: move creation of "refs/" into the files backend
    @@ Commit message
         seems a lot more sensible to have it this way round than to require
         callers to create the directory themselves.
     
    +    An alternative to this would be to create "refs/" in `refs_init_db()`
    +    directly. This feels conceptually unclean though as the creation of the
    +    refdb is now cluttered across different callsites. Furthermore, both the
    +    "files" and the upcoming "reftable" backend write backend-specific data
    +    into the "refs/" directory anyway, so splitting up this logic would only
    +    make it harder to reason about.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## refs/files-backend.c ##
    @@ refs/files-backend.c: static int files_init_db(struct ref_store *ref_store,
      
     
      ## setup.c ##
    -@@ setup.c: void create_reference_database(const char *initial_branch, int quiet)
    +@@ setup.c: void create_reference_database(unsigned int ref_storage_format,
      	struct strbuf err = STRBUF_INIT;
      	int reinit = is_reinit();
      
    @@ setup.c: void create_reference_database(const char *initial_branch, int quiet)
     -	safe_create_dir(git_path("refs"), 1);
     -	adjust_shared_perm(git_path("refs"));
     -
    + 	repo_set_ref_storage_format(the_repository, ref_storage_format);
      	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
      		die("failed to set up refs db: %s", err.buf);
    - 
3:  3cf6ceb274 ! 3:  9e99efeaa3 refs/files: skip creation of "refs/{heads,tags}" for worktrees
    @@ Commit message
     
         The files ref backend will create both "refs/heads" and "refs/tags" in
         the Git directory. While this logic makes sense for normal repositories,
    -    it does not fo worktrees because those refs are "common" refs that would
    -    always be contained in the main repository's ref database.
    +    it does not for worktrees because those refs are "common" refs that
    +    would always be contained in the main repository's ref database.
     
         Introduce a new flag telling the backend that it is expected to create a
         per-worktree ref database and skip creation of these dirs in the files
4:  1a6337fc51 = 4:  f2eb020288 builtin/worktree: move setup of commondir file earlier
5:  f344a915e9 ! 5:  5525a9f9c2 worktree: expose interface to look up worktree by name
    @@ worktree.c
      	free (worktrees);
      }
      
    -@@ worktree.c: static struct worktree *get_main_worktree(void)
    +@@ worktree.c: static struct worktree *get_main_worktree(int skip_reading_head)
      	return worktree;
      }
      
    --static struct worktree *get_linked_worktree(const char *id)
    -+struct worktree *get_linked_worktree(const char *id)
    +-static struct worktree *get_linked_worktree(const char *id,
    +-					    int skip_reading_head)
    ++struct worktree *get_linked_worktree(const char *id,
    ++				     int skip_reading_head)
      {
      	struct worktree *worktree = NULL;
      	struct strbuf path = STRBUF_INIT;
    @@ worktree.h: struct worktree *find_worktree(struct worktree **list,
     + * Look up the worktree corresponding to `id`, or NULL of no such worktree
     + * exists.
     + */
    -+struct worktree *get_linked_worktree(const char *id);
    ++struct worktree *get_linked_worktree(const char *id,
    ++				     int skip_reading_head);
     +
      /*
       * Return the worktree corresponding to `path`, or NULL if no such worktree
6:  aae969301f ! 6:  240dc40de1 builtin/worktree: create refdb via ref backend
    @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam
     -	strbuf_reset(&sb);
     -	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
     -	write_file(sb.buf, "%s", oid_to_hex(null_oid()));
    -+	wt = get_linked_worktree(name);
    ++	wt = get_linked_worktree(name, 1);
     +	if (!wt) {
     +		ret = error(_("could not find created worktree '%s'"), name);
     +		goto done;

base-commit: 1b2234079b24da99dd78e4ce4bfe338a2a841aed
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 1/6] refs: prepare `refs_init_db()` for initializing worktree refs

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4469 bytes --]

The purpose of `refs_init_db()` is to initialize the on-disk files of a
new ref database. The function is quite inflexible right now though, as
callers can neither specify the `struct ref_store` nor can they pass any
flags.

Refactor the interface to accept both of these. This will be required so
that we can start initializing per-worktree ref databases via the ref
backend instead of open-coding the initialization in "worktree.c".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.c                | 6 ++----
 refs.h                | 2 +-
 refs/debug.c          | 4 ++--
 refs/files-backend.c  | 4 +++-
 refs/packed-backend.c | 1 +
 refs/refs-internal.h  | 4 +++-
 setup.c               | 2 +-
 7 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/refs.c b/refs.c
index fdbf5f4cb1..254272ba6f 100644
--- a/refs.c
+++ b/refs.c
@@ -1944,11 +1944,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 }
 
 /* backend functions */
-int refs_init_db(struct strbuf *err)
+int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err)
 {
-	struct ref_store *refs = get_main_ref_store(the_repository);
-
-	return refs->be->init_db(refs, err);
+	return refs->be->init_db(refs, flags, err);
 }
 
 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
diff --git a/refs.h b/refs.h
index 916b874ae3..114caa272a 100644
--- a/refs.h
+++ b/refs.h
@@ -126,7 +126,7 @@ int should_autocreate_reflog(const char *refname);
 
 int is_branch(const char *refname);
 
-int refs_init_db(struct strbuf *err);
+int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err);
 
 /*
  * Return the peeled value of the oid currently being iterated via
diff --git a/refs/debug.c b/refs/debug.c
index b9775f2c37..634681ca44 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -33,10 +33,10 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
 	return (struct ref_store *)res;
 }
 
-static int debug_init_db(struct ref_store *refs, struct strbuf *err)
+static int debug_init_db(struct ref_store *refs, int flags, struct strbuf *err)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
-	int res = drefs->refs->be->init_db(drefs->refs, err);
+	int res = drefs->refs->be->init_db(drefs->refs, flags, err);
 	trace_printf_key(&trace_refs, "init_db: %d\n", res);
 	return res;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 43fd0ac760..153efe6662 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3220,7 +3220,9 @@ static int files_reflog_expire(struct ref_store *ref_store,
 	return -1;
 }
 
-static int files_init_db(struct ref_store *ref_store, struct strbuf *err UNUSED)
+static int files_init_db(struct ref_store *ref_store,
+			 int flags UNUSED,
+			 struct strbuf *err UNUSED)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "init_db");
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 8d1090e284..217f052d34 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1246,6 +1246,7 @@ static const char PACKED_REFS_HEADER[] =
 	"# pack-refs with: peeled fully-peeled sorted \n";
 
 static int packed_init_db(struct ref_store *ref_store UNUSED,
+			  int flags UNUSED,
 			  struct strbuf *err UNUSED)
 {
 	/* Nothing to do. */
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 8e9f04cc67..82219829b0 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -529,7 +529,9 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,
 					    const char *gitdir,
 					    unsigned int flags);
 
-typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
+typedef int ref_init_db_fn(struct ref_store *refs,
+			   int flags,
+			   struct strbuf *err);
 
 typedef int ref_transaction_prepare_fn(struct ref_store *refs,
 				       struct ref_transaction *transaction,
diff --git a/setup.c b/setup.c
index 1ab1a66bcb..6c8f656f7c 100644
--- a/setup.c
+++ b/setup.c
@@ -1943,7 +1943,7 @@ void create_reference_database(unsigned int ref_storage_format,
 	adjust_shared_perm(git_path("refs"));
 
 	repo_set_ref_storage_format(the_repository, ref_storage_format);
-	if (refs_init_db(&err))
+	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
 		die("failed to set up refs db: %s", err.buf);
 
 	/*
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 2/6] setup: move creation of "refs/" into the files backend

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 3859 bytes --]

When creating the ref database we unconditionally create the "refs/"
directory in "setup.c". This is a mandatory prerequisite for all Git
repositories regardless of the ref backend in use, because Git will be
unable to detect the directory as a repository if "refs/" doesn't exist.

We are about to add another new caller that will want to create a ref
database when creating worktrees. We would require the same logic to
create the "refs/" directory even though the caller really should not
care about such low-level details. Ideally, the ref database should be
fully initialized after calling `refs_init_db()`.

Move the code to create the directory into the files backend itself to
make it so. This means that future ref backends will also need to have
equivalent logic around to ensure that the directory exists, but it
seems a lot more sensible to have it this way round than to require
callers to create the directory themselves.

An alternative to this would be to create "refs/" in `refs_init_db()`
directly. This feels conceptually unclean though as the creation of the
refdb is now cluttered across different callsites. Furthermore, both the
"files" and the upcoming "reftable" backend write backend-specific data
into the "refs/" directory anyway, so splitting up this logic would only
make it harder to reason about.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c | 17 +++++++++++++++++
 setup.c              | 15 ---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 153efe6662..054ecdbca3 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3228,9 +3228,26 @@ static int files_init_db(struct ref_store *ref_store,
 		files_downcast(ref_store, REF_STORE_WRITE, "init_db");
 	struct strbuf sb = STRBUF_INIT;
 
+	/*
+	 * We need to create a "refs" dir in any case so that older versions of
+	 * Git can tell that this is a repository. This serves two main purposes:
+	 *
+	 * - Clients will know to stop walking the parent-directory chain when
+	 *   detecting the Git repository. Otherwise they may end up detecting
+	 *   a Git repository in a parent directory instead.
+	 *
+	 * - Instead of failing to detect a repository with unknown reference
+	 *   format altogether, old clients will print an error saying that
+	 *   they do not understand the reference format extension.
+	 */
+	strbuf_addf(&sb, "%s/refs", ref_store->gitdir);
+	safe_create_dir(sb.buf, 1);
+	adjust_shared_perm(sb.buf);
+
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
+	strbuf_reset(&sb);
 	files_ref_path(refs, &sb, "refs/heads");
 	safe_create_dir(sb.buf, 1);
 
diff --git a/setup.c b/setup.c
index 6c8f656f7c..abb271e017 100644
--- a/setup.c
+++ b/setup.c
@@ -1927,21 +1927,6 @@ void create_reference_database(unsigned int ref_storage_format,
 	struct strbuf err = STRBUF_INIT;
 	int reinit = is_reinit();
 
-	/*
-	 * We need to create a "refs" dir in any case so that older versions of
-	 * Git can tell that this is a repository. This serves two main purposes:
-	 *
-	 * - Clients will know to stop walking the parent-directory chain when
-	 *   detecting the Git repository. Otherwise they may end up detecting
-	 *   a Git repository in a parent directory instead.
-	 *
-	 * - Instead of failing to detect a repository with unknown reference
-	 *   format altogether, old clients will print an error saying that
-	 *   they do not understand the reference format extension.
-	 */
-	safe_create_dir(git_path("refs"), 1);
-	adjust_shared_perm(git_path("refs"));
-
 	repo_set_ref_storage_format(the_repository, ref_storage_format);
 	if (refs_init_db(get_main_ref_store(the_repository), 0, &err))
 		die("failed to set up refs db: %s", err.buf);
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 3/6] refs/files: skip creation of "refs/{heads,tags}" for worktrees

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

The files ref backend will create both "refs/heads" and "refs/tags" in
the Git directory. While this logic makes sense for normal repositories,
it does not for worktrees because those refs are "common" refs that
would always be contained in the main repository's ref database.

Introduce a new flag telling the backend that it is expected to create a
per-worktree ref database and skip creation of these dirs in the files
backend when the flag is set. No other backends (currently) need
worktree-specific logic, so this is the only required change to start
creating per-worktree ref databases via `refs_init_db()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.h               |  2 ++
 refs/files-backend.c | 22 ++++++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/refs.h b/refs.h
index 114caa272a..c2dfe451a1 100644
--- a/refs.h
+++ b/refs.h
@@ -126,6 +126,8 @@ int should_autocreate_reflog(const char *refname);
 
 int is_branch(const char *refname);
 
+#define REFS_INIT_DB_IS_WORKTREE (1 << 0)
+
 int refs_init_db(struct ref_store *refs, int flags, struct strbuf *err);
 
 /*
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 054ecdbca3..6dae37e351 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3221,7 +3221,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 }
 
 static int files_init_db(struct ref_store *ref_store,
-			 int flags UNUSED,
+			 int flags,
 			 struct strbuf *err UNUSED)
 {
 	struct files_ref_store *refs =
@@ -3245,15 +3245,21 @@ static int files_init_db(struct ref_store *ref_store,
 	adjust_shared_perm(sb.buf);
 
 	/*
-	 * Create .git/refs/{heads,tags}
+	 * There is no need to create directories for common refs when creating
+	 * a worktree ref store.
 	 */
-	strbuf_reset(&sb);
-	files_ref_path(refs, &sb, "refs/heads");
-	safe_create_dir(sb.buf, 1);
+	if (!(flags & REFS_INIT_DB_IS_WORKTREE)) {
+		/*
+		 * Create .git/refs/{heads,tags}
+		 */
+		strbuf_reset(&sb);
+		files_ref_path(refs, &sb, "refs/heads");
+		safe_create_dir(sb.buf, 1);
 
-	strbuf_reset(&sb);
-	files_ref_path(refs, &sb, "refs/tags");
-	safe_create_dir(sb.buf, 1);
+		strbuf_reset(&sb);
+		files_ref_path(refs, &sb, "refs/tags");
+		safe_create_dir(sb.buf, 1);
+	}
 
 	strbuf_release(&sb);
 	return 0;
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 4/6] builtin/worktree: move setup of commondir file earlier

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

Shuffle around how we create supporting worktree files so that we first
ensure that the worktree has all link files ("gitdir", "commondir")
before we try to initialize the ref database by writing "HEAD". This
will be required by a subsequent commit where we start to initialize the
ref database via `refs_init_db()`, which will require an initialized
`struct worktree *`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/worktree.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4ac1621541..58937a2a68 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -495,6 +495,10 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_realpath(&realpath, get_git_common_dir(), 1);
 	write_file(sb_git.buf, "gitdir: %s/worktrees/%s",
 		   realpath.buf, name);
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
+	write_file(sb.buf, "../..");
+
 	/*
 	 * This is to keep resolve_ref() happy. We need a valid HEAD
 	 * or is_git_directory() will reject the directory. Any value which
@@ -505,9 +509,6 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
 	write_file(sb.buf, "%s", oid_to_hex(null_oid()));
-	strbuf_reset(&sb);
-	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
-	write_file(sb.buf, "../..");
 
 	/*
 	 * If the current worktree has sparse-checkout enabled, then copy
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 5/6] worktree: expose interface to look up worktree by name

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]

Our worktree interfaces do not provide a way to look up a worktree by
its name. Expose `get_linked_worktree()` to allow for this usecase. As
callers are responsible for freeing this worktree, introduce a new
function `free_worktree()` that does so.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 worktree.c | 27 ++++++++++++++++-----------
 worktree.h | 12 ++++++++++++
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/worktree.c b/worktree.c
index cc34a3419b..5d5dd46609 100644
--- a/worktree.c
+++ b/worktree.c
@@ -12,18 +12,23 @@
 #include "wt-status.h"
 #include "config.h"
 
+void free_worktree(struct worktree *worktree)
+{
+	if (!worktree)
+		return;
+	free(worktree->path);
+	free(worktree->id);
+	free(worktree->head_ref);
+	free(worktree->lock_reason);
+	free(worktree->prune_reason);
+	free(worktree);
+}
+
 void free_worktrees(struct worktree **worktrees)
 {
 	int i = 0;
-
-	for (i = 0; worktrees[i]; i++) {
-		free(worktrees[i]->path);
-		free(worktrees[i]->id);
-		free(worktrees[i]->head_ref);
-		free(worktrees[i]->lock_reason);
-		free(worktrees[i]->prune_reason);
-		free(worktrees[i]);
-	}
+	for (i = 0; worktrees[i]; i++)
+		free_worktree(worktrees[i]);
 	free (worktrees);
 }
 
@@ -75,8 +80,8 @@ static struct worktree *get_main_worktree(int skip_reading_head)
 	return worktree;
 }
 
-static struct worktree *get_linked_worktree(const char *id,
-					    int skip_reading_head)
+struct worktree *get_linked_worktree(const char *id,
+				     int skip_reading_head)
 {
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
diff --git a/worktree.h b/worktree.h
index ce45b66de9..f14784a2ff 100644
--- a/worktree.h
+++ b/worktree.h
@@ -57,6 +57,13 @@ struct worktree *find_worktree(struct worktree **list,
 			       const char *prefix,
 			       const char *arg);
 
+/*
+ * Look up the worktree corresponding to `id`, or NULL of no such worktree
+ * exists.
+ */
+struct worktree *get_linked_worktree(const char *id,
+				     int skip_reading_head);
+
 /*
  * Return the worktree corresponding to `path`, or NULL if no such worktree
  * exists.
@@ -134,6 +141,11 @@ void repair_worktrees(worktree_repair_fn, void *cb_data);
  */
 void repair_worktree_at_path(const char *, worktree_repair_fn, void *cb_data);
 
+/*
+ * Free up the memory for a worktree.
+ */
+void free_worktree(struct worktree *);
+
 /*
  * Free up the memory for worktree(s)
  */
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 6/6] builtin/worktree: create refdb via ref backend

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4400 bytes --]

When creating a worktree we create the worktree's ref database manually
by first writing a "HEAD" file so that the directory is recognized as a
Git repository by other commands, and then running git-update-ref(1) or
git-symbolic-ref(1) to write the actual value. But while this is fine
for the files backend, this logic simply assumes too much about how the
ref backend works and will leave behind an invalid ref database once any
other ref backend lands.

Refactor the code to instead use `refs_init_db()` to initialize the ref
database so that git-worktree(1) itself does not need to know about how
to initialize it. This will allow future ref backends to customize how
the per-worktree ref database is set up. Furthermore, as we now already
have a worktree ref store around, we can also avoid spawning external
commands to write the HEAD reference and instead use the refs API to do
so.

Note that we do not have an equivalent to passing the `--quiet` flag to
git-symbolic-ref(1) as we did before. This flag does not have an effect
anyway though, as git-symbolic-ref(1) only honors it when reading a
symref, but never when writing one.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/worktree.c | 48 +++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 58937a2a68..dd10446d81 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -416,7 +416,6 @@ static int add_worktree(const char *path, const char *refname,
 	struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT;
 	const char *name;
-	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strvec child_env = STRVEC_INIT;
 	unsigned int counter = 0;
 	int len, ret;
@@ -424,7 +423,8 @@ static int add_worktree(const char *path, const char *refname,
 	struct commit *commit = NULL;
 	int is_branch = 0;
 	struct strbuf sb_name = STRBUF_INIT;
-	struct worktree **worktrees;
+	struct worktree **worktrees, *wt = NULL;
+	struct ref_store *wt_refs;
 
 	worktrees = get_worktrees();
 	check_candidate_path(path, opts->force, worktrees, "add");
@@ -500,15 +500,26 @@ static int add_worktree(const char *path, const char *refname,
 	write_file(sb.buf, "../..");
 
 	/*
-	 * This is to keep resolve_ref() happy. We need a valid HEAD
-	 * or is_git_directory() will reject the directory. Any value which
-	 * looks like an object ID will do since it will be immediately
-	 * replaced by the symbolic-ref or update-ref invocation in the new
-	 * worktree.
+	 * Set up the ref store of the worktree and create the HEAD reference.
 	 */
-	strbuf_reset(&sb);
-	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
-	write_file(sb.buf, "%s", oid_to_hex(null_oid()));
+	wt = get_linked_worktree(name, 1);
+	if (!wt) {
+		ret = error(_("could not find created worktree '%s'"), name);
+		goto done;
+	}
+	wt_refs = get_worktree_ref_store(wt);
+
+	ret = refs_init_db(wt_refs, REFS_INIT_DB_IS_WORKTREE, &sb);
+	if (ret)
+		goto done;
+
+	if (!is_branch && commit)
+		ret = refs_update_ref(wt_refs, NULL, "HEAD", &commit->object.oid,
+				      NULL, 0, UPDATE_REFS_MSG_ON_ERR);
+	else
+		ret = refs_create_symref(wt_refs, "HEAD", symref.buf, NULL);
+	if (ret)
+		goto done;
 
 	/*
 	 * If the current worktree has sparse-checkout enabled, then copy
@@ -527,22 +538,6 @@ static int add_worktree(const char *path, const char *refname,
 
 	strvec_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
-	cp.git_cmd = 1;
-
-	if (!is_branch && commit) {
-		strvec_pushl(&cp.args, "update-ref", "HEAD",
-			     oid_to_hex(&commit->object.oid), NULL);
-	} else {
-		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
-			     symref.buf, NULL);
-		if (opts->quiet)
-			strvec_push(&cp.args, "--quiet");
-	}
-
-	strvec_pushv(&cp.env, child_env.v);
-	ret = run_command(&cp);
-	if (ret)
-		goto done;
 
 	if (opts->orphan &&
 	    (ret = make_worktree_orphan(refname, opts, &child_env)))
@@ -588,6 +583,7 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_release(&sb_git);
 	strbuf_release(&sb_name);
 	strbuf_release(&realpath);
+	free_worktree(wt);
 	return ret;
 }
 
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/6] worktree: initialize refdb via ref backends

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this is the second version of my patch series that refactors the
> initialization of worktree refdbs to use the refs API.
>
> Changes compared to v1:
>
>   - Improved commit messages.
>
>   - This series is now based on `ps/refstorage-extension`, 1b2234079b
>     (t9500: write "extensions.refstorage" into config, 2023-12-29).
>     While there is no functional dependency between those series,
>     merging both topics causes a merge conflict.
>

This looks good to me now. Thanks Patrick!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 0/6] worktree: initialize refdb via ref backends

[Junio C Hamano]

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

> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> this is the second version of my patch series that refactors the
>> initialization of worktree refdbs to use the refs API.
>>
>> Changes compared to v1:
>>
>>   - Improved commit messages.
>>
>>   - This series is now based on `ps/refstorage-extension`, 1b2234079b
>>     (t9500: write "extensions.refstorage" into config, 2023-12-29).
>>     While there is no functional dependency between those series,
>>     merging both topics causes a merge conflict.
>>
>
> This looks good to me now. Thanks Patrick!

Thanks, both.