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

Re: [PATCH 12/12] hash: stop depending on `the_repository` in `null_oid()`

From
Justin Tobler <jltobler@gmail.com>
Date
Mar 4, 2025, 20:16 UTC
Message-ID
<yl7t44tp5zhygppxrusdpiehjydncv3b6bmxhp2hgzk6xjwvbr@cjpzvpwo4vxk>
In-Reply-To
<20250303-b4-pks-objects-without-the-repository-v1-12-c5dd43f2476e@pks.im>
On 25/03/03 09:47AM, Patrick Steinhardt wrote:
Show 36 quoted lines
> The `null_oid()` function returns the object ID that only consists of
> zeroes. Naturally, this ID also depends on the hash algorithm used, as
> the number of zeroes is different between SHA1 and SHA256. Consequently,
> the function returns the hash-algorithm-specific null object ID.
> 
> This is currently done by depending on `the_hash_algo`, which implicitly
> makes us depend on `the_repository`. Refactor the function to instead
> pass in the hash algorithm for which we want to retrieve the null object
> ID. Adapt callsites accordingly by passing in `the_repository`, thus
> bubbling up the dependency on that global variable by one layer.
> 
> There are a couple of trivial exceptions for subsystems that already got
> rid of `the_repository`. These subsystems instead use the repository
> that is available via the calling context:
> 
>   - "builtin/grep.c"
>   - "grep.c"
>   - "refs/debug.c"
> 
> There is also a single non-trivial exception with "diff-no-index.c".
> Here we know that we may not have a repository initialized at all, so we
> cannot rely on `the_repository`. Instead, we adapt `diff_no_index()` to
> get a `struct git_hash_algo` as parameter. The only caller is located in
> "builtin/diff.c", where we know to call `repo_set_hash_algo()` in case
> we're running outside of a Git repository. Consequently, it is fine to
> continue passing `the_repository->hash_algo` even in this case.
> 
> This means that we could in theory just not bother about this edge case
> at all and just use `the_repository` in "diff-no-index.c". But doing so
> would feel misdesigned.
> 
> Remove the `USE_THE_REPOSITORY_VARIABLE` preprocessor define in
> "hash.c".
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
Show 5 quoted lines
> -int diff_no_index(struct rev_info *revs,
> -		  int implicit_no_index,
> -		  int argc, const char **argv)
> +int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,
> +		  int implicit_no_index, int argc, const char **argv)

As mentioned in the commit message, diff_no_index() may not have a repository initialized, so the git_hash_algo should be explicitly passed.

Makes sense.
Show 12 quoted lines
>  {
>  	int i, no_index;
>  	int ret = 1;
> @@ -354,7 +354,7 @@ int diff_no_index(struct rev_info *revs,
>  	setup_diff_pager(&revs->diffopt);
>  	revs->diffopt.flags.exit_with_status = 1;
>  
> -	if (queue_diff(&revs->diffopt, paths[0], paths[1], 0))
> +	if (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0))
>  		goto out;
>  	diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
>  	diffcore_std(&revs->diffopt);
[snip]

The other changes in this patch largely consist of just updating null_oid() and its call sites to recieve the git_hash_algo explicitly.

Looks good to me.
-Justin
Previous: Patrick SteinhardtNext: Karthik Nayak
Message 23 of 73 in “Stop depending on `the_repository` in object-related subsystems”
  1. 00/12 Stop depending on `the_repository` in object-related subsystemsPatrick Steinhardt, Mar 3, 2025
  2. 01/12 csum-file: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  3. Karthik NayakMar 6, 2025
  4. 03/12 pack-write: stop depending on `the_repository` and `the_hash_algo`Patrick Steinhardt, Mar 3, 2025
  5. Justin ToblerMar 4, 2025
  6. 02/12 object: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  7. Karthik NayakMar 6, 2025
  8. Patrick SteinhardtMar 6, 2025
  9. 05/12 pack-check: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  10. Karthik NayakMar 6, 2025
  11. 04/12 environment: move access to "core.bigFileThreshold" into repo settingsPatrick Steinhardt, Mar 3, 2025
  12. Justin ToblerMar 4, 2025
  13. Patrick SteinhardtMar 6, 2025
  14. 06/12 pack-revindex: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  15. 07/12 pack-bitmap-write: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  16. 08/12 object-file-convert: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  17. Justin ToblerMar 4, 2025
  18. 09/12 delta-islands: stop depending on `the_repository`Patrick Steinhardt, Mar 3, 2025
  19. Justin ToblerMar 4, 2025
  20. 10/12 object-file: split out logic regarding hash algorithmsPatrick Steinhardt, Mar 3, 2025
  21. 11/12 hash: fix "-Wsign-compare" warningsPatrick Steinhardt, Mar 3, 2025
  22. 12/12 hash: stop depending on `the_repository` in `null_oid()`Patrick Steinhardt, Mar 3, 2025
  23. Justin ToblerMar 4, 2025
  24. Karthik NayakMar 6, 2025
  25. 00/12 Stop depending on `the_repository` in object-related subsystemsPatrick Steinhardt, Mar 6, 2025
  26. 01/12 csum-file: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  27. 02/12 object: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  28. 03/12 pack-write: stop depending on `the_repository` and `the_hash_algo`Patrick Steinhardt, Mar 6, 2025
  29. 04/12 environment: move access to "core.bigFileThreshold" into repo settingsPatrick Steinhardt, Mar 6, 2025
  30. 05/12 pack-check: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  31. 06/12 pack-revindex: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  32. 07/12 pack-bitmap-write: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  33. 08/12 object-file-convert: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  34. 09/12 delta-islands: stop depending on `the_repository`Patrick Steinhardt, Mar 6, 2025
  35. 11/12 hash: fix "-Wsign-compare" warningsPatrick Steinhardt, Mar 6, 2025
  36. 10/12 object-file: split out logic regarding hash algorithmsPatrick Steinhardt, Mar 6, 2025
  37. 12/12 hash: stop depending on `the_repository` in `null_oid()`Patrick Steinhardt, Mar 6, 2025
  38. Junio C HamanoMar 6, 2025
  39. Patrick SteinhardtMar 7, 2025
  40. Junio C HamanoMar 7, 2025
  41. Karthik NayakMar 6, 2025
  42. 00/12 Stop depending on `the_repository` in object-related subsystemsPatrick Steinhardt, Mar 7, 2025
  43. 02/12 object: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  44. 01/12 csum-file: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  45. 03/12 pack-write: stop depending on `the_repository` and `the_hash_algo`Patrick Steinhardt, Mar 7, 2025
  46. 05/12 pack-check: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  47. 04/12 environment: move access to "core.bigFileThreshold" into repo settingsPatrick Steinhardt, Mar 7, 2025
  48. 06/12 pack-revindex: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  49. 07/12 pack-bitmap-write: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  50. 08/12 object-file-convert: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  51. 09/12 delta-islands: stop depending on `the_repository`Patrick Steinhardt, Mar 7, 2025
  52. 10/12 object-file: split out logic regarding hash algorithmsPatrick Steinhardt, Mar 7, 2025
  53. 11/12 hash: fix "-Wsign-compare" warningsPatrick Steinhardt, Mar 7, 2025
  54. 12/12 hash: stop depending on `the_repository` in `null_oid()`Patrick Steinhardt, Mar 7, 2025
  55. Elijah NewrenMar 8, 2025
  56. Patrick SteinhardtMar 10, 2025
  57. Elijah NewrenMar 10, 2025
  58. Junio C HamanoMar 10, 2025
  59. Elijah NewrenMar 8, 2025
  60. 00/12 Stop depending on `the_repository` in object-related subsystemsPatrick Steinhardt, Mar 10, 2025
  61. 01/12 csum-file: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  62. 02/12 object: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  63. 03/12 pack-write: stop depending on `the_repository` and `the_hash_algo`Patrick Steinhardt, Mar 10, 2025
  64. 04/12 environment: move access to "core.bigFileThreshold" into repo settingsPatrick Steinhardt, Mar 10, 2025
  65. 05/12 pack-check: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  66. 06/12 pack-revindex: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  67. 07/12 pack-bitmap-write: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  68. 08/12 object-file-convert: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  69. 09/12 delta-islands: stop depending on `the_repository`Patrick Steinhardt, Mar 10, 2025
  70. 11/12 hash: fix "-Wsign-compare" warningsPatrick Steinhardt, Mar 10, 2025
  71. 10/12 object-file: split out logic regarding hash algorithmsPatrick Steinhardt, Mar 10, 2025
  72. 12/12 hash: stop depending on `the_repository` in `null_oid()`Patrick Steinhardt, Mar 10, 2025
  73. Elijah NewrenMar 10, 2025

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.