Thread (3 messages) flat view 3 messages, 3 authors, 2016-06-15

Re: [PATCH] post-receive-email: do not call sendmail if no mail was generated

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:22

Lars Noschinski [off-list ref] writes:
contrib/hooks/post-receive-email used to call the send_mail function
(and thus, /usr/sbin/sendmail), even if generate_mail generated no
output.  This is problematic, as the sendmail binary provided by exim4
generates an error mail if provided with an empty input.
I actually have a bigger question, not about the implementation but about
the cause.

If generate_email results in an empty output in this codepath:

	# Check if we've got anyone to send to
	if [ -z "$recipients" ]; then
		...
		echo >&2 "*** $config_name is not set so no email will be sent"
		echo >&2 "*** for $refname update $oldrev->$newrev"
		exit 0
	fi

shouldn't we rather receive an error e-mail than let the
misconfiguration go undetected?

Before this check, I do not see anywhere generate_email would return nor
exit, and after this check, there is a call to generate_email_header and
that guarantees that the output from the generate_email function is not
empty, so it looks to me that triggering this check is the only case your
patch would change the behaviour of the script.

It looks to me that your exim error mail is actually reporting a
legitimate problem you would want to fix in your configuration.
quoted hunk
Therefore, we now read one line ourselves and use the result to decide
if we really want to call /usr/sbin/sendmail.
---
 contrib/hooks/post-receive-email |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

Two things changed:

 - we do not read the whole mail in a shell variable
 - the decision whether to call sendmail is based on the output generated
   by generate_mail, not its return code
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 2a66063..c855c31 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -637,6 +637,16 @@ show_new_revisions()
 
 send_mail()
 {
+	OIFS=$IFS
+	IFS='
+'
+	read FIRSTLINE || exit 1
Shouldn't this be merely a "return"?  The caller looks like this:

	while read oldrev newrev refname
	do
		generate_email $oldrev $newrev $refname | send_mail
	done

and you would not want to stop after punting to report on the first ref.
quoted hunk
+	(printf $FIRSTLINE'\n'; cat) | call_sendmail
+	IFS=$OLD_IFS
+}
+
+call_sendmail()
+{
 	if [ -n "$envelopesender" ]; then
 		/usr/sbin/sendmail -t -f "$envelopesender"
 	else
@@ -644,6 +654,7 @@ send_mail()
 	fi
 }
 
+
 # ---------------------------- main()
 
 # --- Constants
Why?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help