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

Bug#391356: tasksel: support debconf-apt-progress for task scripts



Package: tasksel
Version: 2.55
Severity: wishlist
Tags: patch

On Wed, Sep 13, 2006 at 11:26:51AM -0400, Joey Hess wrote:
> Colin Watson wrote:
> > However, it isn't quite perfect, because the progress bar endpoints for
> > the task scripts are all wrong. In order to fix this, tasksel would need
> > to be able to take the endpoints passed to it and divide them down
> > somehow based on how many task scripts it had to run, etc. This is
> > probably doable, but it's made difficult because at the moment we're
> > passing the debconf-apt-progress arguments wrapped up in a single
> > --debconf-apt-progress option. What do you think about unpacking that
> > into --debconf-apt-from, --debconf-apt-to, --debconf-apt-logstderr, or
> > something along those lines?
> 
> If you want to do that, it'll also solve the issue of the
> desktop.preinst currently hanging when apt decides it needs to prompt
> for a CD during it.

The attached patch attempts to do this. Its main flaw is that, because
desktop.preinst invokes apt-get multiple times, each of those
invocations will use the same progress bar endpoints. This is
unfortunately difficult to fix without some really nasty code in the
preinst. Installing all packages in a single run won't work in this case
either, because discover1 and xresprobe need to be installed before
xserver-xorg's config script is run. So I'm not quite sure what to do
here.

Still, here's what I have so far. Comments?

-- 
Colin Watson                                       [cjwatson@debian.org]
=== modified file 'info/desktop.preinst'
--- info/desktop.preinst	2006-07-26 15:03:42 +0000
+++ info/desktop.preinst	2006-10-06 08:15:28 +0000
@@ -5,9 +5,11 @@
 # autoconfiguration work better. Need to install them before X is
 # preconfigured. Note that they are also listed as part of the task, which
 # will take care of their removal when the task is removed.
-for pkg in discover1 xresprobe; do
+# We also install the X server early on so that other packages (e.g.
+# usplash) can get at the resolution it configures.
+for pkg in discover1 xresprobe xserver-xorg; do
 	if apt-cache show "$pkg" >/dev/null 2>&1 &&
-	   ! dpkg --get-selections | grep "$pkg" | grep -q install; then
-		apt-get -q -y -f install $pkg >/dev/null || true
+	   ! dpkg --get-selections | egrep "^$pkg[[:space:]]+install"; then
+		apt-get "$@" -q -y -f install $pkg >/dev/null || true
 	fi
 done

=== modified file 'tasksel.pl'
--- tasksel.pl	2006-07-24 15:21:59 +0000
+++ tasksel.pl	2006-10-06 08:15:28 +0000
@@ -4,6 +4,7 @@
 # Licensed under the GPL, version 2 or higher.
 use Locale::gettext;
 use Getopt::Long;
+use POSIX qw(floor);
 use warnings;
 use strict;
 textdomain('tasksel');
@@ -338,21 +339,63 @@
 	return (grep { $_->{task} eq $name } @_)[0];
 }
 
-sub task_script {
+sub find_task_script {
 	my $task=shift;
 	my $script=shift;
 
 	my $path="$infodir/$task.$script";
 	if (-e $path && -x _) {
-		my $ret=run($path);
-		if ($ret != 0) {
-			warning("$path exited with nonzero code $ret");
-			return 0;
-		}
+		return $path;
+	} else {
+		return undef;
+	}
+}
+
+sub run_task_script {
+	my $path=pop;
+	my @prefix=@_;
+
+	my $ret=run(@prefix, $path);
+	if ($ret != 0) {
+		warning("$path exited with nonzero code $ret");
+		return 0;
 	}
 	return 1;
 }
 
+sub debconf_apt_command {
+	my $dap=shift;
+	my $from=shift;
+	my $to=shift;
+	my $options=shift;
+
+	my @cmd=($dap, '--from', $from, '--to', $to);
+	push @cmd, @$options if defined $options;
+	push @cmd, '--';
+	return @cmd;
+}
+
+# Works out the region of a progress bar needed for a given invocation of
+# debconf-apt-progress, and returns the appropriate debconf-apt-progress
+# command.
+sub task_region {
+	my $dap=shift;
+	my $start=shift;
+	my $end=shift;
+	my $options=shift;
+	my $cur_region=shift;
+	my $step_region=shift;
+	my $num_regions=shift;
+
+	if (defined $start and defined $end and $num_regions) {
+		my $from=floor($start + ($end - $start) * $cur_region / $num_regions + 0.5);
+		my $to=floor($start + ($end - $start) * ($cur_region + $step_region) / $num_regions + 0.5);
+		return debconf_apt_command($dap, $from, $to, $options);
+	} else {
+		return;
+	}
+}
+
 sub usage {
 	print STDERR gettext(q{Usage:
 tasksel install <task>
@@ -372,6 +415,7 @@
 	Getopt::Long::Configure ("bundling");
 	if (! GetOptions(\%ret, "test|t", "new-install", "list-tasks",
 		   "task-packages=s@", "task-desc=s",
+		   "debconf-apt-from=i", "debconf-apt-to=i",
 		   "debconf-apt-progress=s")) {
 		usage();
 		exit(1);
@@ -550,6 +594,9 @@
 		}
 	}
 	
+	my $dap;
+	my ($dap_from, $dap_to);
+	my @dap_options;
 	my @aptitude;
 	if ($manual_selection) {
 		# Manaul selection and task installs, as best
@@ -558,27 +605,63 @@
 		@aptitude="aptitude";
 	}
 	elsif (-x "/usr/bin/debconf-apt-progress") {
-		@aptitude="debconf-apt-progress";
-		push @aptitude, split(' ', $options{'debconf-apt-progress'})
+		$dap="debconf-apt-progress";
+		if (exists $options{'debconf-apt-from'} and
+		    exists $options{'debconf-apt-to'}) {
+			$dap_from=$options{'debconf-apt-from'};
+			$dap_to=$options{'debconf-apt-to'};
+		}
+		push @dap_options, split(' ', $options{'debconf-apt-progress'})
 			if exists $options{'debconf-apt-progress'};
-		push @aptitude, qw{-- aptitude -q};
+		@aptitude=qw{aptitude -q};
 	}
 	else {
 		@aptitude="aptitude";
 	}
 
+	# We arbitrarily allocate 5% of the progress bar to task scripts,
+	# divided equally among all the scripts. (If you change this, the
+	# new percentage should divide equally into 100.)
+	my $num_regions=0;
+	foreach my $task (@tasks_remove) {
+		if (defined find_task_script($task->{task}, "prerm")) {
+			++$num_regions;
+		}
+		if (defined find_task_script($task->{task}, "postrm")) {
+			++$num_regions;
+		}
+	}
+	foreach my $task (@tasks_install) {
+		if (defined find_task_script($task->{task}, "preinst")) {
+			++$num_regions;
+		}
+		if (defined find_task_script($task->{task}, "postinst")) {
+			++$num_regions;
+		}
+	}
+	$num_regions*=20;
+	my $cur_region=0;
+
 	# Task removal..
 	if (@tasks_remove) {
 		my @packages_remove=map { task_packages($_, 0) } @tasks_remove;
 		foreach my $task (@tasks_remove) {
-			task_script($task->{task}, "prerm");
+			my $path=find_task_script($task->{task}, "prerm");
+			if (defined $path) {
+				my @prefix=task_region($dap, $dap_from, $dap_to, \@dap_options, $cur_region++, 1, $num_regions);
+				run_task_script(@prefix, $path);
+			}
 		}
 		my $ret=run(@aptitude, "-y", "remove", @packages_remove);
 		if ($ret != 0) {
 			error gettext("aptitude failed")." ($ret)";
 		}
 		foreach my $task (@tasks_remove) {
-			task_script($task->{task}, "postrm");
+			my $path=find_task_script($task->{task}, "postrm");
+			if (defined $path) {
+				my @prefix=task_region($dap, $dap_from, $dap_to, \@dap_options, $cur_region++, 1, $num_regions);
+				run_task_script(@prefix, $path);
+			}
 		}
 	}
 	
@@ -586,7 +669,11 @@
 	if (@tasks_install || $manual_selection) {
 		my @packages_install=map {task_packages($_, 1) } @tasks_install;
 		foreach my $task (@tasks_install) {
-			task_script($task->{task}, "preinst");
+			my $path=find_task_script($task->{task}, "preinst");
+			if (defined $path) {
+				my @prefix=task_region($dap, $dap_from, $dap_to, \@dap_options, $cur_region++, 1, $num_regions);
+				run_task_script(@prefix, $path);
+			}
 		}
 		# If the user selected no other tasks and manual package
 		# selection, run aptitude w/o the --visual-preview parameter.
@@ -600,15 +687,32 @@
 			if ($manual_selection) {
 				unshift @packages_install, "--visual-preview";
 			}
-			my $ret=run(@aptitude, "--without-recommends",
-			                       "-y", "install",
-					       @packages_install);
+			my @prefix;
+			if (defined $dap_from and defined $dap_to) {
+				if ($num_regions) {
+					# The main aptitude run takes up the
+					# remaining 95% of the progress bar.
+					@prefix=task_region($dap, $dap_from, $dap_to, \@dap_options, $cur_region, $num_regions * 0.95, $num_regions);
+					$cur_region=floor($cur_region + $num_regions * 0.95 + 0.5);
+				} else {
+					@prefix=debconf_apt_command($dap, $dap_from, $dap_to, \@dap_options);
+				}
+			} elsif (defined $dap) {
+				@prefix=($dap, @dap_options, '--');
+			}
+			my $ret=run(@prefix, @aptitude, "--without-recommends",
+			                                "-y", "install",
+			                                @packages_install);
 			if ($ret != 0) {
 				error gettext("aptitude failed")." ($ret)";
 			}
 		}
 		foreach my $task (@tasks_install) {
-			task_script($task->{task}, "postinst");
+			my $path=find_task_script($task->{task}, "postinst");
+			if (defined $path) {
+				my @prefix=task_region($dap, $dap_from, $dap_to, \@dap_options, $cur_region++, 1, $num_regions);
+				run_task_script(@prefix, $path);
+			}
 		}
 	}
 }

=== modified file 'tasksel.pod'
--- tasksel.pod	2005-12-04 21:45:25 +0000
+++ tasksel.pod	2006-10-06 08:19:49 +0000
@@ -42,10 +42,20 @@
 
 outputs the extended description of the given task
 
+=item B<--debconf-apt-from> waypoint
+
+Start the debconf-apt-progress bar here.
+
+=item B<--debconf-apt-to> waypoint
+
+End the debconf-apt-progress bar here.
+
 =item B<--debconf-apt-progress> options
 
 Pass the specified options to the debconf-apt-progress command that tasksel
-runs.
+runs. These will be appended to any B<--from> and B<--to> options
+constructed by tasksel itself based on B<--debconf-apt-from> and
+B<--debconf-apt-to> options.
 
 =back
 


Reply to: