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

Re: [PATCH 04/12] environment: move access to "core.bigFileThreshold" into repo settings

From
Justin Tobler <jltobler@gmail.com>
Date
Mar 4, 2025, 19:32 UTC
Message-ID
<rjhdutonrott3a3gpmq6t7qc7qtc4covk7gnp7kqvt3nh2676d@l3wkgwyotv2t>
In-Reply-To
<20250303-b4-pks-objects-without-the-repository-v1-4-c5dd43f2476e@pks.im>
On 25/03/03 09:47AM, Patrick Steinhardt wrote:
Show 12 quoted lines
> The "core.bigFileThreshold" setting is stored in a global variable and
> populated via `git_default_core_config()`. This may cause issues in
> the case where one is handling multiple different repositories in a
> single process with different values for that config key, as we may or
> may not see the correct value in that case. Furthermore, global state
> blocks our path towards libification.
> 
> Refactor the code so that we instead store the value in `struct
> repo_settings`, where the value is computed as-needed and cached.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
Show 29 quoted lines
> diff --git a/repo-settings.c b/repo-settings.c
> index 67e9cfd2e63..4129f8fb2b4 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -20,6 +20,13 @@ static void repo_cfg_int(struct repository *r, const char *key, int *dest,
>  		*dest = def;
>  }
>  
> +static void repo_cfg_ulong(struct repository *r, const char *key, unsigned long *dest,
> +			   unsigned long def)
> +{
> +	if (repo_config_get_ulong(r, key, dest))
> +		*dest = def;
> +}
> +
>  void prepare_repo_settings(struct repository *r)
>  {
>  	int experimental;
> @@ -151,6 +158,19 @@ void repo_settings_clear(struct repository *r)
>  	r->settings = empty;
>  }
>  
> +unsigned long repo_settings_get_big_file_threshold(struct repository *repo)
> +{
> +	if (!repo->settings.big_file_threshold)
> +		repo_cfg_ulong(repo, "core.bigfilethreshold",
> +			       &repo->settings.big_file_threshold, 512 * 1024 * 1024);
> +	return repo->settings.big_file_threshold;
> +}

Ok, if big_file_threshold is not already set, the config is read checking for a value. If there is no explictly configured value, we fallback to the default.

This matches the existing behavior.
Show 60 quoted lines
> +
> +void repo_settings_set_big_file_threshold(struct repository *repo, unsigned long value)
> +{
> +	repo->settings.big_file_threshold = value;
> +}
> +
>  enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo)
>  {
>  	const char *value;
> diff --git a/repo-settings.h b/repo-settings.h
> index ddc11967e01..2bf24b25973 100644
> --- a/repo-settings.h
> +++ b/repo-settings.h
> @@ -64,6 +64,7 @@ struct repo_settings {
>  	size_t delta_base_cache_limit;
>  	size_t packed_git_window_size;
>  	size_t packed_git_limit;
> +	unsigned long big_file_threshold;
>  
>  	char *hooks_path;
>  };
> @@ -88,6 +89,10 @@ int repo_settings_get_warn_ambiguous_refs(struct repository *repo);
>  /* Read the value for "core.hooksPath". */
>  const char *repo_settings_get_hooks_path(struct repository *repo);
>  
> +/* Read and set the value for "core.bigFileThreshold". */
> +unsigned long repo_settings_get_big_file_threshold(struct repository *repo);
> +void repo_settings_set_big_file_threshold(struct repository *repo, unsigned long value);
> +
>  /* Read, set or reset the value for "core.sharedRepository". */
>  int repo_settings_get_shared_repository(struct repository *repo);
>  void repo_settings_set_shared_repository(struct repository *repo, int value);
> diff --git a/streaming.c b/streaming.c
> index 38839511afc..018b794d252 100644
> --- a/streaming.c
> +++ b/streaming.c
> @@ -431,7 +431,8 @@ static int istream_source(struct git_istream *st,
>  		st->open = open_istream_loose;
>  		return 0;
>  	case OI_PACKED:
> -		if (!oi.u.packed.is_delta && big_file_threshold < size) {
> +		if (!oi.u.packed.is_delta &&
> +		    repo_settings_get_big_file_threshold(the_repository) < size) {
>  			st->u.in_pack.pack = oi.u.packed.pack;
>  			st->u.in_pack.pos = oi.u.packed.offset;
>  			st->open = open_istream_pack_non_delta;
> diff --git a/t/t1050-large.sh b/t/t1050-large.sh
> index c71932b0242..5be273611ad 100755
> --- a/t/t1050-large.sh
> +++ b/t/t1050-large.sh
> @@ -6,7 +6,8 @@ test_description='adding and checking out large blobs'
>  . ./test-lib.sh
>  
>  test_expect_success 'core.bigFileThreshold must be non-negative' '
> -	test_must_fail git -c core.bigFileThreshold=-1 rev-parse >out 2>err &&
> +	: >input &&
> +	test_must_fail git -c core.bigFileThreshold=-1 hash-object input >out 2>err &&
>  	grep "bad numeric config value" err &&
>  	test_must_be_empty out
>  '

I assume the test is updated because core.bigFileThreshold is now only computed as-needed. Is that correct?

It may be worth mentioning in the commit message.
-Justin
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 12 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.