Make refdb_fs (hopefully) fully aware of per worktree refs #6387
Conversation
Signed-off-by: Sven Strickroth <[email protected]>
59b4385 to
21e5903
Compare
Fixes issue isse libgit2#5492. Signed-off-by: Sven Strickroth <[email protected]>
|
@ethomson Any chance to get a review? |
| @@ -863,6 +867,49 @@ static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) | |||
| error = git_vector_insert(&iter->loose, ref_dup); | |||
| } | |||
|
|
|||
| if (!error && git_repository_is_worktree(backend->repo) == 1) { | |||
| git_iterator_free(fsit); | |||
There was a problem hiding this comment.
I think from a layering perspective, the filesystem refdb is not the right place for this — that would mean that every reference db would have to handle the per-corktree references themselves. I think that we should hoist the worktree logic into refs.c instead. In theory, the reference database should be able to operate without any knowledge of a repository (indeed, should be able to operate without a repository instance at all).
Reference databases should just handle the storage of references, and the git_reference family of functions should handle the business logic, including understanding whether there's a worktree or not.
Let me know if this makes sense — I can take a stab at an implementation on the references if you'd like.
There was a problem hiding this comment.
Good point.
However, here, both parts are within the filesystem.
There was a problem hiding this comment.
Not sure that I understand? Are you saying that since worktrees are in the filesystem and the default git-compatible ref database is also on the filesystem, that this is a reasonable separation of concerns?
There was a problem hiding this comment.
Er, no, you're saying that the worktree reference storage living in the commondir is actually an implementation detail of the git-compatible ref database implementation itself. 🤔
That's a bit disappointing, but I think that you may be correct given our current database architecture.
|
Thanks for the fix! I hope to come back and maybe re-think the abstraction around worktree refs databases, so that each refdb backend doesn't need to think about worktrees. In the meantime, this is great. 🎉 |
This makes refdb_fs fully aware of per worktree refs (fixes issue #5492).
The code has some duplication but does the trick.