hooks: complain if multiple Exec options in alpm hook hooks: test for reassign Exec
hook: Make reassignment to Exec yield a warning hooks: more consistency checks in hook definition hooks: fixed typo in test descriptions hooks: refactored warnings into one common message hooks: using tabs for indentation fixed 2 space leaks when overwriting hook definitions hook: better macro definitions, more consistent usage of them Signed-off-by: Stefan Klinger <[email protected]> --- lib/libalpm/hook.c | 58 ++++++++++++++-------------- test/pacman/tests/TESTS | 4 ++ test/pacman/tests/hook-description-reused.py | 23 +++++++++++ test/pacman/tests/hook-exec-reused.py | 22 +++++++++++ test/pacman/tests/hook-type-reused.py | 22 +++++++++++ test/pacman/tests/hook-when-reused.py | 22 +++++++++++ 6 files changed, 123 insertions(+), 28 deletions(-) create mode 100644 test/pacman/tests/hook-description-reused.py create mode 100644 test/pacman/tests/hook-exec-reused.py create mode 100644 test/pacman/tests/hook-type-reused.py create mode 100644 test/pacman/tests/hook-when-reused.py diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index 4ec2a906..fdb6d475 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -30,6 +30,10 @@ #include "trans.h" #include "util.h" + +#define error(...) do { _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; } while (0) +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__) + enum _alpm_hook_op_t { ALPM_HOOK_OP_INSTALL = (1 << 0), ALPM_HOOK_OP_UPGRADE = (1 << 1), @@ -103,20 +107,17 @@ static int _alpm_trigger_validate(alpm_handle_t *handle, if(trigger->targets == NULL) { ret = -1; - _alpm_log(handle, ALPM_LOG_ERROR, - _("Missing trigger targets in hook: %s\n"), file); + error(_("Missing trigger targets in hook: %s\n"), file); } if(trigger->type == 0) { ret = -1; - _alpm_log(handle, ALPM_LOG_ERROR, - _("Missing trigger type in hook: %s\n"), file); + error(_("Missing trigger type in hook: %s\n"), file); } if(trigger->op == 0) { ret = -1; - _alpm_log(handle, ALPM_LOG_ERROR, - _("Missing trigger operation in hook: %s\n"), file); + error(_("Missing trigger operation in hook: %s\n"), file); } return ret; @@ -142,14 +143,12 @@ static int _alpm_hook_validate(alpm_handle_t *handle, if(hook->cmd == NULL) { ret = -1; - _alpm_log(handle, ALPM_LOG_ERROR, - _("Missing Exec option in hook: %s\n"), file); + error(_("Missing Exec option in hook: %s\n"), file); } if(hook->when == 0) { ret = -1; - _alpm_log(handle, ALPM_LOG_ERROR, - _("Missing When option in hook: %s\n"), file); + error(_("Missing When option in hook: %s\n"), file); } else if(hook->when != ALPM_HOOK_PRE_TRANSACTION && hook->abort_on_fail) { _alpm_log(handle, ALPM_LOG_WARNING, _("AbortOnFail set for PostTransaction hook: %s\n"), file); @@ -266,8 +265,6 @@ static int _alpm_hook_parse_cb(const char *file, int line, alpm_handle_t *handle = ctx->handle; struct _alpm_hook_t *hook = ctx->hook; -#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; - if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); } else if(!section) { @@ -296,6 +293,9 @@ static int _alpm_hook_parse_cb(const char *file, int line, error(_("hook %s line %d: invalid value %s\n"), file, line, value); } } else if(strcmp(key, "Type") == 0) { + if(t->type != 0) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "Type"); + } if(strcmp(value, "Package") == 0) { t->type = ALPM_HOOK_TYPE_PACKAGE; } else if(strcmp(value, "File") == 0) { @@ -312,6 +312,9 @@ static int _alpm_hook_parse_cb(const char *file, int line, } } else if(strcmp(section, "Action") == 0) { if(strcmp(key, "When") == 0) { + if(hook->when != 0) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "When"); + } if(strcmp(value, "PreTransaction") == 0) { hook->when = ALPM_HOOK_PRE_TRANSACTION; } else if(strcmp(value, "PostTransaction") == 0) { @@ -320,6 +323,10 @@ static int _alpm_hook_parse_cb(const char *file, int line, error(_("hook %s line %d: invalid value %s\n"), file, line, value); } } else if(strcmp(key, "Description") == 0) { + if(hook->desc != NULL) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "Description"); + FREE(hook->desc); + } STRDUP(hook->desc, value, return 1); } else if(strcmp(key, "Depends") == 0) { char *val; @@ -330,12 +337,15 @@ static int _alpm_hook_parse_cb(const char *file, int line, } else if(strcmp(key, "NeedsTargets") == 0) { hook->needs_targets = 1; } else if(strcmp(key, "Exec") == 0) { + if(hook->cmd != NULL) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "Exec"); + _alpm_wordsplit_free(hook->cmd); + } if((hook->cmd = _alpm_wordsplit(value)) == NULL) { if(errno == EINVAL) { error(_("hook %s line %d: invalid value %s\n"), file, line, value); } else { - error(_("hook %s line %d: unable to set option (%s)\n"), - file, line, strerror(errno)); + error(_("hook %s line %d: unable to set option (%s)\n"), file, line, strerror(errno)); } } } else { @@ -343,8 +353,6 @@ static int _alpm_hook_parse_cb(const char *file, int line, } } -#undef error - return 0; } @@ -594,8 +602,7 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook) for(i = hook->depends; i; i = i->next) { if(!alpm_find_satisfier(pkgs, i->data)) { - _alpm_log(handle, ALPM_LOG_ERROR, _("unable to run hook %s: %s\n"), - hook->name, _("could not satisfy dependencies")); + error(_("unable to run hook %s: %s\n"), hook->name, _("could not satisfy dependencies")); return -1; } } @@ -629,8 +636,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) DIR *d; 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)); + error(_("could not open directory: %s: %s\n"), (char *)i->data, strerror(ENAMETOOLONG)); ret = -1; continue; } @@ -640,8 +646,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) if(errno == ENOENT) { continue; } else { - _alpm_log(handle, ALPM_LOG_ERROR, - _("could not open directory: %s: %s\n"), path, strerror(errno)); + error(_("could not open directory: %s: %s\n"), path, strerror(errno)); ret = -1; continue; } @@ -657,8 +662,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) } if((name_len = strlen(entry->d_name)) >= PATH_MAX - dirlen) { - _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file: %s%s: %s\n"), - path, entry->d_name, strerror(ENAMETOOLONG)); + error(_("could not open file: %s%s: %s\n"), path, entry->d_name, strerror(ENAMETOOLONG)); ret = -1; continue; } @@ -676,8 +680,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) } if(stat(path, &buf) != 0) { - _alpm_log(handle, ALPM_LOG_ERROR, - _("could not stat file %s: %s\n"), path, strerror(errno)); + error(_("could not stat file %s: %s\n"), path, strerror(errno)); ret = -1; continue; } @@ -703,8 +706,7 @@ int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when) hooks = alpm_list_add(hooks, ctx.hook); } if(errno != 0) { - _alpm_log(handle, ALPM_LOG_ERROR, _("could not read directory: %s: %s\n"), - (char *) i->data, strerror(errno)); + error(_("could not read directory: %s: %s\n"), (char *) i->data, strerror(errno)); ret = -1; } diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 11d1c38c..dd191175 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -54,6 +54,8 @@ TESTS += test/pacman/tests/fileconflict030.py TESTS += test/pacman/tests/fileconflict031.py TESTS += test/pacman/tests/fileconflict032.py TESTS += test/pacman/tests/hook-abortonfail.py +TESTS += test/pacman/tests/hook-description-reused.py +TESTS += test/pacman/tests/hook-exec-reused.py TESTS += test/pacman/tests/hook-exec-with-arguments.py TESTS += test/pacman/tests/hook-file-change-packages.py TESTS += test/pacman/tests/hook-file-remove-trigger-match.py @@ -64,7 +66,9 @@ TESTS += test/pacman/tests/hook-pkg-postinstall-trigger-match.py TESTS += test/pacman/tests/hook-pkg-remove-trigger-match.py TESTS += test/pacman/tests/hook-pkg-upgrade-trigger-match.py TESTS += test/pacman/tests/hook-target-list.py +TESTS += test/pacman/tests/hook-type-reused.py TESTS += test/pacman/tests/hook-upgrade-trigger-no-match.py +TESTS += test/pacman/tests/hook-when-reused.py TESTS += test/pacman/tests/ignore001.py TESTS += test/pacman/tests/ignore002.py TESTS += test/pacman/tests/ignore003.py diff --git a/test/pacman/tests/hook-description-reused.py b/test/pacman/tests/hook-description-reused.py new file mode 100644 index 00000000..87637e80 --- /dev/null +++ b/test/pacman/tests/hook-description-reused.py @@ -0,0 +1,23 @@ +self.description = "Hook using multiple 'Description's" + +self.add_hook("hook", + """ + [Trigger] + Type = Package + Operation = Install + Target = foo + + [Action] + Description = lala + Description = foobar + When = PreTransaction + Exec = /bin/date + """); + +sp = pmpkg("foo") +self.addpkg2db("sync", sp) + +self.args = "-S foo" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=warning.*overwriting previous definition of Description") diff --git a/test/pacman/tests/hook-exec-reused.py b/test/pacman/tests/hook-exec-reused.py new file mode 100644 index 00000000..38423177 --- /dev/null +++ b/test/pacman/tests/hook-exec-reused.py @@ -0,0 +1,22 @@ +self.description = "Hook using multiple 'Exec's" + +self.add_hook("hook", + """ + [Trigger] + Type = Package + Operation = Install + Target = foo + + [Action] + When = PostTransaction + Exec = /bin/date + Exec = /bin/date + """); + +sp = pmpkg("foo") +self.addpkg2db("sync", sp) + +self.args = "-S foo" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=warning.*overwriting previous definition of Exec") diff --git a/test/pacman/tests/hook-type-reused.py b/test/pacman/tests/hook-type-reused.py new file mode 100644 index 00000000..472c8caf --- /dev/null +++ b/test/pacman/tests/hook-type-reused.py @@ -0,0 +1,22 @@ +self.description = "Hook using multiple 'Type's" + +self.add_hook("hook", + """ + [Trigger] + Type = Package + Type = File + Operation = Install + Target = foo + + [Action] + When = PostTransaction + Exec = /bin/date + """); + +sp = pmpkg("foo") +self.addpkg2db("sync", sp) + +self.args = "-S foo" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=warning.*overwriting previous definition of Type") diff --git a/test/pacman/tests/hook-when-reused.py b/test/pacman/tests/hook-when-reused.py new file mode 100644 index 00000000..85a93db8 --- /dev/null +++ b/test/pacman/tests/hook-when-reused.py @@ -0,0 +1,22 @@ +self.description = "Hook using multiple 'When's" + +self.add_hook("hook", + """ + [Trigger] + Type = Package + Operation = Install + Target = foo + + [Action] + When = PreTransaction + Exec = /bin/date + When = PostTransaction + """); + +sp = pmpkg("foo") +self.addpkg2db("sync", sp) + +self.args = "-S foo" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=warning.*overwriting previous definition of When") -- 2.11.0
