threads / patch / 42906

patch, 2 partsgit-submodule: forward exit code of git-submodule--helper more faithfully

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

## tl;dr

9 messages between Jul 22, 2016 and Jul 22, 2016. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Johannes Sixt· Jul 22, 2016, 19:14 UTC · lore

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(-)
Show changes to 3 files +15 −15

git-submodule.sh, t/t5815-submodule-protos.sh, t/t7400-submodule-basic.sh

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
Johannes Sixt· Jul 22, 2016, 19:15 UTC · re: Johannes Sixt · lore

[PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

'git submodule--helper update-clone' has logic to retry failed clones a second time. For this purpose, there is a list of submodules to clone, and a second list that is filled with the submodules to retry. Within these lists, the submodules are identified by an index as if both lists were just appended.

This works nicely except when the second clone attempt fails as well. To report an error, the identifying index must be adjusted by an offset so that it can be used as an index into the second list. However, the calculation uses the logical total length of the lists so that the result always points one past the end of the second list.

Pick the correct index.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin/submodule--helper.c | 2 +-
 t/t5815-submodule-protos.sh | 4 ++--
 t/t7400-submodule-basic.sh  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
Show changes to 3 files +5 −5

builtin/submodule--helper.c, t/t5815-submodule-protos.sh, t/t7400-submodule-basic.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b22352b..6f6d67a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -795,7 +795,7 @@ static int update_clone_task_finished(int result,
 		suc->failed_clones[suc->failed_clones_nr++] = ce;
 		return 0;
 	} else {
-		idx = suc->current - suc->list.nr;
+		idx -= suc->list.nr;
 		ce  = suc->failed_clones[idx];
 		strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
 			    ce->name);
diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 112cf40..06f55a1 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_failure 'clone with recurse-submodules fails' '
+test_expect_success '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_failure 'update of ext not allowed' '
+test_expect_success '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 7c8b90b..b77cce8 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_failure 'update should fail when path is used by a file' '
+test_expect_success 'update should fail when path is used by a file' '
 	echo hello >expect &&
 
 	echo "hello" >init &&
@@ -361,7 +361,7 @@ test_expect_failure 'update should fail when path is used by a file' '
 	test_cmp expect init
 '
 
-test_expect_failure 'update should fail when path is used by a nonempty directory' '
+test_expect_success 'update should fail when path is used by a nonempty directory' '
 	echo hello >expect &&
 
 	rm -fr init &&
-- 
2.9.0.443.ga8520ad
Stefan Beller· Jul 22, 2016, 19:30 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

On Fri, Jul 22, 2016 at 12:15 PM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 31 quoted lines
> 'git submodule--helper update-clone' has logic to retry failed clones
> a second time. For this purpose, there is a list of submodules to clone,
> and a second list that is filled with the submodules to retry. Within
> these lists, the submodules are identified by an index as if both lists
> were just appended.
>
> This works nicely except when the second clone attempt fails as well. To
> report an error, the identifying index must be adjusted by an offset so
> that it can be used as an index into the second list. However, the
> calculation uses the logical total length of the lists so that the result
> always points one past the end of the second list.
>
> Pick the correct index.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  builtin/submodule--helper.c | 2 +-
>  t/t5815-submodule-protos.sh | 4 ++--
>  t/t7400-submodule-basic.sh  | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b22352b..6f6d67a 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -795,7 +795,7 @@ static int update_clone_task_finished(int result,
>                 suc->failed_clones[suc->failed_clones_nr++] = ce;
>                 return 0;
>         } else {
> -               idx = suc->current - suc->list.nr;
> +               idx -= suc->list.nr;

The fix is the same as in http://thread.gmane.org/gmane.comp.version-control.git/299995 There we have an additional check, which may make sense to use here as well, specifically when having the patch 1 which propagates the exit code.

The approach to tests is different though. I like yours better than mine, as it doesn't add more tests, but strengthens existing tests.

Show 50 quoted lines
>                 ce  = suc->failed_clones[idx];
>                 strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
>                             ce->name);
> diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
> index 112cf40..06f55a1 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_failure 'clone with recurse-submodules fails' '
> +test_expect_success '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_failure 'update of ext not allowed' '
> +test_expect_success '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 7c8b90b..b77cce8 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_failure 'update should fail when path is used by a file' '
> +test_expect_success 'update should fail when path is used by a file' '
>         echo hello >expect &&
>
>         echo "hello" >init &&
> @@ -361,7 +361,7 @@ test_expect_failure 'update should fail when path is used by a file' '
>         test_cmp expect init
>  '
>
> -test_expect_failure 'update should fail when path is used by a nonempty directory' '
> +test_expect_success 'update should fail when path is used by a nonempty directory' '
>         echo hello >expect &&
>
>         rm -fr init &&
> --
> 2.9.0.443.ga8520ad
>
Junio C Hamano· Jul 22, 2016, 19:52 UTC · re: Stefan Beller · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

Stefan Beller <sbeller@google.com> writes:
> The approach to tests is different though. I like yours better than mine,
> as it doesn't add more tests, but strengthens existing tests.

So... are you retracting http://thread.gmane.org/gmane.comp.version-control.git/299995 and instead giving an Ack to these two?

Stefan Beller· Jul 22, 2016, 20:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

On Fri, Jul 22, 2016 at 12:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> The approach to tests is different though. I like yours better than mine,
>> as it doesn't add more tests, but strengthens existing tests.
>
> So... are you retracting
> http://thread.gmane.org/gmane.comp.version-control.git/299995 and
> instead giving an Ack to these two?
>
I like this series better
* for the approach
* for the tests
* for the commit message
So I do think this should be applied instead of what I sent.

I am tempted to send a squash proposal like: (broken whitespaces)

Show changes to builtin/submodule--helper.c +2 −0
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6f6d67a..77be97e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -796,6 +796,8 @@ static int update_clone_task_finished(int result,
  return 0;
  } else {
  idx -= suc->list.nr;
+ if (idx >= suc->failed_clones_nr)
+ die("BUG: idx too large???");
  ce  = suc->failed_clones[idx];
  strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
     ce->name);
Junio C Hamano· Jul 22, 2016, 19:39 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

Johannes Sixt <j6t@kdbg.org> writes:
Show 16 quoted lines
> 'git submodule--helper update-clone' has logic to retry failed clones
> a second time. For this purpose, there is a list of submodules to clone,
> and a second list that is filled with the submodules to retry. Within
> these lists, the submodules are identified by an index as if both lists
> were just appended.
>
> This works nicely except when the second clone attempt fails as well. To
> report an error, the identifying index must be adjusted by an offset so
> that it can be used as an index into the second list. However, the
> calculation uses the logical total length of the lists so that the result
> always points one past the end of the second list.
>
> Pick the correct index.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
Looks very similar to
  http://public-inbox.org/git/20160721013923.17435-1-sbeller%40google.com/
but these two patch series looks more thorough.
I expect I'd queue these two instead, after seeing Acks from Stefan.
Thanks.
Stefan Beller· Jul 22, 2016, 19:50 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

On Fri, Jul 22, 2016 at 12:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> 'git submodule--helper update-clone' has logic to retry failed clones
>> a second time. For this purpose, there is a list of submodules to clone,
>> and a second list that is filled with the submodules to retry. Within
>> these lists, the submodules are identified by an index as if both lists
>> were just appended.
>>
>> This works nicely except when the second clone attempt fails as well. To
>> report an error, the identifying index must be adjusted by an offset so
>> that it can be used as an index into the second list. However, the
>> calculation uses the logical total length of the lists so that the result
>> always points one past the end of the second list.
>>
>> Pick the correct index.
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
>
> Looks very similar to
>
>   http://public-inbox.org/git/20160721013923.17435-1-sbeller%40google.com/
>
> but these two patch series looks more thorough.
>
> I expect I'd queue these two instead, after seeing Acks from Stefan.
>
> Thanks.
Sure. Please take these 2 patches instead of mine.

Thanks, Stefan

Junio C Hamano· Jul 22, 2016, 20:00 UTC · re: Stefan Beller · lore

Re: [PATCH 2/2] submodule-helper: fix indexing in clone retry error reporting path

Stefan Beller <sbeller@google.com> writes:
Show 5 quoted lines
>> I expect I'd queue these two instead, after seeing Acks from Stefan.
>>
>> Thanks.
>
> Sure. Please take these 2 patches instead of mine.

OK, I've already queued yours but I haven't started today's integration cycle yet (I first pick up new topics and updates and review them in isolation, and then re-review them after rebuilding jch/pu branches in a separate phase), so I'll replace it with these two patches.

Thanks.
Stefan Beller· Jul 22, 2016, 19:27 UTC · re: Johannes Sixt · lore

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

On Fri, Jul 22, 2016 at 12:14 PM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 31 quoted lines
> 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.

The segfault has been addressed in http://thread.gmane.org/gmane.comp.version-control.git/299995 but received no attention yet. The propagation of the exit code makes sense nevertheless.

Thanks!
Show 139 quoted lines
>
> 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

← back to recent threads