Subject: [Question] local paths when USE_THE_REPOSITORY_VARIABLE is not defined

From: Kousik Sanagavarapu <[email-masked]>
Date: Mon, 7 Oct 2024 22:24:49 +0530

Hi,

I have two questions but a bit of a background first -

102de880d2 (path.c: migrate global git_path_* to take a repository
argument, 2018-05-17) made global git_path_* functions take a repo
argument.  The commit msg mentions that this migration doesn't change
the local path functions in various builtins - which were defined using
GIT_PATH_FUNC.  This was also the commit which introduced the macro
REPO_GIT_PATH_FUNC.

Skip to 7ac16649ec (path: hide functions using `the_repository` by
default, 2024-08-13), GIT_PATH_FUNC is hidden under
USE_THE_REPOSITORY_VARIABLE and the REPO_GIT_PATH_FUNC is made its
arbitrary repo equivalent - which can be inferred from the following
portion of the diff

@@ -165,19 +130,10 @@ void report_linked_checkout_garbage(struct repository *r);
 /*
  * You can define a static memoized git path like:
  *
- *    static GIT_PATH_FUNC(git_path_foo, "FOO")
+ *    static REPO_GIT_PATH_FUNC(git_path_foo, "FOO")
  *
  * or use one of the global ones below.
  */
-#define GIT_PATH_FUNC(func, filename) \
-       const char *func(void) \
-       { \
-               static char *ret; \
-               if (!ret) \
-                       ret = git_pathdup(filename); \
-               return ret; \
-       }
-
 #define REPO_GIT_PATH_FUNC(var, filename) \
        const char *git_path_##var(struct repository *r) \
        { \

(the GIT_PATH_FUNC macro is moved to be under USE_THE_REPOSITORY_VARIABLE)

Looking at the expansion of REPO_GIT_PATH_FUNC ...

#define REPO_GIT_PATH_FUNC(var, filename) \
	const char *git_path_##var(struct repository *r) \
	{ \
		if (!r->cached_paths.var) \
			r->cached_paths.var = repo_git_path(r, filename); \
		return r->cached_paths.var; \
	}

It seems that REPO_GIT_PATH_FUNC isn't an exact equivalent of
GIT_PATH_FUNC.  That is, REPO_GIT_PATH_FUNC expects even a local path to be
a field of the "struct repo_path_cache".  An example of a local path is
EDIT_DESCRIPTION from "git branch --edit-description" (which inturn gets
used by "git format-patch").

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?

I got into this when I was trying to refactor builtin/branch.c to be
independent of "the_repository".  It was a very naive approach of just
manual conversion of all the git_* calls to repo_* calls and similar
changes but the compiler started to complain since I overlooked
GIT_PATH_FUNC and some variables in environment.h which are also hidden
under USE_THE_REPOSITORY_VARIABLE.

Which raises another question - why are variables such as
"comment_line_str" and "default_abbrev" hidden under
USE_THE_REPOSITORY_VARIABLE?[1]  They don't seem to be dependent on
"the_repository"?  Again, I might be missing something here but am not
sure what.

By the way I don't expect this "naive approach" to be the right method
of doing this - I was just tinkering to get to know
USE_THE_REPOSITORY_VARIABLE better - since builtin/branch.c also calls
into ref-filter which heavily relies on "the_repository" so changes
there also would be appropriate for the complete picture.

[1] See

- f2d70847bd (environment: reorder header to split out `the_repository`-free
  section, 2024-09-12)

- 673af418d0 (environment: guard state depending on a repository, 2024-09-12)