# [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME 7 messages from 2012-07-24 to 2012-07-24. Participants: Jeff King, Matthieu Moy. Thread: https://gitlist.dev/t/31083 ## Jeff King, 2012-07-24 11:53 Subject: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME Message-ID: <20120724115305.GA7328@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120724115305.GA7328%40sigill.intra.peff.net ``` Now that git respects XDG_CONFIG_HOME for some lookups, we must be sure to cleanse the test environment. Otherwise, the user's XDG_CONFIG_HOME could influence the test results. Signed-off-by: Jeff King --- [oops, re-sending because I forgot to cc git@vger] You can test this trivially with: XDG_CONFIG_HOME=/whatever ./t1306-xdg-files.sh t/test-lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..5e7f435 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -61,6 +61,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e ' my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env); print join("\n", @vars); ') +unset XDG_CONFIG_HOME GIT_AUTHOR_EMAIL=author@example.com GIT_AUTHOR_NAME='A U Thor' GIT_COMMITTER_EMAIL=committer@example.com -- 1.7.11.3.4.g9f70dbb ``` ## Jeff King, 2012-07-24 11:53 Subject: [PATCH 2/3] attr: make sure we have an xdg path before using it Message-ID: <20120724115357.GA7353@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120724115357.GA7353%40sigill.intra.peff.net In-Reply-To: <20120724115305.GA7328@sigill.intra.peff.net> ``` If we don't have a core.attributesfile configured, we fall back to checking XDG config. This is usually $HOME/.config/attributes. However, if $HOME is unset, then home_config_paths will return NULL, and we end up calling fopen(NULL). Depending on your system, this may or may not cause the accompanying test to fail (e.g., on Linux and glibc, the address will go straight to open, which will return EFAULT). However, valgrind will reliably notice the error. Signed-off-by: Jeff King --- [oops, resending because I forgot to cc git@vger] This is another instance of Matthieu's e3ebc35 (config: fix several access(NULL) calls, 2012-07-12). I guess it wasn't caught because of the lack of a test without HOME set (I found it because t5541 can trigger it in a very roundabout way). attr.c | 12 +++++++----- t/t1306-xdg-files.sh | 6 ++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/attr.c b/attr.c index aef93d8..b52efb5 100644 --- a/attr.c +++ b/attr.c @@ -520,11 +520,13 @@ static void bootstrap_attr_stack(void) home_config_paths(NULL, &xdg_attributes_file, "attributes"); git_attributes_file = xdg_attributes_file; } - elem = read_attr_from_file(git_attributes_file, 1); - if (elem) { - elem->origin = NULL; - elem->prev = attr_stack; - attr_stack = elem; + if (git_attributes_file) { + elem = read_attr_from_file(git_attributes_file, 1); + if (elem) { + elem->origin = NULL; + elem->prev = attr_stack; + attr_stack = elem; + } } if (!is_bare_repository() || direction == GIT_ATTR_INDEX) { diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 3c75c3f..1569596 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -106,6 +106,12 @@ test_expect_success 'Checking attributes in the XDG attributes file' ' test_cmp expected actual ' +test_expect_success 'Checking XDG attributes when HOME is unset' ' + >expected && + (sane_unset HOME && + git check-attr -a f >actual) && + test_cmp expected actual +' test_expect_success 'Checking attributes in both XDG and local attributes files' ' echo "f -attr_f" >.gitattributes && -- 1.7.11.3.4.g9f70dbb ``` ## Jeff King, 2012-07-24 11:54 Subject: [PATCH 3/3] t1306: check that XDG_CONFIG_HOME works Message-ID: <20120724115422.GB7353@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120724115422.GB7353%40sigill.intra.peff.net In-Reply-To: <20120724115305.GA7328@sigill.intra.peff.net> ``` This should override $HOME/.config, but we never actually tested it. Signed-off-by: Jeff King --- t/t1306-xdg-files.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 1569596..4447949 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -113,6 +113,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' ' test_cmp expected actual ' +test_expect_success 'Prefer XDG_CONFIG_HOME to HOME/.config' ' + echo "f: attr_f: foo" >expected && + mkdir -p foo/git && + echo "f attr_f=foo" >foo/git/attributes && + XDG_CONFIG_HOME="foo" git check-attr -a f >actual && + test_cmp expected actual +' + test_expect_success 'Checking attributes in both XDG and local attributes files' ' echo "f -attr_f" >.gitattributes && echo "f: attr_f: unset" >expected && -- 1.7.11.3.4.g9f70dbb ``` ## Matthieu Moy, 2012-07-24 12:06 Subject: Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME Message-ID: URL: https://gitlist.dev/e/vpqpq7l8iss.fsf%40bauges.imag.fr In-Reply-To: <20120724115305.GA7328@sigill.intra.peff.net> ``` Thanks (for the 3 patches, all of them look good). the "unset XDG_CONFIG_HOME" part was already discussed here: http://thread.gmane.org/gmane.comp.version-control.git/201609 But Michael did not continue the thread. I think your solution (unset $XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better. In the thread above, I also proposed checking that $XDG_CONFIG_HOME was taken into account, but for the "git config" part (while you test the attributes part). I think it makes sense to add stg like this to your PATCH 3: diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 3c75c3f..f1ea9f1 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -38,6 +38,19 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\'' test_cmp expected actual ' +test_expect_success '"$XDG_CONFIG_HOME overrides $HOME/.config/git' ' + mkdir -p "$HOME"/xdg/git/ && + echo "[user]" >"$HOME"/xdg/git/config && + echo " name = in_xdg" >>"$HOME"/xdg/git/config && + echo in_xdg >expected && + ( + XDG_CONFIG_HOME="$HOME"/xdg/ && + export XDG_CONFIG_HOME && + git config --get-all user.name >actual + ) && + test_cmp expected actual +' + test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' ' >.gitconfig && -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Matthieu Moy, 2012-07-24 12:26 Subject: [PATCH] ignore: make sure we have an xdg path before using it Message-ID: <1343132811-2296-1-git-send-email-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/1343132811-2296-1-git-send-email-Matthieu.Moy%40imag.fr In-Reply-To: <20120724115357.GA7353@sigill.intra.peff.net> ``` Commit e3ebc35 (config: fix several access(NULL) calls, 2012-07-12) was fixing access(NULL) calls when trying to access $HOME/.config/git/config, but missed the ones when trying to access $HOME/.config/git/ignore. Fix and test this. Signed-off-by: Matthieu Moy --- This can be appended to Jeff's serie. I thought if we had 3 bug instances and already fixed 2, why not fix the (hopefully last) one ;-). dir.c | 2 +- t/t1306-xdg-files.sh | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index a772c6d..240bf0c 100644 --- a/dir.c +++ b/dir.c @@ -1313,7 +1313,7 @@ void setup_standard_excludes(struct dir_struct *dir) } if (!access(path, R_OK)) add_excludes_from_file(dir, path); - if (!access(excludes_file, R_OK)) + if (excludes_file && !access(excludes_file, R_OK)) add_excludes_from_file(dir, excludes_file); } diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 097184b..6d9e8cd 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -108,6 +108,13 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' ' test_must_fail git add to_be_excluded ' +test_expect_success 'Checking XDG ignore file when HOME is unset' ' + >expected && + (sane_unset HOME && + git config --unset core.excludesfile && + git ls-files --exclude-standard --ignored >actual) && + test_cmp expected actual +' test_expect_success 'Checking attributes in the XDG attributes file' ' echo foo >f && -- 1.7.11.2.402.g04c525e.dirty ``` ## Jeff King, 2012-07-24 12:27 Subject: Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME Message-ID: <20120724122710.GB12066@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120724122710.GB12066%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Jul 24, 2012 at 02:06:43PM +0200, Matthieu Moy wrote: > Thanks (for the 3 patches, all of them look good). > > the "unset XDG_CONFIG_HOME" part was already discussed here: > > http://thread.gmane.org/gmane.comp.version-control.git/201609 > > But Michael did not continue the thread. I think your solution (unset > $XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better. Yeah, setting it to $HOME/.config/git is actively wrong; I agree with the reasoning in that thread (which I did not read until just now). > In the thread above, I also proposed checking that $XDG_CONFIG_HOME was > taken into account, but for the "git config" part (while you test the > attributes part). Yeah, I see. > I think it makes sense to add stg like this to your PATCH 3: Agreed. And one for the exclude section, too, just for completeness. Revised patch 3 is below. -- >8 -- Subject: [PATCHv2 3/3] t1306: check that XDG_CONFIG_HOME works This should override $HOME/.config, but we never actually tested it. --- t/t1306-xdg-files.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 1569596..a62c3fb 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -38,6 +38,13 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\'' test_cmp expected actual ' +test_expect_success '"$XDG_CONFIG_HOME overrides $HOME/.config/git' ' + mkdir -p "$HOME"/xdg/git && + echo "[user]name = in_xdg" >"$HOME"/xdg/git/config && + echo in_xdg >expected && + XDG_CONFIG_HOME="$HOME"/xdg git config --get-all user.name >actual && + test_cmp expected actual +' test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' ' >.gitconfig && @@ -80,6 +87,17 @@ test_expect_success 'Exclusion of a file in the XDG ignore file' ' test_must_fail git add to_be_excluded ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/ignore' ' + mkdir -p "$HOME"/xdg/git && + echo content >excluded_by_xdg_only && + echo excluded_by_xdg_only >"$HOME"/xdg/git/ignore && + test_when_finished "git read-tree --empty" && + (XDG_CONFIG_HOME="$HOME/xdg" && + export XDG_CONFIG_HOME && + git add to_be_excluded && + test_must_fail git add excluded_by_xdg_only + ) +' test_expect_success 'Exclusion in both XDG and local ignore files' ' echo to_be_excluded >.gitignore && @@ -113,6 +131,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' ' test_cmp expected actual ' +test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/attributes' ' + mkdir -p "$HOME"/xdg/git && + echo "f attr_f=xdg" >"$HOME"/xdg/git/attributes && + echo "f: attr_f: xdg" >expected && + XDG_CONFIG_HOME="$HOME/xdg" git check-attr -a f >actual && + test_cmp expected actual +' + test_expect_success 'Checking attributes in both XDG and local attributes files' ' echo "f -attr_f" >.gitattributes && echo "f: attr_f: unset" >expected && ``` ## Jeff King, 2012-07-24 13:32 Subject: Re: [PATCH] ignore: make sure we have an xdg path before using it Message-ID: <20120724133227.GA14422@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120724133227.GA14422%40sigill.intra.peff.net In-Reply-To: <1343132811-2296-1-git-send-email-Matthieu.Moy@imag.fr> ``` On Tue, Jul 24, 2012 at 02:26:51PM +0200, Matthieu Moy wrote: > Commit e3ebc35 (config: fix several access(NULL) calls, 2012-07-12) was > fixing access(NULL) calls when trying to access $HOME/.config/git/config, > but missed the ones when trying to access $HOME/.config/git/ignore. Fix > and test this. > > Signed-off-by: Matthieu Moy > --- > This can be appended to Jeff's serie. I thought if we had 3 bug > instances and already fixed 2, why not fix the (hopefully last) > one ;-). Thanks. Looks obviously correct to me. -Peff ```