"Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> writes:
> The patch extends git config --file interface to allow read config from
> stdin.
Thanks. The external interface proposed by this change that behaves
the way your new test expects is a good addition to the system. I
would describe it as:
Subject: config: teach "git config --file -" to read from the standard input
I however think the patch implements it at the level that is too low
in the callchain. It will affect a lot more than the dash given to
"git config --file -". Fortunately, it does not make it possible
for users to make this mistake
[include]
path = -
and scratch their heads, wondering why "git config" is not answering
until they hit ^D. But that is _only_ because we check if a file
whose name is "-" actually exists in the current directory before
falling into this codepath (and usually no such file exists). If
such a funnily-named file does exist, we read from that file, not
the standard input. So that "include" codepath happens to be safe,
but who knows what dragons lie in other codepaths that call this
function.
I recall that an earlier implementation of "git diff --no-index"
that made "-" read one side to be compared from the standard input
had exactly the same issue of comparing filename with "-", which we
had to fix with code reorganization recently. I'd prefer to see
this update to "git config --file -" done the right way from the
start.
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 967359344dab..f1a63075e34f 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -484,6 +484,10 @@ test_expect_success 'alternative GIT_CONFIG (--file)' '
> test_cmp expect output
> '
>
> +test_expect_success 'alternative GIT_CONFIG (--file=-)' '
> + git config --file - -l < other-config > output &&
Please leave no SP between redirection operator and its file, i.e.
git config --file - --list <other-config >output
Thanks.
--
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