| Message ID | ae013eaa4aba0d68172ff03dbe9f2c2bca596285.1703754513.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] refs: prepare `refs_init_db()` for initializing worktree refs | expand |
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); }
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
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);
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(-)