In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de --- lib/libalpm/hook.c | 15 ++++++++++++++- 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, 107 insertions(+), 1 deletion(-) 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 ccde225e..51e74484 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -267,6 +267,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, struct _alpm_hook_t *hook = ctx->hook;
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__);
if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); @@ -296,6 +297,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 Type\n"), file, line); + } if(strcmp(value, "Package") == 0) { t->type = ALPM_HOOK_TYPE_PACKAGE; } else if(strcmp(value, "File") == 0) { @@ -312,6 +316,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 When\n"), file, line); + } if(strcmp(value, "PreTransaction") == 0) { hook->when = ALPM_HOOK_PRE_TRANSACTION; } else if(strcmp(value, "PostTransaction") == 0) { @@ -320,6 +327,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, "Description") == 0) { + if(hook->desc != NULL) { + warning(_("hook %s line %d: overwriting previous definition of Description\n"), file, line); + } STRDUP(hook->desc, value, return 1); } else if(strcmp(key, "Depends") == 0) { char *val; @@ -330,7 +340,10 @@ 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 = _alpm_wordsplit(value)) == NULL) { + if(hook->cmd != NULL) { + warning(_("hook %s line %d: overwriting previous definition of Exec\n"), file, line); + } + if((hook->cmd = _alpm_wordsplit(value)) == NULL) { if(errno == EINVAL) { error(_("hook %s line %d: invalid value %s\n"), file, line, value); } else { diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 2d877962..4329ca90 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -53,6 +53,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 @@ -63,7 +65,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..5826a30e --- /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..6fe53151 --- /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..8c4dfd0f --- /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..07c3bc5e --- /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")
Hi
On Mon, Jan 2, 2017 at 4:19 PM, Stefan Klinger git@stefan-klinger.de wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de
lib/libalpm/hook.c | 15 ++++++++++++++- 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, 107 insertions(+), 1 deletion(-) 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
Looks good to me but I have not tested it.
One nitpick below.
diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c index ccde225e..51e74484 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c [...] 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..5826a30e --- /dev/null +++ b/test/pacman/tests/hook-description-reused.py @@ -0,0 +1,23 @@ +self.description = "Hook using multiple Description's"
The 's looks like a possessive 's to me (instead of a plural). Writing
+self.description = "Hook using multiple 'Description's"
reads better to me (the same is true for all other self.descriptions further down of course).
Hey, I gave you a fair nitpick-warning! :P
Cheers,
Silvan
+self.add_hook("hook",
"""[Trigger]Type = PackageOperation = InstallTarget = foo[Action]Description = lalaDescription = foobarWhen = PreTransactionExec = /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..6fe53151 --- /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 = PackageOperation = InstallTarget = foo[Action]When = PostTransactionExec = /bin/dateExec = /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..8c4dfd0f --- /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 = PackageType = FileOperation = InstallTarget = foo[Action]When = PostTransactionExec = /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..07c3bc5e --- /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 = PackageOperation = InstallTarget = foo[Action]When = PreTransactionExec = /bin/dateWhen = 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
On 2017-Jan-03, Silvan Jegen wrote with possible deletions:
On Mon, Jan 2, 2017 at 4:19 PM, Stefan Klinger git@stefan-klinger.de wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Looks good to me but I have not tested it.
One nitpick below.
+self.description = "Hook using multiple Description's"
The 's looks like a possessive 's to me (instead of a plural). Writing
+self.description = "Hook using multiple 'Description's"
That's how it was intended. I'll fix it.
On 01/02/2017 10:19 AM, Stefan Klinger wrote:
if(t->type != 0) {warning(_("hook %s line %d: overwriting previous definition of Type\n"), file, line);}
...
if(hook->when != 0) {warning(_("hook %s line %d: overwriting previous definition of When\n"), file, line);}
...
if(hook->desc != NULL) {warning(_("hook %s line %d: overwriting previous definition of Description\n"), file, line);}
...
if(hook->cmd != NULL) {warning(_("hook %s line %d: overwriting previous definition of Exec\n"), file, line);}
Look at all the times error is used. ;) You should be formatting the warning with the definition type as well.
This allows a single translation to be used for all four.
On 2017-Jan-03, Eli Schwartz wrote with possible deletions:
On 01/02/2017 10:19 AM, Stefan Klinger wrote:
warning(_("hook %s line %d: overwriting previous definition of Type\n"), file, line);warning(_("hook %s line %d: overwriting previous definition of When\n"), file, line);warning(_("hook %s line %d: overwriting previous definition of Description\n"), file, line);warning(_("hook %s line %d: overwriting previous definition of Exec\n"), file, line);Look at all the times error is used. ;) You should be formatting the warning with the definition type as well.
Yeah, I had that before but was unsure whether it's appreciated to do formatting with a constant value at runtime. But I'll change that.
Also, I'd like to change
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1;
to
#define error(...) do { _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; } while (0)
see [1]. Also move `error` and `warning` both to the top and use `error` everywhere where appropriate.
____________________ [1] http://stackoverflow.com/questions/257418/do-while-0-what-is-it-good-for#ans...
On 01/02/17 at 04:19pm, Stefan Klinger wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de
lib/libalpm/hook.c | 15 ++++++++++++++- 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, 107 insertions(+), 1 deletion(-) 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 ccde225e..51e74484 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -267,6 +267,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, struct _alpm_hook_t *hook = ctx->hook;
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__);
if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); @@ -296,6 +297,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 Type\n"), file, line);}
We use tabs for indenting C files, not spaces.
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de --- lib/libalpm/hook.c | 16 +++++++++++++++- 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, 108 insertions(+), 1 deletion(-) 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 ccde225e..4b3fa3cb 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -267,6 +267,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, struct _alpm_hook_t *hook = ctx->hook;
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__);
if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); @@ -296,6 +297,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 +316,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 +327,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, "Description") == 0) { + if(hook->desc != NULL) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "Description"); + } STRDUP(hook->desc, value, return 1); } else if(strcmp(key, "Depends") == 0) { char *val; @@ -330,12 +340,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"); + } 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)); + file, line, strerror(errno)); } } } else { @@ -344,6 +357,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, }
#undef error +#undef warning
return 0; } diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 2d877962..4329ca90 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -53,6 +53,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 @@ -63,7 +65,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")
On 2017-Jan-03, Andrew Gregory wrote with possible deletions:
We use tabs for indenting C files, not spaces.
Ok, didn't think of that.
Do you have a style-checking script I could use?
Do you prefer a patch with all commits squashed into one, or rather many small commits (that you could squash yourself)?
On 04/01/17 02:01, git@stefan-klinger.de wrote:
Do you prefer a patch with all commits squashed into one, or rather many small commits (that you could squash yourself)?
Just one squashed patch thanks.
A
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de --- lib/libalpm/hook.c | 16 +++++++++++++++- 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, 108 insertions(+), 1 deletion(-) 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..dd20a55c 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -267,6 +267,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, struct _alpm_hook_t *hook = ctx->hook;
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__);
if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); @@ -296,6 +297,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 +316,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 +327,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, "Description") == 0) { + if(hook->desc != NULL) { + warning(_("hook %s line %d: overwriting previous definition of %s\n"), file, line, "Description"); + } STRDUP(hook->desc, value, return 1); } else if(strcmp(key, "Depends") == 0) { char *val; @@ -330,12 +340,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"); + } 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)); + file, line, strerror(errno)); } } } else { @@ -344,6 +357,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, }
#undef error +#undef warning
return 0; } 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")
On 03/01/17 01:19, Stefan Klinger wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de
lib/libalpm/hook.c | 16 +++++++++++++++- 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, 108 insertions(+), 1 deletion(-) 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
Looks good.
A
On 03/01/17 01:19, Stefan Klinger wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de
On closer review.... y ou have exposed two memory leaks here. Please fix and resubmit.
Thanks, Allan
<snip>
} 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 +340,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));
file, line, strerror(errno)); } }
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@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")
On 01/02/17 at 04:19pm, Stefan Klinger wrote:
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@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__)
The error macro was written specifically for the section of code where it was used. Using it elsewhere causes those functions to return early. They were specifically written not to so that we could present all relevant errors to the user at once.
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);
NACK
}
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);
NACK
}
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);
NACK
}
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);
NACK
}
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);
NACK
} 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");
Please try to keep code wrapped close to 80 columns.
} 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");
Ditto.
} 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");
Ditto.
FREE(hook->desc);} STRDUP(hook->desc, value, return 1);@@ -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");
Ditto.
_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));
Ditto.
} } } 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"));
This is the only place the error macro could actually be used, and you left the return statement.
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));
NACK
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));
NACK
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));
NACK
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));
NACK
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));
NACK
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 = PackageOperation = InstallTarget = foo[Action]Description = lalaDescription = foobarWhen = PreTransactionExec = /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 = PackageOperation = InstallTarget = foo[Action]When = PostTransactionExec = /bin/dateExec = /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 = PackageType = FileOperation = InstallTarget = foo[Action]When = PostTransactionExec = /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 = PackageOperation = InstallTarget = foo[Action]When = PreTransactionExec = /bin/dateWhen = 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
On 15/01/17 00:39, Andrew Gregory wrote:
On 01/02/17 at 04:19pm, Stefan Klinger wrote:
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@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__)
The error macro was written specifically for the section of code where it was used. Using it elsewhere causes those functions to return early. They were specifically written not to so that we could present all relevant errors to the user at once.
In addition, please label resubmitted patches as "v2" in the subject line, and don't expand the scope of the patch in a revision (unless specifically asked to). Two patches would have been better, because I could have accepted the first one with the fixes I had asked for.
Allan
On 01/12/17 at 09:39pm, Allan McRae wrote:
On 03/01/17 01:19, Stefan Klinger wrote:
In hook definition files, repeated assignment to Description, Exec, Type, and When silently overwrote previous settings. This yields a warning now.
Signed-off-by: Stefan Klinger git@stefan-klinger.de
On closer review.... y ou have exposed two memory leaks here. Please fix and resubmit.
Thanks, Allan
<snip>
} 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 +340,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));
file, line, strerror(errno));
While you're at it, undo this change.
} } } else {
Signed-off-by: Stefan Klinger git@stefan-klinger.de --- lib/libalpm/hook.c | 16 ++++++++++++++++ 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, 109 insertions(+) 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..865c11d8 100644 --- a/lib/libalpm/hook.c +++ b/lib/libalpm/hook.c @@ -267,6 +267,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, struct _alpm_hook_t *hook = ctx->hook;
#define error(...) _alpm_log(handle, ALPM_LOG_ERROR, __VA_ARGS__); return 1; +#define warning(...) _alpm_log(handle, ALPM_LOG_WARNING, __VA_ARGS__);
if(!section && !key) { error(_("error while reading hook %s: %s\n"), file, strerror(errno)); @@ -296,6 +297,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 +316,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 +327,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,6 +341,10 @@ 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); @@ -344,6 +359,7 @@ static int _alpm_hook_parse_cb(const char *file, int line, }
#undef error +#undef warning
return 0; } diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 309eb17e..a9b4288c 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -55,6 +55,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 @@ -65,7 +67,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")
Any feedback on this?
On 03/08/2018 07:57 AM, git@stefan-klinger.de wrote:
Any feedback on this?
https://lists.archlinux.org/pipermail/pacman-dev/2018-March/022366.html
You're not being ignored, we all are. :D
Just wait until Allan has free time to catch up on the patch queue.
On 08/03/18 22:57, git@stefan-klinger.de wrote:
Any feedback on this?
At a glance, the patch looks good and addresses the concerns raised for the original patch.
I'll get to pulling it soon...
A
On 26/02/18 03:36, Stefan Klinger wrote:
Signed-off-by: Stefan Klinger git@stefan-klinger.de
lib/libalpm/hook.c | 16 ++++++++++++++++ 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, 109 insertions(+) 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
Thanks - patch looks good. Even better due to the inclusion of test suite additions!
A
pacman-dev@lists.archlinux.org