| Message ID | cover.1703754513.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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).
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
diff --cc setup.c index f2d55994e2,4712bba6f8..0000000000 --- a/setup.c diff --cc worktree.c index 085f2cc41a,9702ed0308..0000000000 --- a/worktree.c 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
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 +++ 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); /* +++ 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; 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