git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] git-submodule: forward exit code of git-submodule--helper more faithfully

From
Johannes Sixt <j6t@kdbg.org>
Date
Jul 22, 2016, 19:14 UTC
Message-ID
<8c0e116b-b604-ee83-197a-538eedf6e0ea@kdbg.org>

git-submodule--helper is invoked as the upstream of a pipe in several places. Usually, the failure of a program in this position is not detected by the shell. For this reason, the code inserts a token in the output stream when git-submodule--helper fails that is detected downstream, where the shell script is quit with exit code 1.

There happens to be a bug in git-submodule--helper that leads to a segmentation fault. The test suite triggers the crash in several places, all of which are protected by 'test_must_fail'. But due to the inspecific exit code 1, the crash remains undiagnosed.

Extend the failure protocol such that git-submodule--helper's exit code is passed downstream (only in the case of failure). This enables the downstream to use it as its own exit code, and 'test_must_fail' to identify the segmentation fault as an unexpected failure.

The bug itself is fixed in the next commit.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
When you run ./t7400-submodule-basic.sh -v, you will notice this output:

fatal: destination path '/home/jsixt/Src/git/git/t/trash directory.t7400-submodule-basic/init' already exists and is not an empty directory. fatal: clone of './.subrepo' into submodule path '/home/jsixt/Src/git/git/t/trash directory.t7400-submodule-basic/init' failed Failed to clone 'init'. Retry scheduled fatal: destination path '/home/jsixt/Src/git/git/t/trash directory.t7400-submodule-basic/init' already exists and is not an empty directory. fatal: clone of './.subrepo' into submodule path '/home/jsixt/Src/git/git/t/trash directory.t7400-submodule-basic/init' failed /home/jsixt/Src/git/git/git-submodule: line 494: 21757 Segmentation fault git submodule--helper update-clone ${GIT_QUIET:+--quiet} ${wt_prefix:+--prefix "$wt_prefix"} ${prefix:+--recursive-prefix "$prefix"} ${update:+--update "$update"} ${reference:+--reference "$reference"} ${depth:+--depth "$depth"} ${recommend_shallow:+"$recommend_shallow"} ${jobs:+$jobs} "$@" ok 32 - update should fail when path is used by a file

Note the segmentation fault. This mini-series addresses the issue.

Noticed on Windows because it "visualizes" segfaults even for command line programs.

 git-submodule.sh            | 22 +++++++++++-----------
 t/t5815-submodule-protos.sh |  4 ++--
 t/t7400-submodule-basic.sh  |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..0a0e12d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -49,7 +49,7 @@ die_if_unmatched ()
 {
 	if test "$1" = "#unmatched"
 	then
-		exit 1
+		exit ${2:-1}
 	fi
 }
 
@@ -312,11 +312,11 @@ cmd_foreach()
 
 	{
 		git submodule--helper list --prefix "$wt_prefix" ||
-		echo "#unmatched"
+		echo "#unmatched" $?
 	} |
 	while read mode sha1 stage sm_path
 	do
-		die_if_unmatched "$mode"
+		die_if_unmatched "$mode" "$sha1"
 		if test -e "$sm_path"/.git
 		then
 			displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
@@ -423,11 +423,11 @@ cmd_deinit()
 
 	{
 		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched"
+		echo "#unmatched" $?
 	} |
 	while read mode sha1 stage sm_path
 	do
-		die_if_unmatched "$mode"
+		die_if_unmatched "$mode" "$sha1"
 		name=$(git submodule--helper name "$sm_path") || exit
 
 		displaypath=$(git submodule--helper relative-path "$sm_path" "$wt_prefix")
@@ -581,12 +581,12 @@ cmd_update()
 		${depth:+--depth "$depth"} \
 		${recommend_shallow:+"$recommend_shallow"} \
 		${jobs:+$jobs} \
-		"$@" || echo "#unmatched"
+		"$@" || echo "#unmatched" $?
 	} | {
 	err=
 	while read mode sha1 stage just_cloned sm_path
 	do
-		die_if_unmatched "$mode"
+		die_if_unmatched "$mode" "$sha1"
 
 		name=$(git submodule--helper name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
@@ -994,11 +994,11 @@ cmd_status()
 
 	{
 		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched"
+		echo "#unmatched" $?
 	} |
 	while read mode sha1 stage sm_path
 	do
-		die_if_unmatched "$mode"
+		die_if_unmatched "$mode" "$sha1"
 		name=$(git submodule--helper name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
 		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
@@ -1075,11 +1075,11 @@ cmd_sync()
 	cd_to_toplevel
 	{
 		git submodule--helper list --prefix "$wt_prefix" "$@" ||
-		echo "#unmatched"
+		echo "#unmatched" $?
 	} |
 	while read mode sha1 stage sm_path
 	do
-		die_if_unmatched "$mode"
+		die_if_unmatched "$mode" "$sha1"
 		name=$(git submodule--helper name "$sm_path")
 		url=$(git config -f .gitmodules --get submodule."$name".url)
 
diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 06f55a1..112cf40 100755
--- a/t/t5815-submodule-protos.sh
+++ b/t/t5815-submodule-protos.sh
@@ -18,7 +18,7 @@ test_expect_success 'setup repository with submodules' '
 	git commit -m "add submodules"
 '
 
-test_expect_success 'clone with recurse-submodules fails' '
+test_expect_failure 'clone with recurse-submodules fails' '
 	test_must_fail git clone --recurse-submodules . dst
 '
 
@@ -32,7 +32,7 @@ test_expect_success 'update of ssh allowed' '
 	git -C dst submodule update ssh-module
 '
 
-test_expect_success 'update of ext not allowed' '
+test_expect_failure 'update of ext not allowed' '
 	test_must_fail git -C dst submodule update ext-module
 '
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b77cce8..7c8b90b 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -352,7 +352,7 @@ test_expect_success 'sync should fail with unknown submodule' '
 	test_failure_with_unknown_submodule sync
 '
 
-test_expect_success 'update should fail when path is used by a file' '
+test_expect_failure 'update should fail when path is used by a file' '
 	echo hello >expect &&
 
 	echo "hello" >init &&
@@ -361,7 +361,7 @@ test_expect_success 'update should fail when path is used by a file' '
 	test_cmp expect init
 '
 
-test_expect_success 'update should fail when path is used by a nonempty directory' '
+test_expect_failure 'update should fail when path is used by a nonempty directory' '
 	echo hello >expect &&
 
 	rm -fr init &&

-- 
2.9.0.443.ga8520ad
Next: Johannes Sixt
Message 1 of 9 in “git-submodule: forward exit code of git-submodule--helper more faithfully”
  1. 1/2 git-submodule: forward exit code of git-submodule--helper more faithfullyJohannes Sixt, Jul 22, 2016
  2. 2/2 submodule-helper: fix indexing in clone retry error reporting pathJohannes Sixt, Jul 22, 2016
  3. Stefan BellerJul 22, 2016
  4. Junio C HamanoJul 22, 2016
  5. Stefan BellerJul 22, 2016
  6. Junio C HamanoJul 22, 2016
  7. Stefan BellerJul 22, 2016
  8. Junio C HamanoJul 22, 2016
  9. Stefan BellerJul 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.