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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2021, 18:55 UTC
Message-ID: <xmqqr1jt3kcr.fsf@gitster.g>
In-Reply-To: <YGYMG3mArYSeMr9D@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 15 quoted lines

>> 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.