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

Re: Remaining triggers changes in Ubuntu


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.


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

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.


> @@ -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!


Reply to: