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

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



Hi Guillem,

thanks for the review.

On Tue, 07 Jun 2011, Guillem Jover wrote:
> 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.

Yeah, it's probably best to keep the semantics intact.

>From the questions I asked, there seems to be 5-6 packages which sort of
rely on this behaviour, it's not many but a bit too much for dpkg to
Breaks them and impose some complicated upgrade scenario.

Maybe we can consider creating "interest-await" and "interest-noawait".
And in wheezy+1 we would add a lintian warning for people still using
"interest". That way people have to consider the question at least.
Not sure if it's worth the effort though.

I have integrated all your other suggestions. The updated patch is
attached.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)
commit 572b672b810c57a418ec38760b107a80430e4fe0
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/doc/triggers.txt b/doc/triggers.txt
index d10aab0..77668ed 100644
--- a/doc/triggers.txt
+++ b/doc/triggers.txt
@@ -46,7 +46,8 @@ will not be processed immediately when it is activated; processing is
 deferred until it is convenient (as described below).
 
 At a trigger activation, the interested packages(s) are added to the
-triggering package's list of triggers-awaited packages; the triggering
+triggering package's list of triggers-awaited packages (unless the
+trigger has been configured to not require it); the triggering
 package is said to await the trigger processing.
 
 A package which has pending triggers, or which awaits triggers, is not
diff --git a/lib/dpkg/triglib.c b/lib/dpkg/triglib.c
index 50b6012..65efcb6 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,
+	                        enum trig_options opts);
 };
 
 static const struct trigkindinfo tki_explicit, tki_file, tki_unknown;
@@ -318,7 +319,8 @@ trk_unknown_activate_done(void)
 }
 
 static void DPKG_ATTR_NORET
-trk_unknown_interest_change(const char *trig, struct pkginfo *pkg, int signum)
+trk_unknown_interest_change(const char *trig, struct pkginfo *pkg, int signum,
+                            enum trig_options opts)
 {
 	ohshit(_("invalid or unknown syntax in trigger name `%.250s'"
 	         " (in trigger interests for package `%.250s')"),
@@ -393,13 +395,21 @@ trk_explicit_activate_awaiter(struct pkginfo *aw)
 		        trk_explicit_fn.buf);
 
 	while (trk_explicit_fgets(buf, sizeof(buf)) >= 0) {
+		char *slash;
+		bool noawait = false;
+		slash = strchr(buf, '/');
+		if (slash && strcmp("/noawait", slash) == 0) {
+			noawait = true;
+			*slash = '\0';
+		}
 		emsg = pkg_name_is_illegal(buf, NULL);
 		if (emsg)
 			ohshit(_("trigger interest file `%.250s' syntax error; "
 			         "illegal package name `%.250s': %.250s"),
 			       trk_explicit_fn.buf, buf, emsg);
 		pend = pkg_db_find(buf);
-		trig_record_activation(pend, aw, trk_explicit_trig);
+		trig_record_activation(pend, noawait ? NULL : aw,
+		                       trk_explicit_trig);
 	}
 }
 
@@ -435,7 +445,8 @@ trk_explicit_interest_remove(const char *newfilename)
 }
 
 static void
-trk_explicit_interest_change(const char *trig,  struct pkginfo *pkg, int signum)
+trk_explicit_interest_change(const char *trig,  struct pkginfo *pkg, int signum,
+                             enum trig_options opts)
 {
 	static struct varbuf newfn;
 	char buf[1024];
@@ -453,13 +464,16 @@ trk_explicit_interest_change(const char *trig,  struct pkginfo *pkg, int signum)
 	push_cleanup(cu_closestream, ~ehflag_normaltidy, NULL, 0, 1, nf);
 
 	while (trk_explicit_f && trk_explicit_fgets(buf, sizeof(buf)) >= 0) {
-		if (!strcmp(buf, pkg->name))
+		int len = strlen(pkg->name);
+		if (strncmp(buf, pkg->name, len) == 0 &&
+		    (buf[len] == '\0' || buf[len] == '/'))
 			continue;
 		fprintf(nf, "%s\n", buf);
 		empty = false;
 	}
 	if (signum > 0) {
-		fprintf(nf, "%s\n", pkg->name);
+		fprintf(nf, "%s%s\n", pkg->name,
+		        (opts == trig_noawait) ? "/noawait" : "");
 		empty = false;
 	}
 
@@ -506,7 +520,8 @@ static int filetriggers_edited = -1;
  * but die if already present.
  */
 static void
-trk_file_interest_change(const char *trig, struct pkginfo *pkg, int signum)
+trk_file_interest_change(const char *trig, struct pkginfo *pkg, int signum,
+                         enum trig_options opts)
 {
 	struct filenamenode *fnn;
 	struct trigfileint **search, *tfi;
@@ -530,6 +545,7 @@ trk_file_interest_change(const char *trig, struct pkginfo *pkg, int signum)
 	tfi = nfmalloc(sizeof(*tfi));
 	tfi->pkg = pkg;
 	tfi->fnn = fnn;
+	tfi->options = opts;
 	tfi->samefile_next = *trigh.namenode_interested(fnn);
 	*trigh.namenode_interested(fnn) = tfi;
 
@@ -537,6 +553,7 @@ trk_file_interest_change(const char *trig, struct pkginfo *pkg, int signum)
 	goto edited;
 
 found:
+	tfi->options = opts;
 	if (signum > 1)
 		ohshit(_("duplicate file trigger interest for filename `%.250s' "
 		         "and package `%.250s'"), trig, pkg->name);
@@ -570,8 +587,9 @@ trig_file_interests_update(void)
 	push_cleanup(cu_closestream, ~ehflag_normaltidy, NULL, 0, 1, nf);
 
 	for (tfi = filetriggers.head; tfi; tfi = tfi->inoverall.next)
-		fprintf(nf, "%s %s\n", trigh.namenode_name(tfi->fnn),
-		        tfi->pkg->name);
+		fprintf(nf, "%s %s%s\n", trigh.namenode_name(tfi->fnn),
+		        tfi->pkg->name,
+		        (tfi->options == trig_noawait) ? "/noawait" : "");
 
 	if (ferror(nf))
 		ohshite(_("unable to write new file triggers file `%.250s'"),
@@ -629,19 +647,26 @@ trig_file_interests_ensure(void)
 
 	push_cleanup(cu_closestream, ~0, NULL, 0, 1, f);
 	while (fgets_checked(linebuf, sizeof(linebuf), f, triggersfilefile) >= 0) {
+		char *slash;
+		enum trig_options trig_opts = trig_await;
 		space = strchr(linebuf, ' ');
 		if (!space || linebuf[0] != '/')
 			ohshit(_("syntax error in file triggers file `%.250s'"),
 			       triggersfilefile);
 		*space++ = '\0';
 
+		slash = strchr(space, '/');
+		if (slash && strcmp("/noawait", slash) == 0) {
+			trig_opts = trig_noawait;
+			*slash = '\0';
+		}
 		emsg = pkg_name_is_illegal(space, NULL);
 		if (emsg)
 			ohshit(_("file triggers record mentions illegal "
 			         "package name `%.250s' (for interest in file "
 				 "`%.250s'): %.250s"), space, linebuf, emsg);
 		pkg = pkg_db_find(space);
-		trk_file_interest_change(linebuf, pkg, +2);
+		trk_file_interest_change(linebuf, pkg, +2, trig_opts);
 	}
 	pop_cleanup(ehflag_normaltidy);
 ok:
@@ -666,7 +691,8 @@ trig_file_activate(struct filenamenode *trig, struct pkginfo *aw)
 
 	for (tfi = *trigh.namenode_interested(trig); tfi;
 	     tfi = tfi->samefile_next)
-		trig_record_activation(tfi->pkg, aw, trigh.namenode_name(trig));
+		trig_record_activation(tfi->pkg, (tfi->options == trig_noawait) ?
+		                       NULL : aw, trigh.namenode_name(trig));
 }
 
 static void
@@ -698,39 +724,41 @@ static const struct trigkindinfo tki_file = {
 /*---------- Trigger control info file. ----------*/
 
 static void
-trig_cicb_interest_change(const char *trig, struct pkginfo *pkg, int signum)
+trig_cicb_interest_change(const char *trig, struct pkginfo *pkg, int signum,
+                          enum trig_options opts)
 {
 	const struct trigkindinfo *tki = trig_classify_byname(trig);
 
 	assert(filetriggers_edited >= 0);
-	tki->interest_change(trig, pkg, signum);
+	tki->interest_change(trig, pkg, signum, opts);
 }
 
 void
-trig_cicb_interest_delete(const char *trig, void *user)
+trig_cicb_interest_delete(const char *trig, void *user, enum trig_options opts)
 {
-	trig_cicb_interest_change(trig, user, -1);
+	trig_cicb_interest_change(trig, user, -1, opts);
 }
 
 void
-trig_cicb_interest_add(const char *trig, void *user)
+trig_cicb_interest_add(const char *trig, void *user, enum trig_options opts)
 {
-	trig_cicb_interest_change(trig, user, +1);
+	trig_cicb_interest_change(trig, user, +1, opts);
 }
 
 void
-trig_cicb_statuschange_activate(const char *trig, void *user)
+trig_cicb_statuschange_activate(const char *trig, void *user,
+                                enum trig_options opts)
 {
 	struct pkginfo *aw = user;
 
 	trig_activate_start(trig);
-	dtki->activate_awaiter(aw);
+	dtki->activate_awaiter((opts == trig_noawait) ? NULL : aw);
 	dtki->activate_done();
 }
 
 static void
 parse_ci_call(const char *file, const char *cmd, trig_parse_cicb *cb,
-              const char *trig, void *user)
+              const char *trig, void *user, enum trig_options opts)
 {
 	const char *emsg;
 
@@ -740,7 +768,7 @@ parse_ci_call(const char *file, const char *cmd, trig_parse_cicb *cb,
 		         "syntax in trigger name `%.250s': %.250s"),
 		       file, trig, emsg);
 	if (cb)
-		cb(trig, user);
+		cb(trig, user, opts);
 }
 
 void
@@ -775,9 +803,13 @@ trig_parse_ci(const char *file, trig_parse_cicb *interest,
 		while (cisspace(*spc))
 			spc++;
 		if (!strcmp(cmd, "interest")) {
-			parse_ci_call(file, cmd, interest, spc, user);
+			parse_ci_call(file, cmd, interest, spc, user, trig_await);
+		} else if (!strcmp(cmd, "interest-noawait")) {
+			parse_ci_call(file, cmd, interest, spc, user, trig_noawait);
 		} else if (!strcmp(cmd, "activate")) {
-			parse_ci_call(file, cmd, activate, spc, user);
+			parse_ci_call(file, cmd, activate, spc, user, trig_await);
+		} else if (!strcmp(cmd, "activate-noawait")) {
+			parse_ci_call(file, cmd, activate, spc, user, trig_noawait);
 		} else {
 			ohshit(_("triggers ci file contains unknown directive `%.250s'"),
 			       cmd);
diff --git a/lib/dpkg/triglib.h b/lib/dpkg/triglib.h
index 9c06ac3..250c241 100644
--- a/lib/dpkg/triglib.h
+++ b/lib/dpkg/triglib.h
@@ -38,9 +38,15 @@ DPKG_BEGIN_DECLS
 
 const char *trig_name_is_illegal(const char *p);
 
+enum trig_options {
+	trig_await,
+	trig_noawait
+};
+
 struct trigfileint {
 	struct pkginfo *pkg;
 	struct filenamenode *fnn;
+	enum trig_options options;
 	struct trigfileint *samefile_next;
 	struct {
 		struct trigfileint *next, *prev;
@@ -83,10 +89,11 @@ void trig_fixup_awaiters(enum modstatdb_rw cstatus);
 void trig_file_interests_ensure(void);
 void trig_file_interests_save(void);
 
-typedef void trig_parse_cicb(const char *trig, void *user);
-void trig_cicb_interest_delete(const char *trig, void *user);
-void trig_cicb_interest_add(const char *trig, void *user);
-void trig_cicb_statuschange_activate(const char *trig, void *user);
+typedef void trig_parse_cicb(const char *trig, void *user, enum trig_options to);
+void trig_cicb_interest_delete(const char *trig, void *user, enum trig_options to);
+void trig_cicb_interest_add(const char *trig, void *user, enum trig_options to);
+void trig_cicb_statuschange_activate(const char *trig, void *user,
+                                     enum trig_options to);
 void trig_parse_ci(const char *file, trig_parse_cicb *interest,
                    trig_parse_cicb *activate, void *user);
 
diff --git a/man/deb-triggers.5 b/man/deb-triggers.5
index f869cd2..6c8df02 100644
--- a/man/deb-triggers.5
+++ b/man/deb-triggers.5
@@ -21,19 +21,32 @@ The trigger control directives currently supported are:
 .I trigger-name
 .PP
 .in +5
+.B interest-noawait
+.I trigger-name
+.PP
+.in +5
 Specifies that the package is interested in the named trigger. All
 triggers in which a package is interested must be listed using this
-directive in the triggers control file.
+directive in the triggers control file. The "noawait" variant does
+not put the triggering packages in triggers-awaited state. This should
+be used when the functionality provided by the trigger is not crucial.
 .PP
 .in +5
 .B activate
 .I trigger-name
 .PP
 .in +5
+.B activate-noawait
+.I trigger-name
+.PP
+.in +5
 Arranges that changes to this package's state will activate the
 specified trigger. The trigger will be activated at the start of
 the following operations: unpack, configure, remove (including for
 the benefit of a conflicting package), purge and deconfigure.
+The "noawait" variant does not put the triggering packages in
+triggers-awaited state. This should be used when the functionality
+provided by the trigger is not crucial.
 .PP
 .in +5
 If this package disappears during the unpacking of another package
@@ -45,6 +58,11 @@ versions of the package will be activated.
 .PP
 Unknown directives are an error which will prevent installation of the
 package.
+.PP
+The "\-noawait" variants are only supported by dpkg 1.16.1 or newer, and
+will lead to errors if used with an older dpkg. It is thus recommended
+to add a "Pre-Depends: dpkg (>= 1.16.1)" to any package that wish to use
+those directives.
 .
 .SH SEE ALSO
 .BR dpkg\-trigger (1),
diff --git a/src/trigproc.c b/src/trigproc.c
index 07bf346..0291ad4 100644
--- a/src/trigproc.c
+++ b/src/trigproc.c
@@ -345,7 +345,8 @@ trigproc(struct pkginfo *pkg)
 /*========== Transitional global activation. ==========*/
 
 static void
-transitional_interest_callback_ro(const char *trig, void *user)
+transitional_interest_callback_ro(const char *trig, void *user,
+                                  enum trig_options opts)
 {
 	struct pkginfo *pend = user;
 
@@ -357,12 +358,13 @@ transitional_interest_callback_ro(const char *trig, void *user)
 }
 
 static void
-transitional_interest_callback(const char *trig, void *user)
+transitional_interest_callback(const char *trig, void *user,
+                               enum trig_options opts)
 {
 	struct pkginfo *pend = user;
 
-	trig_cicb_interest_add(trig, pend);
-	transitional_interest_callback_ro(trig, user);
+	trig_cicb_interest_add(trig, pend, opts);
+	transitional_interest_callback_ro(trig, user, opts);
 }
 
 /*

Reply to: