[pacman-dev] [PATCH] hooks: warn if reassignment overwrites previous setting
Stefan Klinger
git at stefan-klinger.de
Mon Jan 2 15:19:37 UTC 2017
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 <git at stefan-klinger.de>
---
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
More information about the pacman-dev
mailing list