[pacman-dev] Pacman Hooks
Hello, I used this https://wiki.archlinux.org/index.php/User:Allan/Pacman_Hooks as a Project to learn c (so keep that in mind when you criticize the patch :-) ). Anyway, here's an example config file: /etc/pacman.d/hooks.conf: [testhook] Package = lynx nano Package = vim When = PreRemove [fonts] File = usr/share/fonts/* When = PostUpgrade --------------------------------------------------- You can define more than one Package that triggers a hook, either separated by spaces in the "Package ="line, or in a separate "Package =" line or a mix of both. The "File = " argument can be anything that get matched by fnmatch(...). You can also use "Package" and "File" within the same hook. In this case either one of them will trigger the hook (in other words: Package || File). "When" can be one of the following: PreRemove, PostRemove, PreUpgrade, PostUpgrade, PreInstall or PostInstall. For each hook, there needs to be a corresponding file in /etc/pacman.d/hooks.d. pacman.d ├── hooks.conf ├── hooks.d │ ├── fonts* │ └── testhook* Things i don't like, but i don't really now how to fix those: - The global variable HOOK_LIST - The paths to hooks.d and hooks.conf are hardcoded - The parsing of the config file is really ugly - The "File" argument can't begin with a / because it is matched against alpm_pkg_get_data and that file-strings don't have the leading / Greetings, Sascha Kruse P.s I don't mind, if you say, that this is too broken to get included.
From 9ac289d0c495a05239441a8ffe33975f32c3ef97 Mon Sep 17 00:00:00 2001 From: Sascha Kruse <knopwob@googlemail.com> Date: Sat, 29 Jan 2011 15:00:55 +0100 Subject: [PATCH] hooks
--- lib/libalpm/Makefile.am | 1 + lib/libalpm/add.c | 5 + lib/libalpm/alpm.c | 1 + lib/libalpm/hooks.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/hooks.h | 43 +++++++++ lib/libalpm/remove.c | 3 + lib/libalpm/trans.c | 2 + 7 files changed, 272 insertions(+), 0 deletions(-) create mode 100644 lib/libalpm/hooks.c create mode 100644 lib/libalpm/hooks.h diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am index da663cb..f23da73 100644 --- a/lib/libalpm/Makefile.am +++ b/lib/libalpm/Makefile.am @@ -38,6 +38,7 @@ libalpm_la_SOURCES = \ graph.h \ group.h group.c \ handle.h handle.c \ + hooks.h hooks.c \ log.h log.c \ package.h package.c \ remove.h remove.c \ diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index d4c56de..5bb0b0f 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -48,6 +48,7 @@ #include "deps.h" #include "remove.h" #include "handle.h" +#include "hooks.h" /** Add a file target to the transaction. * @param target the name of the file target to add @@ -516,6 +517,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, newpkg->origin_data.file, "pre_upgrade", newpkg->version, oldpkg->version, trans); } + _trigger_hooks(newpkg, "PreUpgrade"); } else { is_upgrade = 0; @@ -528,6 +530,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, newpkg->origin_data.file, "pre_install", newpkg->version, NULL, trans); } + _trigger_hooks(newpkg, "PreInstall"); } /* we override any pre-set reason if we have alldeps or allexplicit set */ @@ -696,9 +699,11 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, scriptlet, "post_upgrade", alpm_pkg_get_version(newpkg), oldpkg ? alpm_pkg_get_version(oldpkg) : NULL, trans); + _trigger_hooks(newpkg, "PostUpgrade"); } else { _alpm_runscriptlet(handle->root, scriptlet, "post_install", alpm_pkg_get_version(newpkg), NULL, trans); + _trigger_hooks(newpkg, "PostInstall"); } } diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 2a9f460..e72de6d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -63,6 +63,7 @@ int SYMEXPORT alpm_initialize(void) fetchConnectionCacheInit(5, 1); #endif + _parse_hooks_config(); return(0); } diff --git a/lib/libalpm/hooks.c b/lib/libalpm/hooks.c new file mode 100644 index 0000000..e1ecfab --- /dev/null +++ b/lib/libalpm/hooks.c @@ -0,0 +1,217 @@ +/* + * package.h + * + * Copyright (c) 2006-2011 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include "config.h" + +#include <alpm.h> +#include <alpm_list.h> +#include <stdio.h> +#include <string.h> +#include <ctype.h> +#include <stdlib.h> +#include <fnmatch.h> +/* pacman */ +#include "package.h" +#include "util.h" +#include "hooks.h" + + +int _run_hook(hook_t *hook) +{ + char *cmd; + int retval; + + cmd = malloc(strlen("/etc/pacman.d/hooks.d/") + strlen(hook->name) + 1); + sprintf(cmd, "/etc/pacman.d/hooks.d/%s", hook->name); + + retval = system(cmd); + if(retval == -1) { + } else if(retval != 0) { + } + return (retval); +} + +void _run_postponed_hooks(void) +{ + hook_t *hook; + alpm_list_t *i; + + for(i = HOOK_LIST; i; i = alpm_list_next(i)) { + hook = alpm_list_getdata(i); + if(hook->triggered) { + _run_hook(hook); + } + } +} + +void _trigger_hooks(pmpkg_t *pkg, char *when) +{ + hook_t *hook; + alpm_list_t *i, *j, *k; + alpm_list_t *pkgfiles; + const char *pkgname; + + pkgname = alpm_pkg_get_name(pkg); + pkgfiles = alpm_pkg_get_files(pkg); + + for(i = HOOK_LIST; i; i = alpm_list_next(i)) { + hook = alpm_list_getdata(i); + + /* trigger package based hooks */ + if(strcmp(hook->when, when) == 0 || + strcmp(hook->when, "Transaction") == 0) { + for(j = hook->pkg_triggers; j; j = alpm_list_next(j)) { + if(strcmp(pkgname, alpm_list_getdata(j)) == 0) { + if(strcmp(when, "Install") == 0 + || strcmp(when, "PostInstall") == 0 + || strcmp(when, "PreRemove") == 0) { + _run_hook(hook); + return; + } else { + hook->triggered++; + return; + } + } + } + + /* trigger file based hooks */ + for(j = hook->file_triggers; j; j = alpm_list_next(j)) { + for(k = pkgfiles; k; k = alpm_list_next(k)) { + if(fnmatch(alpm_list_getdata(j), alpm_list_getdata(k), 0) == 0) { + if(strcmp(when, "Install") == 0 + || strcmp(when, "PostInstall") == 0 + || strcmp(when, "PreRemove") == 0) { + _run_hook(hook); + return; + } else { + hook->triggered++; + return; + } + } + } + } + } + } +} + +int _parse_hooks_config(void) +{ + FILE *config; + char linebuf[128]; + char *cur, *look_ahead, *tmp, *key, *value; + + hook_t *tmp_hook = NULL; + + + config = fopen("/etc/pacman.d/hooks.conf", "r"); + + HOOK_LIST = NULL; + + if(config == NULL) { + return (1); + } + + while(fgets(linebuf, 127, config)) { + /* skip over whitespaces */ + for(cur = linebuf; isspace(*cur) ; cur++); + + /* ignore lines beginning with '#' */ + if(*cur == '#') { + continue; + } + if(*cur == '\0') { + continue; + } + + if(*cur == '[') { + /* we enter a new section */ + if(tmp_hook != NULL) { + /* ignore packages without name */ + if(tmp_hook->name != NULL) { + HOOK_LIST = alpm_list_add(HOOK_LIST, tmp_hook); + } + } + tmp_hook = malloc(sizeof(hook_t)); + tmp_hook->name = NULL; + tmp_hook->pkg_triggers = NULL; + tmp_hook->file_triggers = NULL; + tmp_hook->triggered = 0; + tmp_hook->when = 0; + + cur++; + for(look_ahead = cur; *look_ahead != ']' && *look_ahead != '\0'; look_ahead++); + if(*look_ahead == '\0') { + return (2); + } + + tmp_hook->name = malloc(look_ahead - cur + 1); + strncpy(tmp_hook->name, cur, look_ahead - cur); + tmp_hook->name[look_ahead - cur] = '\0'; + continue; + } + + key = cur; + value = strstr(linebuf, "="); + if(value == NULL) { + return (2); + } + + value++; + for(; isspace(*value) && *value != '\0'; value++); + if(*value == '\0') { + return 2; + } + + if(strncmp("Package", key, strlen("Package")) == 0) { + while(*value != '\0') { + for(look_ahead = value; isalnum(*look_ahead); look_ahead++); + tmp = malloc(look_ahead - value + 1); + strncpy(tmp, value, look_ahead - value); + tmp[look_ahead - value] = '\0'; + tmp_hook->pkg_triggers = alpm_list_add(tmp_hook->pkg_triggers, tmp); + tmp = NULL; + value = look_ahead+1; + /* skip whitespaces */ + for(; isspace(*value) ; value++); + } + } + + if(strncmp("File", key, strlen("File")) == 0) { + for(look_ahead = value; *look_ahead != '\n'; look_ahead++); + tmp = malloc(look_ahead - value + 1); + strncpy(tmp, value, look_ahead - value); + tmp[look_ahead - value + 1] = '\0'; + tmp_hook->file_triggers = alpm_list_add(tmp_hook->file_triggers, tmp); + } + if(strncmp("When", key, strlen("When")) == 0) { + for(look_ahead = value; isalnum(*look_ahead); look_ahead++); + tmp_hook->when = malloc(look_ahead - value + 1); + strncpy(tmp_hook->when, value, look_ahead - value); + tmp_hook->when[look_ahead - value] = '\0'; + } + } + if(tmp_hook != NULL) { + /* ignore hooks without name */ + if(tmp_hook->name != NULL) { + HOOK_LIST = alpm_list_add(HOOK_LIST, tmp_hook); + } + } + return 0; +} +/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/hooks.h b/lib/libalpm/hooks.h new file mode 100644 index 0000000..15bd47e --- /dev/null +++ b/lib/libalpm/hooks.h @@ -0,0 +1,43 @@ +/* + * package.h + * + * Copyright (c) 2006-2011 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef _PM_HOOKS_H +#define _PM_HOOKS_H + +#include <alpm.h> + +typedef struct hook { + char *name; + alpm_list_t *pkg_triggers; + alpm_list_t *file_triggers; + int triggered; + char *when; +} hook_t; + +/* global Variables */ +alpm_list_t *HOOK_LIST; + + +void _run_postponed_hooks(void); +void _trigger_hooks(pmpkg_t *pkg, char *when); +int _parse_hooks_config(void); +#endif /* _PM_HOOKS_H */ + +/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 5fba0b0..3bd32e2 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -45,6 +45,7 @@ #include "deps.h" #include "handle.h" #include "alpm.h" +#include "hooks.h" int SYMEXPORT alpm_remove_target(const char *target) { @@ -409,6 +410,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) _alpm_runscriptlet(handle->root, scriptlet, "pre_remove", alpm_pkg_get_version(info), NULL, trans); } + _trigger_hooks(info, "PreRemove"); if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { alpm_list_t *files = alpm_pkg_get_files(info); @@ -454,6 +456,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) _alpm_runscriptlet(handle->root, scriptlet, "post_remove", alpm_pkg_get_version(info), NULL, trans); } + _trigger_hooks(info, "PostRemve"); /* remove the package from the database */ _alpm_log(PM_LOG_DEBUG, "updating database\n"); diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 804ab7a..64b6e7d 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -45,6 +45,7 @@ #include "sync.h" #include "alpm.h" #include "deps.h" +#include "hooks.h" /** \addtogroup alpm_trans Transaction Functions * @brief Functions to manipulate libalpm transactions @@ -209,6 +210,7 @@ int SYMEXPORT alpm_trans_commit(alpm_list_t **data) return(-1); } } + _run_postponed_hooks(); trans->state = STATE_COMMITED; -- 1.7.3.5
On Sat, Jan 29, 2011 at 8:36 AM, Sascha Kruse <knopwob@googlemail.com> wrote:
Hello,
I used this https://wiki.archlinux.org/index.php/User:Allan/Pacman_Hooks as a Project to learn c (so keep that in mind when you criticize the patch :-) ). Welcome!
Anyway, here's an example config file:
/etc/pacman.d/hooks.conf: [testhook]
Package = lynx nano Package = vim
When = PreRemove
[fonts] File = usr/share/fonts/* When = PostUpgrade --------------------------------------------------- You can define more than one Package that triggers a hook, either separated by spaces in the "Package ="line, or in a separate "Package =" line or a mix of both. The "File = " argument can be anything that get matched by fnmatch(...). You can also use "Package" and "File" within the same hook. In this case either one of them will trigger the hook (in other words: Package || File). "When" can be one of the following: PreRemove, PostRemove, PreUpgrade, PostUpgrade, PreInstall or PostInstall. So are these on a per-package or per-total-operation basis? A huge part of hooks is something like installing a load of font packages; any command that these would normally run is postponed until the end of the transaction and then only needs to run once. Same for icon-cache, info reindexing, etc. or whatever other things you see in 5 install scripts of packages.
For each hook, there needs to be a corresponding file in /etc/pacman.d/hooks.d. pacman.d ├── hooks.conf ├── hooks.d │ ├── fonts* │ └── testhook*
Things i don't like, but i don't really now how to fix those: - The global variable HOOK_LIST It belongs on the handle object, more than likely- this is where everything global is captured.
- The paths to hooks.d and hooks.conf are hardcoded - The parsing of the config file is really ugly And also, seeing that the patch contains zero changes in src/...we never ever parse config stuff in the backend, it needs to get passed through by the frontend.
- The "File" argument can't begin with a / because it is matched against alpm_pkg_get_data and that file-strings don't have the leading / This is a must-have, your logic is backwards. We never assume the install root is /; file paths in all install scripts should be written this way too, not to mention everywhere in the library. So nothing wrong here, it would be wrong if you did any prefix whatsoever.
Greetings,
Sascha Kruse
P.s I don't mind, if you say, that this is too broken to get included.
From 9ac289d0c495a05239441a8ffe33975f32c3ef97 Mon Sep 17 00:00:00 2001 From: Sascha Kruse <knopwob@googlemail.com> Date: Sat, 29 Jan 2011 15:00:55 +0100 Subject: [PATCH] hooks
--- lib/libalpm/Makefile.am | 1 + lib/libalpm/add.c | 5 + lib/libalpm/alpm.c | 1 + lib/libalpm/hooks.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/hooks.h | 43 +++++++++ lib/libalpm/remove.c | 3 + lib/libalpm/trans.c | 2 + 7 files changed, 272 insertions(+), 0 deletions(-) create mode 100644 lib/libalpm/hooks.c create mode 100644 lib/libalpm/hooks.h
diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am index da663cb..f23da73 100644 --- a/lib/libalpm/Makefile.am +++ b/lib/libalpm/Makefile.am @@ -38,6 +38,7 @@ libalpm_la_SOURCES = \ graph.h \ group.h group.c \ handle.h handle.c \ + hooks.h hooks.c \ log.h log.c \ package.h package.c \ remove.h remove.c \ diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index d4c56de..5bb0b0f 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -48,6 +48,7 @@ #include "deps.h" #include "remove.h" #include "handle.h" +#include "hooks.h"
/** Add a file target to the transaction. * @param target the name of the file target to add @@ -516,6 +517,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, newpkg->origin_data.file, "pre_upgrade", newpkg->version, oldpkg->version, trans); } + _trigger_hooks(newpkg, "PreUpgrade");
If the function is in another file, it should be named _alpm, which is the standard prefix for "private" alpm functions.
} else { is_upgrade = 0;
@@ -528,6 +530,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, newpkg->origin_data.file, "pre_install", newpkg->version, NULL, trans); } + _trigger_hooks(newpkg, "PreInstall"); }
/* we override any pre-set reason if we have alldeps or allexplicit set */ @@ -696,9 +699,11 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, scriptlet, "post_upgrade", alpm_pkg_get_version(newpkg), oldpkg ? alpm_pkg_get_version(oldpkg) : NULL, trans); + _trigger_hooks(newpkg, "PostUpgrade"); } else { _alpm_runscriptlet(handle->root, scriptlet, "post_install", alpm_pkg_get_version(newpkg), NULL, trans); + _trigger_hooks(newpkg, "PostInstall"); } }
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 2a9f460..e72de6d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -63,6 +63,7 @@ int SYMEXPORT alpm_initialize(void) fetchConnectionCacheInit(5, 1); #endif
+ _parse_hooks_config(); DBPath, all other things, etc. haven't been set here so this is a non-starter. But as I already mentioned anyway, I think the config needs to come from the frontend so this will likely go away anyway.
return(0); }
diff --git a/lib/libalpm/hooks.c b/lib/libalpm/hooks.c new file mode 100644 index 0000000..e1ecfab --- /dev/null +++ b/lib/libalpm/hooks.c @@ -0,0 +1,217 @@ +/* + * package.h + * + * Copyright (c) 2006-2011 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> No need for the earlier copyright if this is a new file.
+ * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include "config.h" + +#include <alpm.h> +#include <alpm_list.h> +#include <stdio.h> +#include <string.h> +#include <ctype.h> +#include <stdlib.h> +#include <fnmatch.h> +/* pacman */ Um, we're in the backend, so this comment is definitely a no-go. +#include "package.h" +#include "util.h" +#include "hooks.h" + + +int _run_hook(hook_t *hook) static +{ + char *cmd; + int retval; + + cmd = malloc(strlen("/etc/pacman.d/hooks.d/") + strlen(hook->name) + 1); Use MALLOC macros please. + sprintf(cmd, "/etc/pacman.d/hooks.d/%s", hook->name); + + retval = system(cmd); Err...take a look at util.c:_alpm_run_chroot() and the places we use that. You have tremendously oversimplified the setup necessary to run external commands and have them operate on the correct root and paths.
+ if(retval == -1) { + } else if(retval != 0) { + } I'll consider this work in progress?
+ return (retval); +} + +void _run_postponed_hooks(void) static
+{ + hook_t *hook; + alpm_list_t *i; + + for(i = HOOK_LIST; i; i = alpm_list_next(i)) { + hook = alpm_list_getdata(i); hook can be local inside this loop.
+ if(hook->triggered) { + _run_hook(hook); + } + } +} + +void _trigger_hooks(pmpkg_t *pkg, char *when) const char *when +{ + hook_t *hook; + alpm_list_t *i, *j, *k; + alpm_list_t *pkgfiles; + const char *pkgname; + + pkgname = alpm_pkg_get_name(pkg); + pkgfiles = alpm_pkg_get_files(pkg); + + for(i = HOOK_LIST; i; i = alpm_list_next(i)) { + hook = alpm_list_getdata(i); + + /* trigger package based hooks */ + if(strcmp(hook->when, when) == 0 || + strcmp(hook->when, "Transaction") == 0) { Not following this logic...you are comparing to the passed in value and to the static string "Transaction"? Where did that come from?
+ for(j = hook->pkg_triggers; j; j = alpm_list_next(j)) { + if(strcmp(pkgname, alpm_list_getdata(j)) == 0) { + if(strcmp(when, "Install") == 0 + || strcmp(when, "PostInstall") == 0 + || strcmp(when, "PreRemove") == 0) { + _run_hook(hook); + return; + } else { + hook->triggered++; + return; + } + } + } + + /* trigger file based hooks */ + for(j = hook->file_triggers; j; j = alpm_list_next(j)) { + for(k = pkgfiles; k; k = alpm_list_next(k)) { + if(fnmatch(alpm_list_getdata(j), alpm_list_getdata(k), 0) == 0) { j, k- please use more descriptive variables, and use a local var to prevent calling getdata() multiple times on the same list variable.
+ if(strcmp(when, "Install") == 0 + || strcmp(when, "PostInstall") == 0 + || strcmp(when, "PreRemove") == 0) { + _run_hook(hook); + return; + } else { + hook->triggered++; + return; + } + } + } + } + } + } +} +
+int _parse_hooks_config(void) +{ + FILE *config; + char linebuf[128]; + char *cur, *look_ahead, *tmp, *key, *value; + + hook_t *tmp_hook = NULL; + + + config = fopen("/etc/pacman.d/hooks.conf", "r"); + + HOOK_LIST = NULL; + + if(config == NULL) { + return (1); + } + + while(fgets(linebuf, 127, config)) { + /* skip over whitespaces */ + for(cur = linebuf; isspace(*cur) ; cur++); + + /* ignore lines beginning with '#' */ + if(*cur == '#') { + continue; + } + if(*cur == '\0') { + continue; + } + + if(*cur == '[') { + /* we enter a new section */ + if(tmp_hook != NULL) { + /* ignore packages without name */ + if(tmp_hook->name != NULL) { + HOOK_LIST = alpm_list_add(HOOK_LIST, tmp_hook); + } + } + tmp_hook = malloc(sizeof(hook_t)); + tmp_hook->name = NULL; + tmp_hook->pkg_triggers = NULL; + tmp_hook->file_triggers = NULL; + tmp_hook->triggered = 0; + tmp_hook->when = 0; + + cur++; + for(look_ahead = cur; *look_ahead != ']' && *look_ahead != '\0'; look_ahead++); + if(*look_ahead == '\0') { + return (2); + } + + tmp_hook->name = malloc(look_ahead - cur + 1); + strncpy(tmp_hook->name, cur, look_ahead - cur); + tmp_hook->name[look_ahead - cur] = '\0'; + continue; + } + + key = cur; + value = strstr(linebuf, "="); + if(value == NULL) { + return (2); + } + + value++; + for(; isspace(*value) && *value != '\0'; value++); + if(*value == '\0') { + return 2; + } + + if(strncmp("Package", key, strlen("Package")) == 0) { + while(*value != '\0') { + for(look_ahead = value; isalnum(*look_ahead); look_ahead++); + tmp = malloc(look_ahead - value + 1); + strncpy(tmp, value, look_ahead - value); + tmp[look_ahead - value] = '\0'; + tmp_hook->pkg_triggers = alpm_list_add(tmp_hook->pkg_triggers, tmp); + tmp = NULL; + value = look_ahead+1; + /* skip whitespaces */ + for(; isspace(*value) ; value++); + } + } + + if(strncmp("File", key, strlen("File")) == 0) { + for(look_ahead = value; *look_ahead != '\n'; look_ahead++); + tmp = malloc(look_ahead - value + 1); + strncpy(tmp, value, look_ahead - value); + tmp[look_ahead - value + 1] = '\0'; + tmp_hook->file_triggers = alpm_list_add(tmp_hook->file_triggers, tmp); + } + if(strncmp("When", key, strlen("When")) == 0) { + for(look_ahead = value; isalnum(*look_ahead); look_ahead++); + tmp_hook->when = malloc(look_ahead - value + 1); + strncpy(tmp_hook->when, value, look_ahead - value); + tmp_hook->when[look_ahead - value] = '\0'; + } + } + if(tmp_hook != NULL) { + /* ignore hooks without name */ + if(tmp_hook->name != NULL) { + HOOK_LIST = alpm_list_add(HOOK_LIST, tmp_hook); + } + } + return 0; +} +/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/hooks.h b/lib/libalpm/hooks.h new file mode 100644 index 0000000..15bd47e --- /dev/null +++ b/lib/libalpm/hooks.h @@ -0,0 +1,43 @@ +/* + * package.h + * + * Copyright (c) 2006-2011 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef _PM_HOOKS_H +#define _PM_HOOKS_H + +#include <alpm.h> + +typedef struct hook { + char *name; + alpm_list_t *pkg_triggers; + alpm_list_t *file_triggers; + int triggered; + char *when; +} hook_t; + +/* global Variables */ +alpm_list_t *HOOK_LIST; + + +void _run_postponed_hooks(void); +void _trigger_hooks(pmpkg_t *pkg, char *when); +int _parse_hooks_config(void); +#endif /* _PM_HOOKS_H */ + +/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 5fba0b0..3bd32e2 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -45,6 +45,7 @@ #include "deps.h" #include "handle.h" #include "alpm.h" +#include "hooks.h"
int SYMEXPORT alpm_remove_target(const char *target) { @@ -409,6 +410,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) _alpm_runscriptlet(handle->root, scriptlet, "pre_remove", alpm_pkg_get_version(info), NULL, trans); } + _trigger_hooks(info, "PreRemove");
if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { alpm_list_t *files = alpm_pkg_get_files(info); @@ -454,6 +456,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) _alpm_runscriptlet(handle->root, scriptlet, "post_remove", alpm_pkg_get_version(info), NULL, trans); } + _trigger_hooks(info, "PostRemve"); Typo is probably going to not work out well. We should probably be using some sort of enum so we can be type-safe anyway.
/* remove the package from the database */ _alpm_log(PM_LOG_DEBUG, "updating database\n"); diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 804ab7a..64b6e7d 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -45,6 +45,7 @@ #include "sync.h" #include "alpm.h" #include "deps.h" +#include "hooks.h"
/** \addtogroup alpm_trans Transaction Functions * @brief Functions to manipulate libalpm transactions @@ -209,6 +210,7 @@ int SYMEXPORT alpm_trans_commit(alpm_list_t **data) return(-1); } } + _run_postponed_hooks(); Is the ldconfig call around here? I would expect it to be as that is
Yes, we do not want to reinvent parsing. This needs to find a way to be incorporated into our parsing framework in the frontend. the closest thing we have to a delayed hook right now.
trans->state = STATE_COMMITED;
-- 1.7.3.5
2011/1/29 Dan McGee <dpmcgee@gmail.com>:
Anyway, here's an example config file:
/etc/pacman.d/hooks.conf: [testhook]
Package = lynx nano Package = vim
When = PreRemove
[fonts] File = usr/share/fonts/* When = PostUpgrade --------------------------------------------------- You can define more than one Package that triggers a hook, either separated by spaces in the "Package ="line, or in a separate "Package =" line or a mix of both. The "File = " argument can be anything that get matched by fnmatch(...). You can also use "Package" and "File" within the same hook. In this case either one of them will trigger the hook (in other words: Package || File). "When" can be one of the following: PreRemove, PostRemove, PreUpgrade, PostUpgrade, PreInstall or PostInstall. So are these on a per-package or per-total-operation basis? A huge
On Sat, Jan 29, 2011 at 8:36 AM, Sascha Kruse <knopwob@googlemail.com> wrote: part of hooks is something like installing a load of font packages; any command that these would normally run is postponed until the end of the transaction and then only needs to run once. Same for icon-cache, info reindexing, etc. or whatever other things you see in 5 install scripts of packages.
I think the Post* hooks should be a per-total-oparation basis. I'm not sure about the Pre* hooks, though. (I had this mixed up in the actual code, but i'll fix that).
Things i don't like, but i don't really now how to fix those: - The global variable HOOK_LIST It belongs on the handle object, more than likely- this is where everything global is captured.
Okay, i'll look into that.
- The paths to hooks.d and hooks.conf are hardcoded - The parsing of the config file is really ugly And also, seeing that the patch contains zero changes in src/...we never ever parse config stuff in the backend, it needs to get passed through by the frontend.
I wasn't sure where to put this. I had in mind that pacman is just a frontend to libalpm, but the hooks should be used by every package manager that is build upon libalpm since packages will depend on this feature. So basically, every package manager that uses libalpm but isn't a wrapper around pacman has to parse the config files itself?
- The "File" argument can't begin with a / because it is matched against alpm_pkg_get_data and that file-strings don't have the leading / This is a must-have, your logic is backwards. We never assume the install root is /; file paths in all install scripts should be written this way too, not to mention everywhere in the library. So nothing wrong here, it would be wrong if you did any prefix whatsoever.
I get your point, but i think it would be confusing for the users. I would intuitively write File = /usr/share/fonts/* instead of File = usr/share/fonts/*. So maybe we should accept both and in the latter case ignore the leading / ?
/** Add a file target to the transaction. * @param target the name of the file target to add @@ -516,6 +517,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, size_t pkg_current, _alpm_runscriptlet(handle->root, newpkg->origin_data.file, "pre_upgrade", newpkg->version, oldpkg->version, trans); } + _trigger_hooks(newpkg, "PreUpgrade"); If the function is in another file, it should be named _alpm, which is the standard prefix for "private" alpm functions.
Okay, I'll change that.
return(0); }
diff --git a/lib/libalpm/hooks.c b/lib/libalpm/hooks.c new file mode 100644 index 0000000..e1ecfab --- /dev/null +++ b/lib/libalpm/hooks.c @@ -0,0 +1,217 @@ +/* + * package.h + * + * Copyright (c) 2006-2011 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> No need for the earlier copyright if this is a new file.
The copyright comment was just copy&paste from another file. I'll change that.
+#include "package.h" +#include "util.h" +#include "hooks.h" + + +int _run_hook(hook_t *hook) static +{ + char *cmd; + int retval; + + cmd = malloc(strlen("/etc/pacman.d/hooks.d/") + strlen(hook->name) + 1); Use MALLOC macros please. + sprintf(cmd, "/etc/pacman.d/hooks.d/%s", hook->name); + + retval = system(cmd); Err...take a look at util.c:_alpm_run_chroot() and the places we use that. You have tremendously oversimplified the setup necessary to run external commands and have them operate on the correct root and paths.
+ if(retval == -1) { + } else if(retval != 0) { + } I'll consider this work in progress?
Oh, yes. I had the calling of the XferCommand in pacman.c as an example for that. And i forgot to substitute the pm_fprintf that was between the if else. But I guess I can't just use printf but I have to figure out how error messages get passed to the frontend instead.
+ pkgname = alpm_pkg_get_name(pkg); + pkgfiles = alpm_pkg_get_files(pkg); + + for(i = HOOK_LIST; i; i = alpm_list_next(i)) { + hook = alpm_list_getdata(i); + + /* trigger package based hooks */ + if(strcmp(hook->when, when) == 0 || + strcmp(hook->when, "Transaction") == 0) { Not following this logic...you are comparing to the passed in value and to the static string "Transaction"? Where did that come from?
The "Transaction" is another "When". Hooks with "When = Transaction" will be triggered every time, the package or file is touched. It's for hooks that needs to be triggered by every Pre* and Post*. I forgot to mention that in the description above, sorry for that. And to follow the point mentioned above: I think this should be a per-total-operations basis. (... big snip ...)
@@ -454,6 +456,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) _alpm_runscriptlet(handle->root, scriptlet, "post_remove", alpm_pkg_get_version(info), NULL, trans); } + _trigger_hooks(info, "PostRemve"); Typo is probably going to not work out well. We should probably be using some sort of enum so we can be type-safe anyway.
I'll put the enum in hooks.h and change the rest accordingly.
/* remove the package from the database */ _alpm_log(PM_LOG_DEBUG, "updating database\n"); diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 804ab7a..64b6e7d 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -45,6 +45,7 @@ #include "sync.h" #include "alpm.h" #include "deps.h" +#include "hooks.h"
/** \addtogroup alpm_trans Transaction Functions * @brief Functions to manipulate libalpm transactions @@ -209,6 +210,7 @@ int SYMEXPORT alpm_trans_commit(alpm_list_t **data) return(-1); } } + _run_postponed_hooks();
Is the ldconfig call around here? I would expect it to be as that is the closest thing we have to a delayed hook right now.
No, that is in lib/libalpm/add.c and remove.c Thank you for your comments. I hope I can post a better version soon. Greetings, Sascha Kruse
On Sat, Jan 29, 2011 at 2:20 PM, Sascha Kruse <knopwob@googlemail.com> wrote:
2011/1/29 Dan McGee <dpmcgee@gmail.com>:
- The "File" argument can't begin with a / because it is matched against alpm_pkg_get_data and that file-strings don't have the leading / This is a must-have, your logic is backwards. We never assume the install root is /; file paths in all install scripts should be written
On Sat, Jan 29, 2011 at 8:36 AM, Sascha Kruse <knopwob@googlemail.com> wrote: this way too, not to mention everywhere in the library. So nothing wrong here, it would be wrong if you did any prefix whatsoever.
I get your point, but i think it would be confusing for the users. I would intuitively write File = /usr/share/fonts/* instead of File = usr/share/fonts/*. So maybe we should accept both and in the latter case ignore the leading / ?
Maybe I wasn't clear. We will *not* take a patch that prefixes paths with anything, anywhere, anytime. pactest is a perfect example of relative dbpaths and dbroots that this needs to work for. -Dan
2011/1/29 Dan McGee <dpmcgee@gmail.com>:
On Sat, Jan 29, 2011 at 2:20 PM, Sascha Kruse <knopwob@googlemail.com> wrote:
2011/1/29 Dan McGee <dpmcgee@gmail.com>:
- The "File" argument can't begin with a / because it is matched against alpm_pkg_get_data and that file-strings don't have the leading / This is a must-have, your logic is backwards. We never assume the install root is /; file paths in all install scripts should be written
On Sat, Jan 29, 2011 at 8:36 AM, Sascha Kruse <knopwob@googlemail.com> wrote: this way too, not to mention everywhere in the library. So nothing wrong here, it would be wrong if you did any prefix whatsoever.
I get your point, but i think it would be confusing for the users. I would intuitively write File = /usr/share/fonts/* instead of File = usr/share/fonts/*. So maybe we should accept both and in the latter case ignore the leading / ?
Maybe I wasn't clear.
We will *not* take a patch that prefixes paths with anything, anywhere, anytime. pactest is a perfect example of relative dbpaths and dbroots that this needs to work for.
Okay, understood. I hope i don't get annoying, but i have another question. At the moment I'm looking into the config parsing. Currenty, pacman parses the pacman.conf (pacman.c:_parseconfig(...)) into [section] key = value And it assumes, that if section != "options" the section is a repo. To use pacmans config parsing i would like to generalize this a bit. My first idea is to change the syntax of the pacman.conf to [section] {subsection} key = value {subsection} ... So a real world pacman.conf would look like [options] ... [hooks] {fonts} FIle = usr/share/fonts/* When = PostInstall [servers] {core} Server = .... But the big disadvantage of this would be, well, that the syntax of pacman.conf changes. My other idea is to generalize the _parseconfig(...) function so that it can parse an arbitrary config file with the given syntax. The result would be a alpm_list_t filled with: typedef struct section { char *name; alpm_list_t *key_pairs; } section_t; with typedef struct key_pair { char *key; char *value; } key_pair_t; This new _parseconfig(..) would then be used by a new parse_pm_config() and parse_hooks_config().In this case we would have a separate pacman.conf and hooks.conf. If this solution is used, should i move the _parseconfig(..) to a separate *.c file, or should it stay in pacman.c? I prefer the generalized _parseconfig(..) but i wanted to hear your opinion first, before changing the config parsing. Greetings, Sascha
On 30/01/11 09:05, Sascha Kruse wrote:
I hope i don't get annoying, but i have another question. At the moment I'm looking into the config parsing. Currenty, pacman parses the pacman.conf (pacman.c:_parseconfig(...)) into [section] key = value
And it assumes, that if section != "options" the section is a repo. To use pacmans config parsing i would like to generalize this a bit. My first idea is to change the syntax of the pacman.conf to [section] {subsection} key = value {subsection} ...
So a real world pacman.conf would look like [options] ... [hooks] {fonts} FIle = usr/share/fonts/* When = PostInstall [servers] {core} Server = ....
But the big disadvantage of this would be, well, that the syntax of pacman.conf changes.
My other idea is to generalize the _parseconfig(...) function so that it can parse an arbitrary config file with the given syntax. The result would be a alpm_list_t filled with:
typedef struct section { char *name; alpm_list_t *key_pairs; } section_t;
with
typedef struct key_pair { char *key; char *value; } key_pair_t;
This new _parseconfig(..) would then be used by a new parse_pm_config() and parse_hooks_config().In this case we would have a separate pacman.conf and hooks.conf. If this solution is used, should i move the _parseconfig(..) to a separate *.c file, or should it stay in pacman.c?
I prefer the generalized _parseconfig(..) but i wanted to hear your opinion first, before changing the config parsing.
I am very much in favour of the second option. From a distributions point of view, I would guess that a separate pacman-hooks package would be preferable as this would allow updating the hooks as needed, my like Arch does with the mirrorlist file. Thus obviously requires keeping the hooks config separate. As far as the idea for implementation goes, I think it seems a reasonable approach but I need to think on it some more. Allan
participants (3)
-
Allan McRae
-
Dan McGee
-
Sascha Kruse