From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: "Kirill A. Shutemov" <kirill@shutemov•name>,
git@vger•kernel.org, Eric Sunshine <sunshine@sunshineco•com>
Subject: Re: [PATCH] config: teach "git config --file -" to read from the standard input
Date: Tue, 18 Feb 2014 15:02:46 -0800 [thread overview]
Message-ID: <xmqqy518ausp.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: xmqq7g8sfd19.fsf@gitster.dls.corp.google.com
Junio C Hamano <gitster@pobox•com> 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
prev parent reply other threads:[~2014-02-18 23:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-14 23:37 [PATCH 1/3] builtin/config.c: rename check_blob_write() -> check_write() Kirill A. Shutemov
2014-02-14 23:37 ` [PATCH 2/3] config: change git_config_with_options() interface Kirill A. Shutemov
2014-02-14 23:37 ` [PATCH 3/3] config: teach "git config --file -" to read from the standard input Kirill A. Shutemov
2014-02-16 2:12 ` Eric Sunshine
2014-02-16 12:13 ` [PATCH] " Kirill A. Shutemov
2014-02-18 8:41 ` Jeff King
2014-02-18 19:15 ` Junio C Hamano
2014-02-18 23:02 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy518ausp.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=kirill@shutemov$(echo .)name \
--cc=peff@peff$(echo .)net \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox