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

Remaining triggers changes in Ubuntu



I recently finished off the merge of dpkg 1.14.18 into Ubuntu (yes, I
know, I should also merge 1.14.19, but I was already part-way through
this and it was easier to do that as a separate step). I went through
the entire merge line-by-line, with some assistance from Ian on IRC at
one point. Here are the remaining triggers-related changes in Ubuntu.

I also have an item on my to-do list to deal with
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=432893, which showed up
at one point during the merge. The bulk of the fix for this seems to be
here:

  http://git.debian.org/?p=dpkg/dpkg.git;a=commitdiff;h=4b2bc864ce70972800e9995deb97b8ff936a61fe;hp=c372e7e8b203c2bc052f488960f078a5baac03b7

Is it possible that somebody could review this, as it seems to have
slipped through the cracks during the 1.15.0 argument?


  [ Ian Jackson ]
  * Rename triggers/Deferred to triggers/Unincorp to fix upgrades from
    early versions of trigger support in Ubuntu.

I mention this solely for completeness; I doubt it's necessary in Debian
(and it probably isn't necessary in Ubuntu any more either, but it's not
difficult to retain and I generally only remove this kind of thing if it
actively causes a problem).

diff -Nru dpkg-1.14.18/debian/dpkg.postinst dpkg-1.14.18ubuntu1/debian/dpkg.postinst
--- dpkg-1.14.18/debian/dpkg.postinst	2008-04-09 07:35:16.000000000 +0100
+++ dpkg-1.14.18ubuntu1/debian/dpkg.postinst	2008-05-01 13:47:54.000000000 +0100
@@ -72,6 +72,14 @@
 
 	move_info_directory
 	remove_info_symlink
+
+	if test -f /var/lib/dpkg/triggers/Unincorp; then
+	    # Upgrade from broken trigger interest recorder
+	    #  (bugs.launchpad.net/133172).  We remove this
+	    #  old stale file:
+	    rm -f /var/lib/dpkg/triggers/Deferred
+	fi
+
 	;;
 
     abort-upgrade|abort-deconfigure|abort-remove)


  [ Ian Jackson ]
  * Avoid closing fsys tarfile pipe twice even in normal operation -
    normally EBADF but might sometimes close some other desired fd and
    cause hideous doom.
  * Avoid duplicate attempts to [f]close in obscure error situations which
    might conceiveably close wrong fds.

This did actually show up as an Ubuntu bug report from somebody who
encountered it in real life, as I recall, so it's not theoretical.

diff -Nru dpkg-1.14.18/lib/cleanup.c dpkg-1.14.18ubuntu1/lib/cleanup.c
--- dpkg-1.14.18/lib/cleanup.c	2008-04-09 07:35:16.000000000 +0100
+++ dpkg-1.14.18ubuntu1/lib/cleanup.c	2008-05-01 00:43:45.000000000 +0100
@@ -29,8 +29,10 @@
 {
 	int *p1 = (int *)argv[0];
 
-	close(p1[0]);
-	close(p1[1]);
+	if (p1[0] >= 0)
+		close(p1[0]);
+	if (p1[1] >= 0)
+		close(p1[1]);
 }
 
 void
@@ -46,7 +48,8 @@
 {
 	DIR *d = (DIR *)(argv[0]);
 
-	closedir(d);
+	if (d)
+		closedir(d);
 }
 
 void
@@ -54,6 +57,17 @@
 {
 	int ip = *(int *)argv[0];
 
-	close(ip);
+	if (ip >= 0)
+		close(ip);
+}
+
+int
+ferror_fclose_pop_cleanup(FILE *f)
+{
+	int r1, r2;
+	r1 = ferror(f);
+	pop_cleanup(ehflag_normaltidy);
+	r2 = fclose(f);
+	return r1 ? r1 : r2;
 }
 
diff -Nru dpkg-1.14.18/lib/dpkg.h dpkg-1.14.18ubuntu1/lib/dpkg.h
--- dpkg-1.14.18/lib/dpkg.h	2008-04-09 07:35:16.000000000 +0100
+++ dpkg-1.14.18ubuntu1/lib/dpkg.h	2008-05-01 00:43:03.000000000 +0100
@@ -237,6 +237,15 @@
 void cu_closedir(int argc, void **argv);
 void cu_closefd(int argc, void **argv);
 
+int ferror_fclose_pop_cleanup(FILE *f);
+ /* calls ferror and fclose on f, and pop_cleanup
+  *   file= fopen
+  *   push_cleanup(cu_closefile,~ehflag_normaltidy, 0,0, 1,(void*)file
+  * return is 0 on success or EOF if fclose or ferror failed
+  * all three calls are always made regardless, and in the right order
+  * so you can just call ohshite if this returns EOF
+  */
+
 /*** lock.c ***/
 
 void lock_file(int *lockfd, const char *filename,
diff -Nru dpkg-1.14.18/lib/triglib.c dpkg-1.14.18ubuntu1/lib/triglib.c
--- dpkg-1.14.18/lib/triglib.c	2008-04-09 07:35:16.000000000 +0100
+++ dpkg-1.14.18ubuntu1/lib/triglib.c	2008-05-01 14:02:52.000000000 +0100
@@ -355,10 +355,9 @@
 	}
 	if (signum > 0)
 		fprintf(nf, "%s\n", pkg->name);
-	if (ferror(nf) || fclose(nf))
+	if (ferror_fclose_pop_cleanup(nf))
 		ohshite(_("unable to write new trigger interest file `%.250s'"),
 		        newfn.buf);
-	pop_cleanup(ehflag_normaltidy);
 
 	if (rename(newfn.buf, trk_explicit_fn.buf))
 		ohshite(_("unable to install new trigger interest file `%.250s'"),
@@ -449,10 +448,9 @@
 		fprintf(nf, "%s %s\n", trigh.namenode_name(tfi->fnn),
 		        tfi->pkg->name);
 
-	if (ferror(nf) || fclose(nf))
+	if (ferror_fclose_pop_cleanup(nf))
 		ohshite(_("unable to write new file triggers file `%.250s'"),
 		        triggersnewfilefile);
-	pop_cleanup(ehflag_normaltidy);
 
 	if (rename(triggersnewfilefile, triggersfilefile))
 		ohshite(_("unable to install new file triggers file as `%.250s'"),
@@ -708,7 +706,7 @@
 		abort();
 	}
 
-	/* Right, that's it. New (empty) Unincopr can be installed. */
+	/* Right, that's it. New (empty) Unincorp can be installed. */
 	trigdef_process_done();
 }
 


  [ Colin Watson ]
  * Add a few more comments around obscure bits of trigger handling code
    which confused both me and Ian during the Ubuntu merge.

diff -Nru dpkg-1.14.18/lib/dbmodify.c dpkg-1.14.18ubuntu1/lib/dbmodify.c
--- dpkg-1.14.18/lib/dbmodify.c	2008-04-09 07:35:16.000000000 +0100
+++ dpkg-1.14.18ubuntu1/lib/dbmodify.c	2008-05-01 13:26:41.000000000 +0100
@@ -253,6 +253,9 @@
 
   onerr_abort++;
 
+  /* Clear pending triggers here so that only code that sets the status
+   * to interesting (for triggers) values has to care about triggers.
+   */
   if (pkg->status != stat_triggerspending &&
       pkg->status != stat_triggersawaited)
     pkg->trigpend_head = NULL;
diff -Nru dpkg-1.14.18/src/trigproc.c dpkg-1.14.18ubuntu1/src/trigproc.c
--- dpkg-1.14.18/src/trigproc.c	2008-04-09 07:35:17.000000000 +0100
+++ dpkg-1.14.18ubuntu1/src/trigproc.c	2008-05-01 14:02:52.000000000 +0100
@@ -295,6 +295,9 @@
 		}
 		varbufaddc(&namesarg, 0);
 
+		/* Setting the status to halfconfigured
+		 * causes modstatdb_note to clear pending triggers.
+		 */
 		pkg->status = stat_halfconfigured;
 		modstatdb_note(pkg);
 

Thanks,

-- 
Colin Watson                                       [cjwatson@debian.org]


Reply to: