[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [RFC] Trigger directives that do not put packages in triggers-awaited


On Wed, 2011-05-18 at 22:53:12 +0200, Raphael Hertzog wrote:
> please find attached a patch that implements 2 new directives
> "interest-noawait" and "activate-noawait" for the triggers control file.
> They work exactly like "interest" and "activate" but the package
> that activate the triggers are not put in triggers-awaited status instead
> they are directly configured. Up to now, this was only possible if you
> manually activated triggers with dpkg-trigger. With this patch, the
> package implementing the trigger can also specify that the trigger
> is not important enough to consider that the triggering package is
> not configured.

> My main problem is how to introduce this new feature. For now, I
> have added "-noawait" variants but given they are the most common case, I
> wonder if I shall not change the current directive to not
> put the packages in triggers-awaited and instead to add new directives
> "interest-await" and "activate-await" (or "interest-crucial",
> "activate-crucial") with the current behaviour. That would also reduce the
> number of packages to modify to get the expected benefits... very few
> packages really need to put the triggering package in triggers-awaited.

I don't think that's acceptable. We are not even talking about so many
packages! Changing the semantics in such a way might subtly break stuff
for the cases that actually matter (if it was the other way around I'd
not have any issue with it), there's no way to automate a switch over
or to detect when the wrong semantics are being used, and dpkg being
used beyond Debian/Ubuntu guarantees a transition involving unknown
packages to dpkg will cause external breakage (be it other distributions
or sysadmins, etc). Changing it few weeks after its introduction might
have been okish, but definitely not now, two stable Debian releases
after the fact.

> commit 8fd4917ab6eeff63f3035b8c76b90939ee9ae017
> Author: Raphaël Hertzog <hertzog@debian.org>
> Date:   Sun May 15 01:39:31 2011 +0200
>     dpkg: implement "interest-noawait" and "activate-noawait" trigger commands
>     Those variants do not put triggering packages in triggers-awaited status
>     and thus do not record the package with the corresponding pending triggers
>     in the Triggers-Awaited field.
>     This should be used for triggers which do not provide essential
>     functionality when we can safely consider that the triggering packages
>     are able to satisfy dependencies even if the trigger processing
>     has not yet happened.

> diff --git a/lib/dpkg/triglib.c b/lib/dpkg/triglib.c
> index ae0f5b9..4bbb255 100644
> --- a/lib/dpkg/triglib.c
> +++ b/lib/dpkg/triglib.c
> @@ -253,7 +253,8 @@ struct trigkindinfo {
>  	/* Rest are for everyone: */
>  	void (*activate_awaiter)(struct pkginfo *pkg /* may be NULL */);
>  	void (*activate_done)(void);
> -	void (*interest_change)(const char *name, struct pkginfo *pkg, int signum);
> +	void (*interest_change)(const char *name, struct pkginfo *pkg, int signum,
> +	                        bool noawait);
>  };

Using bools on functions with multiple arguments tends to be pretty
confusing on the call sites, please use an enum instead.

> @@ -381,9 +383,10 @@ trk_explicit_activate_start(void)
>  static void
>  trk_explicit_activate_awaiter(struct pkginfo *aw)
>  {
> -	char buf[1024];
> +	char buf[1024], *slash;
>  	const char *emsg;
>  	struct pkginfo *pend;
> +	bool noawait;
>  	if (!trk_explicit_f)
>  		return;

Move the scope of these two to the while loop, so it's clear their
data is not supposed to be preserved outside the loop or across
iterations. There's another instance of this.

> @@ -393,18 +396,28 @@ trk_explicit_activate_awaiter(struct pkginfo *aw)
>  		        trk_explicit_fn.buf);
>  	while (trk_explicit_fgets(buf, sizeof(buf)) >= 0) {
> +		noawait = false;
> +		slash = index(buf, '/');

index(3) was marked as legacy in POSIX.1-2001, and has been dropped by
POSIX.1-2008, use strchr(3) instead. There's another instance of this.

Anyway the rest I've just skimmed over, will try to take a second look
in few days. Something to consider though are downgrades when the new
directives are on the db.


Reply to: