I'm writing hooks to solve infamous kernel problem. Having version in hook isn't necessary, but will save me lot of work (and lot of time at each hook run).

Воскресенье, 4 сентября 2016, 17:40 UTC от Dave Reisner :

On Sun, Sep 04, 2016 at 08:36:39PM +0000, Sergey Petrenko via pacman-dev wrote:

...

---

Missing seems to be some rationale for this patch. What's the problem you're interested in solving with this feature?

...

lib/libalpm/hook.c | 102 ++++++++++++++++++++---------- test/pacman/tests/TESTS | 1 + test/pacman/tests/hook-target-versions.py | 47 ++++++++++++++ 3 files changed, 118 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..cc6c8bd 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,53 @@ 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; - } - } + + if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) { + continue; + } + + int upgrade_match = 0, install_match = 0; + 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; + } + + char* output = NULL; + size_t 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 +513,38 @@ 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; - } - } + + if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) { + continue; } + + if (!hook->needs_targets) { + return 1; + } + + char* output = NULL; + size_t 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

-- With wish of constant improvement and unstoppable creativity. Sergey Petrenko

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