{"thread":{"id":"24609","subject":"[PATCH] post-receive-email: ensure sent messages are not empty","startedAt":"2010-08-02T20:28:47Z","lastAt":"2010-08-02T22:17:21Z","messageCount":3,"participants":["Kevin P. Fleming","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"146988","messageId":"1280780927-29006-1-git-send-email-kpfleming@digium.com","threadId":"24609","inReplyTo":null,"subject":"[PATCH] post-receive-email: ensure sent messages are not empty","fromName":"Kevin P. Fleming","fromEmail":"kpfleming@digium.com","sentAt":"2010-08-02T20:28:47Z","receivedAt":"2010-08-02T20:28:47Z","isPatch":true,"sender":{"key":"kpfleming@digium.com","avatar":null},"body":"Changes the logic in the script to determine whether an email message\nwill be sent before invoking the send_mail() function; otherwise, if\nthe logic determines that a message will not be sent, send_mail() will\ncause an empty email to be sent.\n\nSigned-off-by: Kevin P. Fleming <kpfleming@digium.com>\n---\n contrib/hooks/post-receive-email |   42 +++++++++++++++++++++++++------------\n 1 files changed, 28 insertions(+), 14 deletions(-)\n\ndiff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email\nindex 30ae63d..b595452 100755\n--- a/contrib/hooks/post-receive-email\n+++ b/contrib/hooks/post-receive-email\n@@ -66,19 +66,10 @@\n # ---------------------------- Functions\n \n #\n-# Top level email generation function.  This decides what type of update\n-# this is and calls the appropriate body-generation routine after outputting\n-# the common header\n+# Function to prepare for email generation. This decides what type\n+# of update this is and whether an email should even be generated.\n #\n-# Note this function doesn't actually generate any email output, that is\n-# taken care of by the functions it calls:\n-#  - generate_email_header\n-#  - generate_create_XXXX_email\n-#  - generate_update_XXXX_email\n-#  - generate_delete_XXXX_email\n-#  - generate_email_footer\n-#\n-generate_email()\n+prep_for_email()\n {\n \t# --- Arguments\n \toldrev=$(git rev-parse $1)\n@@ -171,7 +162,28 @@ generate_email()\n \t\techo >&2 \"*** for $refname update $oldrev->$newrev\"\n \t\texit 0\n \tfi\n+}\n \n+#\n+# Top level email generation function.  This calls the appropriate\n+# body-generation routine after outputting the common header.\n+#\n+# Note this function doesn't actually generate any email output, that is\n+# taken care of by the functions it calls:\n+#  - generate_email_header\n+#  - generate_create_XXXX_email\n+#  - generate_update_XXXX_email\n+#  - generate_delete_XXXX_email\n+#  - generate_email_footer\n+#\n+# Note also that this function cannot 'exit' from the script; when this\n+# function is running (in hook script mode), the send_mail() function\n+# is already executing in another process, connected via a pipe, and\n+# if this function exits without, whatever has been generated to that\n+# point will be sent as an email... even if nothing has been generated.\n+#\n+generate_email()\n+{\n \t# Email parameters\n \t# The email subject will contain the best description of the ref\n \t# that we can build from the parameters\n@@ -687,10 +699,12 @@ if [ -n \"$1\" -a -n \"$2\" -a -n \"$3\" ]; then\n \t# Output to the terminal in command line mode - if someone wanted to\n \t# resend an email; they could redirect the output to sendmail\n \t# themselves\n-\tPAGER= generate_email $2 $3 $1\n+\tprep_for_email $2 $3 $1\n+\tPAGER= generate_email\n else\n \twhile read oldrev newrev refname\n \tdo\n-\t\tgenerate_email $oldrev $newrev $refname | send_mail\n+\t\tprep_for_email $oldrev $newrev $refname\n+\t\tgenerate_email | send_mail\n \tdone\n fi\n-- \n1.7.2\n"},{"id":"147001","messageId":"7vk4o8k73w.fsf@alter.siamese.dyndns.org","threadId":"24609","inReplyTo":"1280780927-29006-1-git-send-email-kpfleming@digium.com","subject":"Re: [PATCH] post-receive-email: ensure sent messages are not empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T22:00:35Z","receivedAt":"2010-08-02T22:00:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kevin P. Fleming\" <kpfleming@digium.com> writes:\n\n> @@ -687,10 +699,12 @@ if [ -n \"$1\" -a -n \"$2\" -a -n \"$3\" ]; then\n>  \t# Output to the terminal in command line mode - if someone wanted to\n>  \t# resend an email; they could redirect the output to sendmail\n>  \t# themselves\n> -\tPAGER= generate_email $2 $3 $1\n> +\tprep_for_email $2 $3 $1\n> +\tPAGER= generate_email\n>  else\n>  \twhile read oldrev newrev refname\n>  \tdo\n> -\t\tgenerate_email $oldrev $newrev $refname | send_mail\n> +\t\tprep_for_email $oldrev $newrev $refname\n> +\t\tgenerate_email | send_mail\n>  \tdone\n\nAs \"prep\" exits, when this is run as a hook to read many updated refs, any\ninappropriate update to one ref will cause messages for later refs from\ngetting sent out.  Earlier such an update may have sent an empty message\nbut at least didn't break messages for other refs, if I am reading the\ncode correctly.  Is that what you really want?\n\nPerhaps you would want to do something like this instead, after adjusting\nthe exit code from the new \"prep\" shell function?\n\n\twhile ...\n        do\n        \tprep_for_email || continue\n                generate_email | send_mail\n\tdone\n"},{"id":"147006","messageId":"4C5743F1.5020806@digium.com","threadId":"24609","inReplyTo":"7vk4o8k73w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] post-receive-email: ensure sent messages are not empty","fromName":"Kevin P. Fleming","fromEmail":"kpfleming@digium.com","sentAt":"2010-08-02T22:17:21Z","receivedAt":"2010-08-02T22:17:21Z","isPatch":true,"sender":{"key":"kpfleming@digium.com","avatar":null},"body":"On 08/02/2010 05:00 PM, Junio C Hamano wrote:\n> \"Kevin P. Fleming\" <kpfleming@digium.com> writes:\n> \n>> @@ -687,10 +699,12 @@ if [ -n \"$1\" -a -n \"$2\" -a -n \"$3\" ]; then\n>>  \t# Output to the terminal in command line mode - if someone wanted to\n>>  \t# resend an email; they could redirect the output to sendmail\n>>  \t# themselves\n>> -\tPAGER= generate_email $2 $3 $1\n>> +\tprep_for_email $2 $3 $1\n>> +\tPAGER= generate_email\n>>  else\n>>  \twhile read oldrev newrev refname\n>>  \tdo\n>> -\t\tgenerate_email $oldrev $newrev $refname | send_mail\n>> +\t\tprep_for_email $oldrev $newrev $refname\n>> +\t\tgenerate_email | send_mail\n>>  \tdone\n> \n> As \"prep\" exits, when this is run as a hook to read many updated refs, any\n> inappropriate update to one ref will cause messages for later refs from\n> getting sent out.  Earlier such an update may have sent an empty message\n> but at least didn't break messages for other refs, if I am reading the\n> code correctly.  Is that what you really want?\n> \n> Perhaps you would want to do something like this instead, after adjusting\n> the exit code from the new \"prep\" shell function?\n> \n> \twhile ...\n>         do\n>         \tprep_for_email || continue\n>                 generate_email | send_mail\n> \tdone\n> \n\nYou are right; instead of prep_for_email using 'exit 0' to stop the\nprocess as was done before, it should just return an exit code to skip\nthe current ref being processed. This was also a bug previously, since\ngenerate_email used 'exit 0' to stop the processing of a particular ref,\nwhich would actually stop processing of any further refs as well.\n\n-- \nKevin P. Fleming\nDigium, Inc. | Director of Software Technologies\n445 Jan Davis Drive NW - Huntsville, AL 35806 - USA\nskype: kpfleming | jabber: kfleming@digium.com\nCheck us out at www.digium.com & www.asterisk.org\n"}]}