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

Re: [PATCH v2 05/20] path: stop relying on `the_repository` when reporting garbage

From
Calvin Wan <calvinwan@google.com>
Date
Aug 14, 2024, 18:28 UTC
Message-ID
<20240814182850.1273188-1-calvinwan@google.com>
In-Reply-To
<bbaa85ebad458c60c59784b690be8be2ddbe76fc.1723540226.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 34 quoted lines
> We access `the_repository` in `report_linked_checkout_garbage()` both
> directly and indirectly via `get_git_dir()`. Remove this dependency by
> instead passing a `struct repository` as parameter.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/count-objects.c | 2 +-
>  path.c                  | 6 +++---
>  path.h                  | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/path.c b/path.c
> index 069db6ff8f..97a07fafc7 100644
> --- a/path.c
> +++ b/path.c
> @@ -365,15 +365,15 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len,
>  		strbuf_addstr(buf, LOCK_SUFFIX);
>  }
>  
> -void report_linked_checkout_garbage(void)
> +void report_linked_checkout_garbage(struct repository *r)
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  	const struct common_dir *p;
>  	int len;
>  
> -	if (!the_repository->different_commondir)
> +	if (!r->different_commondir)
>  		return;
> -	strbuf_addf(&sb, "%s/", get_git_dir());
> +	strbuf_addf(&sb, "%s/", r->gitdir);
>  	len = sb.len;
>  	for (p = common_list; p->path; p++) {
>  		const char *path = p->path;

Callers have two options here for accessing the gitdir: one is including environment.h and calling get_git_dir(). The other is passing in `struct repository *r` and accessing r->gitdir. It's not entirely clear which should be used in what scenario. Sure with the second option the user has the option of passing something in that isn't `the_repository` but practically speaking that's not happening here and also in the large majority of other scenarios.

I'm OK with this patch for the purposes of the series, but do you think in the future we should introduce get_git_dir(struct repository *r) and change get_git_dir() into get_env_git_dir() that simply calls get_git_dir(the_repository)?

Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 49 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.