[pacman-dev] [PATCH v2] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.
Sergey Petrenko
chaoskeeper at mail.ru
Sun Sep 4 19:04:27 UTC 2016
Version can contain spaces??? O_o
I definitely agree with your reasoning. For now I can imagine two concise solutions:
1) Environment variables (mercurial style hooks)
2) Binary hooks (aka plugins)
>Воскресенье, 4 сентября 2016, 18:56 UTC от Andrew Gregory < andrew.gregory.8 at gmail.com >:
>
>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
--
With wish of constant improvement
and unstoppable creativity.
Sergey Petrenko
More information about the pacman-dev
mailing list