[pacman-dev] [PATCH] Add events ALPM_EVENT_HOOK_{START,DONE}
Add events to let frontends know when hooks are being processed (and when it's done), as that might be useful to update the UI. It can also be useful to know when ALPM_EVENT_SCRIPTLET_INFO might be happening in that regard (There's no specific event for running scriptlets, but they do happen at specific times after/before ALPM_EVENT_PACKAGE_OPERATION_{START,DONE}). Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- On a not-quite-related topic, I wonder whether a flag should be added to skip hook processing, much like ALPM_TRANS_FLAG_NOSCRIPTLET ? I guess it might not be necessary, since frontends can simply set the hookdirs to NULL to disable them. Thoughts? I also wondered about the same in the POV of pacman, i.e. an option --nohooks, because it seems currently one can only add hookdirs, and therefore only replace the pacman default (/etc/pacman.d/hooks) (I assume setting it to /dev/null might work to disable things?) but not the system/ALPM one (/usr/share/libalpm/hooks)? Also unless I missed it, the documentation doesn't mention that last directory anywhere? lib/libalpm/alpm.h | 24 +++++++++++++++++++++++- lib/libalpm/hook.c | 11 +++++++++-- lib/libalpm/hook.h | 7 +------ src/pacman/callback.c | 6 ++++++ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6cbcd24..f8e1f25 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -337,6 +337,16 @@ typedef struct _alpm_siglist_t { alpm_sigresult_t *results; } alpm_siglist_t; + +/* + * Hooks + */ + +typedef enum _alpm_hook_when_t { + ALPM_HOOK_PRE_TRANSACTION = 1, + ALPM_HOOK_POST_TRANSACTION +} alpm_hook_when_t; + /* * Logging facilities */ @@ -443,7 +453,11 @@ typedef enum _alpm_event_type_t { ALPM_EVENT_PACNEW_CREATED, /** A .pacsave file was created; See alpm_event_pacsave_created_t for * arguments */ - ALPM_EVENT_PACSAVE_CREATED + ALPM_EVENT_PACSAVE_CREATED, + /** Processing hooks will be started. */ + ALPM_EVENT_HOOK_START, + /** Processing hooks is finished. */ + ALPM_EVENT_HOOK_DONE } alpm_event_type_t; typedef struct _alpm_event_any_t { @@ -534,6 +548,13 @@ typedef struct _alpm_event_pacsave_created_t { const char *file; } alpm_event_pacsave_created_t; +typedef struct _alpm_event_hook_t { + /** Type of event.*/ + alpm_event_type_t type; + /** Type of hooks. */ + alpm_hook_when_t when; +} alpm_event_hook_t; + /** Events. * This is an union passed to the callback, that allows the frontend to know * which type of event was triggered (via type). It is then possible to @@ -550,6 +571,7 @@ typedef union _alpm_event_t { alpm_event_pkgdownload_t pkgdownload; alpm_event_pacnew_created_t pacnew_created; alpm_event_pacsave_created_t pacsave_created; + alpm_event_hook_t hook; } alpm_event_t; /** Event callback. */ diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index f5627af..09f7028 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -52,7 +52,7 @@ struct _alpm_hook_t { alpm_list_t *depends; char **cmd; alpm_list_t *matches; - enum _alpm_hook_when_t when; + alpm_hook_when_t when; int abort_on_fail, needs_targets; }; @@ -608,13 +608,17 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook) } } -int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when) +int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) { + alpm_event_hook_t event = { .when = when }; alpm_list_t *i, *hooks = NULL; const char *suffix = ".hook"; size_t suflen = strlen(suffix); int ret = 0; + event.type = ALPM_EVENT_HOOK_START; + EVENT(handle, &event); + for(i = alpm_list_last(handle->hookdirs); i; i = alpm_list_previous(i)) { int err; char path[PATH_MAX]; @@ -723,6 +727,9 @@ cleanup: alpm_list_free_inner(hooks, (alpm_list_fn_free) _alpm_hook_free); alpm_list_free(hooks); + event.type = ALPM_EVENT_HOOK_DONE; + EVENT(handle, &event); + return ret; } diff --git a/lib/libalpm/hook.h b/lib/libalpm/hook.h index 4894a19..3f05b31 100644 --- a/lib/libalpm/hook.h +++ b/lib/libalpm/hook.h @@ -22,12 +22,7 @@ #include "alpm.h" -enum _alpm_hook_when_t { - ALPM_HOOK_PRE_TRANSACTION = 1, - ALPM_HOOK_POST_TRANSACTION -}; - -int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when); +int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when); #endif /* _ALPM_HOOK_H */ diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 7a21b22..d59de73 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -167,6 +167,11 @@ void cb_event(alpm_event_t *event) return; } switch(event->type) { + case ALPM_EVENT_HOOK_START: + printf(_("running %s hooks...\n"), + event->hook.when == ALPM_HOOK_PRE_TRANSACTION + ? _("pre-transaction") : _("post-transaction")); + break; case ALPM_EVENT_CHECKDEPS_START: printf(_("checking dependencies...\n")); break; @@ -329,6 +334,7 @@ void cb_event(alpm_event_t *event) case ALPM_EVENT_DISKSPACE_DONE: case ALPM_EVENT_RETRIEVE_DONE: case ALPM_EVENT_RETRIEVE_FAILED: + case ALPM_EVENT_HOOK_DONE: /* we can safely ignore those as well */ case ALPM_EVENT_PKGDOWNLOAD_START: case ALPM_EVENT_PKGDOWNLOAD_DONE: -- 2.6.3
On Sun, 6 Dec 2015 18:35:37 +0100 Olivier Brunel <jjk@jjacky.com> wrote:
Add events to let frontends know when hooks are being processed (and when it's done), as that might be useful to update the UI.
It can also be useful to know when ALPM_EVENT_SCRIPTLET_INFO might be happening in that regard (There's no specific event for running scriptlets, but they do happen at specific times after/before ALPM_EVENT_PACKAGE_OPERATION_{START,DONE}).
Signed-off-by: Olivier Brunel <jjk@jjacky.com> ---
Forgot to say, I know there's a freeze for 5.0, but I figured you might wanna add this in, because if for pacman you're fine not having this information, for other frontends not knowing what's happening/that something is running, and out of nowhere getting some SCRIPTLET_INFO events, might be "odd"/problematic to provide good/consistent UI. And since such changes involve API changes, I feel it might be good to have at least those events in the 5.0 release.
On 09/12/15 23:00, Olivier Brunel wrote:
On Sun, 6 Dec 2015 18:35:37 +0100 Olivier Brunel <jjk@jjacky.com> wrote:
Add events to let frontends know when hooks are being processed (and when it's done), as that might be useful to update the UI.
It can also be useful to know when ALPM_EVENT_SCRIPTLET_INFO might be happening in that regard (There's no specific event for running scriptlets, but they do happen at specific times after/before ALPM_EVENT_PACKAGE_OPERATION_{START,DONE}).
Signed-off-by: Olivier Brunel <jjk@jjacky.com> ---
Forgot to say, I know there's a freeze for 5.0, but I figured you might wanna add this in, because if for pacman you're fine not having this information, for other frontends not knowing what's happening/that something is running, and out of nowhere getting some SCRIPTLET_INFO events, might be "odd"/problematic to provide good/consistent UI.
And since such changes involve API changes, I feel it might be good to have at least those events in the 5.0 release.
I intend to include this - in fact, I want pacman to print a message when it runs hooks because there could be quite a wait depending on the hook. I'll get around to look at it soon! A
On 07/12/15 03:35, Olivier Brunel wrote:
Add events to let frontends know when hooks are being processed (and when it's done), as that might be useful to update the UI.
It can also be useful to know when ALPM_EVENT_SCRIPTLET_INFO might be happening in that regard (There's no specific event for running scriptlets, but they do happen at specific times after/before ALPM_EVENT_PACKAGE_OPERATION_{START,DONE}).
Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- On a not-quite-related topic, I wonder whether a flag should be added to skip hook processing, much like ALPM_TRANS_FLAG_NOSCRIPTLET ? I guess it might not be necessary, since frontends can simply set the hookdirs to NULL to disable them. Thoughts?
I also wondered about the same in the POV of pacman, i.e. an option --nohooks, because it seems currently one can only add hookdirs, and therefore only replace the pacman default (/etc/pacman.d/hooks) (I assume setting it to /dev/null might work to disable things?) but not the system/ALPM one (/usr/share/libalpm/hooks)?
Also unless I missed it, the documentation doesn't mention that last directory anywhere?
Agreed - we need: - document --hookdir in pacman.8. It should be mentioned that this is what HookDir is overriding in pacman-conf.8 too. - add a --nohooks option And now, onto patch review. I think _alpm_hook_run needs to exit after reading the hooks directories if there is no hooks. Only if there are hooks should ALPM_EVENT_HOOK_START be called. It would be even better if it was called if there were hooks for pre or post transaction. That would require _alpm_hook_run to first loop over the hooks doing: if(hook && hook->when == when && _alpm_hook_triggered(handle, hook)) { then if any hooks matched, do ALPM_EVENT_HOOK_START and run the hooks. I think that check is fast enough that we do no need a callback until we know some hooks will be run. Andrew: would you agree? Here goes my imaginary output when coreutils triggers an info-page update (before and after) and firefox requires a desktop file database after installation: (2/2) checking keys in keyring [######################] 100% (2/2) checking package integrity [######################] 100% (2/2) loading package files [######################] 100% (2/2) checking for file conflicts [######################] 100% :: Running pre-transaction hooks (1/1) info-pages-remove :: Installing (1/2) installing coreutils [######################] 100% (2/2) installing firefox [######################] 100% :: Running post-transaction hooks (1/2) info-pages-install (2/2) desktop-database What is printed is the hook name (I would like to enable this to be configured in the hook later). Allan
On 12/13/15 at 10:38pm, Allan McRae wrote:
On 07/12/15 03:35, Olivier Brunel wrote:
Add events to let frontends know when hooks are being processed (and when it's done), as that might be useful to update the UI.
It can also be useful to know when ALPM_EVENT_SCRIPTLET_INFO might be happening in that regard (There's no specific event for running scriptlets, but they do happen at specific times after/before ALPM_EVENT_PACKAGE_OPERATION_{START,DONE}).
Signed-off-by: Olivier Brunel <jjk@jjacky.com> --- On a not-quite-related topic, I wonder whether a flag should be added to skip hook processing, much like ALPM_TRANS_FLAG_NOSCRIPTLET ? I guess it might not be necessary, since frontends can simply set the hookdirs to NULL to disable them. Thoughts?
I also wondered about the same in the POV of pacman, i.e. an option --nohooks, because it seems currently one can only add hookdirs, and therefore only replace the pacman default (/etc/pacman.d/hooks) (I assume setting it to /dev/null might work to disable things?) but not the system/ALPM one (/usr/share/libalpm/hooks)?
Also unless I missed it, the documentation doesn't mention that last directory anywhere?
Agreed - we need: - document --hookdir in pacman.8. It should be mentioned that this is what HookDir is overriding in pacman-conf.8 too. - add a --nohooks option
And now, onto patch review.
I think _alpm_hook_run needs to exit after reading the hooks directories if there is no hooks. Only if there are hooks should ALPM_EVENT_HOOK_START be called. It would be even better if it was called if there were hooks for pre or post transaction.
That would require _alpm_hook_run to first loop over the hooks doing: if(hook && hook->when == when && _alpm_hook_triggered(handle, hook)) { then if any hooks matched, do ALPM_EVENT_HOOK_START and run the hooks. I think that check is fast enough that we do no need a callback until we know some hooks will be run. Andrew: would you agree?
Agreed. It has been plenty fast in all of my tests so far. apg
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Olivier Brunel