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=4b2bc864ce70972800e9995deb 97b8ff936a61fe;hp=c372e7e8b203c2bc052f488960f078a5 baac03b7

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
", 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
", 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]


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

Hi,

On Tue, 2008-05-27 at 10:53:18 +0100, Colin Watson wrote:
> 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.

Perfect!

> 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=4b2bc864ce70972800e9995deb 97b8ff936a61fe;hp=c372e7e8b203c2bc052f488960f078a5 baac03b7
>
> Is it possible that somebody could review this, as it seems to have
> slipped through the cracks during the 1.15.0 argument?

Yes, I've in my TODO list to check that one.

> [ 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).

Right, I didn't see the point in merging that, just to carry that early
implementation change, also given that I fixed all references to the
Deferred file, there was no room for confusion in the sources/docs.

> [ 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.

IIRC this one should have been fixed already with commit 48ed1657. The
close/closedir calls should always fail in that case, but I could of
course add the argument validation checks anyway (cannot remember
what's the reason I missed that one).

> * Avoid duplicate attempts to [f]close in obscure error situations which
> might conceiveably close wrong fds.

Again IIRC didn't merge that part because there were similar calls
in other parts of the code and wanted to review and fix all of them.
I added that one to the dpkg TODO list to check latter, so will take
care of it.

> 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.

Sure.

> @@ -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();
> }

Committed this locally as a typo fix.

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

Committed locally as well.

> Thanks,

No, thank you!

regards,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

On Tue, 27 May 2008, Colin Watson wrote:
> 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=4b2bc864ce70972800e9995deb 97b8ff936a61fe;hp=c372e7e8b203c2bc052f488960f078a5 baac03b7
>
> Is it possible that somebody could review this, as it seems to have
> slipped through the cracks during the 1.15.0 argument?

The patch pointed above pre-supposes a prior revert of
ca909d6711804e51f48415d31aa2fc630943cd03 (this was in Ian's trigger branch
as commit 2791b4ca7f567e46a2fef779aa864afd4ccbe3d2).

And it didn't apply cleanly anymore so I created an updated patch, it's
attached. Guillem, please review and apply if it's fine.

I also fixed cu_prerminfavour to not clear the reinstreq flag as Ian
already did it from cu_prermupgrade/cu_prermremove and the same reasons do
apply.

Cheers,
--
Raphaël Hertzog

Le best-seller français mis à jour pour Debian Etch :
http://www.ouaza.com/livre/admin-debian/