{"thread":{"id":"42360","subject":"[PATCH v2 1/5] t1500: be considerate to future potential tests","startedAt":"2016-05-17T19:36:25Z","lastAt":"2016-05-18T17:43:29Z","messageCount":16,"participants":["Eric Sunshine","Junio C Hamano","Jeff King","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"286824","messageId":"20160517193630.10379-1-sunshine@sunshineco.com","threadId":"42360","inReplyTo":null,"subject":"[PATCH v2 0/5] modernize t1500","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:25Z","receivedAt":"2016-05-17T19:36:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This is a re-roll of [1] which modernizes t1500 by updating tests to set\nup their own needed state rather than relying upon manipulation of\nglobal state.\n\nChanges since v1[1]:\n\nIn v1 patch 1/6, which makes test_rev_parse() for-loop driven, the loop\ncontrol has been moved to the top of the loop for improved robustness.\n\nv1 patch 2/6, which tweaked the value of GIT_CONFIG in preparation for\nremoval of global cd's has been squashed into patch 3/6 which actually\nremoves the cd's since the below diff makes perfect sense in the context\nof the latter patch, thus doesn't require its own preparatory patch.\n\n    -cd work || exit 1\n    -GIT_CONFIG=\"$(pwd)\"/../.git/config\n    +GIT_CONFIG=\"$(pwd)/work/../.git/config\"\n\nv1 patch 3/6, which teaches test_rev_parse() the -C option, has been\nrevised to ensure that the argument to -C is always quoted upon use to\navoid problems with whitespace in directory names (though current tests\ndon't have any such directories).\n\nv1 patches 4/6 and 5/6, which teach test_rev_parse() the -b and -g\noptions, no longer assigns shell code to a variable for later\nexecution/evaluation; instead they just execute the code directly.\n\nv1 patch 5/6 ensures that the argument to -g is properly quoted when\nassigned to GIT_DIR to avoid problems with whitespace in directory\nnames.\n\nv1 patch 6/6, which changes \"mv .git repo.git\" to \"cp -R .git repo.git\",\nhas been squashed with Junio's 7/6[2], which moves the \"cp -R\" earlier\nin the script, and now sits at the front of the series. Other\nsetup-related actions have likewise been moved into a common \"setup\"\ntest[3].\n\nMost commit messages have seen minor tweaks.\n\nA v1 to v2 interdiff is included below.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/294088\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/294088/focus=294168\n[3]: http://thread.gmane.org/gmane.comp.version-control.git/294088/focus=294170\n\nEric Sunshine (5):\n  t1500: be considerate to future potential tests\n  t1500: test_rev_parse: facilitate future test enhancements\n  t1500: avoid changing working directory outside of tests\n  t1500: avoid setting configuration options outside of tests\n  t1500: avoid setting environment variables outside of tests\n\n t/t1500-rev-parse.sh | 123 ++++++++++++++++++++++++++-------------------------\n 1 file changed, 63 insertions(+), 60 deletions(-)\n\n--- >8 ---\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex af223ed..39af565 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -7,26 +7,21 @@ test_description='test git rev-parse'\n test_rev_parse () {\n \tdir=\n \tbare=\n-\tenv=\n+\tgitdir=\n \twhile :\n \tdo\n \t\tcase \"$1\" in\n-\t\t-C) dir=\"-C $2\"; shift; shift ;;\n-\t\t-b) bare=\"$2\"; shift; shift ;;\n-\t\t-g) env=\"GIT_DIR=$2; export GIT_DIR\"; shift; shift ;;\n+\t\t-C) dir=\"$2\"; shift; shift ;;\n+\t\t-b) case \"$2\" in\n+\t\t    [tfu]*) bare=\"$2\"; shift; shift ;;\n+\t\t    *) error \"test_rev_parse: bogus core.bare value '$2'\" ;;\n+\t\t    esac ;;\n+\t\t-g) gitdir=\"$2\"; shift; shift ;;\n \t\t-*) error \"test_rev_parse: unrecognized option '$1'\" ;;\n \t\t*) break ;;\n \t\tesac\n \tdone\n \n-\tcase \"$bare\" in\n-\t'') ;;\n-\tt*) bare=\"test_config $dir core.bare true\" ;;\n-\tf*) bare=\"test_config $dir core.bare false\" ;;\n-\tu*) bare=\"test_unconfig $dir core.bare\" ;;\n-\t*) error \"test_rev_parse: unrecognized core.bare value '$bare'\"\n-\tesac\n-\n \tname=$1\n \tshift\n \n@@ -36,35 +31,48 @@ test_rev_parse () {\n \t\t show-prefix \\\n \t\t git-dir\n \tdo\n+\t\ttest $# -eq 0 && break\n \t\texpect=\"$1\"\n \t\ttest_expect_success \"$name: $o\" '\n-\t\t\ttest_when_finished \"sane_unset GIT_DIR\" &&\n-\t\t\teval $env &&\n-\t\t\t$bare &&\n+\t\t\tif test -n \"$gitdir\"\n+\t\t\tthen\n+\t\t\t\ttest_when_finished \"unset GIT_DIR\" &&\n+\t\t\t\tGIT_DIR=\"$gitdir\" &&\n+\t\t\t\texport GIT_DIR\n+\t\t\tfi &&\n+\n+\t\t\tcase \"$bare\" in\n+\t\t\tt*) test_config ${dir:+-C \"$dir\"} core.bare true ;;\n+\t\t\tf*) test_config ${dir:+-C \"$dir\"} core.bare false ;;\n+\t\t\tu*) test_unconfig ${dir:+-C \"$dir\"} core.bare ;;\n+\t\t\tesac &&\n+\n \t\t\techo \"$expect\" >expect &&\n-\t\t\tgit $dir rev-parse --$o >actual &&\n+\t\t\tgit ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n \t\t\ttest_cmp expect actual\n \t\t'\n \t\tshift\n-\t\ttest $# -eq 0 && break\n \tdone\n }\n \n ROOT=$(pwd)\n \n+test_expect_success 'setup' '\n+\tmkdir -p sub/dir work &&\n+\tcp -R .git repo.git\n+'\n+\n test_rev_parse toplevel false false true '' .git\n \n test_rev_parse -C .git .git/ false true false '' .\n test_rev_parse -C .git/objects .git/objects/ false true false '' \"$ROOT/.git\"\n \n-test_expect_success 'setup untracked sub/dir' 'mkdir -p sub/dir'\n test_rev_parse -C sub/dir subdirectory false false true sub/dir/ \"$ROOT/.git\"\n \n test_rev_parse -b t 'core.bare = true' true false false\n \n test_rev_parse -b u 'core.bare undefined' false false true\n \n-test_expect_success 'setup non-local database ../.git' 'mkdir work'\n \n test_rev_parse -C work -g ../.git -b f 'GIT_DIR=../.git, core.bare = false' false false true ''\n \n@@ -72,7 +80,6 @@ test_rev_parse -C work -g ../.git -b t 'GIT_DIR=../.git, core.bare = true' true\n \n test_rev_parse -C work -g ../.git -b u 'GIT_DIR=../.git, core.bare undefined' false false true ''\n \n-test_expect_success 'setup non-local database ../repo.git' 'cp -R .git repo.git'\n \n test_rev_parse -C work -g ../repo.git -b f 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n \n--- >8 ---\n\n\n-- \n2.8.2.703.g78b384c\n"},{"id":"286821","messageId":"20160517193630.10379-2-sunshine@sunshineco.com","threadId":"42360","inReplyTo":"20160517193630.10379-1-sunshine@sunshineco.com","subject":"[PATCH v2 1/5] t1500: be considerate to future potential tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:26Z","receivedAt":"2016-05-17T19:36:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The final batch of git-rev-parse tests work against a non-local object\ndatabase named repo.git. This is done by renaming .git to repo.git and\npointing GIT_DIR at it, but the name is never restored to .git at the\nend of the script, which can be problematic for tests added in the\nfuture. Be more friendly by instead making repo.git a copy of .git.\n\nFurthermore, make it clear that tests in repo.git will be independent\nfrom the results of earlier tests done in .git by initializing repo.git\nearlier in the test sequence.\n\nLikewise, bundle remaining preparation (such as directory creation) into\na common setup test consistent with modern practice.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t1500-rev-parse.sh | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 48ee077..0194f54 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -37,6 +37,11 @@ test_rev_parse() {\n \n ROOT=$(pwd)\n \n+test_expect_success 'setup' '\n+\tmkdir -p sub/dir work &&\n+\tcp -R .git repo.git\n+'\n+\n test_rev_parse toplevel false false true '' .git\n \n cd .git || exit 1\n@@ -45,7 +50,6 @@ cd objects || exit 1\n test_rev_parse .git/objects/ false true false '' \"$ROOT/.git\"\n cd ../.. || exit 1\n \n-mkdir -p sub/dir || exit 1\n cd sub/dir || exit 1\n test_rev_parse subdirectory false false true sub/dir/ \"$ROOT/.git\"\n cd ../.. || exit 1\n@@ -56,7 +60,6 @@ test_rev_parse 'core.bare = true' true false false\n git config --unset core.bare\n test_rev_parse 'core.bare undefined' false false true\n \n-mkdir work || exit 1\n cd work || exit 1\n GIT_DIR=../.git\n GIT_CONFIG=\"$(pwd)\"/../.git/config\n@@ -71,7 +74,6 @@ test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''\n git config --unset core.bare\n test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''\n \n-mv ../.git ../repo.git || exit 1\n GIT_DIR=../repo.git\n GIT_CONFIG=\"$(pwd)\"/../repo.git/config\n \n-- \n2.8.2.703.g78b384c\n"},{"id":"286823","messageId":"20160517193630.10379-3-sunshine@sunshineco.com","threadId":"42360","inReplyTo":"20160517193630.10379-1-sunshine@sunshineco.com","subject":"[PATCH v2 2/5] t1500: test_rev_parse: facilitate future test enhancements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:27Z","receivedAt":"2016-05-17T19:36:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Tests run by test_rev_parse() are nearly identical; each invokes\ngit-rev-parse with a single option and compares the result against an\nexpected value. Such duplication makes it onerous to extend the tests\nsince any change needs to be repeated in each test. Avoid the\nduplication by parameterizing the test and driving it via a for-loop.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t1500-rev-parse.sh | 44 +++++++++++++++++---------------------------\n 1 file changed, 17 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 0194f54..c84f5c3 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -3,38 +3,28 @@\n test_description='test git rev-parse'\n . ./test-lib.sh\n \n-test_rev_parse() {\n+# usage: label is-bare is-inside-git is-inside-work prefix git-dir\n+test_rev_parse () {\n \tname=$1\n \tshift\n \n-\ttest_expect_success \"$name: is-bare-repository\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-bare-repository)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n-\n-\ttest_expect_success \"$name: is-inside-git-dir\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-inside-git-dir)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n-\n-\ttest_expect_success \"$name: is-inside-work-tree\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-inside-work-tree)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n-\n-\ttest_expect_success \"$name: prefix\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --show-prefix)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n-\n-\ttest_expect_success \"$name: git-dir\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --git-dir)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n+\tfor o in is-bare-repository \\\n+\t\t is-inside-git-dir \\\n+\t\t is-inside-work-tree \\\n+\t\t show-prefix \\\n+\t\t git-dir\n+\tdo\n+\t\ttest $# -eq 0 && break\n+\t\texpect=\"$1\"\n+\t\ttest_expect_success \"$name: $o\" '\n+\t\t\techo \"$expect\" >expect &&\n+\t\t\tgit rev-parse --$o >actual &&\n+\t\t\ttest_cmp expect actual\n+\t\t'\n+\t\tshift\n+\tdone\n }\n \n-# label is-bare is-inside-git is-inside-work prefix git-dir\n-\n ROOT=$(pwd)\n \n test_expect_success 'setup' '\n-- \n2.8.2.703.g78b384c\n"},{"id":"286826","messageId":"20160517193630.10379-4-sunshine@sunshineco.com","threadId":"42360","inReplyTo":"20160517193630.10379-1-sunshine@sunshineco.com","subject":"[PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:28Z","receivedAt":"2016-05-17T19:36:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Ideally, each test should be responsible for setting up state it needs\nrather than relying upon transient global state. Toward this end, teach\ntest_rev_parse() to accept a \"-C <dir>\" option to allow callers to\ninstruct it explicitly in which directory its tests should be run. Take\nadvantage of this new option to avoid changing the working directory\noutside of tests.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t1500-rev-parse.sh | 42 +++++++++++++++++++++++-------------------\n 1 file changed, 23 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex c84f5c3..fb2cee8 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -3,8 +3,18 @@\n test_description='test git rev-parse'\n . ./test-lib.sh\n \n-# usage: label is-bare is-inside-git is-inside-work prefix git-dir\n+# usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir\n test_rev_parse () {\n+\tdir=\n+\twhile :\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-C) dir=\"$2\"; shift; shift ;;\n+\t\t-*) error \"test_rev_parse: unrecognized option '$1'\" ;;\n+\t\t*) break ;;\n+\t\tesac\n+\tdone\n+\n \tname=$1\n \tshift\n \n@@ -18,7 +28,7 @@ test_rev_parse () {\n \t\texpect=\"$1\"\n \t\ttest_expect_success \"$name: $o\" '\n \t\t\techo \"$expect\" >expect &&\n-\t\t\tgit rev-parse --$o >actual &&\n+\t\t\tgit ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n \t\t\ttest_cmp expect actual\n \t\t'\n \t\tshift\n@@ -34,15 +44,10 @@ test_expect_success 'setup' '\n \n test_rev_parse toplevel false false true '' .git\n \n-cd .git || exit 1\n-test_rev_parse .git/ false true false '' .\n-cd objects || exit 1\n-test_rev_parse .git/objects/ false true false '' \"$ROOT/.git\"\n-cd ../.. || exit 1\n+test_rev_parse -C .git .git/ false true false '' .\n+test_rev_parse -C .git/objects .git/objects/ false true false '' \"$ROOT/.git\"\n \n-cd sub/dir || exit 1\n-test_rev_parse subdirectory false false true sub/dir/ \"$ROOT/.git\"\n-cd ../.. || exit 1\n+test_rev_parse -C sub/dir subdirectory false false true sub/dir/ \"$ROOT/.git\"\n \n git config core.bare true\n test_rev_parse 'core.bare = true' true false false\n@@ -50,30 +55,29 @@ test_rev_parse 'core.bare = true' true false false\n git config --unset core.bare\n test_rev_parse 'core.bare undefined' false false true\n \n-cd work || exit 1\n GIT_DIR=../.git\n-GIT_CONFIG=\"$(pwd)\"/../.git/config\n+GIT_CONFIG=\"$(pwd)/work/../.git/config\"\n export GIT_DIR GIT_CONFIG\n \n git config core.bare false\n-test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''\n+test_rev_parse -C work 'GIT_DIR=../.git, core.bare = false' false false true ''\n \n git config core.bare true\n-test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''\n+test_rev_parse -C work 'GIT_DIR=../.git, core.bare = true' true false false ''\n \n git config --unset core.bare\n-test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''\n+test_rev_parse -C work 'GIT_DIR=../.git, core.bare undefined' false false true ''\n \n GIT_DIR=../repo.git\n-GIT_CONFIG=\"$(pwd)\"/../repo.git/config\n+GIT_CONFIG=\"$(pwd)/work/../repo.git/config\"\n \n git config core.bare false\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n+test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n \n git config core.bare true\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n+test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n \n git config --unset core.bare\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n+test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n \n test_done\n-- \n2.8.2.703.g78b384c\n"},{"id":"286825","messageId":"20160517193630.10379-5-sunshine@sunshineco.com","threadId":"42360","inReplyTo":"20160517193630.10379-1-sunshine@sunshineco.com","subject":"[PATCH v2 4/5] t1500: avoid setting configuration options outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:29Z","receivedAt":"2016-05-17T19:36:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Ideally, each test should be responsible for setting up state it needs\nrather than relying upon transient global state. Toward this end, teach\ntest_rev_parse() to accept a \"-b <value>\" option to allow callers to set\n\"core.bare\" explicitly or undefine it. Take advantage of this new option\nto avoid setting \"core.bare\" outside of tests.\n\nUnder the hood, \"-b <value>\" invokes \"test_config -C <dir>\" (or\n\"test_unconfig -C <dir>\"), thus git-config knows explicitly where to\nfind its configuration file. Consequently, the global GIT_CONFIG\nenvironment variable required by the manual git-config invocations\noutside of tests is no longer needed, and is thus dropped.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t1500-rev-parse.sh | 39 ++++++++++++++++++++-------------------\n 1 file changed, 20 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex fb2cee8..5be463f 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -6,10 +6,15 @@ test_description='test git rev-parse'\n # usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir\n test_rev_parse () {\n \tdir=\n+\tbare=\n \twhile :\n \tdo\n \t\tcase \"$1\" in\n \t\t-C) dir=\"$2\"; shift; shift ;;\n+\t\t-b) case \"$2\" in\n+\t\t    [tfu]*) bare=\"$2\"; shift; shift ;;\n+\t\t    *) error \"test_rev_parse: bogus core.bare value '$2'\" ;;\n+\t\t    esac ;;\n \t\t-*) error \"test_rev_parse: unrecognized option '$1'\" ;;\n \t\t*) break ;;\n \t\tesac\n@@ -27,6 +32,12 @@ test_rev_parse () {\n \t\ttest $# -eq 0 && break\n \t\texpect=\"$1\"\n \t\ttest_expect_success \"$name: $o\" '\n+\t\t\tcase \"$bare\" in\n+\t\t\tt*) test_config ${dir:+-C \"$dir\"} core.bare true ;;\n+\t\t\tf*) test_config ${dir:+-C \"$dir\"} core.bare false ;;\n+\t\t\tu*) test_unconfig ${dir:+-C \"$dir\"} core.bare ;;\n+\t\t\tesac &&\n+\n \t\t\techo \"$expect\" >expect &&\n \t\t\tgit ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n \t\t\ttest_cmp expect actual\n@@ -49,35 +60,25 @@ test_rev_parse -C .git/objects .git/objects/ false true false '' \"$ROOT/.git\"\n \n test_rev_parse -C sub/dir subdirectory false false true sub/dir/ \"$ROOT/.git\"\n \n-git config core.bare true\n-test_rev_parse 'core.bare = true' true false false\n+test_rev_parse -b t 'core.bare = true' true false false\n \n-git config --unset core.bare\n-test_rev_parse 'core.bare undefined' false false true\n+test_rev_parse -b u 'core.bare undefined' false false true\n \n GIT_DIR=../.git\n-GIT_CONFIG=\"$(pwd)/work/../.git/config\"\n-export GIT_DIR GIT_CONFIG\n+export GIT_DIR\n \n-git config core.bare false\n-test_rev_parse -C work 'GIT_DIR=../.git, core.bare = false' false false true ''\n+test_rev_parse -C work -b f 'GIT_DIR=../.git, core.bare = false' false false true ''\n \n-git config core.bare true\n-test_rev_parse -C work 'GIT_DIR=../.git, core.bare = true' true false false ''\n+test_rev_parse -C work -b t 'GIT_DIR=../.git, core.bare = true' true false false ''\n \n-git config --unset core.bare\n-test_rev_parse -C work 'GIT_DIR=../.git, core.bare undefined' false false true ''\n+test_rev_parse -C work -b u 'GIT_DIR=../.git, core.bare undefined' false false true ''\n \n GIT_DIR=../repo.git\n-GIT_CONFIG=\"$(pwd)/work/../repo.git/config\"\n \n-git config core.bare false\n-test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n+test_rev_parse -C work -b f 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n \n-git config core.bare true\n-test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n+test_rev_parse -C work -b t 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n \n-git config --unset core.bare\n-test_rev_parse -C work 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n+test_rev_parse -C work -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n \n test_done\n-- \n2.8.2.703.g78b384c\n"},{"id":"286822","messageId":"20160517193630.10379-6-sunshine@sunshineco.com","threadId":"42360","inReplyTo":"20160517193630.10379-1-sunshine@sunshineco.com","subject":"[PATCH v2 5/5] t1500: avoid setting environment variables outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T19:36:30Z","receivedAt":"2016-05-17T19:36:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Ideally, each test should be responsible for setting up state it needs\nrather than relying upon transient global state. Toward this end, teach\ntest_rev_parse() to accept a \"-g <dir>\" option to allow callers to\nspecify the value of the GIT_DIR environment variable explicitly. Take\nadvantage of this new option to avoid polluting the global scope with\nGIT_DIR assignments.\n\nImplementation note: Typically, tests avoid polluting the global state\nby wrapping transient environment variable assignments within a\nsubshell, however, this technique doesn't work here since test_config()\nand test_unconfig() need to know GIT_DIR, as well, but neither function\ncan be used within a subshell. Consequently, GIT_DIR is instead cleared\nmanually via test_when_finished().\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t1500-rev-parse.sh | 24 +++++++++++++++---------\n 1 file changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 5be463f..39af565 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -7,6 +7,7 @@ test_description='test git rev-parse'\n test_rev_parse () {\n \tdir=\n \tbare=\n+\tgitdir=\n \twhile :\n \tdo\n \t\tcase \"$1\" in\n@@ -15,6 +16,7 @@ test_rev_parse () {\n \t\t    [tfu]*) bare=\"$2\"; shift; shift ;;\n \t\t    *) error \"test_rev_parse: bogus core.bare value '$2'\" ;;\n \t\t    esac ;;\n+\t\t-g) gitdir=\"$2\"; shift; shift ;;\n \t\t-*) error \"test_rev_parse: unrecognized option '$1'\" ;;\n \t\t*) break ;;\n \t\tesac\n@@ -32,6 +34,13 @@ test_rev_parse () {\n \t\ttest $# -eq 0 && break\n \t\texpect=\"$1\"\n \t\ttest_expect_success \"$name: $o\" '\n+\t\t\tif test -n \"$gitdir\"\n+\t\t\tthen\n+\t\t\t\ttest_when_finished \"unset GIT_DIR\" &&\n+\t\t\t\tGIT_DIR=\"$gitdir\" &&\n+\t\t\t\texport GIT_DIR\n+\t\t\tfi &&\n+\n \t\t\tcase \"$bare\" in\n \t\t\tt*) test_config ${dir:+-C \"$dir\"} core.bare true ;;\n \t\t\tf*) test_config ${dir:+-C \"$dir\"} core.bare false ;;\n@@ -64,21 +73,18 @@ test_rev_parse -b t 'core.bare = true' true false false\n \n test_rev_parse -b u 'core.bare undefined' false false true\n \n-GIT_DIR=../.git\n-export GIT_DIR\n \n-test_rev_parse -C work -b f 'GIT_DIR=../.git, core.bare = false' false false true ''\n+test_rev_parse -C work -g ../.git -b f 'GIT_DIR=../.git, core.bare = false' false false true ''\n \n-test_rev_parse -C work -b t 'GIT_DIR=../.git, core.bare = true' true false false ''\n+test_rev_parse -C work -g ../.git -b t 'GIT_DIR=../.git, core.bare = true' true false false ''\n \n-test_rev_parse -C work -b u 'GIT_DIR=../.git, core.bare undefined' false false true ''\n+test_rev_parse -C work -g ../.git -b u 'GIT_DIR=../.git, core.bare undefined' false false true ''\n \n-GIT_DIR=../repo.git\n \n-test_rev_parse -C work -b f 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n+test_rev_parse -C work -g ../repo.git -b f 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n \n-test_rev_parse -C work -b t 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n+test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n \n-test_rev_parse -C work -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n+test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n \n test_done\n-- \n2.8.2.703.g78b384c\n"},{"id":"286834","messageId":"xmqqeg90v2ss.fsf@gitster.mtv.corp.google.com","threadId":"42360","inReplyTo":"20160517193630.10379-4-sunshine@sunshineco.com","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-17T20:37:39Z","receivedAt":"2016-05-17T20:37:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> +\t\t\tgit ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n\nThis is kosher POSIX, but I vaguely recall some shells had trouble\nwith the SP between -C and \"$dir\" in the past.  Let's see if anybody\nscreams; hopefully I am misremembering or buggy shells died out.\n\nOther than that, the entire series makes the script easier to grok.\nThanks.\n"},{"id":"286835","messageId":"CAPig+cTAgB6DXoEeJaDVy7_J7QiqNOm6AJqZTiSowO6_jqa8_w@mail.gmail.com","threadId":"42360","inReplyTo":"xmqqeg90v2ss.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T20:48:33Z","receivedAt":"2016-05-17T20:48:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n>\n> This is kosher POSIX, but I vaguely recall some shells had trouble\n> with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n> screams; hopefully I am misremembering or buggy shells died out.\n\nI also am bothered by a vague recollection of some issue (possibly\ninvolving the internal space and lack of quotes around the entire\n${...}), but couldn't remember nor find a reference to the specific\ndetails. Perhaps someone reading the patch has a better memory than I.\n"},{"id":"286838","messageId":"20160517215214.GA16905@sigill.intra.peff.net","threadId":"42360","inReplyTo":"CAPig+cTAgB6DXoEeJaDVy7_J7QiqNOm6AJqZTiSowO6_jqa8_w@mail.gmail.com","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-17T21:52:14Z","receivedAt":"2016-05-17T21:52:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:\n\n> On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Eric Sunshine <sunshine@sunshineco.com> writes:\n> >> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n> >\n> > This is kosher POSIX, but I vaguely recall some shells had trouble\n> > with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n> > screams; hopefully I am misremembering or buggy shells died out.\n> \n> I also am bothered by a vague recollection of some issue (possibly\n> involving the internal space and lack of quotes around the entire\n> ${...}), but couldn't remember nor find a reference to the specific\n> details. Perhaps someone reading the patch has a better memory than I.\n\nProbably:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/265094\n\n-Peff\n"},{"id":"286843","messageId":"CAPig+cQ19DXqZx_D29qeQNz5pTVe-OGWFJG_2xWRX0XWCLk_ZQ@mail.gmail.com","threadId":"42360","inReplyTo":"20160517215214.GA16905@sigill.intra.peff.net","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-17T22:48:16Z","receivedAt":"2016-05-17T22:48:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 17, 2016 at 5:52 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:\n>> On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> > Eric Sunshine <sunshine@sunshineco.com> writes:\n>> >> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n>> >\n>> > This is kosher POSIX, but I vaguely recall some shells had trouble\n>> > with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n>> > screams; hopefully I am misremembering or buggy shells died out.\n>>\n>> I also am bothered by a vague recollection of some issue (possibly\n>> involving the internal space and lack of quotes around the entire\n>> ${...}), but couldn't remember nor find a reference to the specific\n>> details. Perhaps someone reading the patch has a better memory than I.\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/265094\n\nThanks for the link. I just tested with FreeBSD 8, and the shell\nindeed exhibits that broken behavior. The workaround in Kyle's patch\nisn't the prettiest (and is a bit verbose), but it gets the job done.\n\nI can send v3 using that workaround unless Junio wants to patch it locally(?).\n"},{"id":"286844","messageId":"xmqqshxgti68.fsf@gitster.mtv.corp.google.com","threadId":"42360","inReplyTo":"20160517215214.GA16905@sigill.intra.peff.net","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-17T22:48:31Z","receivedAt":"2016-05-17T22:48: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> On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:\n>\n>> On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> > Eric Sunshine <sunshine@sunshineco.com> writes:\n>> >> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n>> >\n>> > This is kosher POSIX, but I vaguely recall some shells had trouble\n>> > with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n>> > screams; hopefully I am misremembering or buggy shells died out.\n>> \n>> I also am bothered by a vague recollection of some issue (possibly\n>> involving the internal space and lack of quotes around the entire\n>> ${...}), but couldn't remember nor find a reference to the specific\n>> details. Perhaps someone reading the patch has a better memory than I.\n>\n> Probably:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/265094\n\nYikes, you're right.  Does anybody know if FreeBSD shell is still\nbuggy?\n"},{"id":"286845","messageId":"20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@webmail.informatik.kit.edu","threadId":"42360","inReplyTo":"xmqqshxgti68.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-05-17T23:06:09Z","receivedAt":"2016-05-17T23:06:09Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Junio C Hamano <gitster@pobox.com>:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:\n>>\n>>> On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>>>>> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n>>>>\n>>>> This is kosher POSIX, but I vaguely recall some shells had trouble\n>>>> with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n>>>> screams; hopefully I am misremembering or buggy shells died out.\n>>>\n>>> I also am bothered by a vague recollection of some issue (possibly\n>>> involving the internal space and lack of quotes around the entire\n>>> ${...}), but couldn't remember nor find a reference to the specific\n>>> details. Perhaps someone reading the patch has a better memory than I.\n>>\n>> Probably:\n>>\n>>  http://thread.gmane.org/gmane.comp.version-control.git/265094\n\nAnd ea10b60c910e (Work around ash \"alternate value\" expansion bug,\n2009-04-18) as well.\n\n    http://thread.gmane.org/gmane.comp.version-control.git/116816\n\n> Yikes, you're right.  Does anybody know if FreeBSD shell is still\n> buggy?\n\ngit-submodule.sh contains a few offending constructs:\n\n\tgit submodule--helper update-clone ${GIT_QUIET:+--quiet} \\\n\t\t${wt_prefix:+--prefix \"$wt_prefix\"} \\\n\t\t${prefix:+--recursive-prefix \"$prefix\"} \\\n\t\t${update:+--update \"$update\"} \\\n\t\t${reference:+--reference \"$reference\"} \\\n\t\t${depth:+--depth \"$depth\"} \\\n\t\t\"$@\" || echo \"#unmatched\"\n\t} | {\n\nThey were added recently in 48308681b072 (git submodule update: have\na dedicated helper for cloning, 2016-02-29), which is not in any\nrelease yet, perhaps that's why noone complained yet.\n"},{"id":"286851","messageId":"CAPig+cRn9fjo6CB9p7fKio=Nav-2WRjiUSkWiM3rqdO-4aREBA@mail.gmail.com","threadId":"42360","inReplyTo":"20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@webmail.informatik.kit.edu","subject":"Re: [PATCH v2 3/5] t1500: avoid changing working directory outside of tests","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-18T03:33:29Z","receivedAt":"2016-05-18T03:33:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 17, 2016 at 7:06 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Quoting Junio C Hamano <gitster@pobox.com>:\n>> Jeff King <peff@peff.net> writes:\n>>> On Tue, May 17, 2016 at 04:48:33PM -0400, Eric Sunshine wrote:\n>>>> On Tue, May 17, 2016 at 4:37 PM, Junio C Hamano <gitster@pobox.com>\n>>>> wrote:\n>>>>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>>>>>> +                     git ${dir:+-C \"$dir\"} rev-parse --$o >actual &&\n>>>>>\n>>>>> This is kosher POSIX, but I vaguely recall some shells had trouble\n>>>>> with the SP between -C and \"$dir\" in the past.  Let's see if anybody\n>>>>> screams; hopefully I am misremembering or buggy shells died out.\n>>>>\n>>>> I also am bothered by a vague recollection of some issue (possibly\n>>>> involving the internal space and lack of quotes around the entire\n>>>> ${...}), but couldn't remember nor find a reference to the specific\n>>>> details. Perhaps someone reading the patch has a better memory than I.\n>>>\n>>> Probably:\n>>>  http://thread.gmane.org/gmane.comp.version-control.git/265094\n>\n> And ea10b60c910e (Work around ash \"alternate value\" expansion bug,\n> 2009-04-18) as well.\n>\n>    http://thread.gmane.org/gmane.comp.version-control.git/116816\n\nThanks for the additional link. I have v3 ready to roll and will send\nit out within the next day if no more actionable review comments\narrive.\n"},{"id":"286893","messageId":"20160518183827.Horde.e7PY7wuh3iXKFZgoQ0oLR3D@webmail.informatik.kit.edu","threadId":"42360","inReplyTo":"20160517193630.10379-3-sunshine@sunshineco.com","subject":"Re: [PATCH v2 2/5] t1500: test_rev_parse: facilitate future test enhancements","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-05-18T16:38:27Z","receivedAt":"2016-05-18T16:38:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Eric Sunshine <sunshine@sunshineco.com>:\n\n> Tests run by test_rev_parse() are nearly identical; each invokes\n> git-rev-parse with a single option and compares the result against an\n> expected value. Such duplication makes it onerous to extend the tests\n> since any change needs to be repeated in each test. Avoid the\n> duplication by parameterizing the test and driving it via a for-loop.\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n>  t/t1500-rev-parse.sh | 44 +++++++++++++++++---------------------------\n>  1 file changed, 17 insertions(+), 27 deletions(-)\n>\n> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> index 0194f54..c84f5c3 100755\n> --- a/t/t1500-rev-parse.sh\n> +++ b/t/t1500-rev-parse.sh\n\n> +\tfor o in is-bare-repository \\\n> +\t\t is-inside-git-dir \\\n> +\t\t is-inside-work-tree \\\n> +\t\t show-prefix \\\n> +\t\t git-dir\n> +\tdo\n> +\t\ttest $# -eq 0 && break\n> +\t\texpect=\"$1\"\n> +\t\ttest_expect_success \"$name: $o\" '\n> +\t\t\techo \"$expect\" >expect &&\n> +\t\t\tgit rev-parse --$o >actual &&\n\nI think that \"--$o\" looks really weird, but that's subjective, of course.\n\nHowever, the idea popped up in an other thread[1] that we might want\nsomething like 'git rev-parse --absolute-path --git-dir', which wouldn't\nreally work with '--$o'.\n\nEven if we don't go that route, perhaps it would be better to list the\noptions to be tested including their doubledash prefix.\n\n\n[1] -  \nhttp://thread.gmane.org/gmane.comp.version-control.git/287449/focus=292585\n"},{"id":"286902","messageId":"CAPig+cTv8UNg+0taNF4B2wytn0d-jGboCntxBy8TPgY+1qW85w@mail.gmail.com","threadId":"42360","inReplyTo":"20160518183827.Horde.e7PY7wuh3iXKFZgoQ0oLR3D@webmail.informatik.kit.edu","subject":"Re: [PATCH v2 2/5] t1500: test_rev_parse: facilitate future test enhancements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-18T17:32:15Z","receivedAt":"2016-05-18T17:32:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 18, 2016 at 12:38 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Quoting Eric Sunshine <sunshine@sunshineco.com>:\n>> +       for o in is-bare-repository \\\n>> +                is-inside-git-dir \\\n>> +                is-inside-work-tree \\\n>> +                show-prefix \\\n>> +                git-dir\n>> +       do\n>> +               test $# -eq 0 && break\n>> +               expect=\"$1\"\n>> +               test_expect_success \"$name: $o\" '\n>> +                       echo \"$expect\" >expect &&\n>> +                       git rev-parse --$o >actual &&\n>\n> I think that \"--$o\" looks really weird, but that's subjective, of course.\n>\n> However, the idea popped up in an other thread[1] that we might want\n> something like 'git rev-parse --absolute-path --git-dir', which wouldn't\n> really work with '--$o'.\n>\n> Even if we don't go that route, perhaps it would be better to list the\n> options to be tested including their doubledash prefix.\n\nAs this series is only about modernizing t1500, I'd prefer to keep the\nconversion faithful to the original which titles each test \"$name:\nis-bare-repository\", \"$name: is-inside-git-dir\", etc., and the current\napproach does so without additional complexity.\n\nI have no objection to upgrading the for-loop items to include the\nleading dashes or updating the logic to support --absolute-path, but\nsuch changes are outside the scope of this series and can easily be\nbuilt atop it. Also, due to severe time constraints, I'd rather not\nre-roll only for a superficial and subjective change such as adding\nleading dashes to for-loop items.\n\nThanks.\n"},{"id":"286903","messageId":"CAPig+cQtt1TcHdRgZcB_fJ7_SEbZLjbKP04Eq=eRCCkU6PnQ1Q@mail.gmail.com","threadId":"42360","inReplyTo":"CAPig+cTv8UNg+0taNF4B2wytn0d-jGboCntxBy8TPgY+1qW85w@mail.gmail.com","subject":"Re: [PATCH v2 2/5] t1500: test_rev_parse: facilitate future test enhancements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-05-18T17:43:29Z","receivedAt":"2016-05-18T17:43:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 18, 2016 at 1:32 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, May 18, 2016 at 12:38 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n>> Quoting Eric Sunshine <sunshine@sunshineco.com>:\n>>> +       for o in is-bare-repository \\\n>>> +                is-inside-git-dir \\\n>>> +                is-inside-work-tree \\\n>>> +                show-prefix \\\n>>> +                git-dir\n>>> +       do\n>>> +               test $# -eq 0 && break\n>>> +               expect=\"$1\"\n>>> +               test_expect_success \"$name: $o\" '\n>>> +                       echo \"$expect\" >expect &&\n>>> +                       git rev-parse --$o >actual &&\n>>\n>> I think that \"--$o\" looks really weird, but that's subjective, of course.\n>>\n>> However, the idea popped up in an other thread[1] that we might want\n>> something like 'git rev-parse --absolute-path --git-dir', which wouldn't\n>> really work with '--$o'.\n>>\n>> Even if we don't go that route, perhaps it would be better to list the\n>> options to be tested including their doubledash prefix.\n>\n> As this series is only about modernizing t1500, I'd prefer to keep the\n> conversion faithful to the original which titles each test \"$name:\n> is-bare-repository\", \"$name: is-inside-git-dir\", etc., and the current\n> approach does so without additional complexity.\n>\n> I have no objection to upgrading the for-loop items to include the\n> leading dashes or updating the logic to support --absolute-path, but\n> such changes are outside the scope of this series and can easily be\n> built atop it. Also, due to severe time constraints, I'd rather not\n> re-roll only for a superficial and subjective change such as adding\n> leading dashes to for-loop items.\n\nOn reflection, I agree with your points and will include the change in\nthe re-roll. Thanks.\n"}]}