[PATCH 1/2] t/t7407: demonstrate that the command called by 'submodule foreach' loses stdin

[PATCH 1/2] t/t7407: demonstrate that the command called by 'submodule foreach' loses stdin

[Brandon Casey]

The user-supplied command spawned by 'submodule foreach' loses its
connection to the original standard input.  Instead, it is connected to the
output of a pipe within the git-submodule script.  This can cause a problem
if the command requires reading from stdin or if it changes its behavior
based on whether stdin is a tty or not (e.g. git shortlog).  Demonstrate
this flaw.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 t/t7407-submodule-foreach.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index e5be13c..e8d21b5 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -292,4 +292,22 @@ test_expect_success 'use "update --recursive nested1" to checkout all submodules
 	)
 '
 
+test_expect_failure 'command passed to foreach retains notion of stdin' '
+	(
+		cd super &&
+		git submodule foreach echo success >../expected &&
+		yes | git submodule foreach "read y && test \"x\$y\" = xy && echo success" >../actual
+	) &&
+	test_cmp expected actual
+'
+
+test_expect_failure 'command passed to foreach --recursive retains notion of stdin' '
+	(
+		cd clone2 &&
+		git submodule foreach --recursive echo success >../expected &&
+		yes | git submodule foreach --recursive "read y && test \"x\$y\" = xy && echo success" >../actual
+	) &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.7.6

[PATCH 2/2] git-submodule.sh: preserve stdin for the command spawned by foreach

[Brandon Casey]

The user-supplied command spawned by 'submodule foreach' loses its
connection to the original standard input.  Instead, it is connected to the
output of a pipe within the git-submodule script.  The user-supplied
command supplied to 'submodule foreach' is spawned within a while loop
which is being piped into.  Due to the way shells implement piping output
to a while loop, a subshell is created with its standard input attached to
the output of the pipe.  This results in all of the commands executed
within the while loop to have their stdins modified in the same way,
including the user-supplied command.

This can cause a problem if the command requires reading from stdin or if
it changes its behavior based on whether stdin is a tty or not.  For
example, this problem was noticed when trying to execute the following:

   git submodule foreach git shortlog --since=two.weeks.ago

which printed a message about entering the first submodule and produced no
further output and exited with a status of zero.  In this case, shortlog
detected that it was not connected to a tty, and since no revision was
supplied as an argument, it attempted to read the list of revisions from
standard input.  Instead, it slurped up the list of submodules that was
being piped to the enclosing while loop and caused that loop to end early
without processing the remaining submodules.

Work around this behavior by saving the original standard input file
descriptor before the while loop, and restoring it when spawning the
user-supplied command.

This fixes the tests in t7407.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 git-submodule.sh             |    6 +++++-
 t/t7407-submodule-foreach.sh |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d189a24..5013270 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -300,6 +300,10 @@ cmd_foreach()
 
 	toplevel=$(pwd)
 
+	# dup stdin so that it can be restored when running the external
+	# command in the subshell (and a recursive call to this function)
+	exec 3<&0
+
 	module_list |
 	while read mode sha1 stage path
 	do
@@ -316,7 +320,7 @@ cmd_foreach()
 				then
 					cmd_foreach "--recursive" "$@"
 				fi
-			) ||
+			) <&3 3<&- ||
 			die "Stopping at '$path'; script returned non-zero status."
 		fi
 	done
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index e8d21b5..835a506 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -292,7 +292,7 @@ test_expect_success 'use "update --recursive nested1" to checkout all submodules
 	)
 '
 
-test_expect_failure 'command passed to foreach retains notion of stdin' '
+test_expect_success 'command passed to foreach retains notion of stdin' '
 	(
 		cd super &&
 		git submodule foreach echo success >../expected &&
@@ -301,7 +301,7 @@ test_expect_failure 'command passed to foreach retains notion of stdin' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'command passed to foreach --recursive retains notion of stdin' '
+test_expect_success 'command passed to foreach --recursive retains notion of stdin' '
 	(
 		cd clone2 &&
 		git submodule foreach --recursive echo success >../expected &&
-- 
1.7.6