* Junio C Hamano <gitster@xxxxxxxxx> [09-09-08 22:15]:
> Lars Noschinski <lars@xxxxxxxxxxxxxxxxxxxx> writes:
>
> > contrib/hooks/post-receive-email used to call the send_mail function
> > (and thus, /usr/sbin/sendmail), even if generate_mail generated no
> > output. This is problematic, as the sendmail binary provided by exim4
> > generates an error mail if provided with an empty input.
>
> I actually have a bigger question, not about the implementation but about
> the cause.
>
> If generate_email results in an empty output in this codepath:
>
> # Check if we've got anyone to send to
> if [ -z "$recipients" ]; then
> ...
> echo >&2 "*** $config_name is not set so no email will be sent"
> echo >&2 "*** for $refname update $oldrev->$newrev"
> exit 0
> fi
>
> shouldn't we rather receive an error e-mail than let the
> misconfiguration go undetected?
Probably not. The error message is displayed to the user who did the
push. Normally (if no explicit From: address is configured), this is the
same user, which would receive the error mail.
> Before this check, I do not see anywhere generate_email would return nor
> exit, and after this check, there is a call to generate_email_header and
> that guarantees that the output from the generate_email function is not
> empty, so it looks to me that triggering this check is the only case your
> patch would change the behaviour of the script.
Actually, there are a two cases in the case statement before, where
generate_email would return:
refs/remotes/*,commit)
# tracking branch
refname_type="tracking branch"
short_refname=${refname##refs/remotes/}
echo >&2 "*** Push-update of tracking branch, $refname"
echo >&2 "*** - no email generated."
exit 0
;;
*)
# Anything else (is there anything else?)
echo >&2 "*** Unknown type of update to $refname ($rev_type)"
echo >&2 "*** - no email generated"
exit 1
;;
i.e. if we are pushing to a branch neither in refs/tags nor refs/heads.
In our setting, the build process pushes to refs/builds, so we can track
code changes between different builds, without displaying a whole lot of
mostly useless branches or tags to the user.
> > diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
> > index 2a66063..c855c31 100755
> > --- a/contrib/hooks/post-receive-email
> > +++ b/contrib/hooks/post-receive-email
> > @@ -637,6 +637,16 @@ show_new_revisions()
> >
> > send_mail()
> > {
> > + OIFS=$IFS
> > + IFS='
> > +'
> > + read FIRSTLINE || exit 1
>
> Shouldn't this be merely a "return"? The caller looks like this:
Yes.
I'll fix it in the next patch (when there are further comments); but you
may fold it in (and add the SOB if forgot), if you prefer.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html