{"thread":{"id":"31083","subject":"[PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME","startedAt":"2012-07-24T11:53:05Z","lastAt":"2012-07-24T13:32:27Z","messageCount":7,"participants":["Jeff King","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"195618","messageId":"20120724115305.GA7328@sigill.intra.peff.net","threadId":"31083","inReplyTo":null,"subject":"[PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-24T11:53:05Z","receivedAt":"2012-07-24T11:53:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that git respects XDG_CONFIG_HOME for some lookups, we\nmust be sure to cleanse the test environment. Otherwise, the\nuser's XDG_CONFIG_HOME could influence the test results.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n[oops, re-sending because I forgot to cc git@vger]\n\nYou can test this trivially with:\n\n  XDG_CONFIG_HOME=/whatever ./t1306-xdg-files.sh\n\n t/test-lib.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex acda33d..5e7f435 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -61,6 +61,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e '\n \tmy @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);\n \tprint join(\"\\n\", @vars);\n ')\n+unset XDG_CONFIG_HOME\n GIT_AUTHOR_EMAIL=author@example.com\n GIT_AUTHOR_NAME='A U Thor'\n GIT_COMMITTER_EMAIL=committer@example.com\n-- \n1.7.11.3.4.g9f70dbb\n"},{"id":"195619","messageId":"20120724115357.GA7353@sigill.intra.peff.net","threadId":"31083","inReplyTo":"20120724115305.GA7328@sigill.intra.peff.net","subject":"[PATCH 2/3] attr: make sure we have an xdg path before using it","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-24T11:53:57Z","receivedAt":"2012-07-24T11:53:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we don't have a core.attributesfile configured, we fall\nback to checking XDG config. This is usually\n$HOME/.config/attributes. However, if $HOME is unset, then\nhome_config_paths will return NULL, and we end up calling\nfopen(NULL).\n\nDepending on your system, this may or may not cause the\naccompanying test to fail (e.g., on Linux and glibc, the\naddress will go straight to open, which will return EFAULT).\nHowever, valgrind will reliably notice the error.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n[oops, resending because I forgot to cc git@vger]\n\nThis is another instance of Matthieu's e3ebc35 (config: fix several\naccess(NULL) calls, 2012-07-12). I guess it wasn't caught\nbecause of the lack of a test without HOME set (I found it\nbecause t5541 can trigger it in a very roundabout way).\n\n attr.c               | 12 +++++++-----\n t/t1306-xdg-files.sh |  6 ++++++\n 2 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex aef93d8..b52efb5 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -520,11 +520,13 @@ static void bootstrap_attr_stack(void)\n \t\thome_config_paths(NULL, &xdg_attributes_file, \"attributes\");\n \t\tgit_attributes_file = xdg_attributes_file;\n \t}\n-\telem = read_attr_from_file(git_attributes_file, 1);\n-\tif (elem) {\n-\t\telem->origin = NULL;\n-\t\telem->prev = attr_stack;\n-\t\tattr_stack = elem;\n+\tif (git_attributes_file) {\n+\t\telem = read_attr_from_file(git_attributes_file, 1);\n+\t\tif (elem) {\n+\t\t\telem->origin = NULL;\n+\t\t\telem->prev = attr_stack;\n+\t\t\tattr_stack = elem;\n+\t\t}\n \t}\n \n \tif (!is_bare_repository() || direction == GIT_ATTR_INDEX) {\ndiff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh\nindex 3c75c3f..1569596 100755\n--- a/t/t1306-xdg-files.sh\n+++ b/t/t1306-xdg-files.sh\n@@ -106,6 +106,12 @@ test_expect_success 'Checking attributes in the XDG attributes file' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'Checking XDG attributes when HOME is unset' '\n+\t>expected &&\n+\t(sane_unset HOME &&\n+\t git check-attr -a f >actual) &&\n+\ttest_cmp expected actual\n+'\n \n test_expect_success 'Checking attributes in both XDG and local attributes files' '\n \techo \"f -attr_f\" >.gitattributes &&\n-- \n1.7.11.3.4.g9f70dbb\n"},{"id":"195621","messageId":"20120724115422.GB7353@sigill.intra.peff.net","threadId":"31083","inReplyTo":"20120724115305.GA7328@sigill.intra.peff.net","subject":"[PATCH 3/3] t1306: check that XDG_CONFIG_HOME works","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-24T11:54:22Z","receivedAt":"2012-07-24T11:54:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This should override $HOME/.config, but we never actually\ntested it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1306-xdg-files.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh\nindex 1569596..4447949 100755\n--- a/t/t1306-xdg-files.sh\n+++ b/t/t1306-xdg-files.sh\n@@ -113,6 +113,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'Prefer XDG_CONFIG_HOME to HOME/.config' '\n+\techo \"f: attr_f: foo\" >expected &&\n+\tmkdir -p foo/git &&\n+\techo \"f attr_f=foo\" >foo/git/attributes &&\n+\tXDG_CONFIG_HOME=\"foo\" git check-attr -a f >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Checking attributes in both XDG and local attributes files' '\n \techo \"f -attr_f\" >.gitattributes &&\n \techo \"f: attr_f: unset\" >expected &&\n-- \n1.7.11.3.4.g9f70dbb\n"},{"id":"195623","messageId":"vpqpq7l8iss.fsf@bauges.imag.fr","threadId":"31083","inReplyTo":"20120724115305.GA7328@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-07-24T12:06:43Z","receivedAt":"2012-07-24T12:06:43Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Thanks (for the 3 patches, all of them look good).\n\nthe \"unset XDG_CONFIG_HOME\" part was already discussed here:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/201609\n\nBut Michael did not continue the thread. I think your solution (unset\n$XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better.\n\nIn the thread above, I also proposed checking that $XDG_CONFIG_HOME was\ntaken into account, but for the \"git config\" part (while you test the\nattributes part).\n\nI think it makes sense to add stg like this to your PATCH 3:\n\n\ndiff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh\nindex 3c75c3f..f1ea9f1 100755\n--- a/t/t1306-xdg-files.sh\n+++ b/t/t1306-xdg-files.sh\n@@ -38,6 +38,19 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\\''\n        test_cmp expected actual\n '\n\n+test_expect_success '\"$XDG_CONFIG_HOME overrides $HOME/.config/git' '\n+       mkdir -p \"$HOME\"/xdg/git/ &&\n+       echo \"[user]\" >\"$HOME\"/xdg/git/config &&\n+       echo \"  name = in_xdg\" >>\"$HOME\"/xdg/git/config &&\n+       echo in_xdg >expected &&\n+       (\n+               XDG_CONFIG_HOME=\"$HOME\"/xdg/ &&\n+               export XDG_CONFIG_HOME &&\n+               git config --get-all user.name >actual\n+       ) &&\n+       test_cmp expected actual\n+'\n+\n\n test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '\n        >.gitconfig &&\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"195626","messageId":"1343132811-2296-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"31083","inReplyTo":"20120724115357.GA7353@sigill.intra.peff.net","subject":"[PATCH] ignore: make sure we have an xdg path before using it","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2012-07-24T12:26:51Z","receivedAt":"2012-07-24T12:26:51Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Commit e3ebc35 (config: fix several access(NULL) calls, 2012-07-12) was\nfixing access(NULL) calls when trying to access $HOME/.config/git/config,\nbut missed the ones when trying to access $HOME/.config/git/ignore. Fix\nand test this.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nThis can be appended to Jeff's serie. I thought if we had 3 bug\ninstances and already fixed 2, why not fix the (hopefully last)\none ;-).\n\n dir.c                | 2 +-\n t/t1306-xdg-files.sh | 7 +++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex a772c6d..240bf0c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1313,7 +1313,7 @@ void setup_standard_excludes(struct dir_struct *dir)\n \t}\n \tif (!access(path, R_OK))\n \t\tadd_excludes_from_file(dir, path);\n-\tif (!access(excludes_file, R_OK))\n+\tif (excludes_file && !access(excludes_file, R_OK))\n \t\tadd_excludes_from_file(dir, excludes_file);\n }\n \ndiff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh\nindex 097184b..6d9e8cd 100755\n--- a/t/t1306-xdg-files.sh\n+++ b/t/t1306-xdg-files.sh\n@@ -108,6 +108,13 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '\n \ttest_must_fail git add to_be_excluded\n '\n \n+test_expect_success 'Checking XDG ignore file when HOME is unset' '\n+\t>expected &&\n+\t(sane_unset HOME &&\n+\t git config --unset core.excludesfile &&\n+\t git ls-files --exclude-standard --ignored >actual) &&\n+\ttest_cmp expected actual\n+'\n \n test_expect_success 'Checking attributes in the XDG attributes file' '\n \techo foo >f &&\n-- \n1.7.11.2.402.g04c525e.dirty\n"},{"id":"195625","messageId":"20120724122710.GB12066@sigill.intra.peff.net","threadId":"31083","inReplyTo":"vpqpq7l8iss.fsf@bauges.imag.fr","subject":"Re: [PATCH 1/3] test-lib.sh: unset XDG_CONFIG_HOME","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-24T12:27:10Z","receivedAt":"2012-07-24T12:27:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 24, 2012 at 02:06:43PM +0200, Matthieu Moy wrote:\n\n> Thanks (for the 3 patches, all of them look good).\n> \n> the \"unset XDG_CONFIG_HOME\" part was already discussed here:\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/201609\n> \n> But Michael did not continue the thread. I think your solution (unset\n> $XDG_CONFIG_HOME instead of setting it to $HOME/.config/git) is better.\n\nYeah, setting it to $HOME/.config/git is actively wrong; I agree with\nthe reasoning in that thread (which I did not read until just now).\n\n> In the thread above, I also proposed checking that $XDG_CONFIG_HOME was\n> taken into account, but for the \"git config\" part (while you test the\n> attributes part).\n\nYeah, I see.\n\n> I think it makes sense to add stg like this to your PATCH 3:\n\nAgreed. And one for the exclude section, too, just for completeness.\n\nRevised patch 3 is below.\n\n-- >8 --\nSubject: [PATCHv2 3/3] t1306: check that XDG_CONFIG_HOME works\n\nThis should override $HOME/.config, but we never actually\ntested it.\n---\n t/t1306-xdg-files.sh | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh\nindex 1569596..a62c3fb 100755\n--- a/t/t1306-xdg-files.sh\n+++ b/t/t1306-xdg-files.sh\n@@ -38,6 +38,13 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\\''\n \ttest_cmp expected actual\n '\n \n+test_expect_success '\"$XDG_CONFIG_HOME overrides $HOME/.config/git' '\n+\tmkdir -p \"$HOME\"/xdg/git &&\n+\techo \"[user]name = in_xdg\" >\"$HOME\"/xdg/git/config &&\n+\techo in_xdg >expected &&\n+\tXDG_CONFIG_HOME=\"$HOME\"/xdg git config --get-all user.name >actual &&\n+\ttest_cmp expected actual\n+'\n \n test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '\n \t>.gitconfig &&\n@@ -80,6 +87,17 @@ test_expect_success 'Exclusion of a file in the XDG ignore file' '\n \ttest_must_fail git add to_be_excluded\n '\n \n+test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/ignore' '\n+\tmkdir -p \"$HOME\"/xdg/git &&\n+\techo content >excluded_by_xdg_only &&\n+\techo excluded_by_xdg_only >\"$HOME\"/xdg/git/ignore &&\n+\ttest_when_finished \"git read-tree --empty\" &&\n+\t(XDG_CONFIG_HOME=\"$HOME/xdg\" &&\n+\t export XDG_CONFIG_HOME &&\n+\t git add to_be_excluded &&\n+\t test_must_fail git add excluded_by_xdg_only\n+\t)\n+'\n \n test_expect_success 'Exclusion in both XDG and local ignore files' '\n \techo to_be_excluded >.gitignore &&\n@@ -113,6 +131,14 @@ test_expect_success 'Checking XDG attributes when HOME is unset' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success '$XDG_CONFIG_HOME overrides $HOME/.config/attributes' '\n+\tmkdir -p \"$HOME\"/xdg/git &&\n+\techo \"f attr_f=xdg\" >\"$HOME\"/xdg/git/attributes &&\n+\techo \"f: attr_f: xdg\" >expected &&\n+\tXDG_CONFIG_HOME=\"$HOME/xdg\" git check-attr -a f >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Checking attributes in both XDG and local attributes files' '\n \techo \"f -attr_f\" >.gitattributes &&\n \techo \"f: attr_f: unset\" >expected &&\n"},{"id":"195628","messageId":"20120724133227.GA14422@sigill.intra.peff.net","threadId":"31083","inReplyTo":"1343132811-2296-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] ignore: make sure we have an xdg path before using it","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-24T13:32:27Z","receivedAt":"2012-07-24T13:32:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 24, 2012 at 02:26:51PM +0200, Matthieu Moy wrote:\n\n> Commit e3ebc35 (config: fix several access(NULL) calls, 2012-07-12) was\n> fixing access(NULL) calls when trying to access $HOME/.config/git/config,\n> but missed the ones when trying to access $HOME/.config/git/ignore. Fix\n> and test this.\n> \n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> This can be appended to Jeff's serie. I thought if we had 3 bug\n> instances and already fixed 2, why not fix the (hopefully last)\n> one ;-).\n\nThanks. Looks obviously correct to me.\n\n-Peff\n"}]}