Subject: Re: [PATCH v8 35/37] git-send-email: use 'git hook run' for 'sendemail-validate'

From: Junio C Hamano <[email-masked]>
Date: Thu, 01 Apr 2021 11:55:48 -0700

Emily Shaffer <[email-masked]> writes:

>> If we classify the failure cases into three using the sample code in
>> the doc, I think the last one is the only case that we know the
>> logic in the hook is making a decision for us.  In the first case,
>> the hook did not even have a chance to decide for us, and in the
>> second case, the hook died with signal, most likely before it had a
>> chance to make a decision.  If we want to be conservative (sending
>> a message out is something you cannot easily undo), then it may make
>> sense to take the first two failure cases, even though the hook may
>> have said it is OK to send it out if it ran successfully, as a denial
>> to be safe, I would think.
>
> Yeah, I tend to agree. In that case I think you are saying: "Please
> split the first case into two and differentiate launch failure from
> signal, but otherwise continue to return all these cases as errors and
> halt the email."

Not exactly.  I do not have a strong opinion either way to split the
first two cases apart or lump them together.  If I were pressed, I
probably would vote for the latter.

The doc's example classfies into three and I think that
classification is logical:

 * Lumping the first two together would make sense with respect to
   deciding what to do when we see a failure. The first two are
   "hook failed to approve or disapprove" case, while the last one
   is "the hook actively disapproved".  The former is not under
   hook's control.

 * Further, treating a failure even from the first "hook failed to
   approve or disapprove" as a signal to stop sending would be more
   conservative.

 * Which leads us to say, with respect to deciding what to do, any
   failure just stops the program from sending.

It is a separate matter how to phrase the diagnoses and hints for
recovery.  It could be that sendmail-validate hook failed to run due
to a simple misconfiguration (e.g. because it lacked the executable
bit).  Giving an error message with strerr would be helpful for the
"hook failed to approve or disapprove" case.