On 25/03/03 09:47AM, Patrick Steinhardt wrote:
> 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@xxxxxx>
> ---
[snip]
> 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.
> +
> +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
