[pacman-dev] [PATCH v2] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

Andrew Gregory andrew.gregory.8 at gmail.com
Sun Sep 4 18:56:29 UTC 2016


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


More information about the pacman-dev mailing list