# [PATCH] post-receive-email: ensure sent messages are not empty 3 messages from 2010-08-02 to 2010-08-02. Participants: Kevin P. Fleming, Junio C Hamano. Thread: https://gitlist.dev/t/24609 ## Kevin P. Fleming, 2010-08-02 20:28 Subject: [PATCH] post-receive-email: ensure sent messages are not empty Message-ID: <1280780927-29006-1-git-send-email-kpfleming@digium.com> URL: https://gitlist.dev/e/1280780927-29006-1-git-send-email-kpfleming%40digium.com ``` Changes the logic in the script to determine whether an email message will be sent before invoking the send_mail() function; otherwise, if the logic determines that a message will not be sent, send_mail() will cause an empty email to be sent. Signed-off-by: Kevin P. Fleming --- contrib/hooks/post-receive-email | 42 +++++++++++++++++++++++++------------ 1 files changed, 28 insertions(+), 14 deletions(-) diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email index 30ae63d..b595452 100755 --- a/contrib/hooks/post-receive-email +++ b/contrib/hooks/post-receive-email @@ -66,19 +66,10 @@ # ---------------------------- Functions # -# Top level email generation function. This decides what type of update -# this is and calls the appropriate body-generation routine after outputting -# the common header +# Function to prepare for email generation. This decides what type +# of update this is and whether an email should even be generated. # -# Note this function doesn't actually generate any email output, that is -# taken care of by the functions it calls: -# - generate_email_header -# - generate_create_XXXX_email -# - generate_update_XXXX_email -# - generate_delete_XXXX_email -# - generate_email_footer -# -generate_email() +prep_for_email() { # --- Arguments oldrev=$(git rev-parse $1) @@ -171,7 +162,28 @@ generate_email() echo >&2 "*** for $refname update $oldrev->$newrev" exit 0 fi +} +# +# Top level email generation function. This calls the appropriate +# body-generation routine after outputting the common header. +# +# Note this function doesn't actually generate any email output, that is +# taken care of by the functions it calls: +# - generate_email_header +# - generate_create_XXXX_email +# - generate_update_XXXX_email +# - generate_delete_XXXX_email +# - generate_email_footer +# +# Note also that this function cannot 'exit' from the script; when this +# function is running (in hook script mode), the send_mail() function +# is already executing in another process, connected via a pipe, and +# if this function exits without, whatever has been generated to that +# point will be sent as an email... even if nothing has been generated. +# +generate_email() +{ # Email parameters # The email subject will contain the best description of the ref # that we can build from the parameters @@ -687,10 +699,12 @@ if [ -n "$1" -a -n "$2" -a -n "$3" ]; then # Output to the terminal in command line mode - if someone wanted to # resend an email; they could redirect the output to sendmail # themselves - PAGER= generate_email $2 $3 $1 + prep_for_email $2 $3 $1 + PAGER= generate_email else while read oldrev newrev refname do - generate_email $oldrev $newrev $refname | send_mail + prep_for_email $oldrev $newrev $refname + generate_email | send_mail done fi -- 1.7.2 ``` ## Junio C Hamano, 2010-08-02 22:00 Subject: Re: [PATCH] post-receive-email: ensure sent messages are not empty Message-ID: <7vk4o8k73w.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vk4o8k73w.fsf%40alter.siamese.dyndns.org In-Reply-To: <1280780927-29006-1-git-send-email-kpfleming@digium.com> ``` "Kevin P. Fleming" writes: > @@ -687,10 +699,12 @@ if [ -n "$1" -a -n "$2" -a -n "$3" ]; then > # Output to the terminal in command line mode - if someone wanted to > # resend an email; they could redirect the output to sendmail > # themselves > - PAGER= generate_email $2 $3 $1 > + prep_for_email $2 $3 $1 > + PAGER= generate_email > else > while read oldrev newrev refname > do > - generate_email $oldrev $newrev $refname | send_mail > + prep_for_email $oldrev $newrev $refname > + generate_email | send_mail > done As "prep" exits, when this is run as a hook to read many updated refs, any inappropriate update to one ref will cause messages for later refs from getting sent out. Earlier such an update may have sent an empty message but at least didn't break messages for other refs, if I am reading the code correctly. Is that what you really want? Perhaps you would want to do something like this instead, after adjusting the exit code from the new "prep" shell function? while ... do prep_for_email || continue generate_email | send_mail done ``` ## Kevin P. Fleming, 2010-08-02 22:17 Subject: Re: [PATCH] post-receive-email: ensure sent messages are not empty Message-ID: <4C5743F1.5020806@digium.com> URL: https://gitlist.dev/e/4C5743F1.5020806%40digium.com In-Reply-To: <7vk4o8k73w.fsf@alter.siamese.dyndns.org> ``` On 08/02/2010 05:00 PM, Junio C Hamano wrote: > "Kevin P. Fleming" writes: > >> @@ -687,10 +699,12 @@ if [ -n "$1" -a -n "$2" -a -n "$3" ]; then >> # Output to the terminal in command line mode - if someone wanted to >> # resend an email; they could redirect the output to sendmail >> # themselves >> - PAGER= generate_email $2 $3 $1 >> + prep_for_email $2 $3 $1 >> + PAGER= generate_email >> else >> while read oldrev newrev refname >> do >> - generate_email $oldrev $newrev $refname | send_mail >> + prep_for_email $oldrev $newrev $refname >> + generate_email | send_mail >> done > > As "prep" exits, when this is run as a hook to read many updated refs, any > inappropriate update to one ref will cause messages for later refs from > getting sent out. Earlier such an update may have sent an empty message > but at least didn't break messages for other refs, if I am reading the > code correctly. Is that what you really want? > > Perhaps you would want to do something like this instead, after adjusting > the exit code from the new "prep" shell function? > > while ... > do > prep_for_email || continue > generate_email | send_mail > done > You are right; instead of prep_for_email using 'exit 0' to stop the process as was done before, it should just return an exit code to skip the current ref being processed. This was also a bug previously, since generate_email used 'exit 0' to stop the processing of a particular ref, which would actually stop processing of any further refs as well. -- Kevin P. Fleming Digium, Inc. | Director of Software Technologies 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA skype: kpfleming | jabber: kfleming@digium.com Check us out at www.digium.com & www.asterisk.org ```