Skip to content

Commit 3050b6d

Browse files
avargitster
avar
authored and
gitster
committed
repo-settings.c: simplify the setup
Simplify the setup code in repo-settings.c in various ways, making the code shorter, easier to read, and requiring fewer hacks to do the same thing as it did before: Since 7211b9e (repo-settings: consolidate some config settings, 2019-08-13) we have memset() the whole "settings" structure to -1 in prepare_repo_settings(), and subsequently relied on the -1 value. Most of the fields did not need to be initialized to -1, and because we were doing that we had the enum labels "UNTRACKED_CACHE_UNSET" and "FETCH_NEGOTIATION_UNSET" purely to reflect the resulting state created this memset() in prepare_repo_settings(). No other code used or relied on them, more on that below. For the rest most of the subsequent "are we -1, then read xyz" can simply be removed by re-arranging what we read first. E.g. when setting the "index.version" setting we should have first read "feature.experimental", so that it (and "feature.manyfiles") can provide a default for our "index.version". Instead the code setting it, added when "feature.manyFiles"[1] was created, was using the UPDATE_DEFAULT_BOOL() macro added in an earlier commit[2]. That macro is now gone, since it was only needed for this pattern of reading things in the wrong order. This also fixes an (admittedly obscure) logic error where we'd conflate an explicit "-1" value in the config with our own earlier memset() -1. We can also remove the UPDATE_DEFAULT_BOOL() wrapper added in [3]. Using it is redundant to simply using the return value from repo_config_get_bool(), which is non-zero if the provided key exists in the config. Details on edge cases relating to the memset() to -1, continued from "more on that below" above: * UNTRACKED_CACHE_KEEP: In [4] the "unset" and "keep" handling for core.untrackedCache was consolidated. But it while we understand the "keep" value, we don't handle it differently than the case of any other unknown value. So let's retain UNTRACKED_CACHE_KEEP and remove the UNTRACKED_CACHE_UNSET setting (which was always implicitly UNTRACKED_CACHE_KEEP before). We don't need to inform any code after prepare_repo_settings() that the setting was "unset", as far as anyone else is concerned it's core.untrackedCache=keep. if "core.untrackedcache" isn't present in the config. * FETCH_NEGOTIATION_UNSET & FETCH_NEGOTIATION_NONE: Since these two two enum fields added in [5] don't rely on the memzero() setting them to "-1" anymore we don't have to provide them with explicit values. 1. c6cc4c5 (repo-settings: create feature.manyFiles setting, 2019-08-13) 2. 31b1de6 (commit-graph: turn on commit-graph by default, 2019-08-13) 3. 31b1de6 (commit-graph: turn on commit-graph by default, 2019-08-13) 4. ad0fb65 (repo-settings: parse core.untrackedCache, 2019-08-13) 5. aaf633c (repo-settings: create feature.experimental setting, 2019-08-13) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent f1bee82 commit 3050b6d

File tree

4 files changed

+68
-60
lines changed

4 files changed

+68
-60
lines changed

‎fetch-negotiator.c‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,5 @@ void fetch_negotiator_init(struct repository *r,
2121
case FETCH_NEGOTIATION_DEFAULT:
2222
default_negotiator_init(negotiator);
2323
return;
24-
case FETCH_NEGOTIATION_NONE:
25-
case FETCH_NEGOTIATION_UNSET:
26-
BUG("FETCH_NEGOTIATION_{NONE,UNSET} used outside of prepare_repo_settings()!");
2724
}
2825
}

‎read-cache.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,9 +1952,13 @@ static void tweak_untracked_cache(struct index_state *istate)
19521952
add_untracked_cache(istate);
19531953
break;
19541954
case UNTRACKED_CACHE_KEEP:
1955+
/*
1956+
* Either an explicit "core.untrackedCache=keep", the
1957+
* default if "core.untrackedCache" isn't configured,
1958+
* or a fallback on an unknown "core.untrackedCache"
1959+
* value.
1960+
*/
19551961
break;
1956-
case UNTRACKED_CACHE_UNSET:
1957-
BUG("UNTRACKED_CACHE_UNSET used outside of prepare_repo_settings()!");
19581962
}
19591963
}
19601964

‎repo-settings.c‎

Lines changed: 56 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,76 @@
33
#include "repository.h"
44
#include "midx.h"
55

6-
#define UPDATE_DEFAULT_BOOL(s,v) do { if (s == -1) { s = v; } } while(0)
6+
static void repo_cfg_bool(struct repository *r, const char *key, int *dest,
7+
int def)
8+
{
9+
if (repo_config_get_bool(r, key, dest))
10+
*dest = def;
11+
}
712

813
void prepare_repo_settings(struct repository *r)
914
{
15+
int experimental;
1016
int value;
1117
char *strval;
18+
int manyfiles;
1219

13-
if (r->settings.initialized)
20+
if (r->settings.initialized++)
1421
return;
1522

1623
/* Defaults */
17-
memset(&r->settings, -1, sizeof(r->settings));
24+
r->settings.index_version = -1;
25+
r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
26+
r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
27+
28+
/* Booleans config or default, cascades to other settings */
29+
repo_cfg_bool(r, "feature.manyfiles", &manyfiles, 0);
30+
repo_cfg_bool(r, "feature.experimental", &experimental, 0);
1831

19-
if (!repo_config_get_bool(r, "core.commitgraph", &value))
20-
r->settings.core_commit_graph = value;
21-
if (!repo_config_get_bool(r, "commitgraph.readchangedpaths", &value))
22-
r->settings.commit_graph_read_changed_paths = value;
23-
if (!repo_config_get_bool(r, "gc.writecommitgraph", &value))
24-
r->settings.gc_write_commit_graph = value;
25-
UPDATE_DEFAULT_BOOL(r->settings.core_commit_graph, 1);
26-
UPDATE_DEFAULT_BOOL(r->settings.commit_graph_read_changed_paths, 1);
27-
UPDATE_DEFAULT_BOOL(r->settings.gc_write_commit_graph, 1);
32+
/* Defaults modified by feature.* */
33+
if (experimental) {
34+
r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
35+
}
36+
if (manyfiles) {
37+
r->settings.index_version = 4;
38+
r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE;
39+
}
40+
41+
/* Boolean config or default, does not cascade (simple) */
42+
repo_cfg_bool(r, "core.commitgraph", &r->settings.core_commit_graph, 1);
43+
repo_cfg_bool(r, "commitgraph.readchangedpaths", &r->settings.commit_graph_read_changed_paths, 1);
44+
repo_cfg_bool(r, "gc.writecommitgraph", &r->settings.gc_write_commit_graph, 1);
45+
repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0);
46+
repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1);
47+
repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
48+
49+
/*
50+
* The GIT_TEST_MULTI_PACK_INDEX variable is special in that
51+
* either it *or* the config sets
52+
* r->settings.core_multi_pack_index if true. We don't take
53+
* the environment variable if it exists (even if false) over
54+
* any config, as in most other cases.
55+
*/
56+
if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))
57+
r->settings.core_multi_pack_index = 1;
2858

59+
/*
60+
* Non-boolean config
61+
*/
2962
if (!repo_config_get_int(r, "index.version", &value))
3063
r->settings.index_version = value;
31-
if (!repo_config_get_maybe_bool(r, "core.untrackedcache", &value)) {
32-
if (value == 0)
33-
r->settings.core_untracked_cache = UNTRACKED_CACHE_REMOVE;
34-
else
35-
r->settings.core_untracked_cache = UNTRACKED_CACHE_WRITE;
36-
} else if (!repo_config_get_string(r, "core.untrackedcache", &strval)) {
37-
if (!strcasecmp(strval, "keep"))
38-
r->settings.core_untracked_cache = UNTRACKED_CACHE_KEEP;
3964

65+
if (!repo_config_get_string(r, "core.untrackedcache", &strval)) {
66+
int v = git_parse_maybe_bool(strval);
67+
68+
/*
69+
* If it's set to "keep", or some other non-boolean
70+
* value then "v < 0". Then we do nothing and keep it
71+
* at the default of UNTRACKED_CACHE_KEEP.
72+
*/
73+
if (v >= 0)
74+
r->settings.core_untracked_cache = v ?
75+
UNTRACKED_CACHE_WRITE : UNTRACKED_CACHE_REMOVE;
4076
free(strval);
4177
}
4278

@@ -45,34 +81,8 @@ void prepare_repo_settings(struct repository *r)
4581
r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
4682
else if (!strcasecmp(strval, "noop"))
4783
r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_NOOP;
48-
else
49-
r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_DEFAULT;
5084
}
5185

52-
if (!repo_config_get_bool(r, "pack.usesparse", &value))
53-
r->settings.pack_use_sparse = value;
54-
UPDATE_DEFAULT_BOOL(r->settings.pack_use_sparse, 1);
55-
56-
value = git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0);
57-
if (value || !repo_config_get_bool(r, "core.multipackindex", &value))
58-
r->settings.core_multi_pack_index = value;
59-
UPDATE_DEFAULT_BOOL(r->settings.core_multi_pack_index, 1);
60-
61-
if (!repo_config_get_bool(r, "feature.manyfiles", &value) && value) {
62-
UPDATE_DEFAULT_BOOL(r->settings.index_version, 4);
63-
UPDATE_DEFAULT_BOOL(r->settings.core_untracked_cache, UNTRACKED_CACHE_WRITE);
64-
}
65-
66-
if (!repo_config_get_bool(r, "fetch.writecommitgraph", &value))
67-
r->settings.fetch_write_commit_graph = value;
68-
UPDATE_DEFAULT_BOOL(r->settings.fetch_write_commit_graph, 0);
69-
70-
if (!repo_config_get_bool(r, "feature.experimental", &value) && value)
71-
UPDATE_DEFAULT_BOOL(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING);
72-
73-
UPDATE_DEFAULT_BOOL(r->settings.core_untracked_cache, UNTRACKED_CACHE_KEEP);
74-
UPDATE_DEFAULT_BOOL(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_DEFAULT);
75-
7686
/*
7787
* This setting guards all index reads to require a full index
7888
* over a sparse index. After suitable guards are placed in the

‎repository.h‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,15 @@ struct submodule_cache;
1313
struct promisor_remote_config;
1414

1515
enum untracked_cache_setting {
16-
UNTRACKED_CACHE_UNSET = -1,
17-
UNTRACKED_CACHE_REMOVE = 0,
18-
UNTRACKED_CACHE_KEEP = 1,
19-
UNTRACKED_CACHE_WRITE = 2
16+
UNTRACKED_CACHE_KEEP,
17+
UNTRACKED_CACHE_REMOVE,
18+
UNTRACKED_CACHE_WRITE,
2019
};
2120

2221
enum fetch_negotiation_setting {
23-
FETCH_NEGOTIATION_UNSET = -1,
24-
FETCH_NEGOTIATION_NONE = 0,
25-
FETCH_NEGOTIATION_DEFAULT = 1,
26-
FETCH_NEGOTIATION_SKIPPING = 2,
27-
FETCH_NEGOTIATION_NOOP = 3,
22+
FETCH_NEGOTIATION_DEFAULT,
23+
FETCH_NEGOTIATION_SKIPPING,
24+
FETCH_NEGOTIATION_NOOP,
2825
};
2926

3027
struct repo_settings {

0 commit comments

Comments
 (0)