On 09/04/16 at 08:56pm, Sergey Petrenko via pacman-dev wrote:
> Variables are now at beginning of block.
>
> ---
I would love to provide more information to hooks about the
transaction, but, without a compelling justification, I'm -1 on this
patch for two reasons. First, I don't like the piecemeal approach.
When the next person decides they need different information, do we
just keep adding Needs<field> options? Second, the "<name> <pkgver>"
format can't be reliably parsed. Both of those fields can contain
spaces.
> lib/libalpm/hook.c | 104
> +++++++++++++++++++++---------
> test/pacman/tests/TESTS | 1 +
> test/pacman/tests/hook-target-versions.py | 47 ++++++++++++++
> 3 files changed, 120 insertions(+), 32 deletions(-)
> create mode 100644 test/pacman/tests/hook-target-versions.py
>
> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
> index ccde225..d37924f 100644
> --- a/lib/libalpm/hook.c
> +++ b/lib/libalpm/hook.c
> @@ -55,7 +55,7 @@ struct _alpm_hook_t {
> char **cmd;
> alpm_list_t *matches;
> alpm_hook_when_t when;
> - int abort_on_fail, needs_targets;
> + int abort_on_fail, needs_targets, needs_versions;
> };
>
> struct _alpm_hook_cb_ctx {
> @@ -90,7 +90,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
> _alpm_wordsplit_free(hook->cmd);
> alpm_list_free_inner(hook->triggers, (alpm_list_fn_free)
> _alpm_trigger_free);
> alpm_list_free(hook->triggers);
> - alpm_list_free(hook->matches);
> + alpm_list_free_inner(hook->matches, free);
> FREELIST(hook->depends);
> free(hook);
> }
> @@ -329,8 +329,11 @@ static int _alpm_hook_parse_cb(const char *file, int
> line,
> hook->abort_on_fail = 1;
> } else if(strcmp(key, "NeedsTargets") == 0) {
> hook->needs_targets = 1;
> + } else if(strcmp(key, "NeedsVersions") == 0) {
> + hook->needs_versions = 1;
> } else if(strcmp(key, "Exec") == 0) {
> - if((hook->cmd = _alpm_wordsplit(value)) == NULL) {
> + if((hook->
> + cmd = _alpm_wordsplit(value)) == NULL) {
> if(errno == EINVAL) {
> error(_("hook %s line %d: invalid value
> %s\n"), file, line, value);
> } else {
> @@ -456,31 +459,54 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t
> *handle,
> static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle,
> struct _alpm_hook_t *hook, struct _alpm_trigger_t *t)
> {
> - alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL;
> + alpm_list_t *add = NULL, *remove = NULL;
>
> if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) {
> alpm_list_t *i;
> for(i = handle->trans->add; i; i = i->next) {
> alpm_pkg_t *pkg = i->data;
> - if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) {
> - if(pkg->oldpkg) {
> - if(t->op & ALPM_HOOK_OP_UPGRADE) {
> - if(hook->needs_targets) {
> - upgrade =
> alpm_list_add(upgrade, pkg->name);
> - } else {
> - return 1;
> - }
> - }
> - } else {
> - if(t->op & ALPM_HOOK_OP_INSTALL) {
> - if(hook->needs_targets) {
> - install =
> alpm_list_add(install, pkg->name);
> - } else {
> - return 1;
> - }
> - }
> + size_t len;
> + char* output = NULL;
> + int upgrade_match = 0, install_match = 0;
> +
> + if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) {
> + continue;
> + }
> +
> + if (pkg->oldpkg) {
> + upgrade_match = (t->op & ALPM_HOOK_OP_UPGRADE);
> + } else {
> + install_match = (t->op & ALPM_HOOK_OP_INSTALL);
> + }
> +
> + if (!upgrade_match && !install_match) {
> + continue;
> + }
> +
> + if (!hook->needs_targets) {
> + return 1;
> + }
> +
> + len = strlen(pkg->name) + 1;
> + if (hook->needs_versions) {
> + if (upgrade_match) {
> + len += strlen(pkg->oldpkg->version) + 1;
> }
> + len += strlen(pkg->version) + 1;
> }
> + CALLOC(output, len, sizeof(char), continue);
> +
> + strcat(output, pkg->name);
> + if (hook->needs_versions) {
> + if (upgrade_match) {
> + strcat(output, " ");
> + strcat(output, pkg->oldpkg->version);
> + }
> + strcat(output, " ");
> + strcat(output, pkg->version);
> + }
> +
> + add = alpm_list_add(add, output);
> }
> }
>
> @@ -488,25 +514,39 @@ static int _alpm_hook_trigger_match_pkg(alpm_handle_t
> *handle,
> alpm_list_t *i;
> for(i = handle->trans->remove; i; i = i->next) {
> alpm_pkg_t *pkg = i->data;
> - if(pkg && _alpm_fnmatch_patterns(t->targets, pkg->name)
> == 0) {
> - if(!alpm_list_find(handle->trans->add, pkg,
> _alpm_pkg_cmp)) {
> - if(hook->needs_targets) {
> - remove = alpm_list_add(remove,
> pkg->name);
> - } else {
> - return 1;
> - }
> - }
> + size_t len;
> + char* output = NULL;
> +
> + if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) {
> + continue;
> }
> +
> + if (!hook->needs_targets) {
> + return 1;
> + }
> +
> + len = strlen(pkg->name) + 1;
> + if (hook->needs_versions) {
> + len += strlen(pkg->version) + 1;
> + }
> + CALLOC(output, len, sizeof(char), continue);
> +
> + strcat(output, pkg->name);
> + if (hook->needs_versions) {
> + strcat(output, " ");
> + strcat(output, pkg->version);
> + }
> +
> + remove = alpm_list_add(remove, output);
> }
> }
>
> /* if we reached this point we either need the target lists or we didn't
> * match anything and the following calls will all be no-ops */
> - hook->matches = alpm_list_join(hook->matches, install);
> - hook->matches = alpm_list_join(hook->matches, upgrade);
> + hook->matches = alpm_list_join(hook->matches, add);
> hook->matches = alpm_list_join(hook->matches, remove);
>
> - return install || upgrade || remove;
> + return add || remove;
> }
>
> static int _alpm_hook_trigger_match(alpm_handle_t *handle,
> diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
> index bd5a0b6..9443e90 100644
> --- a/test/pacman/tests/TESTS
> +++ b/test/pacman/tests/TESTS
> @@ -63,6 +63,7 @@ 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-target-versions.py
> TESTS += test/pacman/tests/hook-upgrade-trigger-no-match.py
> TESTS += test/pacman/tests/ignore001.py
> TESTS += test/pacman/tests/ignore002.py
> diff --git a/test/pacman/tests/hook-target-versions.py
> b/test/pacman/tests/hook-target-versions.py
> new file mode 100644
> index 0000000..fd6d207
> --- /dev/null
> +++ b/test/pacman/tests/hook-target-versions.py
> @@ -0,0 +1,47 @@
> +self.description = "Hook with NeedsTargets"
> +
> +self.add_hook("hook",
> + """
> + [Trigger]
> + Type = Package
> + Operation = Upgrade
> + Target = foo
> +
> + # duplicate trigger to check that duplicate targets are removed
> + [Trigger]
> + Type = Package
> + Operation = Install
> + Target = bar
> +
> + [Trigger]
> + Type = File
> + Operation = Install
> + # matches files in 'file/' but not 'file/' itself
> + Target = somewhere/?*
> +
> + [Action]
> + When = PreTransaction
> + Exec = bin/sh -c 'while read -r tgt; do printf "%s\\n" "$tgt"; done
> > var/log/hook-output'
> + NeedsTargets
> + NeedsVersions
> + """);
> +
> +lp1 = pmpkg("foo")
> +lp1.files = ["file/foo"]
> +self.addpkg2db("local", lp1)
> +
> +p1 = pmpkg("foo", "2.0-1")
> +p1.files = ["file/foo"]
> +self.addpkg(p1)
> +
> +p2 = pmpkg("bar")
> +p2.files = ["file/bar"]
> +self.addpkg(p2)
> +
> +self.args = "-U %s %s" % (p1.filename(), p2.filename())
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PKG_EXIST=foo")
> +self.addrule("""FILE_CONTENTS=var/log/hook-output|bar 1.0-1
> +foo 1.0-1 2.0-1
> +""")
> --
> 2.9.3
