From: Kevin P. Fleming Date: Tue, 07 Dec 2010 19:44:23 GMT Subject: Re: [PATCH] Corrected return values in post-receive-email.prep_for_email Message-ID: <4CFE8E97.4020508@digium.com> In-Reply-To: <7v1v5tqswl.fsf@alter.siamese.dyndns.org> On 12/07/2010 01:34 PM, Junio C Hamano wrote: > "Alan Raison" writes: > >> --- >> contrib/hooks/post-receive-email | 6 +++--- >> 1 files changed, 3 insertions(+), 3 deletions(-) > > No sign-off, no description. > > This is a regression introduced by 53cad69 (post-receive-email: ensure > sent messages are not empty, 2010-09-10), I think. > >> diff --git a/contrib/hooks/post-receive-email >> b/contrib/hooks/post-receive-email >> index 85724bf..020536d 100755 >> --- a/contrib/hooks/post-receive-email >> +++ b/contrib/hooks/post-receive-email >> @@ -150,7 +150,7 @@ prep_for_email() >> # Anything else (is there anything else?) >> echo>&2 "*** Unknown type of update to $refname >> ($rev_type)" >> echo>&2 "*** - no email generated" >> - return 0 >> + return 1 > > This used to "exit 1" before 53cad69 and I agree with the patch that > signalling error with "return 1" is the right thing to do here. > >> ;; >> esac >> >> @@ -166,10 +166,10 @@ prep_for_email() >> esac >> echo>&2 "*** $config_name is not set so no email will be >> sent" >> echo>&2 "*** for $refname update $oldrev->$newrev" >> - return 0 >> + return 1 > > This used to "exit 0" before 53cad69 to cause the program stop before > sending mails. Again, I agree with the patch that signalling error is the > right thing to do here. > >> fi >> >> - return 1 >> + return 0 > > And this obviously is correct. > > Kevin, care to review and Ack? Alan, care to add a few lines of patch > description and sign-off? Acked-by: Kevin P. Fleming Yeah, this is clearly my breakage; our internal version of this script is so different that it has become hard to backport fixes to the upstream version... or I just did a terrible job of it. Alan, while you are in there fixing this, there is a remaining 'exit 0' in prep_for_email (at line 147) that should be 'return 1' instead. -- 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