[PATCH 1/3] Testing: XDG config files: Export a suitable `XDG_CONFIG_HOME' environment variable
The tests in: t/t1306-xdg-files.sh were failing because the git commands were using the environment variable `XDG_CONFIG_HOME' as it was set for the user's usual environment, rather than as set for the testing environment. This commit provides the quickest, simplest hack to make things work; because there is already the setting and exporting of the environment variable `HOME' in: t/test-lib.sh this commit simply adds to that file the setting and exporting of the variable `XDG_CONFIG_HOME' (based on the variable `HOME' that is provided there). However, the existing tests [sometimes] don't use these variables explicitly, so the whole structure of this testing rests on the hope that people maintain the conventions captured by the values of these variables; another commit should fix this instability by using these variables explicitly. (Note: Double quotes are not needed around the value assigned to the variable, as word splitting is not performed). Signed-off-by: Michael Witten mfwit...@gmail.com --- t/test-lib.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index acda33d..69bcc75 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -544,6 +544,9 @@ rm -fr $test || { HOME=$TRASH_DIRECTORY export HOME +XDG_CONFIG_HOME=$HOME/.config +export XDG_CONFIG_HOME + if test -z $TEST_NO_CREATE_REPO; then test_create_repo $test else -- 1.7.11.1.29.gf71be5c -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Testing: XDG config files: Export a suitable `XDG_CONFIG_HOME' environment variable
Hi, Michael Witten wrote: --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -544,6 +544,9 @@ rm -fr $test || { HOME=$TRASH_DIRECTORY export HOME +XDG_CONFIG_HOME=$HOME/.config +export XDG_CONFIG_HOME + I think this is a bad idea. The typical case is for XDG_CONFIG_HOME not to be set, and we need to make sure git works well in the typical case. But the general idea seems sane --- how about unset VISUAL EMAIL LANGUAGE COLUMNS XDG_CONFIG_HOME $(perl -e ' ... ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Testing: XDG config files: Export a suitable `XDG_CONFIG_HOME' environment variable
Michael Witten mfwit...@gmail.com writes: --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -544,6 +544,9 @@ rm -fr $test || { HOME=$TRASH_DIRECTORY export HOME +XDG_CONFIG_HOME=$HOME/.config +export XDG_CONFIG_HOME + Why not just unset XDG_CONFIG_HOME? Your match makes it look like XDG_CONFIG_HOME is required to use the configuration directory, but it is not. To me, the main feature is the ability to use $HOME/.config/git/ as a configuration directory (this is not just a convention, this is a documented feature), and the management of the variable $XDG_CONFIG_HOME is just a bonnus. Before your patches, the correct management of $XDG_CONFIG_HOME to override $HOME/.config/git/ was untested (which is unfortunate, indeed), but after your patch serie, the fact that the default is $HOME/.config/git/ is untested, which IMHO is even worse. Unsetting XDG_CONFIG_HOME and adding one test like this would be better IMHO. 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/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Testing: XDG config files: Export a suitable `XDG_CONFIG_HOME' environment variable
Matthieu Moy matthieu@grenoble-inp.fr writes: Before your patches, the correct management of $XDG_CONFIG_HOME to override $HOME/.config/git/ was untested (which is unfortunate, indeed), but after your patch serie, the fact that the default is $HOME/.config/git/ is untested, which IMHO is even worse. Unsetting XDG_CONFIG_HOME and adding one test like this would be better IMHO. Absolutely. We would want to make sure that the new code does not interfere with established uses when the user does not ask for the new feature (i.e. XDG not set), and also make sure it does what it was meant to do when the feature is called for (i.e. XDG set). It might be true that the set of tests in the series did not test the full spectrum of the latter, but then we would want to see the gap filled by adding missing tests, not by converting tests for former into the ones that test for the latter. Even with the patch below there may be other gaps in the test. For example, core.excludesfile and core.attributesfile must default to the XDG location when they exist, whether these variables are set; we may want to make sure that is not broken in the future. Michael, could you change the direction of the patch and look into filling such gaps? Thanks. 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html