Hi Glen, On Thu, 13 Oct 2022, Glen Choo via GitGitGadget wrote: > From: Glen Choo <chooglen@xxxxxxxxxx> > > Protected config is implemented by reading a fixed set of paths, > which ignores config [include]-s. Replace this implementation with a > call to config_with_options(), which handles [include]-s and saves us > from duplicating the logic of 1) identifying which paths to read and 2) > reading command line config. After reading the patch, I agree that this is the right thing to do. Thank you, Dscho > > As a result, git_configset_add_parameters() is unused, so remove it. It > was introduced alongside protected config in 5b3c650777 (config: learn > `git_protected_config()`, 2022-07-14) as a way to handle command line > config. > > Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx> > --- > [2.38 regression] config: respect includes in protected config > > Thanks for the quick response, all :) > > Changes in v2: > > * Squash patches together (per Junio's comments) > * Use <<-\EOF (per Ævar's comments) > > Cc: Derrick Stolee derrickstolee@xxxxxxxxxx Cc: Taylor Blau > me@xxxxxxxxxxxx Cc: Johannes Schindelin Johannes.Schindelin@xxxxxx Cc: > Junio C Hamano gitster@xxxxxxxxx Cc: Ævar Arnfjörð Bjarmason > avarab@xxxxxxxxx > > [1] > https://lore.kernel.org/git/CAPWNY8W_Tr-WoD-GXBddD5Y8w5Y4w+gDNYQdOAJ1uBwVHuRrsQ@xxxxxxxxxxxxxx > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1360%2Fchooglen%2Fprotected-config%2Frespect-includes-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1360/chooglen/protected-config/respect-includes-v2 > Pull-Request: https://github.com/git/git/pull/1360 > > Range-diff vs v1: > > 1: 8c0f40aed7e < -: ----------- t0033, t0035: test for included config > 2: 0ff5b5741a5 ! 1: 5c398a7f72a config: respect includes in protected config > @@ t/t0033-safe-directory.sh: test_expect_success 'safe.directory=*, but is reset' > expect_rejected_dir > ' > > --test_expect_failure 'safe.directory in included file' ' > +test_expect_success 'safe.directory in included file' ' > - cat >gitconfig-include <<-EOF && > - [safe] > - directory = "$(pwd)" > ++ cat >gitconfig-include <<-EOF && > ++ [safe] > ++ directory = "$(pwd)" > ++ EOF > ++ git config --global --add include.path "$(pwd)/gitconfig-include" && > ++ git status > ++' > ++ > + test_done > > ## t/t0035-safe-bare-repository.sh ## > @@ t/t0035-safe-bare-repository.sh: test_expect_success 'safe.bareRepository on the command line' ' > -c safe.bareRepository=all > ' > > --test_expect_failure 'safe.bareRepository in included file' ' > +test_expect_success 'safe.bareRepository in included file' ' > - cat >gitconfig-include <<-EOF && > - [safe] > - bareRepository = explicit > ++ cat >gitconfig-include <<-\EOF && > ++ [safe] > ++ bareRepository = explicit > ++ EOF > ++ git config --global --add include.path "$(pwd)/gitconfig-include" && > ++ expect_rejected -C outer-repo/bare-repo > ++' > ++ > + test_done > > > config.c | 30 ++++++++---------------------- > t/t0033-safe-directory.sh | 9 +++++++++ > t/t0035-safe-bare-repository.sh | 9 +++++++++ > 3 files changed, 26 insertions(+), 22 deletions(-) > > diff --git a/config.c b/config.c > index cbb5a3bab74..c157fb5ae3f 100644 > --- a/config.c > +++ b/config.c > @@ -2392,11 +2392,6 @@ int git_configset_add_file(struct config_set *cs, const char *filename) > return git_config_from_file(config_set_callback, filename, cs); > } > > -int git_configset_add_parameters(struct config_set *cs) > -{ > - return git_config_from_parameters(config_set_callback, cs); > -} > - > int git_configset_get_value(struct config_set *cs, const char *key, const char **value) > { > const struct string_list *values = NULL; > @@ -2641,24 +2636,15 @@ int repo_config_get_pathname(struct repository *repo, > /* Read values into protected_config. */ > static void read_protected_config(void) > { > - char *xdg_config = NULL, *user_config = NULL, *system_config = NULL; > - > + struct config_options opts = { > + .respect_includes = 1, > + .ignore_repo = 1, > + .ignore_worktree = 1, > + .system_gently = 1, > + }; > git_configset_init(&protected_config); > - > - system_config = git_system_config(); > - git_global_config(&user_config, &xdg_config); > - > - if (system_config) > - git_configset_add_file(&protected_config, system_config); > - if (xdg_config) > - git_configset_add_file(&protected_config, xdg_config); > - if (user_config) > - git_configset_add_file(&protected_config, user_config); > - git_configset_add_parameters(&protected_config); > - > - free(system_config); > - free(xdg_config); > - free(user_config); > + config_with_options(config_set_callback, &protected_config, > + NULL, &opts); > } > > void git_protected_config(config_fn_t fn, void *data) > diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh > index aecb308cf66..dc3496897ab 100755 > --- a/t/t0033-safe-directory.sh > +++ b/t/t0033-safe-directory.sh > @@ -71,4 +71,13 @@ test_expect_success 'safe.directory=*, but is reset' ' > expect_rejected_dir > ' > > +test_expect_success 'safe.directory in included file' ' > + cat >gitconfig-include <<-EOF && > + [safe] > + directory = "$(pwd)" > + EOF > + git config --global --add include.path "$(pwd)/gitconfig-include" && > + git status > +' > + > test_done > diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh > index ecbdc8238db..11c15a48aab 100755 > --- a/t/t0035-safe-bare-repository.sh > +++ b/t/t0035-safe-bare-repository.sh > @@ -51,4 +51,13 @@ test_expect_success 'safe.bareRepository on the command line' ' > -c safe.bareRepository=all > ' > > +test_expect_success 'safe.bareRepository in included file' ' > + cat >gitconfig-include <<-\EOF && > + [safe] > + bareRepository = explicit > + EOF > + git config --global --add include.path "$(pwd)/gitconfig-include" && > + expect_rejected -C outer-repo/bare-repo > +' > + > test_done > > base-commit: 3dcec76d9df911ed8321007b1d197c1a206dc164 > -- > gitgitgadget >
