Junio C Hamano <gitster@xxxxxxxxx> writes: >> I think I preferred the earlier version where you had "<stdin>" in the >> name field, and this hunk could just go away. I know you switched it to >> NULL here to avoid making bogus relative filenames in includes. > > Exactly the same comment here. I really like the way how this > series becomes more polished every time we see it. > >> But that would be pretty straightforward to fix by splitting the "name" >> field into an additional "path" field. It looks like "git config --blob" >> has the same problem (it will try relative lookups in the filesystem, >> even though that is nonsensical from the blob). So we could fix the >> existing bug at the same time. :) >> >> Perhaps this could go at the start of your series? > > Sounds like the right thing to do. > > Thanks. And [PATCH 3/3] rebased on the others may look like this. builtin/config.c | 11 +++++++++++ cache.h | 1 + config.c | 42 ++++++++++++++++++++++++++++-------------- t/t1300-repo-config.sh | 17 +++++++++++++++-- t/t1305-config-include.sh | 16 +++++++++++++++- 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index de41ba5..5677c94 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -360,6 +360,9 @@ static int get_colorbool(int print) static void check_write(void) { + if (given_config_source.use_stdin) + die("writing to stdin is not supported"); + if (given_config_source.blob) die("writing config blobs is not supported"); } @@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + if (given_config_source.file && + !strcmp(given_config_source.file, "-")) { + given_config_source.file = NULL; + given_config_source.use_stdin = 1; + } + if (use_global_config) { char *user_config = NULL; char *xdg_config = NULL; @@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (!given_config_source.file && nongit) die("not in a git directory"); + if (given_config_source.use_stdin) + die("editing stdin is not supported"); if (given_config_source.blob) die("editing blobs is not supported"); git_config(git_default_config, NULL); diff --git a/cache.h b/cache.h index 9d94bd6..4db19b5 100644 --- a/cache.h +++ b/cache.h @@ -1147,6 +1147,7 @@ extern int update_server_info(int); #define CONFIG_GENERIC_ERROR 7 struct git_config_source { + unsigned int use_stdin:1; const char *file; const char *blob; }; diff --git a/config.c b/config.c index a21b0dd..9dd0bdb 100644 --- a/config.c +++ b/config.c @@ -1031,24 +1031,36 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data) return ret; } -int git_config_from_file(config_fn_t fn, const char *filename, void *data) +static int do_config_from_file(config_fn_t fn, + const char *filename, FILE *f, + const char *label, void *data) { - int ret; - FILE *f = fopen(filename, "r"); + struct config_source top; - ret = -1; - if (f) { - struct config_source top; + top.u.file = f; + top.name = label; + top.path = filename; + top.die_on_error = 1; + top.do_fgetc = config_file_fgetc; + top.do_ungetc = config_file_ungetc; + top.do_ftell = config_file_ftell; + + return do_config_from(&top, fn, data); +} - top.u.file = f; - top.name = top.path = filename; - top.die_on_error = 1; - top.do_fgetc = config_file_fgetc; - top.do_ungetc = config_file_ungetc; - top.do_ftell = config_file_ftell; +static int git_config_from_stdin(config_fn_t fn, void *data) +{ + return do_config_from_file(fn, NULL, stdin, "<stdin>", data); +} - ret = do_config_from(&top, fn, data); +int git_config_from_file(config_fn_t fn, const char *filename, void *data) +{ + int ret = -1; + FILE *f; + f = fopen(filename, "r"); + if (f) { + ret = do_config_from_file(fn, filename, f, filename, data); fclose(f); } return ret; @@ -1190,7 +1202,9 @@ int git_config_with_options(config_fn_t fn, void *data, * If we have a specific filename, use it. Otherwise, follow the * regular lookup sequence. */ - if (config_source && config_source->file) + if (config_source && config_source->use_stdin) + return git_config_from_stdin(fn, data); + else if (config_source && config_source->file) return git_config_from_file(fn, config_source->file, data); else if (config_source && config_source->blob) return git_config_from_blob_ref(fn, config_source->blob, data); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 9673593..c9c426c 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -475,15 +475,28 @@ ein.bahn=strasse EOF test_expect_success 'alternative GIT_CONFIG' ' - GIT_CONFIG=other-config git config -l >output && + GIT_CONFIG=other-config git config --list >output && test_cmp expect output ' test_expect_success 'alternative GIT_CONFIG (--file)' ' - git config --file other-config -l > output && + git config --file other-config --list >output && test_cmp expect output ' +test_expect_success 'alternative GIT_CONFIG (--file=-)' ' + git config --file - --list <other-config >output && + test_cmp expect output +' + +test_expect_success 'setting a value in stdin is an error' ' + test_must_fail git config --file - some.value foo +' + +test_expect_success 'editing stdin is an error' ' + test_must_fail git config --file - --edit +' + test_expect_success 'refer config from subdirectory' ' mkdir x && ( diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 6edd38b..9ba2ba1 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' ' test_expect_success 'absolute includes from command line work' ' echo "[test]one = 1" >one && echo 1 >expect && - git -c include.path="$PWD/one" config test.one >actual && + git -c include.path="$(pwd)/one" config test.one >actual && test_cmp expect actual ' @@ -138,6 +138,20 @@ test_expect_success 'relative includes from blobs fail' ' test_must_fail git config --blob=$blob test.one ' +test_expect_success 'absolute includes from stdin work' ' + echo "[test]one = 1" >one && + echo 1 >expect && + echo "[include]path=\"$(pwd)/one\"" | + git config --file - test.one >actual && + test_cmp expect actual +' + +test_expect_success 'relative includes from stdin line fail' ' + echo "[test]one = 1" >one && + echo "[include]path=one" | + test_must_fail git config --file - test.one +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html