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 <[email protected]>
> ---
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.
> 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.
> + int abort_on_fail;
> /** position of hook being run */
> size_t position;
> /** total hooks being run */
> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
> index acd571d..41dacbe 100644
> --- a/lib/libalpm/hook.c
> +++ b/lib/libalpm/hook.c
> @@ -731,17 +731,20 @@ int _alpm_hook_run(alpm_handle_t *handle,
> alpm_hook_when_t when)
>
> for(i = hooks_triggered; i; i = i->next, hook_event.position++) {
> struct _alpm_hook_t *hook = i->data;
> + int r;
> _alpm_log(handle, ALPM_LOG_DEBUG, "running hook %s\n",
> hook->name);
>
> hook_event.type = ALPM_EVENT_HOOK_RUN_START;
> + hook_event.abort_on_fail = hook->abort_on_fail;
> hook_event.desc = hook->name;
> EVENT(handle, &hook_event);
>
> - if(_alpm_hook_run_hook(handle, hook) != 0 &&
> hook->abort_on_fail) {
> + r = _alpm_hook_run_hook(handle, hook);
> + if(r != 0 && hook->abort_on_fail) {
> ret = -1;
> }
>
> - hook_event.type = ALPM_EVENT_HOOK_RUN_DONE;
> + hook_event.type = (r != 0) ? ALPM_EVENT_HOOK_RUN_FAILED :
> ALPM_EVENT_HOOK_RUN_DONE;
> EVENT(handle, &hook_event);
> }
>
> diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> index ae725d3..02d9b64 100644
> --- a/src/pacman/callback.c
> +++ b/src/pacman/callback.c
> @@ -184,6 +184,10 @@ void cb_event(alpm_event_t *event)
> digits, e->total, e->desc);
> }
> break;
> + case ALPM_EVENT_HOOK_RUN_FAILED:
> + if (event->hook_run.abort_on_fail)
> + printf(_("Hook failed; Abort transaction.\n"));
Always use braces.
> + break;
> case ALPM_EVENT_CHECKDEPS_START:
> printf(_("checking dependencies...\n"));
> break;
> --
> 2.6.4
