On Wed, Oct 09, 2024 at 05:42:01AM +0200, Patrick Steinhardt wrote: > 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. Got it. Thanks again for the nice explanations.
