[pacman-dev] [PATCH] alpm: Add event ALPM_EVENT_HOOK_RUN_FAILED

[Andrew Gregory]

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

>  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