{"thread":{"id":"25984","subject":"[PATCH] Corrected return values in post-receive-email.prep_for_email","startedAt":"2010-12-07T16:43:15Z","lastAt":"2010-12-09T17:38:55Z","messageCount":9,"participants":["Alan Raison","Thiago Farina","Junio C Hamano","Kevin P. Fleming"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"157497","messageId":"002501cb962c$5fa3aa40$1eeafec0$@me.uk","threadId":"25984","inReplyTo":null,"subject":"[PATCH] Corrected return values in post-receive-email.prep_for_email","fromName":"Alan Raison","fromEmail":"alan@theraisons.me.uk","sentAt":null,"receivedAt":"2010-12-07T16:43:15Z","isPatch":true,"sender":{"key":"alan@theraisons.me.uk","avatar":"https://avatars.githubusercontent.com/u/785745?v=4"},"body":"---\n contrib/hooks/post-receive-email |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/hooks/post-receive-email\nb/contrib/hooks/post-receive-email\nindex 85724bf..020536d 100755\n--- a/contrib/hooks/post-receive-email\n+++ b/contrib/hooks/post-receive-email\n@@ -150,7 +150,7 @@ prep_for_email()\n \t\t\t# Anything else (is there anything else?)\n \t\t\techo >&2 \"*** Unknown type of update to $refname\n($rev_type)\"\n \t\t\techo >&2 \"***  - no email generated\"\n-\t\t\treturn 0\n+\t\t\treturn 1\n \t\t\t;;\n \tesac\n \n@@ -166,10 +166,10 @@ prep_for_email()\n \t\tesac\n \t\techo >&2 \"*** $config_name is not set so no email will be\nsent\"\n \t\techo >&2 \"*** for $refname update $oldrev->$newrev\"\n-\t\treturn 0\n+\t\treturn 1\n \tfi\n \n-\treturn 1\n+\treturn 0\n }\n \n #\n-- \n1.7.3.1.msysgit.0\n"},{"id":"157498","messageId":"AANLkTikYnDNRPVd-wd4+3jsX2fBbjxODEGATN5dD7t1E@mail.gmail.com","threadId":"25984","inReplyTo":"002501cb962c$5fa3aa40$1eeafec0$@me.uk","subject":"Re: [PATCH] Corrected return values in post-receive-email.prep_for_email","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-07T16:50:26Z","receivedAt":"2010-12-07T16:50:26Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Care to explain in the change log message why the return value should\nbe 1 instead of 0?\n\nOn Tue, Dec 7, 2010 at 2:32 PM, Alan Raison <alan@theraisons.me.uk> wrote:\n> ---\n>  contrib/hooks/post-receive-email |    6 +++---\n>  1 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/contrib/hooks/post-receive-email\n> b/contrib/hooks/post-receive-email\n> index 85724bf..020536d 100755\n> --- a/contrib/hooks/post-receive-email\n> +++ b/contrib/hooks/post-receive-email\n> @@ -150,7 +150,7 @@ prep_for_email()\n>                        # Anything else (is there anything else?)\n>                        echo >&2 \"*** Unknown type of update to $refname\n> ($rev_type)\"\n>                        echo >&2 \"***  - no email generated\"\n> -                       return 0\n> +                       return 1\n>                        ;;\n>        esac\n>\n> @@ -166,10 +166,10 @@ prep_for_email()\n>                esac\n>                echo >&2 \"*** $config_name is not set so no email will be\n> sent\"\n>                echo >&2 \"*** for $refname update $oldrev->$newrev\"\n> -               return 0\n> +               return 1\n>        fi\n>\n> -       return 1\n> +       return 0\n>  }\n>\n>  #\n> --\n> 1.7.3.1.msysgit.0\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"157500","messageId":"002c01cb9631$972d6690$c58833b0$@me.uk","threadId":"25984","inReplyTo":"AANLkTikYnDNRPVd-wd4+3jsX2fBbjxODEGATN5dD7t1E@mail.gmail.com","subject":"RE: [PATCH] Corrected return values in post-receive-email.prep_for_email","fromName":"Alan Raison","fromEmail":"alan@theraisons.me.uk","sentAt":null,"receivedAt":"2010-12-07T17:06:23Z","isPatch":true,"sender":{"key":"alan@theraisons.me.uk","avatar":"https://avatars.githubusercontent.com/u/785745?v=4"},"body":"In the main loop (lines 734 and 738 in the current master) the && and || operations assume true==0 and false==1; in line with shell defaults.\n\nI tested it on a sourceforge shell (I think using Bash); error conditions reported an error to standard error, then proceeded to generate the email; if prep_for_email succeeded then no mail was sent.\n\nHTH\n\nAlan\n\n-----Original Message-----\nFrom: Thiago Farina [mailto:tfransosi@gmail.com] \nSent: 07 December 2010 16:50\nTo: Alan Raison\nCc: git@vger.kernel.org\nSubject: Re: [PATCH] Corrected return values in post-receive-email.prep_for_email\n\nCare to explain in the change log message why the return value should\nbe 1 instead of 0?\n\nOn Tue, Dec 7, 2010 at 2:32 PM, Alan Raison <alan@theraisons.me.uk> wrote:\n> ---\n>  contrib/hooks/post-receive-email |    6 +++---\n>  1 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/contrib/hooks/post-receive-email\n> b/contrib/hooks/post-receive-email\n> index 85724bf..020536d 100755\n> --- a/contrib/hooks/post-receive-email\n> +++ b/contrib/hooks/post-receive-email\n> @@ -150,7 +150,7 @@ prep_for_email()\n>                        # Anything else (is there anything else?)\n>                        echo >&2 \"*** Unknown type of update to $refname\n> ($rev_type)\"\n>                        echo >&2 \"***  - no email generated\"\n> -                       return 0\n> +                       return 1\n>                        ;;\n>        esac\n>\n> @@ -166,10 +166,10 @@ prep_for_email()\n>                esac\n>                echo >&2 \"*** $config_name is not set so no email will be\n> sent\"\n>                echo >&2 \"*** for $refname update $oldrev->$newrev\"\n> -               return 0\n> +               return 1\n>        fi\n>\n> -       return 1\n> +       return 0\n>  }\n>\n>  #\n> --\n> 1.7.3.1.msysgit.0\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"157525","messageId":"7v1v5tqswl.fsf@alter.siamese.dyndns.org","threadId":"25984","inReplyTo":"002501cb962c$5fa3aa40$1eeafec0$@me.uk","subject":"Re: [PATCH] Corrected return values in post-receive-email.prep_for_email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-07T19:34:02Z","receivedAt":"2010-12-07T19:34:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alan Raison\" <alan@theraisons.me.uk> writes:\n\n> ---\n>  contrib/hooks/post-receive-email |    6 +++---\n>  1 files changed, 3 insertions(+), 3 deletions(-)\n\nNo sign-off, no description.\n\nThis is a regression introduced by 53cad69 (post-receive-email: ensure\nsent messages are not empty, 2010-09-10), I think.\n\n> diff --git a/contrib/hooks/post-receive-email\n> b/contrib/hooks/post-receive-email\n> index 85724bf..020536d 100755\n> --- a/contrib/hooks/post-receive-email\n> +++ b/contrib/hooks/post-receive-email\n> @@ -150,7 +150,7 @@ prep_for_email()\n>  \t\t\t# Anything else (is there anything else?)\n>  \t\t\techo >&2 \"*** Unknown type of update to $refname\n> ($rev_type)\"\n>  \t\t\techo >&2 \"***  - no email generated\"\n> -\t\t\treturn 0\n> +\t\t\treturn 1\n\nThis used to \"exit 1\" before 53cad69 and I agree with the patch that\nsignalling error with \"return 1\" is the right thing to do here.\n\n>  \t\t\t;;\n>  \tesac\n>  \n> @@ -166,10 +166,10 @@ prep_for_email()\n>  \t\tesac\n>  \t\techo >&2 \"*** $config_name is not set so no email will be\n> sent\"\n>  \t\techo >&2 \"*** for $refname update $oldrev->$newrev\"\n> -\t\treturn 0\n> +\t\treturn 1\n\nThis used to \"exit 0\" before 53cad69 to cause the program stop before\nsending mails.  Again, I agree with the patch that signalling error is the\nright thing to do here.\n\n>  \tfi\n>  \n> -\treturn 1\n> +\treturn 0\n\nAnd this obviously is correct.\n\nKevin, care to review and Ack?  Alan, care to add a few lines of patch\ndescription and sign-off?\n\nThanks.\n"},{"id":"157530","messageId":"4CFE8E97.4020508@digium.com","threadId":"25984","inReplyTo":"7v1v5tqswl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Corrected return values in post-receive-email.prep_for_email","fromName":"Kevin P. Fleming","fromEmail":"kpfleming@digium.com","sentAt":"2010-12-07T19:44:23Z","receivedAt":"2010-12-07T19:44:23Z","isPatch":true,"sender":{"key":"kpfleming@digium.com","avatar":null},"body":"On 12/07/2010 01:34 PM, Junio C Hamano wrote:\n> \"Alan Raison\"<alan@theraisons.me.uk>  writes:\n>\n>> ---\n>>   contrib/hooks/post-receive-email |    6 +++---\n>>   1 files changed, 3 insertions(+), 3 deletions(-)\n>\n> No sign-off, no description.\n>\n> This is a regression introduced by 53cad69 (post-receive-email: ensure\n> sent messages are not empty, 2010-09-10), I think.\n>\n>> diff --git a/contrib/hooks/post-receive-email\n>> b/contrib/hooks/post-receive-email\n>> index 85724bf..020536d 100755\n>> --- a/contrib/hooks/post-receive-email\n>> +++ b/contrib/hooks/post-receive-email\n>> @@ -150,7 +150,7 @@ prep_for_email()\n>>   \t\t\t# Anything else (is there anything else?)\n>>   \t\t\techo>&2 \"*** Unknown type of update to $refname\n>> ($rev_type)\"\n>>   \t\t\techo>&2 \"***  - no email generated\"\n>> -\t\t\treturn 0\n>> +\t\t\treturn 1\n>\n> This used to \"exit 1\" before 53cad69 and I agree with the patch that\n> signalling error with \"return 1\" is the right thing to do here.\n>\n>>   \t\t\t;;\n>>   \tesac\n>>\n>> @@ -166,10 +166,10 @@ prep_for_email()\n>>   \t\tesac\n>>   \t\techo>&2 \"*** $config_name is not set so no email will be\n>> sent\"\n>>   \t\techo>&2 \"*** for $refname update $oldrev->$newrev\"\n>> -\t\treturn 0\n>> +\t\treturn 1\n>\n> This used to \"exit 0\" before 53cad69 to cause the program stop before\n> sending mails.  Again, I agree with the patch that signalling error is the\n> right thing to do here.\n>\n>>   \tfi\n>>\n>> -\treturn 1\n>> +\treturn 0\n>\n> And this obviously is correct.\n>\n> Kevin, care to review and Ack?  Alan, care to add a few lines of patch\n> description and sign-off?\n\nAcked-by: Kevin P. Fleming <kpfleming@digium.com>\n\nYeah, this is clearly my breakage; our internal version of this script \nis so different that it has become hard to backport fixes to the \nupstream version... or I just did a terrible job of it.\n\nAlan, while you are in there fixing this, there is a remaining 'exit 0' \nin prep_for_email (at line 147) that should be 'return 1' instead.\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"},{"id":"157686","messageId":"004201cb97a4$6127cc60$23776520$@me.uk","threadId":"25984","inReplyTo":"4CFE8E97.4020508@digium.com","subject":"[PATCH] Corrected return values in prep_for_email;","fromName":"Alan Raison","fromEmail":"alan@theraisons.me.uk","sentAt":null,"receivedAt":"2010-12-09T12:47:36Z","isPatch":true,"sender":{"key":"alan@theraisons.me.uk","avatar":"https://avatars.githubusercontent.com/u/785745?v=4"},"body":">From ebe98d1c682f268b39a7eaf3ef529accbf0ac61c Mon Sep 17 00:00:00 2001\nFrom: Alan Raison <alan@theraisons.me.uk>\nDate: Mon, 6 Dec 2010 15:49:21 +0000\nSubject: [PATCH] Corrected return values in prep_for_email;\n\nFunction was returning 0 for failure and 1 for success which was breaking\nthe logic in the main loop.\n\nCorrected to return 0 for success, 1 for failure.  Function now also returns\nin all cases, rather than exiting.\n---\n contrib/hooks/post-receive-email |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/hooks/post-receive-email\nb/contrib/hooks/post-receive-email\nindex 85724bf..f99ea95 100755\n--- a/contrib/hooks/post-receive-email\n+++ b/contrib/hooks/post-receive-email\n@@ -144,13 +144,13 @@ prep_for_email()\n \t\t\tshort_refname=${refname##refs/remotes/}\n \t\t\techo >&2 \"*** Push-update of tracking branch,\n$refname\"\n \t\t\techo >&2 \"***  - no email generated.\"\n-\t\t\texit 0\n+\t\t\treturn 1\n \t\t\t;;\n \t\t*)\n \t\t\t# Anything else (is there anything else?)\n \t\t\techo >&2 \"*** Unknown type of update to $refname\n($rev_type)\"\n \t\t\techo >&2 \"***  - no email generated\"\n-\t\t\treturn 0\n+\t\t\treturn 1\n \t\t\t;;\n \tesac\n \n@@ -166,10 +166,10 @@ prep_for_email()\n \t\tesac\n \t\techo >&2 \"*** $config_name is not set so no email will be\nsent\"\n \t\techo >&2 \"*** for $refname update $oldrev->$newrev\"\n-\t\treturn 0\n+\t\treturn 1\n \tfi\n \n-\treturn 1\n+\treturn 0\n }\n \n #\n-- \n1.7.3.1.msysgit.0\n"},{"id":"157695","messageId":"4D00F388.5090806@digium.com","threadId":"25984","inReplyTo":"004201cb97a4$6127cc60$23776520$@me.uk","subject":"Re: [PATCH] Corrected return values in prep_for_email;","fromName":"Kevin P. Fleming","fromEmail":"kpfleming@digium.com","sentAt":"2010-12-09T15:19:36Z","receivedAt":"2010-12-09T15:19:36Z","isPatch":true,"sender":{"key":"kpfleming@digium.com","avatar":null},"body":"On 12/09/2010 07:24 AM, Alan Raison wrote:\n>  From ebe98d1c682f268b39a7eaf3ef529accbf0ac61c Mon Sep 17 00:00:00 2001\n> From: Alan Raison<alan@theraisons.me.uk>\n> Date: Mon, 6 Dec 2010 15:49:21 +0000\n> Subject: [PATCH] Corrected return values in prep_for_email;\n>\n> Function was returning 0 for failure and 1 for success which was breaking\n> the logic in the main loop.\n>\n> Corrected to return 0 for success, 1 for failure.  Function now also returns\n> in all cases, rather than exiting.\n\nYour commit message will need a Signed-Off-By line, but...\n\nAcked-By: Kevin P. Fleming <kpfleming@digium.com>\n\n> ---\n>   contrib/hooks/post-receive-email |    8 ++++----\n>   1 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/hooks/post-receive-email\n> b/contrib/hooks/post-receive-email\n> index 85724bf..f99ea95 100755\n> --- a/contrib/hooks/post-receive-email\n> +++ b/contrib/hooks/post-receive-email\n> @@ -144,13 +144,13 @@ prep_for_email()\n>   \t\t\tshort_refname=${refname##refs/remotes/}\n>   \t\t\techo>&2 \"*** Push-update of tracking branch,\n> $refname\"\n>   \t\t\techo>&2 \"***  - no email generated.\"\n> -\t\t\texit 0\n> +\t\t\treturn 1\n>   \t\t\t;;\n>   \t\t*)\n>   \t\t\t# Anything else (is there anything else?)\n>   \t\t\techo>&2 \"*** Unknown type of update to $refname\n> ($rev_type)\"\n>   \t\t\techo>&2 \"***  - no email generated\"\n> -\t\t\treturn 0\n> +\t\t\treturn 1\n>   \t\t\t;;\n>   \tesac\n>\n> @@ -166,10 +166,10 @@ prep_for_email()\n>   \t\tesac\n>   \t\techo>&2 \"*** $config_name is not set so no email will be\n> sent\"\n>   \t\techo>&2 \"*** for $refname update $oldrev->$newrev\"\n> -\t\treturn 0\n> +\t\treturn 1\n>   \tfi\n>\n> -\treturn 1\n> +\treturn 0\n>   }\n>\n>   #\n\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"},{"id":"157698","messageId":"004301cb97ba$90772630$b1657290$@me.uk","threadId":"25984","inReplyTo":"4CFE8E97.4020508@digium.com","subject":"[PATCH] Corrected return values in prep_for_email;","fromName":"Alan Raison","fromEmail":"alan@theraisons.me.uk","sentAt":null,"receivedAt":"2010-12-09T16:00:54Z","isPatch":true,"sender":{"key":"alan@theraisons.me.uk","avatar":"https://avatars.githubusercontent.com/u/785745?v=4"},"body":"Function was returning 0 for failure and 1 for success which was breaking\nthe logic in the main loop.\n\nCorrected to return 0 for success, 1 for failure.  Function now also returns\nin all cases, rather than exiting.\n\nAcked-By: Kevin P. Fleming <kpfleming@digium.com>\nSigned-Off-By: Alan Raison <alan@theraisons.me.uk>\n---\n contrib/hooks/post-receive-email |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/hooks/post-receive-email\nb/contrib/hooks/post-receive-email\nindex 85724bf..f99ea95 100755\n--- a/contrib/hooks/post-receive-email\n+++ b/contrib/hooks/post-receive-email\n@@ -144,13 +144,13 @@ prep_for_email()\n \t\t\tshort_refname=${refname##refs/remotes/}\n \t\t\techo >&2 \"*** Push-update of tracking branch,\n$refname\"\n \t\t\techo >&2 \"***  - no email generated.\"\n-\t\t\texit 0\n+\t\t\treturn 1\n \t\t\t;;\n \t\t*)\n \t\t\t# Anything else (is there anything else?)\n \t\t\techo >&2 \"*** Unknown type of update to $refname\n($rev_type)\"\n \t\t\techo >&2 \"***  - no email generated\"\n-\t\t\treturn 0\n+\t\t\treturn 1\n \t\t\t;;\n \tesac\n \n@@ -166,10 +166,10 @@ prep_for_email()\n \t\tesac\n \t\techo >&2 \"*** $config_name is not set so no email will be\nsent\"\n \t\techo >&2 \"*** for $refname update $oldrev->$newrev\"\n-\t\treturn 0\n+\t\treturn 1\n \tfi\n \n-\treturn 1\n+\treturn 0\n }\n \n #\n-- \n1.7.3.1.msysgit.0\n"},{"id":"157703","messageId":"7v7hfihmmo.fsf@alter.siamese.dyndns.org","threadId":"25984","inReplyTo":"004301cb97ba$90772630$b1657290$@me.uk","subject":"Re: [PATCH] Corrected return values in prep_for_email;","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-09T17:38:55Z","receivedAt":"2010-12-09T17:38:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alan Raison\" <alan@theraisons.me.uk> writes:\n\n> Function was returning 0 for failure and 1 for success which was breaking\n> the logic in the main loop.\n>\n> Corrected to return 0 for success, 1 for failure.  Function now also returns\n> in all cases, rather than exiting.\n\nThanks, will apply.\n\n> Acked-By: Kevin P. Fleming <kpfleming@digium.com>\n> Signed-Off-By: Alan Raison <alan@theraisons.me.uk>\n\nJust for reference---the order of events is that you signed-off first and\nthen Kevin acked the result, so the above is backwards.\n\nAlso your patch was linewrapped, but I can fix it up---no need to resend,\nbut please tell your MUA not to corrupt patches next time.\n"}]}