| Message ID | 20250306143629.1267358-2-usmanakinyemi202@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | stop using the_repository global variable. | expand |
Usman Akinyemi <usmanakinyemi202@gmail.com> writes: > void repo_config(struct repository *repo, config_fn_t fn, void *data) > { > + if (!repo) { > + read_very_early_config(fn, data); > + return; > + } > git_config_check_init(repo); > configset_iter(repo->config, fn, data); > } > diff --git a/config.h b/config.h > index 5c730c4f89..1e5b22dfc4 100644 > --- a/config.h > +++ b/config.h > @@ -219,6 +219,9 @@ void read_very_early_config(config_fn_t cb, void *data); > * repo-specific one; by overwriting, the higher-priority repo-specific > * value is left at the end). > * > + * In cases where the repository variable is NULL, repo_config() will > + * call read_early_config(). > + * early or very early? I am wondering if we should describe the effect we want out of the design more prominently than the way we try to obtain the effect here. In other words, instead of (rather, in addition to) saying that we call helper X, wouldn't it be more helpful to future developers why we call X, to convey the intent, so that they know how to adjust when for example what X does change or X even disappears? E.g., When repo==NULL, skip reading the per-repository configuration file but still use the system- and globa- configuration, by calling X. Note that this ignores one-time configuration override "git -c var=val" given from the command line. The only use case the feature to allow passing repo==NULL was designed for is to support handling "git foo -h" (which lets git.c:run_builtin() to pass NULL and have the cmd_foo() call repo_config() before calling parse_options() to notice "-h", give help and exit) for a command that ordinarily require a repository, so this limitation may be OK (but if needed you are welcome to fix it). That way, folks who are planning to update read_veriy_early_config() so that it pays attention to the "git -c var=val" in the future will be rest assured that they won't be breaking this caller with their planned change. Of course I didn't spend enough brainpower to make the above comment more concise and to the point, which the final version should be, but hopefully you got the idea. Thanks.
diff --git a/config.c b/config.c index 36f76fafe5..c5181fd23b 100644 --- a/config.c +++ b/config.c @@ -2526,6 +2526,10 @@ void repo_config_clear(struct repository *repo) void repo_config(struct repository *repo, config_fn_t fn, void *data) { + if (!repo) { + read_very_early_config(fn, data); + return; + } git_config_check_init(repo); configset_iter(repo->config, fn, data); } diff --git a/config.h b/config.h index 5c730c4f89..1e5b22dfc4 100644 --- a/config.h +++ b/config.h @@ -219,6 +219,9 @@ void read_very_early_config(config_fn_t cb, void *data); * repo-specific one; by overwriting, the higher-priority repo-specific * value is left at the end). * + * In cases where the repository variable is NULL, repo_config() will + * call read_early_config(). + * * Unlike git_config_from_file(), this function respects includes. */ void repo_config(struct repository *r, config_fn_t fn, void *);
The `repo` value can be NULL if a builtin command is run outside any repository. The current implementation of `repo_config()` will fail if `repo` is NULL. If the `repo` is NULL the `repo_config()` can ignore the repository configuration but it should read the other configuration sources like the system-side configuration instead of failing. Teach the `repo_config()` to allow `repo` to be NULL by calling the `read_very_early_config()` which read config but only enumerate system and global settings. This will be useful in the following commits. Suggested-by: Junio C Hamano <gitster@pobox.com> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> --- config.c | 4 ++++ config.h | 3 +++ 2 files changed, 7 insertions(+)