Re: [PATCH] Corrected return values in post-receive-email.prep_for_email
- From
- Kevin P. Fleming <kpfleming@digium.com>
- Date
- Dec 7, 2010, 19:44 UTC
- 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:
Show 51 quoted lines
> "Alan Raison"<alan@theraisons.me.uk> 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 <kpfleming@digium.com>
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