[pacman-dev] [PATCH] alpm: Abort ASAP on failure in pre-transaction hooks

[Andrew Gregory]

On 12/15/15 at 12:07am, Olivier Brunel wrote:
> There is no need to run all/remaining pre-transaction hooks as soon as a failure
> has occured, which will lead to aborting the transaction.
> 
> So as soon as an error in the first phase (reading directories/parsing files)
> occurs, or a hook flagged abort_on_fail does fail, we stop processing and
> return.
> 
> (For post-transaction hooks, all hooks are run regardless since there's no
> aborting.)
> 
> Signed-off-by: Olivier Brunel <jjk at jjacky.com>
> ---
> As discussed earlier with Andrew, this "replaces" the previous "alpm: Add event
> ALPM_EVENT_HOOK_RUN_FAILED" patch I sent.

Remove the first two checks that break out of the parsing loop and I'm
good with this.  Going ahead with parsing after errors will allow us
to give the user all parsing errors at once instead of only providing
a single error and potentially forcing the user into a run pacman, fix
hook, run pacman, fix next hook, ...  cycle.

>  lib/libalpm/hook.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
> index acd571d..9b204fb 100644
> --- a/lib/libalpm/hook.c
> +++ b/lib/libalpm/hook.c
> @@ -624,6 +624,10 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
>  		struct dirent entry, *result;
>  		DIR *d;
>  
> +		if(ret != 0 && when == ALPM_HOOK_PRE_TRANSACTION) {
> +			break;
> +		}
> +
>  		if((dirlen = strlen(i->data)) >= PATH_MAX) {
>  			_alpm_log(handle, ALPM_LOG_ERROR, _("could not open directory: %s: %s\n"),
>  					(char *)i->data, strerror(ENAMETOOLONG));
> @@ -648,6 +652,10 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
>  			struct stat buf;
>  			size_t name_len;
>  
> +			if(ret != 0 && when == ALPM_HOOK_PRE_TRANSACTION) {
> +				break;
> +			}
> +
>  			if(strcmp(entry.d_name, ".") == 0 || strcmp(entry.d_name, "..") == 0) {
>  					continue;
>  			}
> @@ -708,6 +716,10 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
>  		closedir(d);
>  	}
>  
> +	if(ret != 0 && when == ALPM_HOOK_PRE_TRANSACTION) {
> +		goto cleanup;
> +	}
> +
>  	hooks = alpm_list_msort(hooks, alpm_list_count(hooks),
>  			(alpm_list_fn_cmp)_alpm_hook_cmp);
>  
> @@ -743,6 +755,10 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
>  
>  		hook_event.type = ALPM_EVENT_HOOK_RUN_DONE;
>  		EVENT(handle, &hook_event);
> +
> +		if(ret != 0 && when == ALPM_HOOK_PRE_TRANSACTION) {
> +			break;
> +		}
>  	}
>  
>  	event.type = ALPM_EVENT_HOOK_DONE;
> -- 
> 2.6.4