{"thread":{"id":"27731","subject":"[PATCH 1/2] t/t7407: demonstrate that the command called by 'submodule foreach' loses stdin","startedAt":"2011-06-30T00:34:57Z","lastAt":"2011-06-30T00:34:58Z","messageCount":2,"participants":["Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"170687","messageId":"1309394098-18618-1-git-send-email-drafnel@gmail.com","threadId":"27731","inReplyTo":null,"subject":"[PATCH 1/2] t/t7407: demonstrate that the command called by 'submodule foreach' loses stdin","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-06-30T00:34:57Z","receivedAt":"2011-06-30T00:34:57Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The user-supplied command spawned by 'submodule foreach' loses its\nconnection to the original standard input.  Instead, it is connected to the\noutput of a pipe within the git-submodule script.  This can cause a problem\nif the command requires reading from stdin or if it changes its behavior\nbased on whether stdin is a tty or not (e.g. git shortlog).  Demonstrate\nthis flaw.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n t/t7407-submodule-foreach.sh |   18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex e5be13c..e8d21b5 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -292,4 +292,22 @@ test_expect_success 'use \"update --recursive nested1\" to checkout all submodules\n \t)\n '\n \n+test_expect_failure 'command passed to foreach retains notion of stdin' '\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule foreach echo success >../expected &&\n+\t\tyes | git submodule foreach \"read y && test \\\"x\\$y\\\" = xy && echo success\" >../actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure 'command passed to foreach --recursive retains notion of stdin' '\n+\t(\n+\t\tcd clone2 &&\n+\t\tgit submodule foreach --recursive echo success >../expected &&\n+\t\tyes | git submodule foreach --recursive \"read y && test \\\"x\\$y\\\" = xy && echo success\" >../actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.7.6\n"},{"id":"170688","messageId":"1309394098-18618-2-git-send-email-drafnel@gmail.com","threadId":"27731","inReplyTo":"1309394098-18618-1-git-send-email-drafnel@gmail.com","subject":"[PATCH 2/2] git-submodule.sh: preserve stdin for the command spawned by foreach","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-06-30T00:34:58Z","receivedAt":"2011-06-30T00:34:58Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The user-supplied command spawned by 'submodule foreach' loses its\nconnection to the original standard input.  Instead, it is connected to the\noutput of a pipe within the git-submodule script.  The user-supplied\ncommand supplied to 'submodule foreach' is spawned within a while loop\nwhich is being piped into.  Due to the way shells implement piping output\nto a while loop, a subshell is created with its standard input attached to\nthe output of the pipe.  This results in all of the commands executed\nwithin the while loop to have their stdins modified in the same way,\nincluding the user-supplied command.\n\nThis can cause a problem if the command requires reading from stdin or if\nit changes its behavior based on whether stdin is a tty or not.  For\nexample, this problem was noticed when trying to execute the following:\n\n   git submodule foreach git shortlog --since=two.weeks.ago\n\nwhich printed a message about entering the first submodule and produced no\nfurther output and exited with a status of zero.  In this case, shortlog\ndetected that it was not connected to a tty, and since no revision was\nsupplied as an argument, it attempted to read the list of revisions from\nstandard input.  Instead, it slurped up the list of submodules that was\nbeing piped to the enclosing while loop and caused that loop to end early\nwithout processing the remaining submodules.\n\nWork around this behavior by saving the original standard input file\ndescriptor before the while loop, and restoring it when spawning the\nuser-supplied command.\n\nThis fixes the tests in t7407.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n git-submodule.sh             |    6 +++++-\n t/t7407-submodule-foreach.sh |    4 ++--\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex d189a24..5013270 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -300,6 +300,10 @@ cmd_foreach()\n \n \ttoplevel=$(pwd)\n \n+\t# dup stdin so that it can be restored when running the external\n+\t# command in the subshell (and a recursive call to this function)\n+\texec 3<&0\n+\n \tmodule_list |\n \twhile read mode sha1 stage path\n \tdo\n@@ -316,7 +320,7 @@ cmd_foreach()\n \t\t\t\tthen\n \t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n \t\t\t\tfi\n-\t\t\t) ||\n+\t\t\t) <&3 3<&- ||\n \t\t\tdie \"Stopping at '$path'; script returned non-zero status.\"\n \t\tfi\n \tdone\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex e8d21b5..835a506 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -292,7 +292,7 @@ test_expect_success 'use \"update --recursive nested1\" to checkout all submodules\n \t)\n '\n \n-test_expect_failure 'command passed to foreach retains notion of stdin' '\n+test_expect_success 'command passed to foreach retains notion of stdin' '\n \t(\n \t\tcd super &&\n \t\tgit submodule foreach echo success >../expected &&\n@@ -301,7 +301,7 @@ test_expect_failure 'command passed to foreach retains notion of stdin' '\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'command passed to foreach --recursive retains notion of stdin' '\n+test_expect_success 'command passed to foreach --recursive retains notion of stdin' '\n \t(\n \t\tcd clone2 &&\n \t\tgit submodule foreach --recursive echo success >../expected &&\n-- \n1.7.6\n"}]}