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

[PATCH v7 0/9] packfile: avoid using the 'the_repository' global variable

From
Karthik Nayak <karthik.188@gmail.com>
Date
Nov 11, 2024, 11:14 UTC
Message-ID
<cover.1731323350.git.karthik.188@gmail.com>
In-Reply-To
<cover.1729504640.git.karthik.188@gmail.com>

The `packfile.c` file uses the global variable 'the_repository' extensively throughout the code. Let's remove all usecases of this, by modifying the required functions to accept a 'struct repository' instead. This is to clean up usage of global state.

The first 3 patches are mostly internal to `packfile.c`, we add the repository field to the `packed_git` struct and this is used to clear up some useages of the global variables.

The next 3 patches are more disruptive, they modify the function definition of `odb_pack_name`, `has_object[_kept]_pack` and `for_each_packed_object` to receive a repository, helping remove other usages of 'the_repository' variable.

Finally, the last two patches deal with global config values. These values are localized.

For v5 onwards, I've rebased the series off the new master: 8f8d6eee53 (The seventh batch, 2024-11-01), as a dependency for this series 'jk/dumb-http-finalize' was merged to master. I've found no conflicts while merging with seen & next. But since this series does touch multiple files, there could be future conflicts.

Changes in v7:
- Cleanup stale commit message.
- Add missing space in `if` statement.
- Fix typo s/incase/in case/. 
Changes in v6:
- Lazy load repository settings in packfile.c. This ensures that the settings are
available for sure and we do not rely on callees setting it.
- Use `size_t` for `delta_base_cache_limit`.
Changes in v5:
- Move packed_git* settings to repo_settings to ensure we don't keep reparsing the
settings in `use_pack`.
Changes in v4:
- Renamed the repository field within `packed_git` and `multi_pack_index` from
`r` to `repo`, while keeping function parameters to be `r`.
- Fixed bad braces. 
Changes in v3:
- Improved commit messages. In the first commit to talk about how packed_git
struct could also be part of the alternates of a repository. In the 7th commit
to talk about the motive behind removing the global variable.
- Changed 'packed_git->repo' to 'packed_git->r' to keep it consistent with the
rest of the code base.
- Replaced 'the_repository' with locally available access to the repository
struct in multiple regions.
- Removed unecessary inclusion of the 'repository.h' header file by forward
declaring the 'repository' struct.
- Replace memcpy with hashcpy.
- Change the logic in the 7th patch to use if else statements.
- Added an extra commit to cleanup `pack-bitmap.c`. 
Karthik Nayak (9):
  packfile: add repository to struct `packed_git`
  packfile: use `repository` from `packed_git` directly
  packfile: pass `repository` to static function in the file
  packfile: pass down repository to `odb_pack_name`
  packfile: pass down repository to `has_object[_kept]_pack`
  packfile: pass down repository to `for_each_packed_object`
  config: make `delta_base_cache_limit` a non-global variable
  config: make `packed_git_(limit|window_size)` non-global variables
  midx: add repository to `multi_pack_index` struct
 builtin/cat-file.c       |   7 +-
 builtin/count-objects.c  |   2 +-
 builtin/fast-import.c    |  15 ++--
 builtin/fsck.c           |  20 +++---
 builtin/gc.c             |   8 ++-
 builtin/index-pack.c     |  20 ++++--
 builtin/pack-objects.c   |  11 +--
 builtin/pack-redundant.c |   2 +-
 builtin/repack.c         |   2 +-
 builtin/rev-list.c       |   2 +-
 commit-graph.c           |   4 +-
 config.c                 |  22 ------
 connected.c              |   3 +-
 diff.c                   |   3 +-
 environment.c            |   3 -
 environment.h            |   1 -
 fsck.c                   |   2 +-
 http.c                   |   4 +-
 list-objects.c           |   7 +-
 midx-write.c             |   2 +-
 midx.c                   |   3 +-
 midx.h                   |   3 +
 object-store-ll.h        |   9 ++-
 pack-bitmap.c            |  90 +++++++++++++++---------
 pack-objects.h           |   3 +-
 pack-write.c             |   1 +
 pack.h                   |   1 +
 packfile.c               | 148 +++++++++++++++++++++++----------------
 packfile.h               |  18 +++--
 promisor-remote.c        |   2 +-
 prune-packed.c           |   2 +-
 reachable.c              |   4 +-
 repo-settings.c          |  14 ++++
 repo-settings.h          |   5 ++
 revision.c               |  13 ++--
 tag.c                    |   2 +-
 36 files changed, 268 insertions(+), 190 deletions(-)
Range-diff against v6:
 -:  ---------- >  1:  6c00e25c86 packfile: add repository to struct `packed_git`
 -:  ---------- >  2:  70fc8a79af packfile: use `repository` from `packed_git` directly
 -:  ---------- >  3:  167a1f3a11 packfile: pass `repository` to static function in the file
 -:  ---------- >  4:  b7cfe78217 packfile: pass down repository to `odb_pack_name`
 -:  ---------- >  5:  5566f5554c packfile: pass down repository to `has_object[_kept]_pack`
 -:  ---------- >  6:  1b26e45a9b packfile: pass down repository to `for_each_packed_object`
 1:  89313cfed4 !  7:  1bdc34f4d8 config: make `delta_base_cache_limit` a non-global variable
    @@ builtin/gc.c: static void gc_config(struct gc_config *cfg)
      {
      	const char *value;
      	char *owned = NULL;
    -+	unsigned long longval;
    ++	unsigned long ulongval;
      
      	if (!git_config_get_value("gc.packrefs", &value)) {
      		if (value && !strcmp(value, "notbare"))
    @@ builtin/gc.c: static void gc_config(struct gc_config *cfg)
      	git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold);
      	git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size);
      
    -+	if(!git_config_get_ulong("core.deltabasecachelimit", &longval))
    -+		cfg->delta_base_cache_limit = longval;
    ++	if (!git_config_get_ulong("core.deltabasecachelimit", &ulongval))
    ++		cfg->delta_base_cache_limit = ulongval;
     +
      	if (!git_config_get_string("gc.repackfilter", &owned)) {
      		free(cfg->repack_filter);
 2:  3a8e3b88df !  8:  7b6baa89ac config: make `packed_git_(limit|window_size)` non-global variables
    @@ Commit message
         this file, let's change it from being a global config variable to a
         local variable for the subsystem.
     
    -    We do this by introducing a new local `packfile_config` struct in
    -    `packfile.c` and also adding the required function to parse the said
    -    config. We then use this within `packfile.c` to obtain the variables.
    -
         With this, we rid `packfile.c` from all global variable usage and this
         means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from
         the file.
    @@ packfile.c: unsigned char *use_pack(struct packed_git *p,
      			off_t len;
     +			struct repo_settings *settings;
     +
    -+			/* lazy load the settings incase it hasn't been setup */
    ++			/* lazy load the settings in case it hasn't been setup */
     +			prepare_repo_settings(p->repo);
     +			settings = &p->repo->settings;
     +
 3:  2f9a146978 =  9:  a3667d87ec midx: add repository to `multi_pack_index` struct
-- 
2.47.0
Previous: karthik nayakNext: Karthik Nayak
Message 105 of 171 in “packfile: avoid using the 'the_repository' global variable”
  1. 00/20 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Oct 21, 2024
  2. 01/20 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Oct 21, 2024
  3. Taylor BlauOct 21, 2024
  4. karthik nayakOct 22, 2024
  5. Taylor BlauOct 22, 2024
  6. 02/20 packfile: pass down repository to `unuse_one_window`Karthik Nayak, Oct 21, 2024
  7. Taylor BlauOct 21, 2024
  8. 03/20 packfile: pass down repository to `close_one_pack`Karthik Nayak, Oct 21, 2024
  9. 04/20 packfile: pass down repository to `add_packed_git`Karthik Nayak, Oct 21, 2024
  10. 05/20 packfile: pass down repository to `unpack_object_header`Karthik Nayak, Oct 21, 2024
  11. 06/20 packfile: pass down repository to `get_delta_base`Karthik Nayak, Oct 21, 2024
  12. 07/20 packfile: use provided repository in `packed_object_info`Karthik Nayak, Oct 21, 2024
  13. 08/20 packfile: pass down repository to `unpack_compressed_entry`Karthik Nayak, Oct 21, 2024
  14. 09/20 packfile: pass down repository to `nth_packed_object_id`Karthik Nayak, Oct 21, 2024
  15. 10/20 packfile: pass down repository to `find_pack_entry_one`Karthik Nayak, Oct 21, 2024
  16. 11/20 packfile: pass down repository to `fill_pack_entry`Karthik Nayak, Oct 21, 2024
  17. 12/20 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Oct 21, 2024
  18. 13/20 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Oct 21, 2024
  19. 14/20 packfile: pass down repository to `is_promisor_object`Karthik Nayak, Oct 21, 2024
  20. 15/20 object-store: pass down repository to `each_packed_object_fn`Karthik Nayak, Oct 21, 2024
  21. 16/20 packfile: pass down repository to `open_pack_index`Karthik Nayak, Oct 21, 2024
  22. 17/20 packfile: stop using 'the_hash_algo'Karthik Nayak, Oct 21, 2024
  23. 18/20 packfile: pass down repository to `nth_packed_object_offset`Karthik Nayak, Oct 21, 2024
  24. 19/20 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Oct 21, 2024
  25. 20/20 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Oct 21, 2024
  26. Taylor BlauOct 21, 2024
  27. karthik nayakOct 27, 2024
  28. Taylor BlauOct 27, 2024
  29. Jeff KingOct 28, 2024
  30. karthik nayakOct 28, 2024
  31. Taylor BlauOct 28, 2024
  32. 0/8 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Oct 28, 2024
  33. 1/8 packfile: add repository to struct `packed_git`Karthik Nayak, Oct 28, 2024
  34. Taylor BlauOct 28, 2024
  35. karthik nayakOct 29, 2024
  36. Taylor BlauOct 29, 2024
  37. 2/8 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Oct 28, 2024
  38. Taylor BlauOct 28, 2024
  39. karthik nayakOct 29, 2024
  40. 3/8 packfile: pass `repository` to static function in the fileKarthik Nayak, Oct 28, 2024
  41. Taylor BlauOct 28, 2024
  42. 5/8 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Oct 28, 2024
  43. Taylor BlauOct 28, 2024
  44. karthik nayakOct 29, 2024
  45. 4/8 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Oct 28, 2024
  46. Taylor BlauOct 28, 2024
  47. Jeff KingOct 29, 2024
  48. karthik nayakOct 29, 2024
  49. Taylor BlauOct 29, 2024
  50. 6/8 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Oct 28, 2024
  51. 7/8 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Oct 28, 2024
  52. me@ttaylorr.comOct 28, 2024
  53. karthik nayakOct 29, 2024
  54. 8/8 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Oct 28, 2024
  55. Taylor BlauOct 28, 2024
  56. karthik nayakOct 29, 2024
  57. Taylor BlauOct 29, 2024
  58. 0/9 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Oct 31, 2024
  59. 1/9 packfile: add repository to struct `packed_git`Karthik Nayak, Oct 31, 2024
  60. 2/9 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Oct 31, 2024
  61. 3/9 packfile: pass `repository` to static function in the fileKarthik Nayak, Oct 31, 2024
  62. 4/9 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Oct 31, 2024
  63. 5/9 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Oct 31, 2024
  64. 6/9 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Oct 31, 2024
  65. 7/9 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Oct 31, 2024
  66. 8/9 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Oct 31, 2024
  67. Jeff KingNov 1, 2024
  68. Taylor BlauNov 1, 2024
  69. karthik nayakNov 4, 2024
  70. 9/9 midx: add repository to `multi_pack_index` structKarthik Nayak, Oct 31, 2024
  71. Taylor BlauOct 31, 2024
  72. Taylor BlauNov 1, 2024
  73. karthik nayakNov 1, 2024
  74. Jeff KingNov 1, 2024
  75. karthik nayakNov 4, 2024
  76. Jeff KingNov 4, 2024
  77. 0/9 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Nov 4, 2024
  78. 1/9 packfile: add repository to struct `packed_git`Karthik Nayak, Nov 4, 2024
  79. 2/9 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Nov 4, 2024
  80. 3/9 packfile: pass `repository` to static function in the fileKarthik Nayak, Nov 4, 2024
  81. 4/9 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Nov 4, 2024
  82. 5/9 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Nov 4, 2024
  83. 6/9 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Nov 4, 2024
  84. 7/9 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Nov 4, 2024
  85. 8/9 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Nov 4, 2024
  86. Jeff KingNov 4, 2024
  87. karthik nayakNov 5, 2024
  88. 9/9 midx: add repository to `multi_pack_index` structKarthik Nayak, Nov 4, 2024
  89. Jeff KingNov 4, 2024
  90. karthik nayakNov 5, 2024
  91. 0/9 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Nov 7, 2024
  92. 1/9 packfile: add repository to struct `packed_git`Karthik Nayak, Nov 7, 2024
  93. 2/9 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Nov 7, 2024
  94. 3/9 packfile: pass `repository` to static function in the fileKarthik Nayak, Nov 7, 2024
  95. 4/9 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Nov 7, 2024
  96. 5/9 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Nov 7, 2024
  97. 6/9 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Nov 7, 2024
  98. 7/9 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Nov 7, 2024
  99. 8/9 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Nov 7, 2024
  100. Junio C HamanoNov 8, 2024
  101. karthik nayakNov 8, 2024
  102. 9/9 midx: add repository to `multi_pack_index` structKarthik Nayak, Nov 7, 2024
  103. Junio C HamanoNov 8, 2024
  104. karthik nayakNov 8, 2024
  105. 0/9 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Nov 11, 2024
  106. 1/9 packfile: add repository to struct `packed_git`Karthik Nayak, Nov 11, 2024
  107. Toon ClaesNov 13, 2024
  108. karthik nayakNov 13, 2024
  109. Junio C HamanoNov 13, 2024
  110. karthik nayakNov 14, 2024
  111. Taylor BlauNov 20, 2024
  112. karthik nayakNov 21, 2024
  113. 2/9 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Nov 11, 2024
  114. 3/9 packfile: pass `repository` to static function in the fileKarthik Nayak, Nov 11, 2024
  115. 5/9 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Nov 11, 2024
  116. 4/9 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Nov 11, 2024
  117. 6/9 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Nov 11, 2024
  118. Taylor BlauNov 20, 2024
  119. packfile.c: remove unnecessary prepare_packed_git() callTaylor Blau, Nov 20, 2024
  120. Jeff KingNov 21, 2024
  121. 7/9 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Nov 11, 2024
  122. Taylor BlauNov 20, 2024
  123. Jeff KingNov 21, 2024
  124. karthik nayakNov 21, 2024
  125. 8/9 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Nov 11, 2024
  126. 9/9 midx: add repository to `multi_pack_index` structKarthik Nayak, Nov 11, 2024
  127. Jeff KingNov 12, 2024
  128. karthik nayakNov 13, 2024
  129. Taylor BlauNov 20, 2024
  130. karthik nayakNov 21, 2024
  131. 00/10 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Nov 22, 2024
  132. 01/10 packfile: add repository to struct `packed_git`Karthik Nayak, Nov 22, 2024
  133. 03/10 packfile: pass `repository` to static function in the fileKarthik Nayak, Nov 22, 2024
  134. 02/10 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Nov 22, 2024
  135. 04/10 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Nov 22, 2024
  136. 05/10 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Nov 22, 2024
  137. 06/10 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Nov 22, 2024
  138. 07/10 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Nov 22, 2024
  139. Junio C HamanoNov 26, 2024
  140. karthik nayakNov 26, 2024
  141. 08/10 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Nov 22, 2024
  142. 09/10 midx: add repository to `multi_pack_index` structKarthik Nayak, Nov 22, 2024
  143. 10/10 packfile.c: remove unnecessary prepare_packed_git() callKarthik Nayak, Nov 22, 2024
  144. 00/10 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Nov 26, 2024
  145. 01/10 packfile: add repository to struct `packed_git`Karthik Nayak, Nov 26, 2024
  146. Kristoffer HaugsbakkNov 27, 2024
  147. karthik nayakNov 27, 2024
  148. 02/10 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Nov 26, 2024
  149. 03/10 packfile: pass `repository` to static function in the fileKarthik Nayak, Nov 26, 2024
  150. Kristoffer HaugsbakkNov 27, 2024
  151. karthik nayakNov 27, 2024
  152. 04/10 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Nov 26, 2024
  153. 05/10 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Nov 26, 2024
  154. 06/10 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Nov 26, 2024
  155. 08/10 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Nov 26, 2024
  156. 07/10 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Nov 26, 2024
  157. 09/10 midx: add repository to `multi_pack_index` structKarthik Nayak, Nov 26, 2024
  158. 10/10 packfile.c: remove unnecessary prepare_packed_git() callKarthik Nayak, Nov 26, 2024
  159. 00/10 packfile: avoid using the 'the_repository' global variableKarthik Nayak, Dec 3, 2024
  160. 01/10 packfile: add repository to struct `packed_git`Karthik Nayak, Dec 3, 2024
  161. 02/10 packfile: use `repository` from `packed_git` directlyKarthik Nayak, Dec 3, 2024
  162. 03/10 packfile: pass `repository` to static function in the fileKarthik Nayak, Dec 3, 2024
  163. 04/10 packfile: pass down repository to `odb_pack_name`Karthik Nayak, Dec 3, 2024
  164. 05/10 packfile: pass down repository to `has_object[_kept]_pack`Karthik Nayak, Dec 3, 2024
  165. 06/10 packfile: pass down repository to `for_each_packed_object`Karthik Nayak, Dec 3, 2024
  166. 07/10 config: make `delta_base_cache_limit` a non-global variableKarthik Nayak, Dec 3, 2024
  167. 08/10 config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak, Dec 3, 2024
  168. 10/10 packfile.c: remove unnecessary prepare_packed_git() callKarthik Nayak, Dec 3, 2024
  169. 09/10 midx: add repository to `multi_pack_index` structKarthik Nayak, Dec 3, 2024
  170. Kristoffer HaugsbakkDec 3, 2024
  171. Junio C HamanoDec 3, 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.