On Tue, Oct 08, 2024 at 11:29:51PM +0530, Kousik Sanagavarapu wrote: > On Tue, Oct 08, 2024 at 02:23:54PM +0200, Patrick Steinhardt wrote: > > On Mon, Oct 07, 2024 at 10:24:49PM +0530, Kousik Sanagavarapu wrote: > > > Hi, > > > > > > I have two questions but a bit of a background first - > > > > > > [...] > > > > > > So my question is - do we want, in the future in which we are free from > > > the dependency on "the_repository", for all the local paths to be a part > > > of "struct repo_path_cache"? Which in my gut feels wrong - one alternative > > > then is that we will have to refactor REPO_GIT_PATH_FUNC - or am I missing > > > something here? > > > > What I don't quite understand: what is the problem with making it part > > of the `struct repo_path_cache`? Does this cause an actual issue, or is > > it merely that you feel it is unnecessary complexity? > > I feel it is unnecessary complexity. > > $ git grep -E "(static GIT_PATH_FUNC|^GIT_PATH_FUNC)" | wc -l > 65 > > Meaning each of these would have to have an entry in > "struct repo_path_cache" in the world where we don't rely on > "the_repository". Some of these are also not direct ".git/some-file" but > ".git/dir/files" where ".git/dir" is also given by a seperate path func, > like ".git/rebase-merges" and ".git/rebase-merges/head-name". > > So why hold pointers to such filenames instead of just calling > repo_git_path() manually - all these filenames are "local" anyways - unlike > say files such as "SQUASH_MSG"? It does make the interface easier to use at times because you don't have to worry about freeing returned strings. In other situations it likely is unnecessary. In any case, not all cases must strictly be converted to REPO_PATH_FUNC. A refactoring may also decide that using e.g. `repo_git_path()` or `repo_common_pathv()` might be better alternatives. Patrick
