On Tuesday 05 August 2008, Jérémy Bobbio wrote: > Actually, it won't as the test is run inside an "if" expression. > "/usr/bin/[" will simply return false and go on. An unpretty error > message [1] will be displayed but that could be mitigated by > redirecting stderr to /dev/null. > > [1] sh: vendor_id : GenuineIntel: bad number OK, so it won't blow up the whole script. It is still very fragile IMO and papering over something like that by redirecting to /dev/null is also not a sign of good coding. I still don't agree that moving more "main" functionality from the postinst into hook scripts is the correct way to go anyway. Hook scripts should in general be small, short-running scripts. Their primary use is to offer flexibility to _add_ minor functionality, not to execute main functionality. > Anyway, I am open to any solutions that would actually fix #492861. > The one discussed here could be one of them. But I don't see it as > anymore fragile than the current code. Attached (tested) patch is what I had in mind (based on similar code in base-installer). It assigns 1% of the progress bar to each hook script with a maximum of 10% for all hook scripts. For popcon I've just dropped the advancement of the progress bar by d-a-p as it's not really worth it anyway. The patch, even though fairly big, is IMO the least invasive way to fix the issue while preserving relevant existing functionality. It would probably be a good idea if someone would rewrite pkgsel a bit post-lenny, probably more in line with base-installer (although that could use a cleanup too for some parts). On Tuesday 05 August 2008, Jérémy Bobbio wrote: > On Tue, Aug 05, 2008 at 06:45:19PM +0200, Frans Pop wrote: > > In all other cases where we use hook scripts we advance the progress > > bar by some fixed amount per hook script. The only additional feature > > here is that some hook scripts can usefully subdivide their allotted > > bit. > > I have only took a quick look, but are you aware of any other hooks > that allow the use of debconf-apt-progress? The apt-setup generators do something similar (especially 50mirror). > debconf-apt-progress interface really is what make the pre-pkgsel.d > hook special here, as far as I have been able to dug up. I'm not really sure that using d-a-p is really needed at all here.
Index: debian/postinst
===================================================================
--- debian/postinst (revision 55113)
+++ debian/postinst (working copy)
@@ -15,7 +15,7 @@
log "warning: $@"
}
-cleanup () {
+cleanup() {
for divert in $DIVERTS; do
rm -f "/target$divert"
log-output -t pkgsel chroot /target dpkg-divert \
@@ -23,7 +23,7 @@
done
}
-aptfailed () {
+aptfailed() {
ret=$?
if [ "$ret" != 0 ]; then
# In case packages failed to install, try to clean up.
@@ -46,7 +46,7 @@
fi
}
-db_progress START 0 100 debian-installer/pkgsel/title
+db_progress START 0 1000 debian-installer/pkgsel/title
db_progress INFO pkgsel/progress/init
# d-i debconf variables used by packages
@@ -61,7 +61,7 @@
ln -sf /bin/true "/target$divert"
done
-db_progress STEP 1
+db_progress STEP 10
# Unmount the installation CD to allow CD changing; as it may be needed
# again after pkgsel, make sure it is reloaded before exiting
@@ -80,25 +80,35 @@
db_get pkgsel/upgrade
if [ "$RET" = none ] || [ "$suite" = etch ]; then
- tasksel_start=5
+ tasksel_start=50
else
upgrade_type="$RET"
db_progress INFO pkgsel/progress/upgrade
sleep 2 # allow the message to be seen
- in-target sh -c "debconf-apt-progress --from 5 --to 10 --logstderr -- aptitude -q --without-recommends -y -o DPkg::options=--force-confnew '$upgrade_type'" || aptfailed
- tasksel_start=10
+ in-target sh -c "debconf-apt-progress --from 50 --to 100 --logstderr -- aptitude -q --without-recommends -y -o DPkg::options=--force-confnew '$upgrade_type'" || aptfailed
+ tasksel_start=100
fi
partsdir="/usr/lib/pre-pkgsel.d"
if [ -d "$partsdir" ]; then
- for script in `ls "$partsdir"/* 2>/dev/null`; do
+ scriptcount=$(ls "$partsdir"/* 2>/dev/null | wc -l)
+ if [ $scriptcount -lt 10 ]; then
+ scripts_range=$(($scriptcount * 10))
+ else
+ scripts_range=100
+ fi
+ scripts_start=$tasksel_start
+ tasksel_start=$(($scripts_start + $scripts_range))
+
+ scriptcur=0
+ for script in $(ls "$partsdir"/* 2>/dev/null); do
base=$(basename $script | sed 's/[0-9]*//')
if ! db_progress INFO pkgsel/progress/$base; then
db_subst pkgsel/progress/fallback SCRIPT "$base"
db_progress INFO pkgsel/progress/fallback
fi
- if [ -x "$script" ] ; then
+ if [ -x "$script" ]; then
# be careful to preserve exit code
if log-output -t pkgsel "$script"; then
:
@@ -108,14 +118,18 @@
else
error "Unable to execute $script"
fi
+
+ # Advance progress bar
+ scriptcur=$(($scriptcur + 1))
+ db_progress SET $(($scripts_start + $scripts_range * $scriptcur / $scriptcount))
done
fi
db_get pkgsel/include
if [ "$RET" ]; then
- tasksel_end=90
+ tasksel_end=900
else
- tasksel_end=95
+ tasksel_end=950
fi
db_progress INFO pkgsel/progress/tasksel
@@ -126,7 +140,7 @@
# Allow comma-separation so that this can more easily be preseeded
# at the kernel command line.
RET="$(printf '%s' "$RET" | sed 's/,/ /g')"
- in-target sh -c "debconf-apt-progress --from 90 --to 95 --logstderr -- aptitude -q --without-recommends -y install -- $RET" || aptfailed
+ in-target sh -c "debconf-apt-progress --from 900 --to 950 --logstderr -- aptitude -q --without-recommends -y install -- $RET" || aptfailed
fi
db_progress INFO pkgsel/progress/cleanup
@@ -134,7 +148,7 @@
log "cleanup failed"
fi
-db_progress STEP 2
+db_progress STEP 20
if [ -x /target/usr/bin/scrollkeeper-update ]; then
log-output -t pkgsel chroot /target scrollkeeper-update -q || true
@@ -144,7 +158,7 @@
|| true
fi
-db_progress STEP 3
+db_progress STEP 30
db_progress STOP
load_install_cd
Index: pre-pkgsel.d/10popcon
===================================================================
--- pre-pkgsel.d/10popcon (revision 55113)
+++ pre-pkgsel.d/10popcon (working copy)
@@ -4,7 +4,7 @@
# Install popularity-contest but remove it if the user decides not to
# participate.
-if in-target sh -c "debconf-apt-progress --from 1 --to 5 --logstderr -- apt-get -o APT::Install-Recommends=false -q -y -f install popularity-contest"; then
+if in-target sh -c "debconf-apt-progress --no-progress --logstderr -- apt-get -o APT::Install-Recommends=false -q -y -f install popularity-contest"; then
if ! grep -q '^PARTICIPATE=\"*yes\"*' /target/etc/popularity-contest.conf; then
in-target dpkg --purge popularity-contest
fi
Attachment:
signature.asc
Description: This is a digitally signed message part.