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

[PATCH 1/2] send-email: fix missing error message regression

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
May 24, 2021, 23:14 UTC
Message-ID
<patch-1.2-df3a2b8562d-20210524T231047Z-avarab@gmail.com>
In-Reply-To
<cover-0.2-00000000000-20210524T231047Z-avarab@gmail.com>

Fix a regression with the "the editor exited uncleanly, aborting everything" error message going missing after my d21616c0394 (git-send-email: refactor duplicate $? checks into a function, 2021-04-06).

I introduced a $msg variable, but did not actually use it. This caused us to miss the optional error message supplied by the "do_edit" codepath. Fix that, and add tests to check that this works.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 git-send-email.perl   | 12 +++++++++++-
 t/t9001-send-email.sh | 23 +++++++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 175da07d946..170f42fe210 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -219,8 +219,18 @@ sub system_or_msg {
 	my $exit_code = $? >> 8;
 	return unless $signalled or $exit_code;
 
+	my @sprintf_args = ($args->[0], $exit_code);
+	if (defined $msg) {
+		# Quiet the 'redundant' warning category, except we
+		# need to support down to Perl 5.8, so we can't do a
+		# "no warnings 'redundant'", since that category was
+		# introduced in perl 5.22, and asking for it will die
+		# on older perls.
+		no warnings;
+		return sprintf($msg, @sprintf_args);
+	}
 	return sprintf(__("fatal: command '%s' died with exit code %d"),
-		       $args->[0], $exit_code);
+		       @sprintf_args);
 }
 
 sub system_or_die {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 65b30353719..2acf389837c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -644,14 +644,33 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '
 	test_cmp expect actual
 '
 
+test_set_editor "$(pwd)/fake-editor"
+
+test_expect_success $PREREQ 'setup erroring fake editor' '
+	write_script fake-editor <<-\EOF
+	echo >&2 "I am about to error"
+	exit 1
+	EOF
+'
+
+test_expect_success $PREREQ 'fake editor dies with error' '
+	clean_fake_sendmail &&
+	test_must_fail git send-email \
+		--compose --subject foo \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches 2>err &&
+	grep "I am about to error" err &&
+	grep "the editor exited uncleanly, aborting everything" err
+'
+
 test_expect_success $PREREQ 'setup fake editor' '
 	write_script fake-editor <<-\EOF
 	echo fake edit >>"$1"
 	EOF
 '
 
-test_set_editor "$(pwd)/fake-editor"
-
 test_expect_success $PREREQ '--compose works' '
 	clean_fake_sendmail &&
 	git send-email \
-- 
2.32.0.rc1.385.g9db524b96f7
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 6 of 20 in “t9001-send-email.sh: fix expected absolute paths on Windows”
  1. t9001-send-email.sh: fix expected absolute paths on WindowsJohannes Sixt, May 24, 2021
  2. Jonathan NiederMay 24, 2021
  3. Ævar Arnfjörð BjarmasonMay 24, 2021
  4. Ævar Arnfjörð BjarmasonMay 24, 2021
  5. 0/2 send-email: pre-release fixes for v2.32.0Ævar Arnfjörð Bjarmason, May 24, 2021
  6. 1/2 send-email: fix missing error message regressionÆvar Arnfjörð Bjarmason, May 24, 2021
  7. 2/2 send-email: don't needlessly abs_path() the core.hooksPathÆvar Arnfjörð Bjarmason, May 24, 2021
  8. Junio C HamanoMay 25, 2021
  9. Robert FossMay 25, 2021
  10. Ævar Arnfjörð BjarmasonMay 25, 2021
  11. Ævar Arnfjörð BjarmasonMay 25, 2021
  12. Robert FossMay 25, 2021
  13. Junio C HamanoMay 25, 2021
  14. Ævar Arnfjörð BjarmasonMay 25, 2021
  15. Junio C HamanoMay 25, 2021
  16. 0/2 send-email: don't needlessly abs_path() the core.hooksPathÆvar Arnfjörð Bjarmason, May 26, 2021
  17. 1/2 send-email: don't needlessly abs_path() the core.hooksPathÆvar Arnfjörð Bjarmason, May 26, 2021
  18. 2/2 send-email: move "hooks_path" invocation to git-send-email.perlÆvar Arnfjörð Bjarmason, May 26, 2021
  19. Felipe ContrerasMay 26, 2021
  20. Johannes SchindelinJun 2, 2021

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.