| Message ID | 20250206-b4-pks-path-drop-the-repository-v1-0-4e77f0313206@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | path: remove dependency on `the_repository` | expand |
On Thu, Feb 06, 2025 at 08:57:56AM +0100, Patrick Steinhardt wrote: > Hi, > > this patch series removes the dependency on `the_repository` from the > "path" subsystem. The series is structured as follows: > > - Patches 1 to 5 unifyf the interfaces that can be used to retrieve > repository paths (gitdir, commondir, workdir and submodule paths) > and adapts callers accodringly. > > - Patches 6 to 12 drop repository path functions that had an implicit > dependency on `the_repository`. > > - Patches 13 to 16 refactor "path.c"-internal code to stop depending > on `the_repository`. > Thanks for working on this. However, I feel a little hard when reviewing the code for patch 13 to patch 16. They are not so relevant to the previous twelfth patches. One thing I want to recommend is that we may combine the first and the second step. It is a little wired that we first refactor the code then we use the new one. When reading the code, I have to step back to the previous patch to understand something. Thanks, Jialuo
On Fri, Feb 07, 2025 at 12:14:14AM +0800, shejialuo wrote: > On Thu, Feb 06, 2025 at 08:57:56AM +0100, Patrick Steinhardt wrote: > > Hi, > > > > this patch series removes the dependency on `the_repository` from the > > "path" subsystem. The series is structured as follows: > > > > - Patches 1 to 5 unifyf the interfaces that can be used to retrieve > > repository paths (gitdir, commondir, workdir and submodule paths) > > and adapts callers accodringly. > > > > - Patches 6 to 12 drop repository path functions that had an implicit > > dependency on `the_repository`. > > > > - Patches 13 to 16 refactor "path.c"-internal code to stop depending > > on `the_repository`. > > > > Thanks for working on this. However, I feel a little hard when reviewing > the code for patch 13 to patch 16. They are not so relevant to the > previous twelfth patches. Thanks for your review! > One thing I want to recommend is that we may combine the first and the > second step. It is a little wired that we first refactor the code then > we use the new one. When reading the code, I have to step back to the > previous patch to understand something. But those are about very different things. The first step fills in missing interfaces and unifies the calling conventions, whereas the second step removes users of functions that don't have a dependency on `the_repository`. From my perspective, mixing these two steps up with one another would only lead to confusion as we're doing too many things at once. Patrick
Patrick Steinhardt <ps@pks.im> writes: > Hi, > > this patch series removes the dependency on `the_repository` from the > "path" subsystem. The series is structured as follows: > > - Patches 1 to 5 unifyf the interfaces that can be used to retrieve > repository paths (gitdir, commondir, workdir and submodule paths) > and adapts callers accodringly. > > - Patches 6 to 12 drop repository path functions that had an implicit > dependency on `the_repository`. > > - Patches 13 to 16 refactor "path.c"-internal code to stop depending > on `the_repository`. > > Thanks! > > Patrick > I've reviewed the series, apart from some small nits, the series looks great. Thanks for working on this. Karthik [snip]
Hi, this patch series removes the dependency on `the_repository` from the "path" subsystem. The series is structured as follows: - Patches 1 to 5 unifyf the interfaces that can be used to retrieve repository paths (gitdir, commondir, workdir and submodule paths) and adapts callers accodringly. - Patches 6 to 12 drop repository path functions that had an implicit dependency on `the_repository`. - Patches 13 to 16 refactor "path.c"-internal code to stop depending on `the_repository`. Thanks! Patrick --- Patrick Steinhardt (16): path: refactor `repo_common_path()` family of functions path: refactor `repo_git_path()` family of functions path: refactor `repo_worktree_path()` family of functions submodule: refactor `submodule_to_gitdir()` to accept a repo path: refactor `repo_submodule_path()` family of functions path: drop unused `strbuf_git_path()` function path: drop `git_pathdup()` in favor of `repo_git_path()` path: drop `git_path_buf()` in favor of `repo_git_path_replace()` worktree: return allocated string from `get_worktree_git_dir()` path: drop `git_common_path()` in favor of `repo_common_path()` rerere: let `rerere_path()` write paths into a caller-provided buffer path: drop `git_path()` in favor of `repo_git_path()` repo-settings: introduce function to clear struct environment: move access to "core.hooksPath" into repo settings environment: move access to "core.sharedRepository" into repo settings path: adjust last remaining users of `the_repository` bisect.c | 2 +- branch.c | 7 +- builtin/am.c | 2 +- builtin/clone.c | 4 +- builtin/commit.c | 8 +- builtin/config.c | 8 +- builtin/fast-import.c | 4 +- builtin/fsck.c | 10 ++- builtin/gc.c | 25 ++++-- builtin/init-db.c | 8 +- builtin/log.c | 6 +- builtin/notes.c | 9 ++- builtin/rebase.c | 2 +- builtin/receive-pack.c | 4 +- builtin/remote.c | 6 +- builtin/replace.c | 2 +- builtin/rerere.c | 11 ++- builtin/rev-parse.c | 6 +- builtin/submodule--helper.c | 4 +- builtin/tag.c | 2 +- builtin/worktree.c | 43 +++++++--- commit-graph.c | 2 +- compat/precompose_utf8.c | 6 +- config.c | 5 -- copy.c | 4 +- dir.c | 2 +- editor.c | 6 +- environment.c | 27 ------- environment.h | 11 --- hook.c | 3 +- http-backend.c | 2 +- loose.c | 8 +- midx-write.c | 2 +- notes-merge.c | 26 +++--- object-file.c | 14 ++-- pack-bitmap-write.c | 2 +- pack-write.c | 10 ++- path.c | 166 +++++++++++++++++++++++++++----------- path.h | 191 ++++++++++++-------------------------------- reachable.c | 6 +- read-cache.c | 26 ++++-- refs.c | 6 +- refs/files-backend.c | 10 +-- refs/reftable-backend.c | 10 +-- remote.c | 21 ++--- repo-settings.c | 44 +++++++++- repo-settings.h | 14 ++++ repository.c | 6 +- rerere.c | 101 ++++++++++++++++------- rerere.h | 3 +- revision.c | 7 +- server-info.c | 2 +- setup.c | 70 ++++++++-------- shallow.c | 4 +- submodule.c | 15 ++-- submodule.h | 3 +- t/helper/test-ref-store.c | 7 +- tempfile.c | 4 +- tmp-objdir.c | 17 ++-- worktree.c | 49 ++++++++---- worktree.h | 2 +- wt-status.c | 42 ++++++---- 62 files changed, 639 insertions(+), 490 deletions(-) --- base-commit: bc204b742735ae06f65bb20291c95985c9633b7f change-id: 20250204-b4-pks-path-drop-the-repository-9ed0f9956361