# [PATCH] Fix wrong failures in config test 11 messages from 2011-01-10 to 2011-01-10. Participants: Ingo Br ückl, Jonathan Nieder, Junio C Hamano. Thread: https://gitlist.dev/t/26252 ## Ingo Br ückl, 2011-01-10 16:13 Subject: [PATCH] Fix wrong failures in config test Message-ID: <4d2b3198.674034bb.bm000@wupperonline.de> URL: https://gitlist.dev/e/4d2b3198.674034bb.bm000%40wupperonline.de ``` The tests after '--set in alternative GIT_CONFIG' failed because variable GIT_CONFIG was still set. Signed-off-by: Ingo Brückl --- Is it only me (bash 3.2.48(1)-release) experiencing these failures? t/t1300-repo-config.sh | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index d0e5546..d1c9a8f 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -428,6 +428,8 @@ EOF test_expect_success '--set in alternative GIT_CONFIG' 'cmp other-config expect' +unset GIT_CONFIG + cat > .git/config << EOF # Hallo #Bello -- 1.7.3.5 ``` ## Jonathan Nieder, 2011-01-10 16:52 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <20110110165251.GB7714@burratino> URL: https://gitlist.dev/e/20110110165251.GB7714%40burratino In-Reply-To: <4d2b3198.674034bb.bm000@wupperonline.de> ``` Ingo Brückl wrote: > The tests after '--set in alternative GIT_CONFIG' failed because > variable GIT_CONFIG was still set. [...] > Is it only me (bash 3.2.48(1)-release) experiencing these failures? Could you explain the nature of the failures in more detail? What version of git are you building? Is GIT_CONFIG already set in the environment before you run t1300-repo-config.sh (it shouldn't matter)? How does output from "sh t1300-repo-config.sh -v -i" end? If that doesn't end up being helpful, how about "GIT_TRACE=1 sh -x t1300-repo-config.sh -v -i"? Regards, Jonathan ``` ## Ingo Br ückl, 2011-01-10 17:15 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <4d2b3ecd.7c62abb5.bm000@wupperonline.de> URL: https://gitlist.dev/e/4d2b3ecd.7c62abb5.bm000%40wupperonline.de In-Reply-To: <20110110165251.GB7714@burratino> ``` Jonathan Nieder wrote on Mon, 10 Jan 2011 10:52:51 -0600: > Could you explain the nature of the failures in more detail? Yeah, sorry. > What version of git are you building? 1.7.3.5 > Is GIT_CONFIG already set in the environment before you run > t1300-repo-config.sh (it shouldn't matter)? No. > How does output from "sh t1300-repo-config.sh -v -i" end? expecting success: git config --rename-section branch.eins branch.zwei fatal: No such section! not ok - 50 rename section # git config --rename-section branch.eins branch.zwei The problem is that the last 'git config' worked on other-config due to variable GIT_CONFIG. Test 50 should work now on .git/config, but as GIT_CONFIG is still set, it doesn't and fails. Ingo ``` ## Jonathan Nieder, 2011-01-10 17:29 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <20110110172931.GB7882@burratino> URL: https://gitlist.dev/e/20110110172931.GB7882%40burratino In-Reply-To: <4d2b3ecd.7c62abb5.bm000@wupperonline.de> ``` Ingo Brückl wrote: > Jonathan Nieder wrote on Mon, 10 Jan 2011 10:52:51 -0600: >> How does output from "sh t1300-repo-config.sh -v -i" end? > > expecting success: git config --rename-section branch.eins branch.zwei > fatal: No such section! > not ok - 50 rename section > # git config --rename-section branch.eins branch.zwei Thanks. > The problem is that the last 'git config' worked on other-config due to > variable GIT_CONFIG. I'm still missing something. GIT_CONFIG=other-config git config -l > output echo $GIT_CONFIG should result in no output to stdout, right? In other words, the construct envvar=value git command is not supposed to pollute the current environment. It sounds like you've checked that "unset GIT_CONFIG" fixes it; what's left is to explain why GIT_CONFIG had a value in the first place. Confused, Jonathan ``` ## Junio C Hamano, 2011-01-10 18:30 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <7vhbdg6286.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhbdg6286.fsf%40alter.siamese.dyndns.org In-Reply-To: <4d2b3198.674034bb.bm000@wupperonline.de> ``` Ingo Brückl writes: > The tests after '--set in alternative GIT_CONFIG' failed because > variable GIT_CONFIG was still set. > > Signed-off-by: Ingo Brückl > --- > > Is it only me (bash 3.2.48(1)-release) experiencing these failures? > > t/t1300-repo-config.sh | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) t1300 first sources test-lib.sh that explicitly unsets GIT_CONFIG and the tests that might touch GIT_CONFIG all do so by a single-shot assignment to be exported, i.e. GIT_CONFIG=other-config git config anwohner.park ausweis that shouldn't affect the later test, unless the shell is broken. With this patch, can you check which one of the new tests barf on you? t/t1300-repo-config.sh | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index d0e5546..c91d166 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -7,6 +7,10 @@ test_description='Test git config in different settings' . ./test-lib.sh +test_expect_success 'is GIT_CONFIG set (0)?' ' + test "z${GIT_CONFIG+set}" = z +' + test -f .git/config && rm .git/config git config core.penguin "little blue" @@ -399,8 +403,17 @@ cat > expect << EOF ein.bahn=strasse EOF + +test_expect_success 'is GIT_CONFIG set (1)?' ' + test "z${GIT_CONFIG+set}" = z +' + GIT_CONFIG=other-config git config -l > output +test_expect_success 'is GIT_CONFIG set (2)?' ' + test "z${GIT_CONFIG+set}" = z +' + test_expect_success 'alternative GIT_CONFIG' 'cmp output expect' test_expect_success 'alternative GIT_CONFIG (--file)' \ @@ -419,6 +432,10 @@ test_expect_success 'refer config from subdirectory' ' GIT_CONFIG=other-config git config anwohner.park ausweis +test_expect_success 'is GIT_CONFIG set (3)?' ' + test "z${GIT_CONFIG+set}" = z +' + cat > expect << EOF [ein] bahn = strasse @@ -426,6 +443,10 @@ cat > expect << EOF park = ausweis EOF +test_expect_success 'is GIT_CONFIG set (4)?' ' + test "z${GIT_CONFIG+set}" = z +' + test_expect_success '--set in alternative GIT_CONFIG' 'cmp other-config expect' cat > .git/config << EOF ``` ## Ingo Br ückl, 2011-01-10 19:21 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <4d2b5c52.68e3cdc2.bm000@wupperonline.de> URL: https://gitlist.dev/e/4d2b5c52.68e3cdc2.bm000%40wupperonline.de In-Reply-To: <7vhbdg6286.fsf@alter.siamese.dyndns.org> ``` I wrote: > Is it only me [...] experiencing these failures? And the answer is yes. Sorry. As Jonathan and Junio stated, > envvar=value git command > GIT_CONFIG=other-config git config anwohner.park ausweis shouldn't affect the environment of the tests. Unfortunately, I had a shell alias function named git that interfered. In fact it passes to the git program (command git "$@") but sadly does not know about the newly set PATH and (still inexplicably to me) makes the variable set. So it's all my problem. Ingo ``` ## Jonathan Nieder, 2011-01-10 19:42 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <20110110194242.GA11801@burratino> URL: https://gitlist.dev/e/20110110194242.GA11801%40burratino In-Reply-To: <4d2b5c52.68e3cdc2.bm000@wupperonline.de> ``` Ingo Brückl wrote: > As Jonathan and Junio stated, >> envvar=value git command > >> GIT_CONFIG=other-config git config anwohner.park ausweis > > shouldn't affect the environment of the tests. > > Unfortunately, I had a shell alias function named git that interfered. In > fact it passes to the git program (command git "$@") but sadly does not know > about the newly set PATH and (still inexplicably to me) makes the variable > set. For what it's worth, here's what POSIX[1] has to say: When a given simple command is required to be executed [...] the following expansions, assignments, and redirections shall all be performed from the beginning of the command text to the end: [...] If no command name results, variable assignments shall affect the current execution environment. Otherwise, the variable assignments shall be exported for the execution environment of the command and shall not affect the current execution environment (except for special built-ins). I am guessing the expansion of your 'git' alias starts with a special builtin. For the future, it is probably best to guard settings for interactive use with if test "${PS1+set}" then CDPATH=something alias foo=bar alias baz=qux ... fi or even better, case $- in *i*) CDPATH=something ... esac Thanks for explaining. Jonathan [1] http://unix.org/2008edition/ ``` ## Junio C Hamano, 2011-01-10 21:30 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <7v62tw5twb.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v62tw5twb.fsf%40alter.siamese.dyndns.org In-Reply-To: <20110110194242.GA11801@burratino> ``` Jonathan Nieder writes: >> Unfortunately, I had a shell alias function named git that interfered. In >> fact it passes to the git program (command git "$@") but sadly does not know >> about the newly set PATH and (still inexplicably to me) makes the variable >> set. Yuck. I really do not want to do something like this X-<. t/test-lib.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index cb1ca97..df1b4f2 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -77,10 +77,10 @@ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME export EDITOR # Protect ourselves from common misconfiguration to export -# CDPATH into the environment +# CDPATH into the environment and such unset CDPATH - unset GREP_OPTIONS +unalias git >/dev/null 2>&1 || : case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in 1|2|true) ``` ## Jonathan Nieder, 2011-01-10 21:33 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <20110110213308.GA12030@burratino> URL: https://gitlist.dev/e/20110110213308.GA12030%40burratino In-Reply-To: <7v62tw5twb.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano wrote: > Jonathan Nieder writes: >>> Unfortunately, I had a shell alias function named git that interfered. In >>> fact it passes to the git program (command git "$@") but sadly does not know >>> about the newly set PATH and (still inexplicably to me) makes the variable >>> set. > > Yuck. I really do not want to do something like this X-<. Please don't. :) ``` ## Ingo Br ückl, 2011-01-10 21:50 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <4d2b7f25.10be45ba.bm000@wupperonline.de> URL: https://gitlist.dev/e/4d2b7f25.10be45ba.bm000%40wupperonline.de In-Reply-To: <7v62tw5twb.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano wrote: > Yuck. I really do not want to do something like this X-<. And you shouldn't. :-) It's my personal problem (now that I know) to unset the alias. Ingo ``` ## Jonathan Nieder, 2011-01-10 21:59 Subject: Re: [PATCH] Fix wrong failures in config test Message-ID: <20110110215933.GB12030@burratino> URL: https://gitlist.dev/e/20110110215933.GB12030%40burratino In-Reply-To: <4d2b7b68.47102a21.bm000@wupperonline.de> ``` Ingo Brückl wrote: > It's a function (available in login shells and thus during the test suite): The test suite doesn't run in a login shell. As I hinted before, you can put case "$-" in *i*) # interactive shell ;; *) return 0 esac in your .bashrc before the function definition and all should be well. > From what I've learned from you now, if 'git' is an exported bash function, > 'VAR=val git' will always automatically result in VAR being exported I didn't understand at first why this particular vintage of bash makes VAR leak into the current environment. I tried to reproduce it with Debian bash 3.2-4 (which is based on bash 3.2.39(1)-release) with no success. In any event git avoids VAR=val fn when fn is a function for this and possibly other reasons (see [1]). I do not think git ought to guard against a git function (or alias) in the user's environment, even though doing so might lead to a better user experience and less confusion on the mailing list. git does not protect against 'rm' being an alias to 'rm -i' or 'svn' being an alias, either. Regards, Jonathan [1] http://thread.gmane.org/gmane.comp.version-control.git/135766/focus=137095 ```