git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 07/20] path: hide functions using `the_repository` by default

From
Justin Tobler <jltobler@gmail.com>
Date
Aug 9, 2024, 19:43 UTC
Message-ID
<mc4omj4mi23fhmqp4xeafcluzf2iidp4uaf3h3gaxcalm3igvk@a7abzjynshfw>
In-Reply-To
<b4e973a2804ba09149224a2e18a359717228607e.1723013714.git.ps@pks.im>
On 24/08/07 08:57AM, Patrick Steinhardt wrote:
> The path subsytem provides a bunch of legacy functions that compute
s/subsytem/subsystem/
Show 7 quoted lines
> paths relative to the "gitdir" and "commondir" directories of the global
> `the_repository` variable. Use of those functions is discouraged, and it
> is easy to miss the implicit dependency on `the_repository` that calls
> to those functions may cause.
> 
> With `USE_THE_REPOSITORY_VARIABLE`, we have recently introduced a tool
> that allows us to get rid of such functions over time. With this define,
s/define/defined/
Show 9 quoted lines
> we can hide away functions that have such implicit dependency such that
> other subsystems that want to be free of `the_repository` will not use
> them by accident.
> 
> Move all path-related functions that use `the_repository` into a block
> that gets only conditionally compiled depending on whether or not the
> macro has been defined. This also removes all dependencies on that
> variable in "path.c", allowing us to remove the definition of said
> preprocessor macro.

Nice! So now the "path.c" is `the_repository` free. Implicit use of `the_repository` is guarded behind `USE_THE_REPOSITORY_VARIABLE`. This is quite neat. :)

Show 26 quoted lines
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  path.c |  52 +-------------------
>  path.h | 147 ++++++++++++++++++++++++++++++++++++++-------------------
>  2 files changed, 100 insertions(+), 99 deletions(-)
> 
> diff --git a/path.c b/path.c
> index 567eff5253..d073ae6449 100644
> --- a/path.c
> +++ b/path.c
> @@ -2,8 +2,6 @@
>   * Utilities for paths and pathnames
>   */
>  
> -#define USE_THE_REPOSITORY_VARIABLE
> -
>  #include "git-compat-util.h"
>  #include "abspath.h"
>  #include "environment.h"
> @@ -30,7 +28,7 @@ static int get_st_mode_bits(const char *path, int *mode)
>  	return 0;
>  }
>  
> -static struct strbuf *get_pathname(void)
> +struct strbuf *get_pathname(void)

Now that functions with implicit `the_repository` usage are static inlined we are required to expose `get_pathname()` for the same reason as some of the other functions in previous commits.

[snip]
> +# ifdef USE_THE_REPOSITORY_VARIABLE
> +#  include "strbuf.h"
> +#  include "repository.h"

Naive question, what is the purpose of providing the include statements here? Wouldn't they always already be included?

Show 84 quoted lines
> +
> +/*
> + * Return a statically allocated path into the main repository's
> + * (the_repository) common git directory.
> + */
> +__attribute__((format (printf, 1, 2)))
> +static inline const char *git_common_path(const char *fmt, ...)
> +{
> +	struct strbuf *pathname = get_pathname();
> +	va_list args;
> +	va_start(args, fmt);
> +	strbuf_git_common_pathv(pathname, the_repository, fmt, args);
> +	va_end(args);
> +	return pathname->buf;
> +}
> +
> +/*
> + * Construct a path into the main repository's (the_repository) git directory
> + * and place it in the provided buffer `buf`, the contents of the buffer will
> + * be overridden.
> + */
> +__attribute__((format (printf, 2, 3)))
> +static inline char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
> +{
> +	va_list args;
> +	strbuf_reset(buf);
> +	va_start(args, fmt);
> +	repo_git_pathv(the_repository, NULL, buf, fmt, args);
> +	va_end(args);
> +	return buf->buf;
> +}
> +
> +/*
> + * Construct a path into the main repository's (the_repository) git directory
> + * and append it to the provided buffer `sb`.
> + */
> +__attribute__((format (printf, 2, 3)))
> +static inline void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
> +{
> +	va_list args;
> +	va_start(args, fmt);
> +	repo_git_pathv(the_repository, NULL, sb, fmt, args);
> +	va_end(args);
> +}
> +
> +/*
> + * Return a statically allocated path into the main repository's
> + * (the_repository) git directory.
> + */
> +__attribute__((format (printf, 1, 2)))
> +static inline const char *git_path(const char *fmt, ...)
> +{
> +	struct strbuf *pathname = get_pathname();
> +	va_list args;
> +	va_start(args, fmt);
> +	repo_git_pathv(the_repository, NULL, pathname, fmt, args);
> +	va_end(args);
> +	return pathname->buf;
> +}
> +
> +#define GIT_PATH_FUNC(func, filename) \
> +	const char *func(void) \
> +	{ \
> +		static char *ret; \
> +		if (!ret) \
> +			ret = git_pathdup(filename); \
> +		return ret; \
> +	}
> +
> +/*
> + * Return a path into the main repository's (the_repository) git directory.
> + */
> +__attribute__((format (printf, 1, 2)))
> +static inline char *git_pathdup(const char *fmt, ...)
> +{
> +	struct strbuf path = STRBUF_INIT;
> +	va_list args;
> +	va_start(args, fmt);
> +	repo_git_pathv(the_repository, NULL, &path, fmt, args);
> +	va_end(args);
> +	return strbuf_detach(&path, NULL);
> +}
> +
> +# endif /* USE_THE_REPOSITORY_VARIABLE */
I like how it's all consolidated into this one block. Very nice! :)
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 18 of 69 in “Stop using `the_repository` in "config.c"”
  1. 00/20 Stop using `the_repository` in "config.c"Patrick Steinhardt, Aug 7, 2024
  2. 01/20 path: expose `do_git_path()` as `repo_git_pathv()`Patrick Steinhardt, Aug 7, 2024
  3. Justin ToblerAug 9, 2024
  4. Patrick SteinhardtAug 13, 2024
  5. 02/20 path: expose `do_git_common_path()` as `strbuf_git_common_pathv()`Patrick Steinhardt, Aug 7, 2024
  6. Justin ToblerAug 9, 2024
  7. Junio C HamanoAug 9, 2024
  8. Patrick SteinhardtAug 13, 2024
  9. Junio C HamanoAug 13, 2024
  10. 03/20 editor: do not rely on `the_repository` for interactive editsPatrick Steinhardt, Aug 7, 2024
  11. Justin ToblerAug 9, 2024
  12. 04/20 hooks: remove implicit dependency on `the_repository`Patrick Steinhardt, Aug 7, 2024
  13. 05/20 path: stop relying on `the_repository` when reporting garbagePatrick Steinhardt, Aug 7, 2024
  14. 06/20 path: stop relying on `the_repository` in `worktree_git_path()`Patrick Steinhardt, Aug 7, 2024
  15. Justin ToblerAug 9, 2024
  16. Patrick SteinhardtAug 13, 2024
  17. 07/20 path: hide functions using `the_repository` by defaultPatrick Steinhardt, Aug 7, 2024
  18. Justin ToblerAug 9, 2024
  19. Patrick SteinhardtAug 13, 2024
  20. 08/20 config: introduce missing setters that take repo as parameterPatrick Steinhardt, Aug 7, 2024
  21. Justin ToblerAug 9, 2024
  22. Patrick SteinhardtAug 13, 2024
  23. 09/20 config: expose `repo_config_clear()`Patrick Steinhardt, Aug 7, 2024
  24. 10/20 config: pass repo to `git_config_get_index_threads()`Patrick Steinhardt, Aug 7, 2024
  25. 11/20 config: pass repo to `git_config_get_split_index()`Patrick Steinhardt, Aug 7, 2024
  26. 12/20 config: pass repo to `git_config_get_max_percent_split_change()`Patrick Steinhardt, Aug 7, 2024
  27. 13/20 config: pass repo to `git_config_get_expiry()`Patrick Steinhardt, Aug 7, 2024
  28. 14/20 config: pass repo to `git_config_get_expiry_in_days()`Patrick Steinhardt, Aug 7, 2024
  29. Justin ToblerAug 9, 2024
  30. Junio C HamanoAug 9, 2024
  31. 15/20 config: pass repo to `git_die_config()`Patrick Steinhardt, Aug 7, 2024
  32. 16/20 config: pass repo to functions that rename or copy sectionsPatrick Steinhardt, Aug 7, 2024
  33. 17/20 config: don't have setters depend on `the_repository`Patrick Steinhardt, Aug 7, 2024
  34. 18/20 config: don't depend on `the_repository` with branch conditionsPatrick Steinhardt, Aug 7, 2024
  35. Justin ToblerAug 9, 2024
  36. Patrick SteinhardtAug 13, 2024
  37. 19/20 global: prepare for hiding away repo-less config functionsPatrick Steinhardt, Aug 7, 2024
  38. Justin ToblerAug 9, 2024
  39. 20/20 config: hide functions using `the_repository` by defaultPatrick Steinhardt, Aug 7, 2024
  40. Justin ToblerAug 9, 2024
  41. Ghanshyam ThakkarAug 7, 2024
  42. Patrick SteinhardtAug 7, 2024
  43. 00/20 Stop using `the_repository` in "config.c"Patrick Steinhardt, Aug 13, 2024
  44. 01/20 path: expose `do_git_path()` as `repo_git_pathv()`Patrick Steinhardt, Aug 13, 2024
  45. 02/20 path: expose `do_git_common_path()` as `repo_common_pathv()`Patrick Steinhardt, Aug 13, 2024
  46. 03/20 editor: do not rely on `the_repository` for interactive editsPatrick Steinhardt, Aug 13, 2024
  47. 04/20 hooks: remove implicit dependency on `the_repository`Patrick Steinhardt, Aug 13, 2024
  48. 05/20 path: stop relying on `the_repository` when reporting garbagePatrick Steinhardt, Aug 13, 2024
  49. Calvin WanAug 14, 2024
  50. Patrick SteinhardtAug 15, 2024
  51. 06/20 path: stop relying on `the_repository` in `worktree_git_path()`Patrick Steinhardt, Aug 13, 2024
  52. 07/20 path: hide functions using `the_repository` by defaultPatrick Steinhardt, Aug 13, 2024
  53. 08/20 config: introduce missing setters that take repo as parameterPatrick Steinhardt, Aug 13, 2024
  54. 09/20 config: expose `repo_config_clear()`Patrick Steinhardt, Aug 13, 2024
  55. 10/20 config: pass repo to `git_config_get_index_threads()`Patrick Steinhardt, Aug 13, 2024
  56. 11/20 config: pass repo to `git_config_get_split_index()`Patrick Steinhardt, Aug 13, 2024
  57. 12/20 config: pass repo to `git_config_get_max_percent_split_change()`Patrick Steinhardt, Aug 13, 2024
  58. 13/20 config: pass repo to `git_config_get_expiry()`Patrick Steinhardt, Aug 13, 2024
  59. 14/20 config: pass repo to `git_config_get_expiry_in_days()`Patrick Steinhardt, Aug 13, 2024
  60. 15/20 config: pass repo to `git_die_config()`Patrick Steinhardt, Aug 13, 2024
  61. 16/20 config: pass repo to functions that rename or copy sectionsPatrick Steinhardt, Aug 13, 2024
  62. 17/20 config: don't have setters depend on `the_repository`Patrick Steinhardt, Aug 13, 2024
  63. 18/20 config: don't depend on `the_repository` with branch conditionsPatrick Steinhardt, Aug 13, 2024
  64. 19/20 global: prepare for hiding away repo-less config functionsPatrick Steinhardt, Aug 13, 2024
  65. 20/20 config: hide functions using `the_repository` by defaultPatrick Steinhardt, Aug 13, 2024
  66. Junio C HamanoAug 13, 2024
  67. Calvin WanAug 14, 2024
  68. Patrick SteinhardtAug 15, 2024
  69. Justin ToblerAug 15, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.