[pacman-dev] [PATCH 1/6] order hooks by file name
Signed-off-by: Andrew Gregory
Signed-off-by: Andrew Gregory
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- lib/libalpm/hook.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
OK.
Signed-off-by: Andrew Gregory
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- Updated caveats section.
doc/.gitignore | 1 + doc/Makefile.am | 4 ++ doc/alpm-hooks.5.txt | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 doc/alpm-hooks.5.txt
Looks good. Allan
Signed-off-by: Andrew Gregory
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- Replaced wordexp with a custom function that only does word splitting with quote removal.
Looks fine to me, apart from the indentation issues (space vs tab). A
Signed-off-by: Andrew Gregory
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- lib/libalpm/hook.c | 2 +- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 174 ++++++++++++++++++++++++++++++++++++++++++++-------- lib/libalpm/util.h | 5 +- 4 files changed, 154 insertions(+), 29 deletions(-) diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index 45c10e1..b449ce0 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -511,7 +511,7 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook) } }
- return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd); + return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd, NULL, NULL); }
int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when)
OK.
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index a6b1aef..06997a0 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -391,7 +391,7 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath,
_alpm_log(handle, ALPM_LOG_DEBUG, "executing \"%s\"\n", cmdline);
- retval = _alpm_run_chroot(handle, SCRIPTLET_SHELL, argv); + retval = _alpm_run_chroot(handle, SCRIPTLET_SHELL, argv, NULL, NULL);
cleanup: if(scriptfn && unlink(scriptfn)) {
OK.
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 66a2742..eed0293 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -31,6 +31,7 @@ #include
#include #include +#include /* libarchive */ #include
@@ -445,16 +446,40 @@ ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, return files; } +/* write wrapper that ignores SIGPIPE */
write() wrapper ... Also, this comment is worth expanding - why do we need this?
+static ssize_t _alpm_pipe_write(int fd, const void *buf, size_t count) +{ + sigset_t new, old; + ssize_t ret; + sigemptyset(&new); + sigaddset(&new, SIGPIPE); + pthread_sigmask(SIG_BLOCK, &new, &old); + ret = write(fd, buf, count); + pthread_sigmask(SIG_SETMASK, &old, NULL); + return ret; +} +
Why not use signal(SIGPIPE, SIG_IGN)?
+static void _alpm_handle_script_output(alpm_handle_t *handle, const char *line) +{ + alpm_event_scriptlet_info_t event = { + .type = ALPM_EVENT_SCRIPTLET_INFO, + .line = line + }; + alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line); + EVENT(handle, &event); +} + /** Execute a command with arguments in a chroot. * @param handle the context handle * @param cmd command to execute * @param argv arguments to pass to cmd
Needs updated.
* @return 0 on success, 1 on error */ -int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) +int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[], + _alpm_cb_io p2c_cb, void *p2c_ctx)
void *... can this be alpm_list_t?
{ pid_t pid; - int pipefd[2], cwdfd; + int c2p_pipefd[2], p2c_pipefd[2], cwdfd;
child2parent, parent2child - I would like these names expanded.
int retval = 0;
/* save the cwd so we can restore it later */ @@ -476,7 +501,13 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) /* Flush open fds before fork() to avoid cloning buffers */ fflush(NULL);
- if(pipe(pipefd) == -1) { + if(pipe(c2p_pipefd) == -1) { + _alpm_log(handle, ALPM_LOG_ERROR, _("could not create pipe (%s)\n"), strerror(errno)); + retval = 1; + goto cleanup; + } + + if(p2c_cb && pipe(p2c_pipefd) == -1) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not create pipe (%s)\n"), strerror(errno)); retval = 1; goto cleanup;
OK
@@ -495,10 +526,15 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) close(0); close(1); close(2); - while(dup2(pipefd[1], 1) == -1 && errno == EINTR); - while(dup2(pipefd[1], 2) == -1 && errno == EINTR); - close(pipefd[0]); - close(pipefd[1]); + while(dup2(c2p_pipefd[1], 1) == -1 && errno == EINTR); + while(dup2(c2p_pipefd[1], 2) == -1 && errno == EINTR); + if(p2c_cb) { + while(dup2(p2c_pipefd[0], 0) == -1 && errno == EINTR); + close(p2c_pipefd[0]); + close(p2c_pipefd[1]); + } + close(c2p_pipefd[0]); + close(c2p_pipefd[1]); if(cwdfd >= 0) { close(cwdfd); }
OK
@@ -521,29 +557,115 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) } else { /* this code runs for the parent only (wait on the child) */ int status; - FILE *pipe_file; - - close(pipefd[1]); - pipe_file = fdopen(pipefd[0], "r"); - if(pipe_file == NULL) { - close(pipefd[0]); - retval = 1; + char obuf[PIPE_BUF]; /* writes <= PIPE_BUF are guaranteed atomic */ + char ibuf[LINE_MAX + 2]; /* +2 for appending \n\0 */ + ssize_t olen = 0, ilen = 0; + struct pollfd fds[2], *c2p = &(fds[0]), *p2c = &(fds[1]); + nfds_t nfds = 2; + + c2p->fd = c2p_pipefd[0]; + c2p->events = POLLIN; + fcntl(c2p->fd, F_SETFL, O_NONBLOCK); + close(c2p_pipefd[1]); +
OK
+ if(p2c_cb) { + p2c->fd = p2c_pipefd[1]; + p2c->events = POLLOUT; + fcntl(p2c->fd, F_SETFL, O_NONBLOCK); + close(p2c_pipefd[0]); } else { - while(!feof(pipe_file)) { - char line[PATH_MAX]; - alpm_event_scriptlet_info_t event = { - .type = ALPM_EVENT_SCRIPTLET_INFO, - .line = line - }; - if(safe_fgets(line, PATH_MAX, pipe_file) == NULL) { - break; + p2c->fd = -1; + p2c->events = 0; + }
OK And, I am lost from here...
+ +#define STOP_POLLING(p) do { close(p->fd); p->fd = -1; } while(0) + + while((c2p->fd != -1 || p2c->fd != -1) && poll(fds, nfds, -1) > 0) { + if(c2p->revents & POLLIN) { + ssize_t space = LINE_MAX - ilen; + ssize_t nread = read(c2p->fd, ibuf + ilen, space); + if(nread > 0) { + char *newline = memchr(ibuf + ilen, '\n', nread); + ilen += nread; + if(newline) { + while(newline) { + size_t linelen = newline - ibuf + 1; + char old = ibuf[linelen]; + ibuf[linelen] = '\0'; + _alpm_handle_script_output(handle, ibuf); + ibuf[linelen] = old; + + ilen -= linelen; + memmove(ibuf, ibuf + linelen, ilen); + newline = memchr(ibuf, '\n', ilen); + } + } else if(nread == space) { + /* we didn't read a full line, but we're out of space */ + strcpy(ibuf + ilen, "\n"); + _alpm_handle_script_output(handle, ibuf); + ilen = 0; + } + } else if(nread == 0) { + /* end-of-file */ + if(ilen) { + strcpy(ibuf + ilen, "\n"); + _alpm_handle_script_output(handle, ibuf); + } + STOP_POLLING(c2p); + } else if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) { + /* nothing read, try again */ + } else { + /* read error */ + if(ilen) { + strcpy(ibuf + ilen, "\n"); + _alpm_handle_script_output(handle, ibuf); + } + _alpm_log(handle, ALPM_LOG_ERROR, + _("unable to read from pipe (%s)\n"), strerror(errno)); + STOP_POLLING(c2p); + } + } else if(c2p->revents) { + /* anything but POLLIN indicates an error */ + STOP_POLLING(c2p); + } + if(p2c->revents & POLLOUT) { + ssize_t nwrite; + if(olen == 0) { + /* empty buffer, ask the callback for more */ + if((olen = p2c_cb(obuf, PIPE_BUF, p2c_ctx)) == 0) { + /* no more to write, close the pipe */ + STOP_POLLING(p2c); + continue; + } + } + if((nwrite = _alpm_pipe_write(p2c->fd, obuf, olen)) != -1) { + olen -= nwrite; + memmove(obuf, obuf + nwrite, olen); + } else { + if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) { + /* nothing written, try again later */ + } else { + /* something went wrong, close the pipe */ + _alpm_log(handle, ALPM_LOG_ERROR, + _("unable to write to pipe (%s)\n"), strerror(errno)); + STOP_POLLING(p2c); + } } - alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line); - EVENT(handle, &event); + } else if(p2c->revents) { + /* anything but POLLOUT indicates an error */ + STOP_POLLING(p2c); } - fclose(pipe_file); }
... to here. This has gone from being a large function to a massive one - is there any way to split it up into smaller components?
+ if(p2c->fd != -1) { + close(p2c->fd); + } + if(c2p->fd != -1) { + close(c2p->fd); + } + +#undef STOP_POLLING +
OK
while(waitpid(pid, &status, 0) == -1) { if(errno != EINTR) { _alpm_log(handle, ALPM_LOG_ERROR, _("call to waitpid failed (%s)\n"), strerror(errno)); @@ -605,7 +727,7 @@ int _alpm_ldconfig(alpm_handle_t *handle) char arg0[32]; char *argv[] = { arg0, NULL }; strcpy(arg0, "ldconfig"); - return _alpm_run_chroot(handle, LDCONFIG, argv); + return _alpm_run_chroot(handle, LDCONFIG, argv, NULL, NULL); } }
OK
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 95112cf..c0c9ff0 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -119,7 +119,10 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix,
ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, int full_count);
-int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]); +typedef ssize_t (*_alpm_cb_io)(void *buf, ssize_t len, void *ctx); + +int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[], + _alpm_cb_io in_cb, void *in_ctx); int _alpm_ldconfig(alpm_handle_t *handle); int _alpm_str_cmp(const void *s1, const void *s2); char *_alpm_filecache_find(alpm_handle_t *handle, const char *filename);
Signed-off-by: Andrew Gregory
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- doc/alpm-hooks.5.txt | 5 ++ lib/libalpm/hook.c | 126 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 114 insertions(+), 17 deletions(-) diff --git a/doc/alpm-hooks.5.txt b/doc/alpm-hooks.5.txt index 770f466..3729387 100644 --- a/doc/alpm-hooks.5.txt +++ b/doc/alpm-hooks.5.txt @@ -23,6 +23,7 @@ When = PreTransaction|PostTransaction (Required) Exec = <Command> (Required) Depends = <PkgName> (Optional) AbortOnFail (Optional, PreTransaction only) +NeedsTargets (Optional) --------
DESCRIPTION @@ -77,6 +78,10 @@ ACTIONS Causes the transaction to be aborted if the hook exits non-zero. Only applies to PreTransaction hooks.
+*NeedsTargets*:: + Causes the list of matched trigger targets to be passed to the running hook + on 'stdin'. + OVERRIDING HOOKS ----------------
OK.
diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index b449ce0..3a2c33a 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -51,8 +51,9 @@ struct _alpm_hook_t { alpm_list_t *triggers; alpm_list_t *depends; char **cmd; + alpm_list_t *matches; enum _alpm_hook_when_t when; - int abort_on_fail; + int abort_on_fail, needs_targets; };
OK
struct _alpm_hook_cb_ctx { @@ -86,6 +87,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook) _alpm_wordsplit_free(hook->cmd); alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) _alpm_trigger_free); alpm_list_free(hook->triggers); + alpm_list_free(hook->matches); FREELIST(hook->depends); free(hook); }
OK
@@ -319,6 +321,8 @@ static int _alpm_hook_parse_cb(const char *file, int line, hook->depends = alpm_list_add(hook->depends, val); } else if(strcmp(key, "AbortOnFail") == 0) { hook->abort_on_fail = 1; + } else if(strcmp(key, "NeedsTargets") == 0) { + hook->needs_targets = 1; } else if(strcmp(key, "Exec") == 0) { if((hook->cmd = _alpm_wordsplit(value)) == NULL) { if(errno == EINVAL) {
OK
@@ -338,7 +342,8 @@ static int _alpm_hook_parse_cb(const char *file, int line, return 0; }
-static int _alpm_hook_trigger_match_file(alpm_handle_t *handle, struct _alpm_trigger_t *t) +static int _alpm_hook_trigger_match_file(alpm_handle_t *handle, + struct _alpm_hook_t *hook, struct _alpm_trigger_t *t) { alpm_list_t *i, *j, *install = NULL, *upgrade = NULL, *remove = NULL; size_t isize = 0, rsize = 0; @@ -422,15 +427,31 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t *handle, struct _alpm_tri || (t->op & ALPM_HOOK_OP_UPGRADE && upgrade) || (t->op & ALPM_HOOK_OP_REMOVE && remove);
- alpm_list_free(install); - alpm_list_free(upgrade); - alpm_list_free(remove); + if(hook->needs_targets) { +#define _save_matches(_op, _matches) \ + if(t->op & _op && _matches) { \ + hook->matches = alpm_list_join(hook->matches, _matches); \ + } else { \ + alpm_list_free(_matches); \ + } + _save_matches(ALPM_HOOK_OP_INSTALL, install); + _save_matches(ALPM_HOOK_OP_UPGRADE, upgrade); + _save_matches(ALPM_HOOK_OP_REMOVE, remove); +#undef _save_matches + } else { + alpm_list_free(install); + alpm_list_free(upgrade); + alpm_list_free(remove); + }
return ret; }
OK (although I am not a big fan of defines used like that).
-static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle, struct _alpm_trigger_t *t) +static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle, + struct _alpm_hook_t *hook, struct _alpm_trigger_t *t) { + alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL; + if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) { alpm_list_t *i; for(i = handle->trans->add; i; i = i->next) { @@ -438,11 +459,19 @@ static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle, struct _alpm_trig if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) { if(alpm_db_get_pkg(handle->db_local, pkg->name)) { if(t->op & ALPM_HOOK_OP_UPGRADE) { - return 1; + if(hook->needs_targets) { + upgrade = alpm_list_add(upgrade, pkg->name); + } else { + return 1; + } } } else { if(t->op & ALPM_HOOK_OP_INSTALL) { - return 1; + if(hook->needs_targets) { + install = alpm_list_add(install, pkg->name); + } else { + return 1; + } } } } @@ -455,31 +484,47 @@ static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle, struct _alpm_trig alpm_pkg_t *pkg = i->data; if(pkg && _alpm_fnmatch_patterns(t->targets, pkg->name) == 0) { if(!alpm_list_find(handle->trans->add, pkg, _alpm_pkg_cmp)) { - return 1; + if(hook->needs_targets) { + remove = alpm_list_add(remove, pkg->name); + } else { + return 1; + } } } } }
- return 0; + /* if we reached this point we either need the target lists or we didn't + * match anything and the following calls will all be no-ops */ + hook->matches = alpm_list_join(hook->matches, install); + hook->matches = alpm_list_join(hook->matches, upgrade); + hook->matches = alpm_list_join(hook->matches, remove); + + return install || upgrade || remove; }
OK
-static int _alpm_hook_trigger_match(alpm_handle_t *handle, struct _alpm_trigger_t *t) +static int _alpm_hook_trigger_match(alpm_handle_t *handle, + struct _alpm_hook_t *hook, struct _alpm_trigger_t *t) { return t->type == ALPM_HOOK_TYPE_PACKAGE - ? _alpm_hook_trigger_match_pkg(handle, t) - : _alpm_hook_trigger_match_file(handle, t); + ? _alpm_hook_trigger_match_pkg(handle, hook, t) + : _alpm_hook_trigger_match_file(handle, hook, t); }
OK
static int _alpm_hook_triggered(alpm_handle_t *handle, struct _alpm_hook_t *hook) { alpm_list_t *i; + int ret = 0; for(i = hook->triggers; i; i = i->next) { - if(_alpm_hook_trigger_match(handle, i->data)) { - return 1; + if(_alpm_hook_trigger_match(handle, hook, i->data)) { + if(!hook->needs_targets) { + return 1; + } else { + ret = 1; + } } } - return 0; + return ret; }
OK
static int _alpm_hook_cmp(struct _alpm_hook_t *h1, struct _alpm_hook_t *h2) @@ -499,6 +544,44 @@ static alpm_list_t *find_hook(alpm_list_t *haystack, const void *needle) return NULL; }
+static ssize_t _alpm_hook_feed_targets(char *buf, ssize_t needed, alpm_list_t **pos) +{ + size_t remaining = needed, written = 0;; + size_t len; + + while(*pos && (len = strlen((*pos)->data)) + 1 <= remaining) { + memcpy(buf, (*pos)->data, len); + buf[len++] = '\n'; + *pos = (*pos)->next; + buf += len; + remaining -= len; + written += len; + } + + if(*pos && remaining) { + memcpy(buf, (*pos)->data, remaining); + (*pos)->data = (char*) (*pos)->data + remaining; + written += remaining; + } + + return written; +} +
OK
+static alpm_list_t *_alpm_strlist_dedup(alpm_list_t *list) +{ + alpm_list_t *i = list; + while(i) { + alpm_list_t *next = i->next; + while(next && strcmp(i->data, next->data) == 0) { + list = alpm_list_remove_item(list, next); + free(next); + next = i->next; + } + i = next; + } + return list; +} +
OK.
static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook) { alpm_list_t *i, *pkgs = _alpm_db_get_pkgcache(handle->db_local); @@ -511,7 +594,16 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook) } }
- return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd, NULL, NULL); + if(hook->needs_targets) { + alpm_list_t *ctx; + hook->matches = alpm_list_msort(hook->matches, + alpm_list_count(hook->matches), (alpm_list_fn_cmp)strcmp); + ctx = hook->matches = _alpm_strlist_dedup(hook->matches);
Where do the duplicates come from?
+ return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd, + (_alpm_cb_io) _alpm_hook_feed_targets, &ctx); + } else { + return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd, NULL, NULL); + } }
int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when)
On 27/10/15 14:47, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- lib/libalpm/hook.c | 8 ++++++++ 1 file changed, 8 insertions(+)
OK.
participants (2)
-
Allan McRae
-
Andrew Gregory