On Mon, 14 Dec 2015 12:47:26 -0500 Andrew Gregory <andrew.gregory.8@gmail.com> wrote:
On 12/14/15 at 05:55pm, Olivier Brunel wrote:
Also include the abort_on_fail flag so the frontend can let the user know which hook(s) are responsible.
Signed-off-by: Olivier Brunel <jjk@jjacky.com> ---
This seems like overkill. A failed AbortOnFail hook will already generate 3 error messages (assuming the hook itself prints one). We could just label AbortOnFail hooks as such when we print the info during HOOK_RUN_START. Alternatively, and this might be preferable behavior anyway, we could simply stop running PreTransaction hooks as soon as an AbortOnFail hook fails.
But there's no guarantee the hook will actually print anything, so it is good for frontends to (be able to) do so. And this is the only way to tell which hook(s) are causing the transaction to be aborted, so I don't see this as overkill. Now I assumed there was a reason for remaining hooks to still be ran, but if the behavior is to be changed to stop processing hooks right away then it might not be needed indeed, since the faulty hook is always the last one...
lib/libalpm/alpm.h | 6 +++++- lib/libalpm/hook.c | 7 +++++-- src/pacman/callback.c | 4 ++++ 3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index f6e6b86..52cbd51 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -465,7 +465,9 @@ typedef enum _alpm_event_type_t { /** A hook is starting */ ALPM_EVENT_HOOK_RUN_START, /** A hook has finnished runnning */ - ALPM_EVENT_HOOK_RUN_DONE + ALPM_EVENT_HOOK_RUN_DONE, + /** A hook has failed to run. */ + ALPM_EVENT_HOOK_RUN_FAILED } alpm_event_type_t;
typedef struct _alpm_event_any_t { @@ -568,6 +570,8 @@ typedef struct _alpm_event_hook_run_t { alpm_event_type_t type; /** Text to be outputted */ const char *desc; + /** Whether the hook has the AbortOnFail flag set (pre-transaction only) */
This is not strictly true. Setting AbortOnFail on a PostTransaction hook generates a warning, but leaves the flag set. In that case your callback will falsely claim that it aborted the transaction.
Oh, I missed that. Easy enough to set the flag here only for pre-transaction hooks though. (Since it doesn't apply, why are abort_on_fail flags set for post-transaction hooks though?)