[pacman-dev] [PATCH] hooks: warn if reassignment overwrites previous setting

[Silvan Jegen]

Hi

On Mon, Jan 2, 2017 at 4:19 PM, Stefan Klinger <git at 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 at 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 = 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")
> --
> 2.11.0