submodule status: propagate SIGPIPE

From: Phillip Wood <[email protected]>

It has been reported than running

     git submodule status --recurse | grep -q ^+

results in an unexpected error message

fatal: failed to recurse into submodule $submodule

When "git submodule--helper" recurses into a submodule it creates a child process. If that process fails then the error message above is displayed by the parent. In the case above the child is killed by SIGPIPE as "grep -q" exits as soon as it sees the first match. Fix this by propagating SIGPIPE so that it is visible to the process running git. We could propagate other signals but I'm not sure there is much value in doing that. In the common case of the user pressing Ctrl-C or Ctrl-\ then SIGINT or SIGQUIT will be sent to the foreground process group and so the parent process will receive the same signal as the child.

---
submodule status: propagate SIGPIPE

Note that I haven't checked if any other submodule subcommands are affected by this - I'll leave that to someone more familiar with the code.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1799%2Fphillipwood%2Fsubmodule-status-sigpipe-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1799/phillipwood/submodule-status-sigpipe-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1799

 builtin/submodule--helper.c | 7 ++++++-
 t/t7422-submodule-output.sh | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

index a46ffd49b34..a8e497ef3c6 100644
@@ -30,6 +30,7 @@
 #include "advice.h"
 #include "branch.h"
 #include "list-objects-filter-options.h"
+#include <signal.h>
 
 #define OPT_QUIET (1 << 0)
 #define OPT_CACHED (1 << 1)

@@ -695,6 +696,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 
 	if (flags & OPT_RECURSIVE) {
 		struct child_process cpr = CHILD_PROCESS_INIT;
+		int res;
 
 		cpr.git_cmd = 1;
 		cpr.dir = path;

@@ -710,7 +712,10 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 		if (flags & OPT_QUIET)
 			strvec_push(&cpr.args, "--quiet");
 
-		if (run_command(&cpr))
+		res = run_command(&cpr);
+		if (res == SIGPIPE + 128)
+			raise(SIGPIPE);
+		else if (res)
 			die(_("failed to recurse into submodule '%s'"), path);
 	}
 

index ab946ec9405..c1686d6bb5f 100755
@@ -167,4 +167,11 @@ do
 	'
 done
 
+test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' '
+	{ git submodule status --recursive 2>err; echo $?>status; } |
+		grep -q X/S &&
+	test_must_be_empty err &&
+	test_match_signal 13 "$(cat status)"
+'
+
 test_done

base-commit: ed155187b429a2a6b6475efe1767053df37ccfe1

gitgitgadget

Hi Junio

Good point - I really need to figure out how to stop emacs' lsp mode automatically adding includes. I removed its "helpful" addition of <csignal> but forgot to remove <signal.h> as well.

Thanks

Phillip

From: Phillip Wood <[email protected]>

It has been reported than running

     git submodule status --recurse | grep -q ^+

results in an unexpected error message

fatal: failed to recurse into submodule $submodule

When "git submodule--helper" recurses into a submodule it creates a child process. If that process fails then the error message above is displayed by the parent. In the case above the child is killed by SIGPIPE as "grep -q" exits as soon as it sees the first match. Fix this by propagating SIGPIPE so that it is visible to the process running git. We could propagate other signals but I'm not sure there is much value in doing that. In the common case of the user pressing Ctrl-C or Ctrl-\ then SIGINT or SIGQUIT will be sent to the foreground process group and so the parent process will receive the same signal as the child.

---
submodule status: propagate SIGPIPE

Note that I haven't checked if any other submodule subcommands are affected by this - I'll leave that to someone more familiar with the code.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1799%2Fphillipwood%2Fsubmodule-status-sigpipe-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1799/phillipwood/submodule-status-sigpipe-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1799

Range-diff vs v1:

1: 64233d5ee0a ! 1: 169e2c06f1a submodule status: propagate SIGPIPE
@@ Commit message

## builtin/submodule--helper.c ##
-@@
- #include "advice.h"
- #include "branch.h"
- #include "list-objects-filter-options.h"
-+#include <signal.h>
-
- #define OPT_QUIET (1 << 0)
- #define OPT_CACHED (1 << 1)
@@ builtin/submodule--helper.c: static void status_submodule(const char *path, const struct object_id *ce_oid,

if (flags & OPT_RECURSIVE) {

 builtin/submodule--helper.c | 6 +++++-
 t/t7422-submodule-output.sh | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

index a46ffd49b34..4daca494b80 100644
@@ -695,6 +695,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 
 	if (flags & OPT_RECURSIVE) {
 		struct child_process cpr = CHILD_PROCESS_INIT;
+		int res;
 
 		cpr.git_cmd = 1;
 		cpr.dir = path;

@@ -710,7 +711,10 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 		if (flags & OPT_QUIET)
 			strvec_push(&cpr.args, "--quiet");
 
-		if (run_command(&cpr))
+		res = run_command(&cpr);
+		if (res == SIGPIPE + 128)
+			raise(SIGPIPE);
+		else if (res)
 			die(_("failed to recurse into submodule '%s'"), path);
 	}
 

index ab946ec9405..c1686d6bb5f 100755
@@ -167,4 +167,11 @@ do
 	'
 done
 
+test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' '
+	{ git submodule status --recursive 2>err; echo $?>status; } |
+		grep -q X/S &&
+	test_must_be_empty err &&
+	test_match_signal 13 "$(cat status)"
+'
+
 test_done

base-commit: ed155187b429a2a6b6475efe1767053df37ccfe1

gitgitgadget

Thanks. Queued.