Re: Bug#560317: dpkg-reconfigure does not set DPKG_MAINTSCRIPT_PACKAGE (et al)
On Fri, Aug 05, 2011 at 09:57:33PM +0200, Raphael Hertzog wrote:
> On Fri, 05 Aug 2011, Joey Hess wrote:
> > * Nobody has ever addressed my concern that, if dpkg-reconfigure runs
> > dpkg --configure --pending, this will result in it confusingly doing
> > other things than configuring the specified package.
>
> I believe this should simply be documented. I rarely run dpkg-reconfigure
> on a system that's not "clean" from an installation point of view.
>
> But if you really want to try to limit the side effects then you can compare
> the status of all packages before and after having run the scripts and decide
> whether or not you have to run it. You could also try to configure only the
> affected packages but since triggers processing can activate other triggers,
> you might have to fallback to --configure --pending anyway if it turns out
> it was not enough.
I would add a couple of things:
* There's nothing stopping a package using 'dpkg-trigger
--by-package=PACKAGE' in its postinst right now (it's redundant but
permissible), so we might well have this problem already even without
setting these environment variables. Fundamentally since we're
running the postinst by hand it seems as though we're responsible for
cleaning up after it too.
* There already exist packages which test for DEBCONF_RECONFIGURE or
DPKG_RUNNING_VERSION or some such before running dpkg-trigger, and if
set they just perform the action immediately rather than triggering.
That won't be broken by this change, and at least in the case of
self-triggers it's a perfectly reasonable package-level approach.
(Many non-self-triggers are guarded by version checks anyway which
won't fire on reconfigure, such as the perl-major-upgrade one.)
* The dpkg-maintscript-helper issue seems more pressing to me right now
than the dpkg-trigger one, since .maintscript files (for which I
added support to dh_installdeb a while back without thinking about
this bug) are becoming more popular and will cause this kind of
problem. But I agree that we at least need not to make matters worse
regarding dpkg-trigger.
* I do tend to agree with Joey that 'dpkg --configure --pending' seems
a little bit over the top.
I applied Raphaël's first two patches to a local branch, fixed up a
couple of typos in the second, and added a patch of my own which
calculates the set of packages with newly pending triggers after running
maintainer scripts and configures them until there are no newly pending
triggers. This seems to behave well for me and doesn't seem too
unreasonable a solution, particularly since you get "Processing triggers
for PACKAGE ..." at the top of the output from dpkg so it's clear what's
happening. The result of this work is attached, minus changelog which I
can write up if this is acceptable.
Joey, would you be OK with this approach?
Thanks,
--
Colin Watson [cjwatson@debian.org]
>From 4628727338951178308d957b7c8bf9ad927f6bce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Fri, 5 Aug 2011 21:21:13 +0200
Subject: [PATCH 1/3] Set environment variables expected by maintainer scripts
Closes: #560317
---
dpkg-reconfigure | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/dpkg-reconfigure b/dpkg-reconfigure
index e38df69..b433bb0 100755
--- a/dpkg-reconfigure
+++ b/dpkg-reconfigure
@@ -170,6 +170,8 @@ foreach my $pkg (@packages) {
$_=`dpkg --status $pkg`;
my ($version)=m/Version: (.*)\n/;
my ($status)=m/Status: (.*)\n/;
+ my ($package)=m/Package: (.*)\n/;
+ my ($arch)=m/Architecture: (.*)\n/;
if (! $force) {
if (! defined $status || $status =~ m/not-installed$/) {
print STDERR "$0: ".sprintf(gettext("%s is not installed"), $pkg)."\n";
@@ -200,6 +202,11 @@ foreach my $pkg (@packages) {
my $is_confmodule='';
+ # Set environment variables expected by maintainer scripts
+ $ENV{DPKG_MAINTSCRIPT_PACKAGE}=$package;
+ $ENV{DPKG_MAINTSCRIPT_ARCH}=$arch;
+ $ENV{DPKG_MAINTSCRIPT_NAME}=$script;
+
if ($script ne 'config') {
# Test to see if the script uses debconf.
open (IN, "<$infodir/$pkg.$script");
--
1.7.9.1
>From 3489fc62f2ab7b9235ba2f431601e8f3ed0fbafb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hertzog@debian.org>
Date: Fri, 5 Aug 2011 21:27:06 +0200
Subject: [PATCH 2/3] Do not hardcode the path of maintainer scripts
Multi-arch changes the layout for Multi-Arch: same package
and it's thus important to use the official dpkg interface
to retrieve the path.
---
dpkg-reconfigure | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/dpkg-reconfigure b/dpkg-reconfigure
index b433bb0..daa7bee 100755
--- a/dpkg-reconfigure
+++ b/dpkg-reconfigure
@@ -85,8 +85,6 @@ if (exists $ENV{DEBCONF_USE_CDEBCONF} and $ENV{DEBCONF_USE_CDEBCONF} ne '') {
exec "/usr/lib/cdebconf/dpkg-reconfigure", @ARGV;
}
-my $infodir="/var/lib/dpkg/info";
-
use strict;
use Debconf::Db;
use Debconf::Gettext;
@@ -185,8 +183,11 @@ foreach my $pkg (@packages) {
if ($reload) {
# Load up templates just in case they aren't already.
- Debconf::Template->load("$infodir/$pkg.templates", $pkg)
- if -e "$infodir/$pkg.templates";
+ my $templates=`dpkg-query --control-path $pkg templates`;
+ chomp($templates);
+ if ($templates and -e $templates) {
+ Debconf::Template->load($templates, $pkg);
+ }
}
# Simulation of reinstalling a package, without bothering with
@@ -198,7 +199,10 @@ foreach my $pkg (@packages) {
['config', 'reconfigure', $version],
['postinst', 'configure', $version]) {
my $script=shift @$info;
- next unless -x "$infodir/$pkg.$script";
+ my $path_script=`dpkg-query --control-path $pkg $script`;
+ chomp($path_script);
+
+ next unless $path_script and -x $path_script;
my $is_confmodule='';
@@ -209,7 +213,7 @@ foreach my $pkg (@packages) {
if ($script ne 'config') {
# Test to see if the script uses debconf.
- open (IN, "<$infodir/$pkg.$script");
+ open (IN, "<$path_script");
while (<IN>) {
if (/confmodule/i) {
$is_confmodule=1;
@@ -221,8 +225,7 @@ foreach my $pkg (@packages) {
if ($script eq 'config' || $is_confmodule) {
# Start up the confmodule.
- my $confmodule=make_confmodule(
- "$infodir/$pkg.$script", @$info);
+ my $confmodule=make_confmodule($path_script, @$info);
# Make sure any questions the confmodule registers
# are owned by this package.
@@ -241,7 +244,7 @@ foreach my $pkg (@packages) {
Debconf::Db->save;
delete $ENV{DEBIAN_HAS_FRONTEND};
- my $ret=system("$infodir/$pkg.$script", @$info);
+ my $ret=system($path_script, @$info);
if (int($ret / 256) != 0) {
exit int($ret / 256);
}
--
1.7.9.1
>From 7961adc048b505ca80d9f472f5364706010fba3a Mon Sep 17 00:00:00 2001
From: Colin Watson <cjwatson@debian.org>
Date: Fri, 9 Mar 2012 14:32:07 +0000
Subject: [PATCH 3/3] Process any newly pending triggers after running
maintainer scripts
---
dpkg-reconfigure | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/dpkg-reconfigure b/dpkg-reconfigure
index daa7bee..7a31ff0 100755
--- a/dpkg-reconfigure
+++ b/dpkg-reconfigure
@@ -159,6 +159,8 @@ else {
# hysterical raisens.
$ENV{DEBCONF_RECONFIGURE}=1;
+my %initial_triggers=map { $_ => 1 } triggers_pending();
+
foreach my $pkg (@packages) {
# Set default title.
$frontend->default_title($pkg);
@@ -255,6 +257,23 @@ foreach my $pkg (@packages) {
}
}
+# Maintainer scripts may have activated triggers. If so, try to process
+# them.
+my @new_triggers;
+do {
+ @new_triggers=();
+ foreach my $trigpend (triggers_pending()) {
+ push @new_triggers, $trigpend
+ if not exists $initial_triggers{$trigpend};
+ }
+ if (@new_triggers) {
+ my $ret=system("dpkg", "--configure", @new_triggers);
+ if (int($ret / 256) != 0) {
+ exit int($ret / 256);
+ }
+ }
+} while (@new_triggers);
+
$frontend->shutdown;
Debconf::Db->save;
@@ -275,6 +294,22 @@ sub allpackages {
return sort @ret;
}
+# Returns a list of all packages with pending triggers.
+sub triggers_pending {
+ my @ret;
+ local $_;
+
+ open (QUERY, '-|', 'dpkg-query', '-W',
+ '-f', '${Package} ${Triggers-Pending}\n');
+ while (<QUERY>) {
+ my ($pkg, @triggers) = split;
+ push @ret, $pkg if @triggers;
+ }
+ close QUERY;
+
+ return @ret;
+}
+
=head1 AUTHOR
Joey Hess <joeyh@debian.org>
--
1.7.9.1
Reply to: