{"thread":{"id":"36209","subject":"[PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","startedAt":"2014-03-18T12:08:38Z","lastAt":"2014-03-25T04:56:55Z","messageCount":28,"participants":["David Tran","Junio C Hamano","Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"237008","messageId":"1395144518-2489-1-git-send-email-unsignedzero@gmail.com","threadId":"36209","inReplyTo":"244284@gmane.comp.version-control.git","subject":"[PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"David Tran","fromEmail":"unsignedzero@gmail.com","sentAt":"2014-03-18T12:08:38Z","receivedAt":"2014-03-18T12:08:38Z","isPatch":true,"sender":{"key":"unsignedzero@gmail.com","avatar":"https://avatars.githubusercontent.com/u/778125?v=4"},"body":"Originally, the code used subshells instead of FOO=BAR command because\nthe variable would otherwise leak into the surrounding context of the POSIX\nshell when 'command' is a shell function. The subshell was used to hold the\ncontext for the test. Using 'env' in the test function sets the temp variables\nwithout leaking, removing the need of a subshell.\n\nSigned-off-by: David Tran <unsignedzero@gmail.com>\n\n---\n\nLet's see if I replied correctly with send-email. Retrying this again.\nHow do I 'reply' to a thread using send-email?\n\nI am David Tran a graduating CS/Math senior from Sonoma State University,\nUnited States. I would like to work with git for GSoC'14, specifically the line\noptions for git rebase --interactive [1]. I have used git for a few years and\nknow how destructive but important rebase is to git. I have created a few shell\nscripts here and there to make life using bash/zsh easier. I would like to\napply these skills and work with the best.\n\nGithub: unsignedzero\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967\n\n> Oh, really ;-)?\nMissed that.\n\n> Thanks.  Getting closer, I think.\nSlowly but surely.\n\nSigned-off-by: David Tran <unsignedzero@gmail.com>\n---\n t/t1300-repo-config.sh        |   17 ++--------\n t/t1510-repo-setup.sh         |    4 +--\n t/t3200-branch.sh             |   12 +------\n t/t3301-notes.sh              |   22 ++++----------\n t/t3404-rebase-interactive.sh |   65 ++++++++--------------------------------\n t/t3413-rebase-hook.sh        |    6 +---\n t/t4014-format-patch.sh       |   14 ++-------\n t/t5305-include-tag.sh        |    4 +--\n t/t5602-clone-remote-exec.sh  |   13 ++------\n t/t5801-remote-helpers.sh     |    6 +--\n t/t6006-rev-list-format.sh    |    9 ++----\n t/t7006-pager.sh              |   18 ++---------\n 12 files changed, 42 insertions(+), 148 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex c9c426c..3e3f77b 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n '\n\n test_expect_success 'nonexistent configuration' '\n-\t(\n-\t\tGIT_CONFIG=doesnotexist &&\n-\t\texport GIT_CONFIG &&\n-\t\ttest_must_fail git config --list &&\n-\t\ttest_must_fail git config test.xyzzy\n-\t)\n+\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n+\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n '\n\n test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n \tln -s doesnotexist linktonada &&\n \tln -s linktonada linktolinktonada &&\n-\t(\n-\t\tGIT_CONFIG=linktonada &&\n-\t\texport GIT_CONFIG &&\n-\t\ttest_must_fail git config --list &&\n-\t\tGIT_CONFIG=linktolinktonada &&\n-\t\ttest_must_fail git config --list\n-\t)\n+\ttest_must_fail env GIT_CONFIG=linktonada git config --list &&\n+\ttest_must_fail env GIT_CONFIG=linktolinktonada git config --list\n '\n\n test_expect_success 'check split_cmdline return' \"\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex cf2ee78..e1b2a99 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)\n \tsetup_repo 30 \"$here/30\" gitfile true &&\n \t(\n \t\tcd 30 &&\n-\t\tGIT_DIR=.git &&\n-\t\texport GIT_DIR &&\n-\t\ttest_must_fail git symbolic-ref HEAD 2>result\n+\t\ttest_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result\n \t) &&\n \tgrep \"core.bare and core.worktree\" 30/result\n '\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex fcdb867..d45e95c 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n-\t(\n-\t\tEDITOR=./editor &&\n-\t\texport EDITOR &&\n-\t\ttest_must_fail git branch --edit-description no-such-branch\n-\t)\n+\ttest_must_fail env EDITOR=./editor git branch --edit-description no-such-branch\n '\n\n test_expect_success 'refuse --edit-description on unborn branch for now' '\n@@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' '\n \t\techo \"New contents\" >\"$1\"\n \tEOF\n \tgit checkout --orphan unborn &&\n-\t(\n-\t\tEDITOR=./editor &&\n-\t\texport EDITOR &&\n-\t\ttest_must_fail git branch --edit-description\n-\t)\n+\ttest_must_fail env EDITOR=./editor git branch --edit-description\n '\n\n test_expect_success '--merged catches invalid object names' '\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 3bb79a4..cfd67ff 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh\n export GIT_EDITOR\n\n test_expect_success 'cannot annotate non-existing HEAD' '\n-\t(MSG=3 && export MSG && test_must_fail git notes add)\n+\ttest_must_fail env MSG=3 git notes add\n '\n\n test_expect_success setup '\n@@ -32,22 +32,16 @@ test_expect_success setup '\n '\n\n test_expect_success 'need valid notes ref' '\n-\t(MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes add) &&\n-\t(MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes show)\n+\ttest_must_fail env MSG=1 GIT_NOTES_REF=/ git notes show &&\n+\ttest_must_fail env MSG=2 GIT_NOTES_REF=/ git notes show\n '\n\n test_expect_success 'refusing to add notes in refs/heads/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes add)\n+\ttest_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes add\n '\n\n test_expect_success 'refusing to edit notes in refs/remotes/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG GIT_NOTES_REF &&\n-\t test_must_fail git notes edit)\n+\ttest_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes edit\n '\n\n # 1 indicates caught gracefully by die, 128 means git-show barked\n@@ -865,11 +859,7 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n \tgit add a10 &&\n \ttest_tick &&\n \tgit commit -m 10th &&\n-\t(\n-\t\tMSG=\"yet another note\" &&\n-\t\texport MSG &&\n-\t\ttest_must_fail git notes add -c deadbeef\n-\t) &&\n+\ttest_must_fail env MSG=\"yet another note\" git notes add -c deadbeef &&\n \ttest_must_fail git notes list HEAD\n '\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 50e22b1..4c7364a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \t(\n \tset_fake_editor &&\n-\tFAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n-\texport FAKE_LINES &&\n-\ttest_must_fail git rebase -i HEAD^\n+\ttest_must_fail env FAKE_LINES=\"exec_echo_foo_>file1 1\" git rebase -i HEAD^\n \t) &&\n \ttest_cmp_rev master^ HEAD &&\n \tgit reset --hard &&\n@@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n \ttest_when_finished \"git rebase --abort\" &&\n \t(\n \tset_fake_editor &&\n-\tFAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n-\texport FAKE_LINES &&\n-\ttest_must_fail git rebase -i HEAD^ >actual 2>&1\n+\ttest_must_fail env FAKE_LINES=\"exec_this-command-does-not-exist 1\" \\\n+\tgit rebase -i HEAD^ >actual 2>&1\n \t) &&\n \t! grep \"Maybe git-rebase is broken\" actual\n '\n@@ -375,11 +372,7 @@ test_expect_success 'commit message used after conflict' '\n \tgit checkout -b conflict-fixup conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 fixup 3 fixup 4\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i $base\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 fixup 3 fixup 4\" git rebase -i $base &&\n \techo three > conflict &&\n \tgit add conflict &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" EXPECT_HEADER_COUNT=2 \\\n@@ -394,11 +387,7 @@ test_expect_success 'commit message retained after conflict' '\n \tgit checkout -b conflict-squash conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 fixup 3 squash 4\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i $base\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 fixup 3 squash 4\" git rebase -i $base &&\n \techo three > conflict &&\n \tgit add conflict &&\n \tFAKE_COMMIT_AMEND=\"TWICE\" EXPECT_HEADER_COUNT=2 \\\n@@ -469,11 +458,7 @@ test_expect_success 'interrupted squash works as expected' '\n \tgit checkout -b interrupted-squash conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 squash 3 2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD~3\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 squash 3 2\" git rebase -i HEAD~3 &&\n \t(echo one; echo two; echo four) > conflict &&\n \tgit add conflict &&\n \ttest_must_fail git rebase --continue &&\n@@ -487,11 +472,7 @@ test_expect_success 'interrupted squash works as expected (case 2)' '\n \tgit checkout -b interrupted-squash2 conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"3 squash 1 2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD~3\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"3 squash 1 2\" git rebase -i HEAD~3 &&\n \t(echo one; echo four) > conflict &&\n \tgit add conflict &&\n \ttest_must_fail git rebase --continue &&\n@@ -528,11 +509,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n \tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n-\t(\n-\t\tFAKE_COMMIT_MESSAGE=\" \" &&\n-\t\texport FAKE_COMMIT_MESSAGE &&\n-\t\ttest_must_fail git rebase --continue\n-\t) &&\n+\ttest_must_fail env FAKE_COMMIT_MESSAGE=\" \" git rebase --continue\n \ttest $old = $(git rev-parse HEAD) &&\n \tgit rebase --abort\n '\n@@ -547,11 +524,7 @@ test_expect_success 'auto-amend only edited commits after \"edit\"' '\n \techo \"and again\" > file7 &&\n \tgit add file7 &&\n \ttest_tick &&\n-\t(\n-\t\tFAKE_COMMIT_MESSAGE=\"and again\" &&\n-\t\texport FAKE_COMMIT_MESSAGE &&\n-\t\ttest_must_fail git rebase --continue\n-\t) &&\n+\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"and again\" git rebase --continue &&\n \tgit rebase --abort\n '\n\n@@ -559,11 +532,7 @@ test_expect_success 'clean error after failed \"exec\"' '\n \ttest_tick &&\n \ttest_when_finished \"git rebase --abort || :\" &&\n \tset_fake_editor &&\n-\t(\n-\t\tFAKE_LINES=\"1 exec_false\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i HEAD^\n-\t) &&\n+\ttest_must_fail env FAKE_LINES=\"1 exec_false\" git rebase -i HEAD^ &&\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n \ttest_must_fail git rebase --continue 2>error &&\n@@ -947,12 +916,8 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n\n test_expect_success 'rebase -i --root temporary sentinel commit' '\n \tgit checkout B &&\n-\t(\n-\t\tset_fake_editor &&\n-\t\tFAKE_LINES=\"2\" &&\n-\t\texport FAKE_LINES &&\n-\t\ttest_must_fail git rebase -i --root\n-\t) &&\n+\tset_fake_editor &&\n+\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root &&\n \tgit cat-file commit HEAD | grep \"^tree 4b825dc642cb\" &&\n \tgit rebase --abort\n '\n@@ -1042,11 +1007,7 @@ test_expect_success 'rebase -i error on commits with \\ in message' '\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n \ttest_commit TO-REMOVE will-conflict old-content &&\n \ttest_commit \"\\temp\" will-conflict new-content dummy &&\n-\t(\n-\tEDITOR=true &&\n-\texport EDITOR &&\n-\ttest_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error\n-\t) &&\n+\ttest_must_fail env EDITOR=true git rebase -i HEAD^ --onto HEAD^^ 2>error &&\n \ttest_expect_code 1 grep  \"\temp\" error\n '\n\ndiff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\nindex 098b755..b6833e9 100755\n--- a/t/t3413-rebase-hook.sh\n+++ b/t/t3413-rebase-hook.sh\n@@ -118,11 +118,7 @@ test_expect_success 'pre-rebase hook stops rebase (1)' '\n test_expect_success 'pre-rebase hook stops rebase (2)' '\n \tgit checkout test &&\n \tgit reset --hard side &&\n-\t(\n-\t\tEDITOR=:\n-\t\texport EDITOR\n-\t\ttest_must_fail git rebase -i master\n-\t) &&\n+\ttest_must_fail env EDITOR=: git rebase -i master &&\n \ttest \"z$(git symbolic-ref HEAD)\" = zrefs/heads/test &&\n \ttest 0 = $(git rev-list HEAD...side | wc -l)\n '\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 73194b2..9c80633 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -764,22 +764,14 @@ test_expect_success 'format-patch --signature=\"\" suppresses signatures' '\n\n test_expect_success TTY 'format-patch --stdout paginates' '\n \trm -f pager_used &&\n-\t(\n-\t\tGIT_PAGER=\"wc >pager_used\" &&\n-\t\texport GIT_PAGER &&\n-\t\ttest_terminal git format-patch --stdout --all\n-\t) &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git format-patch --stdout --all &&\n \ttest_path_is_file pager_used\n '\n\n  test_expect_success TTY 'format-patch --stdout pagination can be disabled' '\n \trm -f pager_used &&\n-\t(\n-\t\tGIT_PAGER=\"wc >pager_used\" &&\n-\t\texport GIT_PAGER &&\n-\t\ttest_terminal git --no-pager format-patch --stdout --all &&\n-\t\ttest_terminal git -c \"pager.format-patch=false\" format-patch --stdout --all\n-\t) &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git --no-pager format-patch --stdout --all &&\n+\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git -c \"pager.format-patch=false\" format-patch --stdout --all &&\n \ttest_path_is_missing pager_used &&\n \ttest_path_is_missing .git/pager_used\n '\ndiff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\nindex b061864..21517c7 100755\n--- a/t/t5305-include-tag.sh\n+++ b/t/t5305-include-tag.sh\n@@ -45,9 +45,7 @@ test_expect_success 'unpack objects' '\n test_expect_success 'check unpacked result (have commit, no tag)' '\n \tgit rev-list --objects $commit >list.expect &&\n \t(\n-\t\tGIT_DIR=clone.git &&\n-\t\texport GIT_DIR &&\n-\t\ttest_must_fail git cat-file -e $tag &&\n+\t\ttest_must_fail env GIT_DIR=clone.git git cat-file -e $tag &&\n \t\tgit rev-list --objects $commit\n \t) >list.actual &&\n \ttest_cmp list.expect list.actual\ndiff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh\nindex 3f353d9..cbcceab 100755\n--- a/t/t5602-clone-remote-exec.sh\n+++ b/t/t5602-clone-remote-exec.sh\n@@ -12,21 +12,14 @@ test_expect_success setup '\n '\n\n test_expect_success 'clone calls git upload-pack unqualified with no -u option' '\n-\t(\n-\t\tGIT_SSH=./not_ssh &&\n-\t\texport GIT_SSH &&\n-\t\ttest_must_fail git clone localhost:/path/to/repo junk\n-\t) &&\n+\ttest_must_fail env GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk &&\n \techo \"localhost git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n \ttest_cmp expected not_ssh_output\n '\n\n test_expect_success 'clone calls specified git upload-pack with -u option' '\n-\t(\n-\t\tGIT_SSH=./not_ssh &&\n-\t\texport GIT_SSH &&\n-\t\ttest_must_fail git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n-\t) &&\n+\ttest_must_fail env GIT_SSH=./not_ssh \\\n+\t\tgit clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk &&\n \techo \"localhost ./something/bin/git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n \ttest_cmp expected not_ssh_output\n '\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 613f69a..ca19838 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -218,10 +218,8 @@ test_expect_success 'proper failure checks for fetching' '\n '\n\n test_expect_success 'proper failure checks for pushing' '\n-\t(GIT_REMOTE_TESTGIT_FAILURE=1 &&\n-\texport GIT_REMOTE_TESTGIT_FAILURE &&\n-\tcd local &&\n-\ttest_must_fail git push --all\n+\t(cd local &&\n+\ttest_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all\n \t)\n '\n\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 9874403..9d9d9de 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -190,12 +190,9 @@ test_expect_success '%C(auto) respects --no-color' '\n '\n\n test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '\n-\t(\n-\t\tTERM=vt100 && export TERM &&\n-\t\ttest_terminal \\\n-\t\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n-\t\thas_color actual\n-\t)\n+\ttest_terminal env TERM=vt100 \\\n+\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n+\thas_color actual\n '\n\n test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex b9365b4..da958a8 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -146,11 +146,7 @@ test_expect_success 'no color when stdout is a regular file' '\n test_expect_success TTY 'color when writing to a pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git log\n-\t) &&\n+\ttest_terminal env TERM=vt100 git log &&\n \tcolorful paginated.out\n '\n\n@@ -158,11 +154,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '\n \trm -f paginated.out &&\n \ttest_config color.ui auto &&\n \ttest_config color.pager false &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git log\n-\t) &&\n+\ttest_terminal env TERM=vt100 git log &&\n \t! colorful paginated.out\n '\n\n@@ -181,11 +173,7 @@ test_expect_success 'color when writing to a file intended for a pager' '\n test_expect_success TTY 'colors are sent to pager for external commands' '\n \ttest_config alias.externallog \"!git log\" &&\n \ttest_config color.ui auto &&\n-\t(\n-\t\tTERM=vt100 &&\n-\t\texport TERM &&\n-\t\ttest_terminal git -p externallog\n-\t) &&\n+\ttest_terminal env TERM=vt100 git -p externallog &&\n \tcolorful paginated.out\n '\n\n--\n1.7.9\n"},{"id":"237011","messageId":"xmqqd2hj6y5o.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"1395144518-2489-1-git-send-email-unsignedzero@gmail.com","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-18T20:37:39Z","receivedAt":"2014-03-18T20:37:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Tran <unsignedzero@gmail.com> writes:\n\n> Originally, the code used subshells instead of FOO=BAR command\n> because the variable would otherwise leak into the surrounding\n> context of the POSIX shell when 'command' is a shell function.\n> The subshell was used to hold the context for the test. Using\n> 'env' in the test function sets the temp variables without\n> leaking, removing the need of a subshell.\n\nThese are not \"temp variables\" ;-).\n\nYou are improving the way how commands are run under a different\nsettings to environment variables.\n\nHmm, let's try to see if I can do better:\n\n\tSubject: tests: use \"env\" to run commands with temporary env-var settings\n\n\tOrdinarily, we would say \"VAR=VAL command\" to execute a\n\ttested command with environment variable(s) set only for\n\tthat command.  This however does not work if 'command' is a\n\tshell function (most notably 'test_must_fail'); the result\n\tof the assignment is retained and affects later commands.\n\n\tTo avoid this, we used to assign and export environment\n        variables and run such a test in a subshell,\n\n\t\t(\n                \tVAR=VAL && export VAR &&\n                        test_must_fail git command to be tested\n\t\t)\n\n        but with \"env\" utility, we should be able to say\n\n        \ttest_must_fail env VAR=VAL git command to be tested\n\n\twhich is much shorter and easier to read.\n\n> Let's see if I replied correctly with send-email. Retrying this again.\n> How do I 'reply' to a thread using send-email?\n\nLook for --in-reply-to option in \"man git-send-email\".\n\n> Signed-off-by: David Tran <unsignedzero@gmail.com>\n> ---\n>  t/t1300-repo-config.sh        |   17 ++--------\n>  t/t1510-repo-setup.sh         |    4 +--\n>  t/t3200-branch.sh             |   12 +------\n>  t/t3301-notes.sh              |   22 ++++----------\n>  t/t3404-rebase-interactive.sh |   65 ++++++++--------------------------------\n>  t/t3413-rebase-hook.sh        |    6 +---\n>  t/t4014-format-patch.sh       |   14 ++-------\n>  t/t5305-include-tag.sh        |    4 +--\n>  t/t5602-clone-remote-exec.sh  |   13 ++------\n>  t/t5801-remote-helpers.sh     |    6 +--\n>  t/t6006-rev-list-format.sh    |    9 ++----\n>  t/t7006-pager.sh              |   18 ++---------\n>  12 files changed, 42 insertions(+), 148 deletions(-)\n\nThanks.  The numbers look very good ;-)  We love code reduction.\n\n> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> index c9c426c..3e3f77b 100755\n> --- a/t/t1300-repo-config.sh\n> +++ b/t/t1300-repo-config.sh\n> @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n>  '\n>\n>  test_expect_success 'nonexistent configuration' '\n> -\t(\n> -\t\tGIT_CONFIG=doesnotexist &&\n> -\t\texport GIT_CONFIG &&\n> -\t\ttest_must_fail git config --list &&\n> -\t\ttest_must_fail git config test.xyzzy\n> -\t)\n> +\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n> +\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n>  '\n>\n>  test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n>  \tln -s doesnotexist linktonada &&\n>  \tln -s linktonada linktolinktonada &&\n> -\t(\n> -\t\tGIT_CONFIG=linktonada &&\n> -\t\texport GIT_CONFIG &&\n> -\t\ttest_must_fail git config --list &&\n> -\t\tGIT_CONFIG=linktolinktonada &&\n> -\t\ttest_must_fail git config --list\n> -\t)\n> +\ttest_must_fail env GIT_CONFIG=linktonada git config --list &&\n> +\ttest_must_fail env GIT_CONFIG=linktolinktonada git config --list\n>  '\n>\n>  test_expect_success 'check split_cmdline return' \"\n> diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\n> index cf2ee78..e1b2a99 100755\n> --- a/t/t1510-repo-setup.sh\n> +++ b/t/t1510-repo-setup.sh\n> @@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)\n>  \tsetup_repo 30 \"$here/30\" gitfile true &&\n>  \t(\n>  \t\tcd 30 &&\n> -\t\tGIT_DIR=.git &&\n> -\t\texport GIT_DIR &&\n> -\t\ttest_must_fail git symbolic-ref HEAD 2>result\n> +\t\ttest_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result\n>  \t) &&\n>  \tgrep \"core.bare and core.worktree\" 30/result\n>  '\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index fcdb867..d45e95c 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -849,11 +849,7 @@ test_expect_success 'detect typo in branch name when using --edit-description' '\n>  \twrite_script editor <<-\\EOF &&\n>  \t\techo \"New contents\" >\"$1\"\n>  \tEOF\n> -\t(\n> -\t\tEDITOR=./editor &&\n> -\t\texport EDITOR &&\n> -\t\ttest_must_fail git branch --edit-description no-such-branch\n> -\t)\n> +\ttest_must_fail env EDITOR=./editor git branch --edit-description no-such-branch\n>  '\n>\n>  test_expect_success 'refuse --edit-description on unborn branch for now' '\n> @@ -861,11 +857,7 @@ test_expect_success 'refuse --edit-description on unborn branch for now' '\n>  \t\techo \"New contents\" >\"$1\"\n>  \tEOF\n>  \tgit checkout --orphan unborn &&\n> -\t(\n> -\t\tEDITOR=./editor &&\n> -\t\texport EDITOR &&\n> -\t\ttest_must_fail git branch --edit-description\n> -\t)\n> +\ttest_must_fail env EDITOR=./editor git branch --edit-description\n>  '\n>\n>  test_expect_success '--merged catches invalid object names' '\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 3bb79a4..cfd67ff 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -17,7 +17,7 @@ GIT_EDITOR=./fake_editor.sh\n>  export GIT_EDITOR\n>\n>  test_expect_success 'cannot annotate non-existing HEAD' '\n> -\t(MSG=3 && export MSG && test_must_fail git notes add)\n> +\ttest_must_fail env MSG=3 git notes add\n>  '\n>\n>  test_expect_success setup '\n> @@ -32,22 +32,16 @@ test_expect_success setup '\n>  '\n>\n>  test_expect_success 'need valid notes ref' '\n> -\t(MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n> -\t test_must_fail git notes add) &&\n> -\t(MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&\n> -\t test_must_fail git notes show)\n> +\ttest_must_fail env MSG=1 GIT_NOTES_REF=/ git notes show &&\n> +\ttest_must_fail env MSG=2 GIT_NOTES_REF=/ git notes show\n>  '\n>\n>  test_expect_success 'refusing to add notes in refs/heads/' '\n> -\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n> -\t export MSG GIT_NOTES_REF &&\n> -\t test_must_fail git notes add)\n> +\ttest_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes add\n>  '\n>\n>  test_expect_success 'refusing to edit notes in refs/remotes/' '\n> -\t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n> -\t export MSG GIT_NOTES_REF &&\n> -\t test_must_fail git notes edit)\n> +\ttest_must_fail env MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes edit\n>  '\n>\n>  # 1 indicates caught gracefully by die, 128 means git-show barked\n> @@ -865,11 +859,7 @@ test_expect_success 'create note from non-existing note with \"git notes add -c\"\n>  \tgit add a10 &&\n>  \ttest_tick &&\n>  \tgit commit -m 10th &&\n> -\t(\n> -\t\tMSG=\"yet another note\" &&\n> -\t\texport MSG &&\n> -\t\ttest_must_fail git notes add -c deadbeef\n> -\t) &&\n> +\ttest_must_fail env MSG=\"yet another note\" git notes add -c deadbeef &&\n>  \ttest_must_fail git notes list HEAD\n>  '\n>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 50e22b1..4c7364a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n>  \tgit checkout master &&\n>  \t(\n>  \tset_fake_editor &&\n> -\tFAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n> -\texport FAKE_LINES &&\n> -\ttest_must_fail git rebase -i HEAD^\n> +\ttest_must_fail env FAKE_LINES=\"exec_echo_foo_>file1 1\" git rebase -i HEAD^\n>  \t) &&\n>  \ttest_cmp_rev master^ HEAD &&\n>  \tgit reset --hard &&\n> @@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n>  \ttest_when_finished \"git rebase --abort\" &&\n>  \t(\n>  \tset_fake_editor &&\n> -\tFAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n> -\texport FAKE_LINES &&\n> -\ttest_must_fail git rebase -i HEAD^ >actual 2>&1\n> +\ttest_must_fail env FAKE_LINES=\"exec_this-command-does-not-exist 1\" \\\n> +\tgit rebase -i HEAD^ >actual 2>&1\n>  \t) &&\n>  \t! grep \"Maybe git-rebase is broken\" actual\n>  '\n> @@ -375,11 +372,7 @@ test_expect_success 'commit message used after conflict' '\n>  \tgit checkout -b conflict-fixup conflict-branch &&\n>  \tbase=$(git rev-parse HEAD~4) &&\n>  \tset_fake_editor &&\n> -\t(\n> -\t\tFAKE_LINES=\"1 fixup 3 fixup 4\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i $base\n> -\t) &&\n> +\ttest_must_fail env FAKE_LINES=\"1 fixup 3 fixup 4\" git rebase -i $base &&\n>  \techo three > conflict &&\n>  \tgit add conflict &&\n>  \tFAKE_COMMIT_AMEND=\"ONCE\" EXPECT_HEADER_COUNT=2 \\\n> @@ -394,11 +387,7 @@ test_expect_success 'commit message retained after conflict' '\n>  \tgit checkout -b conflict-squash conflict-branch &&\n>  \tbase=$(git rev-parse HEAD~4) &&\n>  \tset_fake_editor &&\n> -\t(\n> -\t\tFAKE_LINES=\"1 fixup 3 squash 4\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i $base\n> -\t) &&\n> +\ttest_must_fail env FAKE_LINES=\"1 fixup 3 squash 4\" git rebase -i $base &&\n>  \techo three > conflict &&\n>  \tgit add conflict &&\n>  \tFAKE_COMMIT_AMEND=\"TWICE\" EXPECT_HEADER_COUNT=2 \\\n> @@ -469,11 +458,7 @@ test_expect_success 'interrupted squash works as expected' '\n>  \tgit checkout -b interrupted-squash conflict-branch &&\n>  \tone=$(git rev-parse HEAD~3) &&\n>  \tset_fake_editor &&\n> -\t(\n> -\t\tFAKE_LINES=\"1 squash 3 2\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i HEAD~3\n> -\t) &&\n> +\ttest_must_fail env FAKE_LINES=\"1 squash 3 2\" git rebase -i HEAD~3 &&\n>  \t(echo one; echo two; echo four) > conflict &&\n>  \tgit add conflict &&\n>  \ttest_must_fail git rebase --continue &&\n> @@ -487,11 +472,7 @@ test_expect_success 'interrupted squash works as expected (case 2)' '\n>  \tgit checkout -b interrupted-squash2 conflict-branch &&\n>  \tone=$(git rev-parse HEAD~3) &&\n>  \tset_fake_editor &&\n> -\t(\n> -\t\tFAKE_LINES=\"3 squash 1 2\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i HEAD~3\n> -\t) &&\n> +\ttest_must_fail env FAKE_LINES=\"3 squash 1 2\" git rebase -i HEAD~3 &&\n>  \t(echo one; echo four) > conflict &&\n>  \tgit add conflict &&\n>  \ttest_must_fail git rebase --continue &&\n> @@ -528,11 +509,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n>  \tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n>  \techo \"edited again\" > file7 &&\n>  \tgit add file7 &&\n> -\t(\n> -\t\tFAKE_COMMIT_MESSAGE=\" \" &&\n> -\t\texport FAKE_COMMIT_MESSAGE &&\n> -\t\ttest_must_fail git rebase --continue\n> -\t) &&\n> +\ttest_must_fail env FAKE_COMMIT_MESSAGE=\" \" git rebase --continue\n>  \ttest $old = $(git rev-parse HEAD) &&\n>  \tgit rebase --abort\n>  '\n> @@ -547,11 +524,7 @@ test_expect_success 'auto-amend only edited commits after \"edit\"' '\n>  \techo \"and again\" > file7 &&\n>  \tgit add file7 &&\n>  \ttest_tick &&\n> -\t(\n> -\t\tFAKE_COMMIT_MESSAGE=\"and again\" &&\n> -\t\texport FAKE_COMMIT_MESSAGE &&\n> -\t\ttest_must_fail git rebase --continue\n> -\t) &&\n> +\ttest_must_fail env FAKE_COMMIT_MESSAGE=\"and again\" git rebase --continue &&\n>  \tgit rebase --abort\n>  '\n>\n> @@ -559,11 +532,7 @@ test_expect_success 'clean error after failed \"exec\"' '\n>  \ttest_tick &&\n>  \ttest_when_finished \"git rebase --abort || :\" &&\n>  \tset_fake_editor &&\n> -\t(\n> -\t\tFAKE_LINES=\"1 exec_false\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i HEAD^\n> -\t) &&\n> +\ttest_must_fail env FAKE_LINES=\"1 exec_false\" git rebase -i HEAD^ &&\n>  \techo \"edited again\" > file7 &&\n>  \tgit add file7 &&\n>  \ttest_must_fail git rebase --continue 2>error &&\n> @@ -947,12 +916,8 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n>\n>  test_expect_success 'rebase -i --root temporary sentinel commit' '\n>  \tgit checkout B &&\n> -\t(\n> -\t\tset_fake_editor &&\n> -\t\tFAKE_LINES=\"2\" &&\n> -\t\texport FAKE_LINES &&\n> -\t\ttest_must_fail git rebase -i --root\n> -\t) &&\n> +\tset_fake_editor &&\n> +\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root &&\n>  \tgit cat-file commit HEAD | grep \"^tree 4b825dc642cb\" &&\n>  \tgit rebase --abort\n>  '\n> @@ -1042,11 +1007,7 @@ test_expect_success 'rebase -i error on commits with \\ in message' '\n>  \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n>  \ttest_commit TO-REMOVE will-conflict old-content &&\n>  \ttest_commit \"\\temp\" will-conflict new-content dummy &&\n> -\t(\n> -\tEDITOR=true &&\n> -\texport EDITOR &&\n> -\ttest_must_fail git rebase -i HEAD^ --onto HEAD^^ 2>error\n> -\t) &&\n> +\ttest_must_fail env EDITOR=true git rebase -i HEAD^ --onto HEAD^^ 2>error &&\n>  \ttest_expect_code 1 grep  \"\temp\" error\n>  '\n>\n> diff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh\n> index 098b755..b6833e9 100755\n> --- a/t/t3413-rebase-hook.sh\n> +++ b/t/t3413-rebase-hook.sh\n> @@ -118,11 +118,7 @@ test_expect_success 'pre-rebase hook stops rebase (1)' '\n>  test_expect_success 'pre-rebase hook stops rebase (2)' '\n>  \tgit checkout test &&\n>  \tgit reset --hard side &&\n> -\t(\n> -\t\tEDITOR=:\n> -\t\texport EDITOR\n> -\t\ttest_must_fail git rebase -i master\n> -\t) &&\n> +\ttest_must_fail env EDITOR=: git rebase -i master &&\n>  \ttest \"z$(git symbolic-ref HEAD)\" = zrefs/heads/test &&\n>  \ttest 0 = $(git rev-list HEAD...side | wc -l)\n>  '\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 73194b2..9c80633 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -764,22 +764,14 @@ test_expect_success 'format-patch --signature=\"\" suppresses signatures' '\n>\n>  test_expect_success TTY 'format-patch --stdout paginates' '\n>  \trm -f pager_used &&\n> -\t(\n> -\t\tGIT_PAGER=\"wc >pager_used\" &&\n> -\t\texport GIT_PAGER &&\n> -\t\ttest_terminal git format-patch --stdout --all\n> -\t) &&\n> +\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git format-patch --stdout --all &&\n>  \ttest_path_is_file pager_used\n>  '\n>\n>   test_expect_success TTY 'format-patch --stdout pagination can be disabled' '\n>  \trm -f pager_used &&\n> -\t(\n> -\t\tGIT_PAGER=\"wc >pager_used\" &&\n> -\t\texport GIT_PAGER &&\n> -\t\ttest_terminal git --no-pager format-patch --stdout --all &&\n> -\t\ttest_terminal git -c \"pager.format-patch=false\" format-patch --stdout --all\n> -\t) &&\n> +\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git --no-pager format-patch --stdout --all &&\n> +\ttest_terminal env GIT_PAGER=\"wc >pager_used\" git -c \"pager.format-patch=false\" format-patch --stdout --all &&\n>  \ttest_path_is_missing pager_used &&\n>  \ttest_path_is_missing .git/pager_used\n>  '\n> diff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\n> index b061864..21517c7 100755\n> --- a/t/t5305-include-tag.sh\n> +++ b/t/t5305-include-tag.sh\n> @@ -45,9 +45,7 @@ test_expect_success 'unpack objects' '\n>  test_expect_success 'check unpacked result (have commit, no tag)' '\n>  \tgit rev-list --objects $commit >list.expect &&\n>  \t(\n> -\t\tGIT_DIR=clone.git &&\n> -\t\texport GIT_DIR &&\n> -\t\ttest_must_fail git cat-file -e $tag &&\n> +\t\ttest_must_fail env GIT_DIR=clone.git git cat-file -e $tag &&\n>  \t\tgit rev-list --objects $commit\n>  \t) >list.actual &&\n>  \ttest_cmp list.expect list.actual\n> diff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh\n> index 3f353d9..cbcceab 100755\n> --- a/t/t5602-clone-remote-exec.sh\n> +++ b/t/t5602-clone-remote-exec.sh\n> @@ -12,21 +12,14 @@ test_expect_success setup '\n>  '\n>\n>  test_expect_success 'clone calls git upload-pack unqualified with no -u option' '\n> -\t(\n> -\t\tGIT_SSH=./not_ssh &&\n> -\t\texport GIT_SSH &&\n> -\t\ttest_must_fail git clone localhost:/path/to/repo junk\n> -\t) &&\n> +\ttest_must_fail env GIT_SSH=./not_ssh git clone localhost:/path/to/repo junk &&\n>  \techo \"localhost git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n>  \ttest_cmp expected not_ssh_output\n>  '\n>\n>  test_expect_success 'clone calls specified git upload-pack with -u option' '\n> -\t(\n> -\t\tGIT_SSH=./not_ssh &&\n> -\t\texport GIT_SSH &&\n> -\t\ttest_must_fail git clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk\n> -\t) &&\n> +\ttest_must_fail env GIT_SSH=./not_ssh \\\n> +\t\tgit clone -u ./something/bin/git-upload-pack localhost:/path/to/repo junk &&\n>  \techo \"localhost ./something/bin/git-upload-pack '\\''/path/to/repo'\\''\" >expected &&\n>  \ttest_cmp expected not_ssh_output\n>  '\n> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\n> index 613f69a..ca19838 100755\n> --- a/t/t5801-remote-helpers.sh\n> +++ b/t/t5801-remote-helpers.sh\n> @@ -218,10 +218,8 @@ test_expect_success 'proper failure checks for fetching' '\n>  '\n>\n>  test_expect_success 'proper failure checks for pushing' '\n> -\t(GIT_REMOTE_TESTGIT_FAILURE=1 &&\n> -\texport GIT_REMOTE_TESTGIT_FAILURE &&\n> -\tcd local &&\n> -\ttest_must_fail git push --all\n> +\t(cd local &&\n> +\ttest_must_fail env GIT_REMOTE_TESTGIT_FAILURE=1 git push --all\n>  \t)\n>  '\n>\n> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\n> index 9874403..9d9d9de 100755\n> --- a/t/t6006-rev-list-format.sh\n> +++ b/t/t6006-rev-list-format.sh\n> @@ -190,12 +190,9 @@ test_expect_success '%C(auto) respects --no-color' '\n>  '\n>\n>  test_expect_success TTY '%C(auto) respects --color=auto (stdout is tty)' '\n> -\t(\n> -\t\tTERM=vt100 && export TERM &&\n> -\t\ttest_terminal \\\n> -\t\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n> -\t\thas_color actual\n> -\t)\n> +\ttest_terminal env TERM=vt100 \\\n> +\t\tgit log --format=$AUTO_COLOR -1 --color=auto >actual &&\n> +\thas_color actual\n>  '\n>\n>  test_expect_success '%C(auto) respects --color=auto (stdout not tty)' '\n> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\n> index b9365b4..da958a8 100755\n> --- a/t/t7006-pager.sh\n> +++ b/t/t7006-pager.sh\n> @@ -146,11 +146,7 @@ test_expect_success 'no color when stdout is a regular file' '\n>  test_expect_success TTY 'color when writing to a pager' '\n>  \trm -f paginated.out &&\n>  \ttest_config color.ui auto &&\n> -\t(\n> -\t\tTERM=vt100 &&\n> -\t\texport TERM &&\n> -\t\ttest_terminal git log\n> -\t) &&\n> +\ttest_terminal env TERM=vt100 git log &&\n>  \tcolorful paginated.out\n>  '\n>\n> @@ -158,11 +154,7 @@ test_expect_success TTY 'colors are suppressed by color.pager' '\n>  \trm -f paginated.out &&\n>  \ttest_config color.ui auto &&\n>  \ttest_config color.pager false &&\n> -\t(\n> -\t\tTERM=vt100 &&\n> -\t\texport TERM &&\n> -\t\ttest_terminal git log\n> -\t) &&\n> +\ttest_terminal env TERM=vt100 git log &&\n>  \t! colorful paginated.out\n>  '\n>\n> @@ -181,11 +173,7 @@ test_expect_success 'color when writing to a file intended for a pager' '\n>  test_expect_success TTY 'colors are sent to pager for external commands' '\n>  \ttest_config alias.externallog \"!git log\" &&\n>  \ttest_config color.ui auto &&\n> -\t(\n> -\t\tTERM=vt100 &&\n> -\t\texport TERM &&\n> -\t\ttest_terminal git -p externallog\n> -\t) &&\n> +\ttest_terminal env TERM=vt100 git -p externallog &&\n>  \tcolorful paginated.out\n>  '\n>\n> --\n> 1.7.9\n"},{"id":"237012","messageId":"CAPig+cS5UDxvCGRm4d840tOfG6pwjHbARuyAWOR+D_Aht79Gzw@mail.gmail.com","threadId":"36209","inReplyTo":"1395144518-2489-1-git-send-email-unsignedzero@gmail.com","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-18T20:52:46Z","receivedAt":"2014-03-18T20:52:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 18, 2014 at 8:08 AM, David Tran <unsignedzero@gmail.com> wrote:\n> Originally, the code used subshells instead of FOO=BAR command because\n> the variable would otherwise leak into the surrounding context of the POSIX\n> shell when 'command' is a shell function. The subshell was used to hold the\n> context for the test. Using 'env' in the test function sets the temp variables\n> without leaking, removing the need of a subshell.\n>\n> Signed-off-by: David Tran <unsignedzero@gmail.com>\n> ---\n>> Oh, really ;-)?\n> Missed that.\n>\n>> Thanks.  Getting closer, I think.\n> Slowly but surely.\n\nGetting better. See below.\n\n> Signed-off-by: David Tran <unsignedzero@gmail.com>\n> ---\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 50e22b1..4c7364a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -104,9 +104,7 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n>         git checkout master &&\n>         (\n>         set_fake_editor &&\n> -       FAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n> -       export FAKE_LINES &&\n> -       test_must_fail git rebase -i HEAD^\n> +       test_must_fail env FAKE_LINES=\"exec_echo_foo_>file1 1\" git rebase -i HEAD^\n>         ) &&\n\nIn a previous review, I asked if this subshell could be dropped or if\nit was required for set_fake_editor. I didn't quite understand your\nresponse, so I tested it myself, and found that the subshell can be\neliminated safely without breaking this or later tests.\n\n>         test_cmp_rev master^ HEAD &&\n>         git reset --hard &&\n> @@ -118,9 +116,8 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n>         test_when_finished \"git rebase --abort\" &&\n>         (\n>         set_fake_editor &&\n> -       FAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n> -       export FAKE_LINES &&\n> -       test_must_fail git rebase -i HEAD^ >actual 2>&1\n> +       test_must_fail env FAKE_LINES=\"exec_this-command-does-not-exist 1\" \\\n> +       git rebase -i HEAD^ >actual 2>&1\n>         ) &&\n\nDitto for this subshell.\n\n>         ! grep \"Maybe git-rebase is broken\" actual\n>  '\n> @@ -528,11 +509,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n>         FAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n>         echo \"edited again\" > file7 &&\n>         git add file7 &&\n> -       (\n> -               FAKE_COMMIT_MESSAGE=\" \" &&\n> -               export FAKE_COMMIT_MESSAGE &&\n> -               test_must_fail git rebase --continue\n> -       ) &&\n> +       test_must_fail env FAKE_COMMIT_MESSAGE=\" \" git rebase --continue\n\nBroken &&-chain.\n\n>         test $old = $(git rev-parse HEAD) &&\n>         git rebase --abort\n>  '\n"},{"id":"237014","messageId":"20140318214536.GA10076@sigill.intra.peff.net","threadId":"36209","inReplyTo":"xmqqd2hj6y5o.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T21:45:36Z","receivedAt":"2014-03-18T21:45:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:\n\n> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> > index c9c426c..3e3f77b 100755\n> > --- a/t/t1300-repo-config.sh\n> > +++ b/t/t1300-repo-config.sh\n> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n> >  '\n> >\n> >  test_expect_success 'nonexistent configuration' '\n> > -\t(\n> > -\t\tGIT_CONFIG=doesnotexist &&\n> > -\t\texport GIT_CONFIG &&\n> > -\t\ttest_must_fail git config --list &&\n> > -\t\ttest_must_fail git config test.xyzzy\n> > -\t)\n> > +\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n> > +\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n> >  '\n\nIsn't GIT_CONFIG here another way of saying:\n\n  test_must_fail git config -f doesnotexist --list\n\nPerhaps that is shorter and more readable still (and there are a few\nsimilar cases in this patch.\n\n-Peff\n"},{"id":"237016","messageId":"xmqqy5075f0k.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"20140318214536.GA10076@sigill.intra.peff.net","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-18T22:16:27Z","receivedAt":"2014-03-18T22:16:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:\n>\n>> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n>> > index c9c426c..3e3f77b 100755\n>> > --- a/t/t1300-repo-config.sh\n>> > +++ b/t/t1300-repo-config.sh\n>> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n>> >  '\n>> >\n>> >  test_expect_success 'nonexistent configuration' '\n>> > -\t(\n>> > -\t\tGIT_CONFIG=doesnotexist &&\n>> > -\t\texport GIT_CONFIG &&\n>> > -\t\ttest_must_fail git config --list &&\n>> > -\t\ttest_must_fail git config test.xyzzy\n>> > -\t)\n>> > +\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n>> > +\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n>> >  '\n>\n> Isn't GIT_CONFIG here another way of saying:\n>\n>   test_must_fail git config -f doesnotexist --list\n>\n> Perhaps that is shorter and more readable still (and there are a few\n> similar cases in this patch.\n\nSurely, but are we assuming that \"git config\" correctly honors the\nequivalence between GIT_CONFIG=file and -f file, or is that also\nsomething we are testing in these tests?\n"},{"id":"237020","messageId":"CAPig+cRHJmzZMahNaz641t8i7di3YinYV=OQPd=-zDo6U3NkQg@mail.gmail.com","threadId":"36209","inReplyTo":"20140318214536.GA10076@sigill.intra.peff.net","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-18T22:36:29Z","receivedAt":"2014-03-18T22:36:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 18, 2014 at 5:45 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Mar 18, 2014 at 01:37:39PM -0700, Junio C Hamano wrote:\n>\n>> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n>> > index c9c426c..3e3f77b 100755\n>> > --- a/t/t1300-repo-config.sh\n>> > +++ b/t/t1300-repo-config.sh\n>> > @@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n>> >  '\n>> >\n>> >  test_expect_success 'nonexistent configuration' '\n>> > -   (\n>> > -           GIT_CONFIG=doesnotexist &&\n>> > -           export GIT_CONFIG &&\n>> > -           test_must_fail git config --list &&\n>> > -           test_must_fail git config test.xyzzy\n>> > -   )\n>> > +   test_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n>> > +   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n>> >  '\n>\n> Isn't GIT_CONFIG here another way of saying:\n>\n>   test_must_fail git config -f doesnotexist --list\n>\n> Perhaps that is shorter and more readable still (and there are a few\n> similar cases in this patch.\n\nSuch a change could be the subject of a separate cleanup patch, but is\ntangental to the GSoC microproject which begat this submission.\n"},{"id":"237022","messageId":"20140318230658.GA10679@sigill.intra.peff.net","threadId":"36209","inReplyTo":"xmqqy5075f0k.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-18T23:06:58Z","receivedAt":"2014-03-18T23:06:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 18, 2014 at 03:16:27PM -0700, Junio C Hamano wrote:\n\n> > Isn't GIT_CONFIG here another way of saying:\n> >\n> >   test_must_fail git config -f doesnotexist --list\n> >\n> > Perhaps that is shorter and more readable still (and there are a few\n> > similar cases in this patch.\n> \n> Surely, but are we assuming that \"git config\" correctly honors the\n> equivalence between GIT_CONFIG=file and -f file, or is that also\n> something we are testing in these tests?\n\nI think we can assume that they are equivalent, and it is not worth\ntesting (and they are equivalent in code since 270a344 (config: stop\nusing config_exclusive_filename, 2012-02-16).\n\nMy recollection is that GIT_CONFIG mostly exists as a historical\nfootnote. Recall that at one time it affected all commands, but that had\nmany problems and was done away with in dc87183 (Only use GIT_CONFIG in\n\"git config\", not other programs, 2008-06-30). I think we left it in\nplace for git-config mostly for backward compatibility, but I didn't see\nthat point explicitly addressed in the list discussion (the main issue\nwas that setting it for things besides \"git config\" is a bad idea, as it\nsuppresses ~/.gitconfig).\n\n-Peff\n"},{"id":"237090","messageId":"xmqqzjkm3xo1.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"20140318230658.GA10679@sigill.intra.peff.net","subject":"Re: [PATCH v2] tests: set temp variables using 'env' in test function instead of subshell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T17:28:46Z","receivedAt":"2014-03-19T17:28:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Mar 18, 2014 at 03:16:27PM -0700, Junio C Hamano wrote:\n>\n>> > Isn't GIT_CONFIG here another way of saying:\n>> >\n>> >   test_must_fail git config -f doesnotexist --list\n>> >\n>> > Perhaps that is shorter and more readable still (and there are a few\n>> > similar cases in this patch.\n>> \n>> Surely, but are we assuming that \"git config\" correctly honors the\n>> equivalence between GIT_CONFIG=file and -f file, or is that also\n>> something we are testing in these tests?\n>\n> I think we can assume that they are equivalent, and it is not worth\n> testing (and they are equivalent in code since 270a344 (config: stop\n> using config_exclusive_filename, 2012-02-16).\n>\n> My recollection is that GIT_CONFIG mostly exists as a historical\n> footnote. Recall that at one time it affected all commands, but that had\n> many problems and was done away with in dc87183 (Only use GIT_CONFIG in\n> \"git config\", not other programs, 2008-06-30). I think we left it in\n> place for git-config mostly for backward compatibility,...\n\nThanks.  Then I think it makes sense to do such a conversion but it\nprobably should be done on top of this patch (we could do it before\nthis patch), not as a part of this patch.\n"},{"id":"237224","messageId":"20140320231159.GA7774@sigill.intra.peff.net","threadId":"36209","inReplyTo":"xmqqzjkm3xo1.fsf@gitster.dls.corp.google.com","subject":"[PATCH 0/12] GIT_CONFIG in the test suite","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:11:59Z","receivedAt":"2014-03-20T23:11:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 19, 2014 at 10:28:46AM -0700, Junio C Hamano wrote:\n\n> [git config --file versus GIT_CONFIG=]\n>\n> Thanks.  Then I think it makes sense to do such a conversion but it\n> probably should be done on top of this patch (we could do it before\n> this patch), not as a part of this patch.\n\nHere's a series that goes on top of what you queued in\ndt/tests-with-env-not-subshell. Once I started cleaning, I noticed a lot\nof room for improvement and modernization in t0001. I hope I didn't get\ntoo carried away.\n\n  [01/12]: t/Makefile: stop setting GIT_CONFIG\n  [02/12]: t/test-lib: drop redundant unset of GIT_CONFIG\n  [03/12]: t: drop useless sane_unset GIT_* calls\n  [04/12]: t: stop using GIT_CONFIG to cross repo boundaries\n  [05/12]: t: prefer \"git config --file\" to GIT_CONFIG with test_must_fail\n  [06/12]: t: prefer \"git config --file\" to GIT_CONFIG\n  [07/12]: t0001: make symlink reinit test more careful\n  [08/12]: t0001: use test_path_is_*\n  [09/12]: t0001: use test_config_global\n  [10/12]: t0001: use test_must_fail\n  [11/12]: t0001: drop useless subshells\n  [12/12]: t0001: drop subshells just for \"cd\"\n\n t/Makefile                      |   4 +-\n t/t0001-init.sh                 | 211 ++++++++++-------------------------\n t/t1300-repo-config.sh          |  28 ++---\n t/t1302-repo-version.sh         |   2 +-\n t/t5701-clone-local.sh          |   6 +-\n t/t7400-submodule-basic.sh      |   5 +-\n t/t9130-git-svn-authors-file.sh |   3 +-\n t/t9154-git-svn-fancy-glob.sh   |   6 +-\n t/t9400-git-cvsserver-server.sh |   1 -\n t/test-lib.sh                   |   1 -\n 10 files changed, 87 insertions(+), 180 deletions(-)\n\n-Peff\n"},{"id":"237225","messageId":"20140320231321.GA8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 01/12] t/Makefile: stop setting GIT_CONFIG","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:13:21Z","receivedAt":"2014-03-20T23:13:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Once upon a time, the setting of GIT_CONFIG in the\nenvironment could affect how tests ran. Commit 9c3796f (Fix\nsetting config variables with an alternative GIT_CONFIG,\n2006-06-20) unconditionally set GIT_CONFIG in the Makefile\nwhen running tests to give us a known starting point.\n\nThis is insufficient for running the tests outside of the\nMakefile, however, and 8565d2d (Make tests independent of\nglobal config files, 2007-02-15) later set GIT_CONFIG\ndirectly in test-lib.sh. At that point the Makefile setting\nwas redundant, but we never removed it. Let's do so now.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 2373a04..8fd1a72 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -36,11 +36,11 @@ test: pre-clean $(TEST_LINT)\n \t$(MAKE) aggregate-results-and-cleanup\n \n prove: pre-clean $(TEST_LINT)\n-\t@echo \"*** prove ***\"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n+\t@echo \"*** prove ***\"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n \t$(MAKE) clean-except-prove-cache\n \n $(T):\n-\t@echo \"*** $@ ***\"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n \n pre-clean:\n \t$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237226","messageId":"20140320231336.GB8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 02/12] t/test-lib: drop redundant unset of GIT_CONFIG","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:13:36Z","receivedAt":"2014-03-20T23:13:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is already handled by the mass GIT_* unsetting added by\n95a1d12 (tests: scrub environment of GIT_* variables,\n2011-03-15).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/test-lib.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1531c24..625f06e 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -649,7 +649,6 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:\n \tfi\n fi\n GIT_TEMPLATE_DIR=\"$GIT_BUILD_DIR\"/templates/blt\n-unset GIT_CONFIG\n GIT_CONFIG_NOSYSTEM=1\n GIT_ATTR_NOSYSTEM=1\n export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237228","messageId":"20140320231433.GC8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 03/12] t: drop useless sane_unset GIT_* calls","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:14:33Z","receivedAt":"2014-03-20T23:14:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Several test scripts manually unset GIT_CONFIG and other\nGIT_* variables. These are generally taken care of for us by\ntest-lib.sh already.\n\nUnsetting these is not only useless, but can be confusing to\na reader, who may wonder why some tests in a script unset\nthem and others do not (t0001 is particularly guilty of this\ninconsistency, probably because many of its tests predate\nthe test-lib.sh environment-cleansing).\n\nNote that we cannot always get rid of such unsetting. For\nexample, t9130 can drop the GIT_CONFIG unset, but not the\nGIT_DIR one, because lib-git-svn.sh sets the latter. And in\nt1000, we unset GIT_TEMPLATE_DIR, which is explicitly\ninitialized by test-lib.sh.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI suppose one could make an argument that test-lib.sh may later change\nthe set of variables it clears, and these unsets are documenting an\nexplicit need of each test. I'd find that more compelling if it were\nactually applied consistently.\n\n t/t0001-init.sh                 | 15 ---------------\n t/t9130-git-svn-authors-file.sh |  1 -\n t/t9400-git-cvsserver-server.sh |  1 -\n 3 files changed, 17 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 9fb582b..ddc8160 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -25,7 +25,6 @@ check_config () {\n \n test_expect_success 'plain' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n \t\tmkdir plain &&\n \t\tcd plain &&\n \t\tgit init\n@@ -35,7 +34,6 @@ test_expect_success 'plain' '\n \n test_expect_success 'plain nested in bare' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n \t\tgit init --bare bare-ancestor.git &&\n \t\tcd bare-ancestor.git &&\n \t\tmkdir plain-nested &&\n@@ -47,7 +45,6 @@ test_expect_success 'plain nested in bare' '\n \n test_expect_success 'plain through aliased command, outside any git repo' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n \t\tHOME=$(pwd)/alias-config &&\n \t\texport HOME &&\n \t\tmkdir alias-config &&\n@@ -65,7 +62,6 @@ test_expect_success 'plain through aliased command, outside any git repo' '\n \n test_expect_failure 'plain nested through aliased command' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n \t\tgit init plain-ancestor-aliased &&\n \t\tcd plain-ancestor-aliased &&\n \t\techo \"[alias] aliasedinit = init\" >>.git/config &&\n@@ -78,7 +74,6 @@ test_expect_failure 'plain nested through aliased command' '\n \n test_expect_failure 'plain nested in bare through aliased command' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n \t\tgit init --bare bare-ancestor-aliased.git &&\n \t\tcd bare-ancestor-aliased.git &&\n \t\techo \"[alias] aliasedinit = init\" >>config &&\n@@ -91,7 +86,6 @@ test_expect_failure 'plain nested in bare through aliased command' '\n \n test_expect_success 'plain with GIT_WORK_TREE' '\n \tif (\n-\t\tsane_unset GIT_DIR &&\n \t\tmkdir plain-wt &&\n \t\tcd plain-wt &&\n \t\tGIT_WORK_TREE=$(pwd) git init\n@@ -104,7 +98,6 @@ test_expect_success 'plain with GIT_WORK_TREE' '\n \n test_expect_success 'plain bare' '\n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&\n \t\tmkdir plain-bare-1 &&\n \t\tcd plain-bare-1 &&\n \t\tgit --bare init\n@@ -114,7 +107,6 @@ test_expect_success 'plain bare' '\n \n test_expect_success 'plain bare with GIT_WORK_TREE' '\n \tif (\n-\t\tsane_unset GIT_DIR GIT_CONFIG &&\n \t\tmkdir plain-bare-2 &&\n \t\tcd plain-bare-2 &&\n \t\tGIT_WORK_TREE=$(pwd) git --bare init\n@@ -128,7 +120,6 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '\n test_expect_success 'GIT_DIR bare' '\n \n \t(\n-\t\tsane_unset GIT_CONFIG &&\n \t\tmkdir git-dir-bare.git &&\n \t\tGIT_DIR=git-dir-bare.git git init\n \t) &&\n@@ -138,7 +129,6 @@ test_expect_success 'GIT_DIR bare' '\n test_expect_success 'init --bare' '\n \n \t(\n-\t\tsane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&\n \t\tmkdir init-bare.git &&\n \t\tcd init-bare.git &&\n \t\tgit init --bare\n@@ -149,7 +139,6 @@ test_expect_success 'init --bare' '\n test_expect_success 'GIT_DIR non-bare' '\n \n \t(\n-\t\tsane_unset GIT_CONFIG &&\n \t\tmkdir non-bare &&\n \t\tcd non-bare &&\n \t\tGIT_DIR=.git git init\n@@ -160,7 +149,6 @@ test_expect_success 'GIT_DIR non-bare' '\n test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '\n \n \t(\n-\t\tsane_unset GIT_CONFIG &&\n \t\tmkdir git-dir-wt-1.git &&\n \t\tGIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init\n \t) &&\n@@ -170,7 +158,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '\n test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '\n \n \tif (\n-\t\tsane_unset GIT_CONFIG &&\n \t\tmkdir git-dir-wt-2.git &&\n \t\tGIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init\n \t)\n@@ -183,8 +170,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '\n test_expect_success 'reinit' '\n \n \t(\n-\t\tsane_unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG &&\n-\n \t\tmkdir again &&\n \t\tcd again &&\n \t\tgit init >out1 2>err1 &&\ndiff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh\nindex c3443ce..a812783 100755\n--- a/t/t9130-git-svn-authors-file.sh\n+++ b/t/t9130-git-svn-authors-file.sh\n@@ -97,7 +97,6 @@ test_expect_success 'fresh clone with svn.authors-file in config' '\n \t\ttest x = x\"$(git config svn.authorsfile)\" &&\n \t\ttest_config=\"$HOME\"/.gitconfig &&\n \t\tsane_unset GIT_DIR &&\n-\t\tsane_unset GIT_CONFIG &&\n \t\tgit config --global \\\n \t\t  svn.authorsfile \"$HOME\"/svn-authors &&\n \t\ttest x\"$HOME\"/svn-authors = x\"$(git config svn.authorsfile)\" &&\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 3edc408..ed98e64 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -25,7 +25,6 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n     test_done\n }\n \n-unset GIT_DIR GIT_CONFIG\n WORKDIR=$(pwd)\n SERVERDIR=$(pwd)/gitcvs.git\n git_config=\"$SERVERDIR/config\"\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237229","messageId":"20140320231524.GD8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:15:24Z","receivedAt":"2014-03-20T23:15:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some tests want to check or set config in another\nrepository. E.g., t1000 creates repositories and makes sure\nthat their core.bare and core.worktree settings are what we\nexpect. We can do this with:\n\n  GIT_CONFIG=$repo/.git/config git config ...\n\nbut it better shows the intent to just enter the repository\nand let \"git config\" do the normal lookups:\n\n  (cd $repo && git config ...)\n\nIn theory, this would cause us to use an extra subshell, but\nin all such cases, we are actually already in a subshell.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t0001-init.sh        | 4 ++--\n t/t5701-clone-local.sh | 6 +++---\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex ddc8160..9b05fdf 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -12,8 +12,8 @@ check_config () {\n \t\techo \"expected a directory $1, a file $1/config and $1/refs\"\n \t\treturn 1\n \tfi\n-\tbare=$(GIT_CONFIG=\"$1/config\" git config --bool core.bare)\n-\tworktree=$(GIT_CONFIG=\"$1/config\" git config core.worktree) ||\n+\tbare=$(cd \"$1\" && git config --bool core.bare)\n+\tworktree=$(cd \"$1\" && git config core.worktree) ||\n \tworktree=unset\n \n \ttest \"$bare\" = \"$2\" && test \"$worktree\" = \"$3\" || {\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex c490368..3c087e9 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -12,8 +12,8 @@ test_expect_success 'preparing origin repository' '\n \t: >file && git add . && git commit -m1 &&\n \tgit clone --bare . a.git &&\n \tgit clone --bare . x &&\n-\ttest \"$(GIT_CONFIG=a.git/config git config --bool core.bare)\" = true &&\n-\ttest \"$(GIT_CONFIG=x/config git config --bool core.bare)\" = true &&\n+\ttest \"$(cd a.git && git config --bool core.bare)\" = true &&\n+\ttest \"$(cd x && git config --bool core.bare)\" = true &&\n \tgit bundle create b1.bundle --all &&\n \tgit bundle create b2.bundle master &&\n \tmkdir dir &&\n@@ -24,7 +24,7 @@ test_expect_success 'preparing origin repository' '\n test_expect_success 'local clone without .git suffix' '\n \tgit clone -l -s a b &&\n \t(cd b &&\n-\ttest \"$(GIT_CONFIG=.git/config git config --bool core.bare)\" = false &&\n+\ttest \"$(git config --bool core.bare)\" = false &&\n \tgit fetch)\n '\n \n-- \n1.9.0.560.g01ceb46\n"},{"id":"237230","messageId":"20140320231554.GE8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 05/12] t: prefer \"git config --file\" to GIT_CONFIG with test_must_fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:15:54Z","receivedAt":"2014-03-20T23:15:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This lets us get rid of an extra \"env\" invocation in the\nmiddle, and is slightly more readable.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe case that started this all...\n\nThis is also the only reason this series needs to go on top of David's\npatch.\n\n t/t1300-repo-config.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex cd23d07..e355aa1 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -961,15 +961,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n '\n \n test_expect_success 'nonexistent configuration' '\n-\ttest_must_fail env GIT_CONFIG=doesnotexist git config --list &&\n-\ttest_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy\n+\ttest_must_fail git config --file=doesnotexist --list &&\n+\ttest_must_fail git config --file=doesnotexist test.xyzzy\n '\n \n test_expect_success SYMLINKS 'symlink to nonexistent configuration' '\n \tln -s doesnotexist linktonada &&\n \tln -s linktonada linktolinktonada &&\n-\ttest_must_fail env GIT_CONFIG=linktonada git config --list &&\n-\ttest_must_fail env GIT_CONFIG=linktolinktonada git config --list\n+\ttest_must_fail git config --file=linktonada --list &&\n+\ttest_must_fail git config --file=linktolinktonada --list\n '\n \n test_expect_success 'check split_cmdline return' \"\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237231","messageId":"20140320231701.GF8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 06/12] t: prefer \"git config --file\" to GIT_CONFIG","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:17:01Z","receivedAt":"2014-03-20T23:17:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Doing:\n\n  GIT_CONFIG=foo git config ...\n\nis equivalent to:\n\n  git config --file=foo ...\n\nThe latter is easier to read and slightly less error-prone,\nbecause of issues with one-shot variables and shell\nfunctions (e.g., you cannot use the former with\ntest_must_fail).\n\nNote that we explicitly leave one case in t1300 which checks\nthe same operation on both GIT_CONFIG and \"git config\n--file\". They are equivalent in the code these days, but\nthis will make sure it remains so.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nUnlike the last patch, this one has no tangible benefits besides \"Peff\nthinks it looks better\". I also tend to think that GIT_CONFIG is\nsomething that it would be nice to get rid of in the long run, but I\ndon't have any immediate plans to do so.\n\n t/t1300-repo-config.sh          | 20 ++++++++++----------\n t/t1302-repo-version.sh         |  2 +-\n t/t7400-submodule-basic.sh      |  5 ++---\n t/t9130-git-svn-authors-file.sh |  2 +-\n t/t9154-git-svn-fancy-glob.sh   |  6 +++---\n 5 files changed, 17 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex e355aa1..85c6637 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -461,7 +461,7 @@ test_expect_success 'new variable inserts into proper section' '\n \ttest_cmp expect .git/config\n '\n \n-test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' '\n+test_expect_success 'alternative --file (non-existing file should fail)' '\n \ttest_must_fail git config --file non-existing-config -l\n '\n \n@@ -495,10 +495,10 @@ test_expect_success 'refer config from subdirectory' '\n \n '\n \n-test_expect_success 'refer config from subdirectory via GIT_CONFIG' '\n+test_expect_success 'refer config from subdirectory via --file' '\n \t(\n \t\tcd x &&\n-\t\tGIT_CONFIG=../other-config git config --get ein.bahn >actual &&\n+\t\tgit config --file=../other-config --get ein.bahn >actual &&\n \t\ttest_cmp expect actual\n \t)\n '\n@@ -510,8 +510,8 @@ cat > expect << EOF\n \tpark = ausweis\n EOF\n \n-test_expect_success '--set in alternative GIT_CONFIG' '\n-\tGIT_CONFIG=other-config git config anwohner.park ausweis &&\n+test_expect_success '--set in alternative file' '\n+\tgit config --file=other-config anwohner.park ausweis &&\n \ttest_cmp expect other-config\n '\n \n@@ -942,11 +942,11 @@ test_expect_success 'inner whitespace kept verbatim' '\n \n test_expect_success SYMLINKS 'symlinked configuration' '\n \tln -s notyet myconfig &&\n-\tGIT_CONFIG=myconfig git config test.frotz nitfol &&\n+\tgit config --file=myconfig test.frotz nitfol &&\n \ttest -h myconfig &&\n \ttest -f notyet &&\n-\ttest \"z$(GIT_CONFIG=notyet git config test.frotz)\" = znitfol &&\n-\tGIT_CONFIG=myconfig git config test.xyzzy rezrov &&\n+\ttest \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n+\tgit config --file=myconfig test.xyzzy rezrov &&\n \ttest -h myconfig &&\n \ttest -f notyet &&\n \tcat >expect <<-\\EOF &&\n@@ -954,8 +954,8 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n \trezrov\n \tEOF\n \t{\n-\t\tGIT_CONFIG=notyet git config test.frotz &&\n-\t\tGIT_CONFIG=notyet git config test.xyzzy\n+\t\tgit config --file=notyet test.frotz &&\n+\t\tgit config --file=notyet test.xyzzy\n \t} >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex 0e47662..0d9388a 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -19,7 +19,7 @@ test_expect_success 'setup' '\n \n \ttest_create_repo \"test\" &&\n \ttest_create_repo \"test2\" &&\n-\tGIT_CONFIG=test2/.git/config git config core.repositoryformatversion 99\n+\tgit config --file=test2/.git/config core.repositoryformatversion 99\n '\n \n test_expect_success 'gitdir selection on normal repos' '\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex c28e8d8..7c88245 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -249,8 +249,7 @@ test_expect_success 'submodule add in subdirectory with relative path should fai\n '\n \n test_expect_success 'setup - add an example entry to .gitmodules' '\n-\tGIT_CONFIG=.gitmodules \\\n-\tgit config submodule.example.url git://example.com/init.git\n+\tgit config --file=.gitmodules submodule.example.url git://example.com/init.git\n '\n \n test_expect_success 'status should fail for unmapped paths' '\n@@ -264,7 +263,7 @@ test_expect_success 'setup - map path in .gitmodules' '\n \tpath = init\n EOF\n \n-\tGIT_CONFIG=.gitmodules git config submodule.example.path init &&\n+\tgit config --file=.gitmodules submodule.example.path init &&\n \n \ttest_cmp expect .gitmodules\n '\ndiff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh\nindex a812783..c44de26 100755\n--- a/t/t9130-git-svn-authors-file.sh\n+++ b/t/t9130-git-svn-authors-file.sh\n@@ -67,7 +67,7 @@ test_expect_success 'fetch fails on ee' '\n \t'\n \n tmp_config_get () {\n-\tGIT_CONFIG=.git/svn/.metadata git config --get \"$1\"\n+\tgit config --file=.git/svn/.metadata --get \"$1\"\n }\n \n test_expect_success 'failure happened without negative side effects' '\ndiff --git a/t/t9154-git-svn-fancy-glob.sh b/t/t9154-git-svn-fancy-glob.sh\nindex b780e0e..a0150f0 100755\n--- a/t/t9154-git-svn-fancy-glob.sh\n+++ b/t/t9154-git-svn-fancy-glob.sh\n@@ -22,7 +22,7 @@ test_expect_success 'add red branch' \"\n \t\"\n \n test_expect_success 'add gre branch' \"\n-\tGIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&\n+\tgit config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&\n \tgit config svn-remote.svn.branches 'branches/{red,gre}:refs/remotes/*' &&\n \tgit svn fetch &&\n \tgit rev-parse refs/remotes/red &&\n@@ -31,7 +31,7 @@ test_expect_success 'add gre branch' \"\n \t\"\n \n test_expect_success 'add green branch' \"\n-\tGIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&\n+\tgit config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&\n \tgit config svn-remote.svn.branches 'branches/{red,green}:refs/remotes/*' &&\n \tgit svn fetch &&\n \tgit rev-parse refs/remotes/red &&\n@@ -40,7 +40,7 @@ test_expect_success 'add green branch' \"\n \t\"\n \n test_expect_success 'add all branches' \"\n-\tGIT_CONFIG=.git/svn/.metadata git config --unset svn-remote.svn.branches-maxRev &&\n+\tgit config --file=.git/svn/.metadata --unset svn-remote.svn.branches-maxRev &&\n \tgit config svn-remote.svn.branches 'branches/*:refs/remotes/*' &&\n \tgit svn fetch &&\n \tgit rev-parse refs/remotes/red &&\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237232","messageId":"20140320231715.GG8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 07/12] t0001: make symlink reinit test more careful","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:17:15Z","receivedAt":"2014-03-20T23:17:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In the final test of t0001, we have a repo whose .git is a\nsymlink to a directory \"here\", and we use\n\"--separate-git-dir\" to migrate that to a .git file pointing\nto a different directory. We check that the data is migrated\nto the new directory and that .git looks like a git-file.\n\nWe also check that \"here\" is not a directory, which is\nslightly misleading. It should not be a directory, but\nneither should it be gone. It is the actual resting place of\nthe git-file, and .git remains a symlink to it.\n\nLet's check that more explicitly, both to make our test more\nrobust, and to make further cleanups in this area more\nobvious.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t0001-init.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 9b05fdf..5245711 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -402,8 +402,8 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '\n \t) &&\n \techo \"gitdir: `pwd`/realgitdir\" >expected &&\n \ttest_cmp expected newdir/.git &&\n-\ttest -d realgitdir/refs &&\n-\t! test -d newdir/here\n+\ttest_cmp expected newdir/here &&\n+\ttest -d realgitdir/refs\n '\n \n test_done\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237233","messageId":"20140320231735.GH8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 08/12] t0001: use test_path_is_*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:17:35Z","receivedAt":"2014-03-20T23:17:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"t0001 predates the test_path_is_* helpers, and uses \"test\n-f\" and \"test -d\" directly. Using the helpers provides\nbetter debugging output, and are a little more robust.\nAs opposed to \"! test -d\", test_path_is_missing will\nactually makes sure the path does not exist at all.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t0001-init.sh | 36 ++++++++++++++++++------------------\n 1 file changed, 18 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 5245711..fdcf4b3 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -199,13 +199,13 @@ test_expect_success 'init with --template (blank)' '\n \t\tcd template-plain &&\n \t\tgit init\n \t) &&\n-\ttest -f template-plain/.git/info/exclude &&\n+\ttest_path_is_file template-plain/.git/info/exclude &&\n \t(\n \t\tmkdir template-blank &&\n \t\tcd template-blank &&\n \t\tgit init --template=\n \t) &&\n-\t! test -f template-blank/.git/info/exclude\n+\ttest_path_is_missing template-blank/.git/info/exclude\n '\n \n test_expect_success 'init with init.templatedir set' '\n@@ -263,7 +263,7 @@ test_expect_success 'init creates a new directory' '\n \trm -fr newdir &&\n \t(\n \t\tgit init newdir &&\n-\t\ttest -d newdir/.git/refs\n+\t\ttest_path_is_dir newdir/.git/refs\n \t)\n '\n \n@@ -271,7 +271,7 @@ test_expect_success 'init creates a new bare directory' '\n \trm -fr newdir &&\n \t(\n \t\tgit init --bare newdir &&\n-\t\ttest -d newdir/refs\n+\t\ttest_path_is_dir newdir/refs\n \t)\n '\n \n@@ -280,7 +280,7 @@ test_expect_success 'init recreates a directory' '\n \t(\n \t\tmkdir newdir &&\n \t\tgit init newdir &&\n-\t\ttest -d newdir/.git/refs\n+\t\ttest_path_is_dir newdir/.git/refs\n \t)\n '\n \n@@ -289,14 +289,14 @@ test_expect_success 'init recreates a new bare directory' '\n \t(\n \t\tmkdir newdir &&\n \t\tgit init --bare newdir &&\n-\t\ttest -d newdir/refs\n+\t\ttest_path_is_dir newdir/refs\n \t)\n '\n \n test_expect_success 'init creates a new deep directory' '\n \trm -fr newdir &&\n \tgit init newdir/a/b/c &&\n-\ttest -d newdir/a/b/c/.git/refs\n+\ttest_path_is_dir newdir/a/b/c/.git/refs\n '\n \n test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shared)' '\n@@ -306,7 +306,7 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar\n \t\t# the repository itself should follow \"shared\"\n \t\tumask 002 &&\n \t\tgit init --bare --shared=0660 newdir/a/b/c &&\n-\t\ttest -d newdir/a/b/c/refs &&\n+\t\ttest_path_is_dir newdir/a/b/c/refs &&\n \t\tls -ld newdir/a newdir/a/b > lsab.out &&\n \t\t! grep -v \"^drwxrw[sx]r-x\" lsab.out &&\n \t\tls -ld newdir/a/b/c > lsc.out &&\n@@ -319,7 +319,7 @@ test_expect_success 'init notices EEXIST (1)' '\n \t(\n \t\t>newdir &&\n \t\ttest_must_fail git init newdir &&\n-\t\ttest -f newdir\n+\t\ttest_path_is_file newdir\n \t)\n '\n \n@@ -329,7 +329,7 @@ test_expect_success 'init notices EEXIST (2)' '\n \t\tmkdir newdir &&\n \t\t>newdir/a\n \t\ttest_must_fail git init newdir/a/b &&\n-\t\ttest -f newdir/a\n+\t\ttest_path_is_file newdir/a\n \t)\n '\n \n@@ -345,15 +345,15 @@ test_expect_success POSIXPERM,SANITY 'init notices EPERM' '\n test_expect_success 'init creates a new bare directory with global --bare' '\n \trm -rf newdir &&\n \tgit --bare init newdir &&\n-\ttest -d newdir/refs\n+\ttest_path_is_dir newdir/refs\n '\n \n test_expect_success 'init prefers command line to GIT_DIR' '\n \trm -rf newdir &&\n \tmkdir otherdir &&\n \tGIT_DIR=otherdir git --bare init newdir &&\n-\ttest -d newdir/refs &&\n-\t! test -d otherdir/refs\n+\ttest_path_is_dir newdir/refs &&\n+\ttest_path_is_missing otherdir/refs\n '\n \n test_expect_success 'init with separate gitdir' '\n@@ -361,7 +361,7 @@ test_expect_success 'init with separate gitdir' '\n \tgit init --separate-git-dir realgitdir newdir &&\n \techo \"gitdir: `pwd`/realgitdir\" >expected &&\n \ttest_cmp expected newdir/.git &&\n-\ttest -d realgitdir/refs\n+\ttest_path_is_dir realgitdir/refs\n '\n \n test_expect_success 're-init on .git file' '\n@@ -375,8 +375,8 @@ test_expect_success 're-init to update git link' '\n \t) &&\n \techo \"gitdir: `pwd`/surrealgitdir\" >expected &&\n \ttest_cmp expected newdir/.git &&\n-\ttest -d surrealgitdir/refs &&\n-\t! test -d realgitdir/refs\n+\ttest_path_is_dir surrealgitdir/refs &&\n+\ttest_path_is_missing realgitdir/refs\n '\n \n test_expect_success 're-init to move gitdir' '\n@@ -388,7 +388,7 @@ test_expect_success 're-init to move gitdir' '\n \t) &&\n \techo \"gitdir: `pwd`/realgitdir\" >expected &&\n \ttest_cmp expected newdir/.git &&\n-\ttest -d realgitdir/refs\n+\ttest_path_is_dir realgitdir/refs\n '\n \n test_expect_success SYMLINKS 're-init to move gitdir symlink' '\n@@ -403,7 +403,7 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '\n \techo \"gitdir: `pwd`/realgitdir\" >expected &&\n \ttest_cmp expected newdir/.git &&\n \ttest_cmp expected newdir/here &&\n-\ttest -d realgitdir/refs\n+\ttest_path_is_dir realgitdir/refs\n '\n \n test_done\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237234","messageId":"20140320231812.GI8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 09/12] t0001: use test_config_global","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:18:12Z","receivedAt":"2014-03-20T23:18:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We hand-set several config options using :\n\n  git config -f $HOME/.gitconfig ...\n\nInstead, we can use \"test_config_global\". Not only is this\nmore readable, but it cleans up for us so that subsequent\ntests aren't polluted by our settings.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t0001-init.sh | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex fdcf4b3..9515da3 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -211,9 +211,8 @@ test_expect_success 'init with --template (blank)' '\n test_expect_success 'init with init.templatedir set' '\n \tmkdir templatedir-source &&\n \techo Content >templatedir-source/file &&\n+\ttest_config_global init.templatedir \"${HOME}/templatedir-source\" &&\n \t(\n-\t\ttest_config=\"${HOME}/.gitconfig\" &&\n-\t\tgit config -f \"$test_config\"  init.templatedir \"${HOME}/templatedir-source\" &&\n \t\tmkdir templatedir-set &&\n \t\tcd templatedir-set &&\n \t\tsane_unset GIT_TEMPLATE_DIR &&\n@@ -225,10 +224,9 @@ test_expect_success 'init with init.templatedir set' '\n '\n \n test_expect_success 'init --bare/--shared overrides system/global config' '\n+\ttest_config_global core.bare false &&\n+\ttest_config_global core.sharedRepository 0640 &&\n \t(\n-\t\ttest_config=\"$HOME\"/.gitconfig &&\n-\t\tgit config -f \"$test_config\" core.bare false &&\n-\t\tgit config -f \"$test_config\" core.sharedRepository 0640 &&\n \t\tmkdir init-bare-shared-override &&\n \t\tcd init-bare-shared-override &&\n \t\tgit init --bare --shared=0666\n@@ -239,9 +237,8 @@ test_expect_success 'init --bare/--shared overrides system/global config' '\n '\n \n test_expect_success 'init honors global core.sharedRepository' '\n+\ttest_config_global core.sharedRepository 0666 &&\n \t(\n-\t\ttest_config=\"$HOME\"/.gitconfig &&\n-\t\tgit config -f \"$test_config\" core.sharedRepository 0666 &&\n \t\tmkdir shared-honor-global &&\n \t\tcd shared-honor-global &&\n \t\tgit init\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237236","messageId":"20140320231950.GJ8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 10/12] t0001: use test_must_fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:19:50Z","receivedAt":"2014-03-20T23:19:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We've hand-rolled several \"if\" statements looking for\nfailures. We can use test_must_fail here, which is shorter\nand more robust.\n\nNote that we modify the commands slightly (to use \"git init\nfoo\" rather than \"cd foo && git init\") to avoid dealing with\na subshell, but this should not affect the outcome.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI'm pretty sure we can actually drop the \"mkdir\" in each of\nthese cases, too, but I was trying to leave things as close\nto the original as possible.\n\n t/t0001-init.sh | 38 +++++++++++---------------------------\n 1 file changed, 11 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 9515da3..4560bba 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -85,15 +85,8 @@ test_expect_failure 'plain nested in bare through aliased command' '\n '\n \n test_expect_success 'plain with GIT_WORK_TREE' '\n-\tif (\n-\t\tmkdir plain-wt &&\n-\t\tcd plain-wt &&\n-\t\tGIT_WORK_TREE=$(pwd) git init\n-\t)\n-\tthen\n-\t\techo Should have failed -- GIT_WORK_TREE should not be used\n-\t\tfalse\n-\tfi\n+\tmkdir plain-wt &&\n+\ttest_must_fail env GIT_WORK_TREE=\"$(pwd)/plain-wt\" git init plain-wt\n '\n \n test_expect_success 'plain bare' '\n@@ -106,15 +99,10 @@ test_expect_success 'plain bare' '\n '\n \n test_expect_success 'plain bare with GIT_WORK_TREE' '\n-\tif (\n-\t\tmkdir plain-bare-2 &&\n-\t\tcd plain-bare-2 &&\n-\t\tGIT_WORK_TREE=$(pwd) git --bare init\n-\t)\n-\tthen\n-\t\techo Should have failed -- GIT_WORK_TREE should not be used\n-\t\tfalse\n-\tfi\n+\tmkdir plain-bare-2 &&\n+\ttest_must_fail \\\n+\t\tenv GIT_WORK_TREE=\"$(pwd)/plain-bare-2\" \\\n+\t\tgit --bare init plain-bare-2\n '\n \n test_expect_success 'GIT_DIR bare' '\n@@ -156,15 +144,11 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '\n '\n \n test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '\n-\n-\tif (\n-\t\tmkdir git-dir-wt-2.git &&\n-\t\tGIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init\n-\t)\n-\tthen\n-\t\techo Should have failed -- --bare should not be used\n-\t\tfalse\n-\tfi\n+\tmkdir git-dir-wt-2.git &&\n+\ttest_must_fail env \\\n+\t\tGIT_WORK_TREE=\"$(pwd)\" \\\n+\t\tGIT_DIR=git-dir-wt-2.git \\\n+\t\tgit --bare init\n '\n \n test_expect_success 'reinit' '\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237237","messageId":"20140320232125.GK8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 11/12] t0001: drop useless subshells","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:21:25Z","receivedAt":"2014-03-20T23:21:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many tests use subshells, but don't actually change the\nshell environment. They were probably cargo-culted from\nearlier tests which did need subshells. Drop the useless\nones.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThese ones should produce no behavior change at all; they're purely\nmechanical \"(foo && bar)\" to \"foo && bar\" (though of course I did them\nby hand, because you need to know that \"foo\" and \"bar\" do not affect the\nenvironment).\n\n t/t0001-init.sh | 61 +++++++++++++++++++++------------------------------------\n 1 file changed, 22 insertions(+), 39 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 4560bba..55a68bc 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -106,11 +106,8 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '\n '\n \n test_expect_success 'GIT_DIR bare' '\n-\n-\t(\n-\t\tmkdir git-dir-bare.git &&\n-\t\tGIT_DIR=git-dir-bare.git git init\n-\t) &&\n+\tmkdir git-dir-bare.git &&\n+\tGIT_DIR=git-dir-bare.git git init &&\n \tcheck_config git-dir-bare.git true unset\n '\n \n@@ -242,36 +239,28 @@ test_expect_success 'init rejects insanely long --template' '\n \n test_expect_success 'init creates a new directory' '\n \trm -fr newdir &&\n-\t(\n-\t\tgit init newdir &&\n-\t\ttest_path_is_dir newdir/.git/refs\n-\t)\n+\tgit init newdir &&\n+\ttest_path_is_dir newdir/.git/refs\n '\n \n test_expect_success 'init creates a new bare directory' '\n \trm -fr newdir &&\n-\t(\n-\t\tgit init --bare newdir &&\n-\t\ttest_path_is_dir newdir/refs\n-\t)\n+\tgit init --bare newdir &&\n+\ttest_path_is_dir newdir/refs\n '\n \n test_expect_success 'init recreates a directory' '\n \trm -fr newdir &&\n-\t(\n-\t\tmkdir newdir &&\n-\t\tgit init newdir &&\n-\t\ttest_path_is_dir newdir/.git/refs\n-\t)\n+\tmkdir newdir &&\n+\tgit init newdir &&\n+\ttest_path_is_dir newdir/.git/refs\n '\n \n test_expect_success 'init recreates a new bare directory' '\n \trm -fr newdir &&\n-\t(\n-\t\tmkdir newdir &&\n-\t\tgit init --bare newdir &&\n-\t\ttest_path_is_dir newdir/refs\n-\t)\n+\tmkdir newdir &&\n+\tgit init --bare newdir &&\n+\ttest_path_is_dir newdir/refs\n '\n \n test_expect_success 'init creates a new deep directory' '\n@@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar\n \n test_expect_success 'init notices EEXIST (1)' '\n \trm -fr newdir &&\n-\t(\n-\t\t>newdir &&\n-\t\ttest_must_fail git init newdir &&\n-\t\ttest_path_is_file newdir\n-\t)\n+\t>newdir &&\n+\ttest_must_fail git init newdir &&\n+\ttest_path_is_file newdir\n '\n \n test_expect_success 'init notices EEXIST (2)' '\n \trm -fr newdir &&\n-\t(\n-\t\tmkdir newdir &&\n-\t\t>newdir/a\n-\t\ttest_must_fail git init newdir/a/b &&\n-\t\ttest_path_is_file newdir/a\n-\t)\n+\tmkdir newdir &&\n+\t>newdir/a\n+\ttest_must_fail git init newdir/a/b &&\n+\ttest_path_is_file newdir/a\n '\n \n test_expect_success POSIXPERM,SANITY 'init notices EPERM' '\n \trm -fr newdir &&\n-\t(\n-\t\tmkdir newdir &&\n-\t\tchmod -w newdir &&\n-\t\ttest_must_fail git init newdir/a/b\n-\t)\n+\tmkdir newdir &&\n+\tchmod -w newdir &&\n+\ttest_must_fail git init newdir/a/b\n '\n \n test_expect_success 'init creates a new bare directory with global --bare' '\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237238","messageId":"20140320232306.GL8479@sigill.intra.peff.net","threadId":"36209","inReplyTo":"20140320231159.GA7774@sigill.intra.peff.net","subject":"[PATCH 12/12] t0001: drop subshells just for \"cd\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-20T23:23:06Z","receivedAt":"2014-03-20T23:23:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many tests do something like:\n\n  (\n\tmkdir foo &&\n\tcd foo &&\n\tgit init\n  )\n\nYou can do the same these days with \"git init foo\", which\nmakes the tests shorter and simpler to read.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nUnlike the last patch, this one _could_ have an affect. I made the\nassumption that \"git init foo\" would behave sanely, but that other\ncomplex things should be left alone. E.g., ones that set GIT_DIR in the\nenvironment to a relative path might be affected based on when git does\nthe \"chdir\".\n\n t/t0001-init.sh | 56 +++++++++-----------------------------------------------\n 1 file changed, 9 insertions(+), 47 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 55a68bc..68549d1 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -24,11 +24,7 @@ check_config () {\n }\n \n test_expect_success 'plain' '\n-\t(\n-\t\tmkdir plain &&\n-\t\tcd plain &&\n-\t\tgit init\n-\t) &&\n+\tgit init plain &&\n \tcheck_config plain/.git false unset\n '\n \n@@ -90,11 +86,7 @@ test_expect_success 'plain with GIT_WORK_TREE' '\n '\n \n test_expect_success 'plain bare' '\n-\t(\n-\t\tmkdir plain-bare-1 &&\n-\t\tcd plain-bare-1 &&\n-\t\tgit --bare init\n-\t) &&\n+\tgit --bare init plain-bare-1 &&\n \tcheck_config plain-bare-1 true unset\n '\n \n@@ -112,12 +104,7 @@ test_expect_success 'GIT_DIR bare' '\n '\n \n test_expect_success 'init --bare' '\n-\n-\t(\n-\t\tmkdir init-bare.git &&\n-\t\tcd init-bare.git &&\n-\t\tgit init --bare\n-\t) &&\n+\tgit init --bare init-bare.git &&\n \tcheck_config init-bare.git true unset\n '\n \n@@ -166,26 +153,14 @@ test_expect_success 'reinit' '\n test_expect_success 'init with --template' '\n \tmkdir template-source &&\n \techo content >template-source/file &&\n-\t(\n-\t\tmkdir template-custom &&\n-\t\tcd template-custom &&\n-\t\tgit init --template=../template-source\n-\t) &&\n+\tgit init --template=../template-source template-custom &&\n \ttest_cmp template-source/file template-custom/.git/file\n '\n \n test_expect_success 'init with --template (blank)' '\n-\t(\n-\t\tmkdir template-plain &&\n-\t\tcd template-plain &&\n-\t\tgit init\n-\t) &&\n+\tgit init template-plain &&\n \ttest_path_is_file template-plain/.git/info/exclude &&\n-\t(\n-\t\tmkdir template-blank &&\n-\t\tcd template-blank &&\n-\t\tgit init --template=\n-\t) &&\n+\tgit init --template= template-blank &&\n \ttest_path_is_missing template-blank/.git/info/exclude\n '\n \n@@ -207,11 +182,7 @@ test_expect_success 'init with init.templatedir set' '\n test_expect_success 'init --bare/--shared overrides system/global config' '\n \ttest_config_global core.bare false &&\n \ttest_config_global core.sharedRepository 0640 &&\n-\t(\n-\t\tmkdir init-bare-shared-override &&\n-\t\tcd init-bare-shared-override &&\n-\t\tgit init --bare --shared=0666\n-\t) &&\n+\tgit init --bare --shared=0666 init-bare-shared-override &&\n \tcheck_config init-bare-shared-override true unset &&\n \ttest x0666 = \\\n \tx`git config -f init-bare-shared-override/config core.sharedRepository`\n@@ -219,22 +190,13 @@ test_expect_success 'init --bare/--shared overrides system/global config' '\n \n test_expect_success 'init honors global core.sharedRepository' '\n \ttest_config_global core.sharedRepository 0666 &&\n-\t(\n-\t\tmkdir shared-honor-global &&\n-\t\tcd shared-honor-global &&\n-\t\tgit init\n-\t) &&\n+\tgit init shared-honor-global &&\n \ttest x0666 = \\\n \tx`git config -f shared-honor-global/.git/config core.sharedRepository`\n '\n \n test_expect_success 'init rejects insanely long --template' '\n-\t(\n-\t\tinsane=$(printf \"x%09999dx\" 1) &&\n-\t\tmkdir test &&\n-\t\tcd test &&\n-\t\ttest_must_fail git init --template=$insane\n-\t)\n+\ttest_must_fail git init --template=$(printf \"x%09999dx\" 1) test\n '\n \n test_expect_success 'init creates a new directory' '\n-- \n1.9.0.560.g01ceb46\n"},{"id":"237357","messageId":"CAPig+cQs7N3yfTdQWaYudfTeYEJYB9rCY==68c1rdRTE9qokcA@mail.gmail.com","threadId":"36209","inReplyTo":"20140320232125.GK8479@sigill.intra.peff.net","subject":"Re: [PATCH 11/12] t0001: drop useless subshells","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-21T20:27:55Z","receivedAt":"2014-03-21T20:27:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 20, 2014 at 7:21 PM, Jeff King <peff@peff.net> wrote:\n> Many tests use subshells, but don't actually change the\n> shell environment. They were probably cargo-culted from\n> earlier tests which did need subshells. Drop the useless\n> ones.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> These ones should produce no behavior change at all; they're purely\n> mechanical \"(foo && bar)\" to \"foo && bar\" (though of course I did them\n> by hand, because you need to know that \"foo\" and \"bar\" do not affect the\n> environment).\n>\n>  t/t0001-init.sh | 61 +++++++++++++++++++++------------------------------------\n>  1 file changed, 22 insertions(+), 39 deletions(-)\n>\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 4560bba..55a68bc 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar\n>\n>  test_expect_success 'init notices EEXIST (2)' '\n>         rm -fr newdir &&\n> -       (\n> -               mkdir newdir &&\n> -               >newdir/a\n> -               test_must_fail git init newdir/a/b &&\n> -               test_path_is_file newdir/a\n> -       )\n> +       mkdir newdir &&\n> +       >newdir/a\n\nBroken &&-chain (though, not introduced by this patch).\n\n> +       test_must_fail git init newdir/a/b &&\n> +       test_path_is_file newdir/a\n>  '\n>\n>  test_expect_success POSIXPERM,SANITY 'init notices EPERM' '\n>         rm -fr newdir &&\n> -       (\n> -               mkdir newdir &&\n> -               chmod -w newdir &&\n> -               test_must_fail git init newdir/a/b\n> -       )\n> +       mkdir newdir &&\n> +       chmod -w newdir &&\n> +       test_must_fail git init newdir/a/b\n>  '\n>\n>  test_expect_success 'init creates a new bare directory with global --bare' '\n> --\n> 1.9.0.560.g01ceb46\n"},{"id":"237373","messageId":"xmqqy503s0s0.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"20140320231433.GC8479@sigill.intra.peff.net","subject":"Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-21T21:24:31Z","receivedAt":"2014-03-21T21:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Several test scripts manually unset GIT_CONFIG and other\n> GIT_* variables. These are generally taken care of for us by\n> test-lib.sh already.\n>\n> Unsetting these is not only useless, but can be confusing to\n> a reader, who may wonder why some tests in a script unset\n> them and others do not (t0001 is particularly guilty of this\n> inconsistency, probably because many of its tests predate\n> the test-lib.sh environment-cleansing).\n\n> Note that we cannot always get rid of such unsetting. For\n> example, t9130 can drop the GIT_CONFIG unset, but not the\n> GIT_DIR one, because lib-git-svn.sh sets the latter. And in\n> t1000, we unset GIT_TEMPLATE_DIR, which is explicitly\n> initialized by test-lib.sh.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I suppose one could make an argument that test-lib.sh may later change\n> the set of variables it clears, and these unsets are documenting an\n> explicit need of each test. I'd find that more compelling if it were\n> actually applied consistently.\n\nHmph.  I am looking at \"git show HEAD^:t/t0001-init.sh\" after\napplying this patch, and it does look consistently done with\nGIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a\ncursory read it is done consistently for tests on non-bare\nrepositories).\n\nSo I would actually agree with your alternative interpretation\n\"Unsetting these is useless, but it does serve documentation\npurpose---without having to see what the state of the environment\nwhen the subprocess is started, the reader can understand what is\nbeing tested\", rather than the one in the log message.\n\nHaving said that, I am perfectly OK with the change to t0001 in this\npatch, if we added at the very beginning of the test sequence a\ncomment that says:\n\n    Below, creation and use of repositories are tested with various\n    combinations of environment settings and command line flags.\n    They are done inside subshells to avoid leaking temporary\n    environment settings to later tests *and* assumes that the\n    initial environment does not have have GIT_DIR, GIT_CONFIG, and\n    GIT_WORK_TREE defined.\n\nor something.\n\n>  t/t0001-init.sh                 | 15 ---------------\n>  t/t9130-git-svn-authors-file.sh |  1 -\n>  t/t9400-git-cvsserver-server.sh |  1 -\n>  3 files changed, 17 deletions(-)\n>\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 9fb582b..ddc8160 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -25,7 +25,6 @@ check_config () {\n>  \n>  test_expect_success 'plain' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n>  \t\tmkdir plain &&\n>  \t\tcd plain &&\n>  \t\tgit init\n> @@ -35,7 +34,6 @@ test_expect_success 'plain' '\n>  \n>  test_expect_success 'plain nested in bare' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n>  \t\tgit init --bare bare-ancestor.git &&\n>  \t\tcd bare-ancestor.git &&\n>  \t\tmkdir plain-nested &&\n> @@ -47,7 +45,6 @@ test_expect_success 'plain nested in bare' '\n>  \n>  test_expect_success 'plain through aliased command, outside any git repo' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n>  \t\tHOME=$(pwd)/alias-config &&\n>  \t\texport HOME &&\n>  \t\tmkdir alias-config &&\n> @@ -65,7 +62,6 @@ test_expect_success 'plain through aliased command, outside any git repo' '\n>  \n>  test_expect_failure 'plain nested through aliased command' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n>  \t\tgit init plain-ancestor-aliased &&\n>  \t\tcd plain-ancestor-aliased &&\n>  \t\techo \"[alias] aliasedinit = init\" >>.git/config &&\n> @@ -78,7 +74,6 @@ test_expect_failure 'plain nested through aliased command' '\n>  \n>  test_expect_failure 'plain nested in bare through aliased command' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE &&\n>  \t\tgit init --bare bare-ancestor-aliased.git &&\n>  \t\tcd bare-ancestor-aliased.git &&\n>  \t\techo \"[alias] aliasedinit = init\" >>config &&\n> @@ -91,7 +86,6 @@ test_expect_failure 'plain nested in bare through aliased command' '\n>  \n>  test_expect_success 'plain with GIT_WORK_TREE' '\n>  \tif (\n> -\t\tsane_unset GIT_DIR &&\n>  \t\tmkdir plain-wt &&\n>  \t\tcd plain-wt &&\n>  \t\tGIT_WORK_TREE=$(pwd) git init\n> @@ -104,7 +98,6 @@ test_expect_success 'plain with GIT_WORK_TREE' '\n>  \n>  test_expect_success 'plain bare' '\n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&\n>  \t\tmkdir plain-bare-1 &&\n>  \t\tcd plain-bare-1 &&\n>  \t\tgit --bare init\n> @@ -114,7 +107,6 @@ test_expect_success 'plain bare' '\n>  \n>  test_expect_success 'plain bare with GIT_WORK_TREE' '\n>  \tif (\n> -\t\tsane_unset GIT_DIR GIT_CONFIG &&\n>  \t\tmkdir plain-bare-2 &&\n>  \t\tcd plain-bare-2 &&\n>  \t\tGIT_WORK_TREE=$(pwd) git --bare init\n> @@ -128,7 +120,6 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '\n>  test_expect_success 'GIT_DIR bare' '\n>  \n>  \t(\n> -\t\tsane_unset GIT_CONFIG &&\n>  \t\tmkdir git-dir-bare.git &&\n>  \t\tGIT_DIR=git-dir-bare.git git init\n>  \t) &&\n> @@ -138,7 +129,6 @@ test_expect_success 'GIT_DIR bare' '\n>  test_expect_success 'init --bare' '\n>  \n>  \t(\n> -\t\tsane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&\n>  \t\tmkdir init-bare.git &&\n>  \t\tcd init-bare.git &&\n>  \t\tgit init --bare\n> @@ -149,7 +139,6 @@ test_expect_success 'init --bare' '\n>  test_expect_success 'GIT_DIR non-bare' '\n>  \n>  \t(\n> -\t\tsane_unset GIT_CONFIG &&\n>  \t\tmkdir non-bare &&\n>  \t\tcd non-bare &&\n>  \t\tGIT_DIR=.git git init\n> @@ -160,7 +149,6 @@ test_expect_success 'GIT_DIR non-bare' '\n>  test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '\n>  \n>  \t(\n> -\t\tsane_unset GIT_CONFIG &&\n>  \t\tmkdir git-dir-wt-1.git &&\n>  \t\tGIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init\n>  \t) &&\n> @@ -170,7 +158,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (1)' '\n>  test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '\n>  \n>  \tif (\n> -\t\tsane_unset GIT_CONFIG &&\n>  \t\tmkdir git-dir-wt-2.git &&\n>  \t\tGIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init\n>  \t)\n> @@ -183,8 +170,6 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '\n>  test_expect_success 'reinit' '\n>  \n>  \t(\n> -\t\tsane_unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG &&\n> -\n>  \t\tmkdir again &&\n>  \t\tcd again &&\n>  \t\tgit init >out1 2>err1 &&\n> diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh\n> index c3443ce..a812783 100755\n> --- a/t/t9130-git-svn-authors-file.sh\n> +++ b/t/t9130-git-svn-authors-file.sh\n> @@ -97,7 +97,6 @@ test_expect_success 'fresh clone with svn.authors-file in config' '\n>  \t\ttest x = x\"$(git config svn.authorsfile)\" &&\n>  \t\ttest_config=\"$HOME\"/.gitconfig &&\n>  \t\tsane_unset GIT_DIR &&\n> -\t\tsane_unset GIT_CONFIG &&\n>  \t\tgit config --global \\\n>  \t\t  svn.authorsfile \"$HOME\"/svn-authors &&\n>  \t\ttest x\"$HOME\"/svn-authors = x\"$(git config svn.authorsfile)\" &&\n> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\n> index 3edc408..ed98e64 100755\n> --- a/t/t9400-git-cvsserver-server.sh\n> +++ b/t/t9400-git-cvsserver-server.sh\n> @@ -25,7 +25,6 @@ perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n>      test_done\n>  }\n>  \n> -unset GIT_DIR GIT_CONFIG\n>  WORKDIR=$(pwd)\n>  SERVERDIR=$(pwd)/gitcvs.git\n>  git_config=\"$SERVERDIR/config\"\n"},{"id":"237374","messageId":"xmqqtxars0ph.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"20140320231524.GD8479@sigill.intra.peff.net","subject":"Re: [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-21T21:26:02Z","receivedAt":"2014-03-21T21:26:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Some tests want to check or set config in another\n> repository. E.g., t1000 creates repositories and makes sure\n> that their core.bare and core.worktree settings are what we\n> expect. We can do this with:\n>\n>   GIT_CONFIG=$repo/.git/config git config ...\n>\n> but it better shows the intent to just enter the repository\n> and let \"git config\" do the normal lookups:\n>\n>   (cd $repo && git config ...)\n>\n> In theory, this would cause us to use an extra subshell, but\n> in all such cases, we are actually already in a subshell.\n\nSure; alternatively we could use \"git -C $there\", but this rewrite\nis fine by me.\n\nThanks.\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  t/t0001-init.sh        | 4 ++--\n>  t/t5701-clone-local.sh | 6 +++---\n>  2 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index ddc8160..9b05fdf 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -12,8 +12,8 @@ check_config () {\n>  \t\techo \"expected a directory $1, a file $1/config and $1/refs\"\n>  \t\treturn 1\n>  \tfi\n> -\tbare=$(GIT_CONFIG=\"$1/config\" git config --bool core.bare)\n> -\tworktree=$(GIT_CONFIG=\"$1/config\" git config core.worktree) ||\n> +\tbare=$(cd \"$1\" && git config --bool core.bare)\n> +\tworktree=$(cd \"$1\" && git config core.worktree) ||\n>  \tworktree=unset\n>  \n>  \ttest \"$bare\" = \"$2\" && test \"$worktree\" = \"$3\" || {\n> diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\n> index c490368..3c087e9 100755\n> --- a/t/t5701-clone-local.sh\n> +++ b/t/t5701-clone-local.sh\n> @@ -12,8 +12,8 @@ test_expect_success 'preparing origin repository' '\n>  \t: >file && git add . && git commit -m1 &&\n>  \tgit clone --bare . a.git &&\n>  \tgit clone --bare . x &&\n> -\ttest \"$(GIT_CONFIG=a.git/config git config --bool core.bare)\" = true &&\n> -\ttest \"$(GIT_CONFIG=x/config git config --bool core.bare)\" = true &&\n> +\ttest \"$(cd a.git && git config --bool core.bare)\" = true &&\n> +\ttest \"$(cd x && git config --bool core.bare)\" = true &&\n>  \tgit bundle create b1.bundle --all &&\n>  \tgit bundle create b2.bundle master &&\n>  \tmkdir dir &&\n> @@ -24,7 +24,7 @@ test_expect_success 'preparing origin repository' '\n>  test_expect_success 'local clone without .git suffix' '\n>  \tgit clone -l -s a b &&\n>  \t(cd b &&\n> -\ttest \"$(GIT_CONFIG=.git/config git config --bool core.bare)\" = false &&\n> +\ttest \"$(git config --bool core.bare)\" = false &&\n>  \tgit fetch)\n>  '\n"},{"id":"237507","messageId":"20140324215638.GH13728@sigill.intra.peff.net","threadId":"36209","inReplyTo":"xmqqy503s0s0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T21:56:38Z","receivedAt":"2014-03-24T21:56:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 21, 2014 at 02:24:31PM -0700, Junio C Hamano wrote:\n\n> > Unsetting these is not only useless, but can be confusing to\n> > a reader, who may wonder why some tests in a script unset\n> > them and others do not (t0001 is particularly guilty of this\n> > inconsistency, probably because many of its tests predate\n> > the test-lib.sh environment-cleansing).\n> [...]\n> > I suppose one could make an argument that test-lib.sh may later change\n> > the set of variables it clears, and these unsets are documenting an\n> > explicit need of each test. I'd find that more compelling if it were\n> > actually applied consistently.\n> \n> Hmph.  I am looking at \"git show HEAD^:t/t0001-init.sh\" after\n> applying this patch, and it does look consistently done with\n> GIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a\n> cursory read it is done consistently for tests on non-bare\n> repositories).\n\nI don't understand why we stop bothering with the unsets starting with\n\"init with --template\". Are those variables not important to the outcome\nof that and later tests, or did the author simply not bother because\nthey are noops?\n\n> So I would actually agree with your alternative interpretation\n> \"Unsetting these is useless, but it does serve documentation\n> purpose---without having to see what the state of the environment\n> when the subprocess is started, the reader can understand what is\n> being tested\", rather than the one in the log message.\n\nI'd agree with that if I were convinced that the presence of them there\nversus the absence of them later was meaningful.\n\n> Having said that, I am perfectly OK with the change to t0001 in this\n> patch, if we added at the very beginning of the test sequence a\n> comment that says:\n> \n>     Below, creation and use of repositories are tested with various\n>     combinations of environment settings and command line flags.\n>     They are done inside subshells to avoid leaking temporary\n>     environment settings to later tests *and* assumes that the\n>     initial environment does not have have GIT_DIR, GIT_CONFIG, and\n>     GIT_WORK_TREE defined.\n> \n> or something.\n\nI do not have a problem with that, as it implicitly covers all of the\ntests following it. I do not think it is particularly necessary, though.\nAssuming we start with a known test environment and avoiding polluting\nit for further tests are basic principles of _all_ test scripts.\n\n-Peff\n"},{"id":"237508","messageId":"20140324220011.GI13728@sigill.intra.peff.net","threadId":"36209","inReplyTo":"xmqqtxars0ph.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 04/12] t: stop using GIT_CONFIG to cross repo boundaries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-24T22:00:11Z","receivedAt":"2014-03-24T22:00:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 21, 2014 at 02:26:02PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Some tests want to check or set config in another\n> > repository. E.g., t1000 creates repositories and makes sure\n> > that their core.bare and core.worktree settings are what we\n> > expect. We can do this with:\n> >\n> >   GIT_CONFIG=$repo/.git/config git config ...\n> >\n> > but it better shows the intent to just enter the repository\n> > and let \"git config\" do the normal lookups:\n> >\n> >   (cd $repo && git config ...)\n> >\n> > In theory, this would cause us to use an extra subshell, but\n> > in all such cases, we are actually already in a subshell.\n> \n> Sure; alternatively we could use \"git -C $there\", but this rewrite\n> is fine by me.\n\nThe existing callers all pass actual $GIT_DIRs, so I initially wrote it\nas \"git --git-dir=$repo config ...\". Doing it as \"-C\" is perhaps nicer,\nas callers could potentially pass a shorter string to the repo root,\nand not bother with adding \"/.git\". However, t0001 needs the actual\n$GIT_DIR (because it looks for things like the refs/ directory in the\nsame function), and the other callers are just passing bare repos.\n\nSo I'm fine with any of them. Feel free to mark it up if you have a\npreference.\n\n-Peff\n"},{"id":"237511","messageId":"xmqqwqfjntf7.fsf@gitster.dls.corp.google.com","threadId":"36209","inReplyTo":"20140324215638.GH13728@sigill.intra.peff.net","subject":"Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-24T22:06:04Z","receivedAt":"2014-03-24T22:06:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I do not have a problem with that, as it implicitly covers all of the\n> tests following it. I do not think it is particularly necessary, though.\n> Assuming we start with a known test environment and avoiding polluting\n> it for further tests are basic principles of _all_ test scripts.\n\nThey should be, but I suspect majority of tests, especially the\nolder ones, do have dependencies on earlier test pieces X-<.\n"},{"id":"237524","messageId":"7v8uryq3jc.fsf@alter.siamese.dyndns.org","threadId":"36209","inReplyTo":"20140324215638.GH13728@sigill.intra.peff.net","subject":"Re: [PATCH 03/12] t: drop useless sane_unset GIT_* calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-25T04:56:55Z","receivedAt":"2014-03-25T04:56:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> Hmph.  I am looking at \"git show HEAD^:t/t0001-init.sh\" after\n>> applying this patch, and it does look consistently done with\n>> GIT_CONFIG and GIT_DIR (I am not sure about GIT_WORK_TREE but from a\n>> cursory read it is done consistently for tests on non-bare\n>> repositories).\n>\n> I don't understand why we stop bothering with the unsets starting with\n> \"init with --template\". Are those variables not important to the outcome\n> of that and later tests, or did the author simply not bother because\n> they are noops?\n\nIf I had to guess (without running \"blame\", so it may well turn out\nthat \"the author\" may turn out to be me ;-), it is simply the author\nnot being careful.\n"}]}