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

Re: fixing the base-installer progress bars



> Famous last words. I think I'm about 50% of the way there currently.

.. 100%.

Although I have not done anything about kernel-installer yet.
Does anyone know of a good reason for kernel-installer to remain a
separate udeb and menu item? I would like to merge it into the
base-installer postinst unless there is a good reason to leave things as
they are.

-- 
see shy jo
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/Makefile /home/joey/src/debian-installer/tools/base-installer.progress-bar/Makefile
--- base-installer-0.044/Makefile	2003-07-18 11:50:44.000000000 -0400
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/Makefile	2003-12-31 21:27:09.000000000 -0500
@@ -8,7 +8,7 @@
 $(BIN): run-debootstrap.c
 	$(CC) $(CFLAGS) -o $@ $^ -ldebconfclient -ldebian-installer
 
-small: CLAGS:=-Os $(CFLAGS)
+small: CFLAGS:=-Os $(CFLAGS)
 small: clean $(BIN)
 	strip --remove-section=.comment --remove-section=.note $(BIN)
 	ls -l $(BIN)
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/README /home/joey/src/debian-installer/tools/base-installer.progress-bar/README
--- base-installer-0.044/README	2002-04-27 15:16:08.000000000 -0400
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/README	2003-12-31 21:42:50.000000000 -0500
@@ -1,9 +1,107 @@
+On progress bars:
 
-The interesting parts are in debian/
+run-debootstrap translates the debootstrap progress stream into debconf
+progress bars. There are essentially two ways the progress information from
+debootstrap can be dislayed. The naive way is to start a new progress bar
+for each new progress id debootstrap dislays; the problem with this
+approach is it gives the user no clue as to the overall progress as they
+just see a bewildering series of progress bars.
 
-Please feel free to change stuff in debian/, but I would prefer to
-receive an email if you change any big things.
+A more sophisticated approach is to use only one logical progress bar, and
+know about various debootstrap waypoints, using each as it appears to
+update the bar to a given point, and using the more detailed progress
+information to keep the bar updating in between. The waypoints we currently
+care about, and where the progress bar will be when they complete are
+listed in waypoints.h.
 
-Happy hacking,
+Say between 30 and 50%, while downloading debs, debootstrap sends us
+progress commands telling us what the current amount done is, and what the
+total amount is. For example:
 
-Tollef Fog Heen, Sat, 27 Apr 2002 21:11:11 +0200
+P: 22422934 31625258 DOWNDEBS
+
+This is 22422934 / 31625258 = 70% of the way done with downloading debs.
+Since the DOWNDEBS waypoint uses 20% of the progress bar, that scales to
+70% of 20, or 14%. So the progress bar should be positioned to 30+14, or
+44%.
+
+Since we want to use the same progress bar for both run-debootstrap and
+for the end of the base-installer postinst, the actual setup of the
+progress bar should happen in base-installer's postinst. Then
+run-debootstrap just updates it to 90%, and finally the end of the
+postinst takes care of updating it to the final waypoint as it does the
+apt-install stuff.
+
+One other thing to look at might be integrating kernel-installer with
+base-installer (why are the udebs separate?), and adding a new "kernel
+install" waypoint, to consolidate yet another progress bar.
+
+
+Appendix A: The debootstrap --debian-installer progress stream.
+
+As far as I know, there is no other documentation of the data deboostrap
+outputs on fd 3 if run with --debian-installer (except its source). 
+But then I have not looked too hard. Here is what I have been able to piece
+together.
+
+The stream consists of a number of lines. Each line is of the form:
+CODE: arg1 [arg2 .. argn]
+
+The codes are:
+
+P (progress)
+arg1 is an integer, the current amount done
+arg2 is the total amount of work that must be done for this progress item
+arg3 is a unique identifier for the progress item. It may be omitted
+sometimes.
+
+P is generally followed by PA and PF to build up a message to dislay. If
+they are left off, an earlier info message can be displayed to indicate what
+is being done.
+
+PA (progress arguments)
+args combine to form the value of one argument
+
+PF (progress format)
+args combine to be a progress format string.
+
+PF (progress format)
+args combine to form a format string. The info arguments are then
+substituted in turn into the %s tokens in the string to form the message to
+display to the user to indicate progress.
+
+I (info)
+arg1 is an identifier for the information message in question
+Generally followed by an IA and IF.
+
+IA (info arguments)
+args combine to form the value of one argument
+
+IF (info format)
+args combine to form a format string. The info arguments are then
+substituted in turn into the %s tokens in the string to form the message to
+display to the user to indicate info.
+
+E (error)
+arg1 is the id of the error
+May be followed by EA, and EF
+
+EA (error arguments)
+args combine to form the value of one argument
+
+EF (error format)
+args combine to form a format string. The info arguments are then
+substituted in turn into the %s tokens in the string to form the error
+message.
+
+W (warning)
+arg1 is the id of the warning
+May be followed by WA, and WF
+
+WA (warning arguments)
+args combine to form the value of one argument
+
+WF (warning format)
+args combine to form a format string. The info arguments are then
+substituted in turn into the %s tokens in the string to form the warning
+message.
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/debian/base-installer.templates /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/base-installer.templates
--- base-installer-0.044/debian/base-installer.templates	2003-12-18 23:11:56.000000000 -0500
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/base-installer.templates	2003-12-31 22:20:16.000000000 -0500
@@ -3,22 +3,21 @@
 _Description: Failed to install the base system
  The base system installation into /target/ failed.
  .
- Please check /target/var/log/debootstrap.log and
- /target/var/log/debootstrap.err.log for the details.
+ Check /var/log/messages or see virtual console 3 for the details.
 
 Template: base-installer/debootstrap/error-exitcode
 Type: error
 _Description: Base system installation error
  The debootstrap program exited with an error (return value ${EXITCODE}).
  .
- You might find more information in /target/var/log/debootstrap.log.
+ Check /var/log/messages or see virtual console 3 for the details.
 
 Template: base-installer/debootstrap/error-abnormal
 Type: error
 _Description: Base system installation error
  The debootstrap program exited abnormally.
  .
- You might find more information in /target/var/log/debootstrap.log.
+ Check /var/log/messages or see virtual console 3 for the details.
 
 Template: base-installer/debootstrap/fallback-error
 Type: error
@@ -27,8 +26,7 @@
  .
  ${ERROR}
  .
- You might find more information in /target/var/log/debootstrap.log
- or /target/var/log/debootstrap.err.log.
+ Check /var/log/messages or see virtual console 3 for the details.
 
 Template: base-installer/debootstrap/error/baddload
 Type: error
@@ -85,37 +83,37 @@
 _Description: Debootstrap Error
  Interrupt caught ... exiting.
 
-Template: base-installer/debootstrap/fallback-progress
+Template: base-installer/progress/installing-debian
 Type: text
-Description: ${PROGRESS}
+Description: Installing Debian base system
 
-Template: base-installer/debootstrap/progress/downrel
+Template: base-installer/debootstrap/section/downrel
 Type: text
-_Description: Retrieving Release file...
+_Description: Retrieving Release file
 
-Template: base-installer/debootstrap/progress/downpkgs
+Template: base-installer/debootstrap/section/sizedebs
 Type: text
-_Description: Retrieving Packages files...
+_Description: Finding package sizes
 
-Template: base-installer/debootstrap/progress/downdebs
+Template: base-installer/debootstrap/section/downpkgs
 Type: text
-_Description: Retrieving packages...
+_Description: Retrieving Packages files
 
-Template: base-installer/debootstrap/progress/downmainpkgs
+Template: base-installer/debootstrap/section/downdebs
 Type: text
-_Description: Retrieving Packages file...
+_Description: Retrieving packages
 
-Template: base-installer/debootstrap/progress/extractpkgs
+Template: base-installer/debootstrap/section/extractpkgs
 Type: text
-_Description: Extracting packages...
+_Description: Extracting packages
 
-Template: base-installer/debootstrap/progress/instbase
+Template: base-installer/debootstrap/section/instbase
 Type: text
-_Description: Installing base system...
+_Description: Installing the base system
 
 Template: base-installer/debootstrap/fallback-info
 Type: text
-Description: ${INFO}...
+Description: ${SECTION}: ${INFO}...
 
 Template: base-installer/debootstrap/info/validating
 Type: text
@@ -129,6 +127,10 @@
 Type: text
 _Description: Extracting ${SUBST0}...
 
+Template: base-installer/debootstrap/info/checkingsize
+Type: text
+Description: Checking package size: ${SUBST0}...
+
 Template: base-installer/debootstrap/info/instcore
 Type: text
 _Description: Installing core packages...
@@ -145,14 +147,6 @@
 Type: text
 _Description: Installing base packages...
 
-Template: base-installer/debootstrap/info/basesuccess
-Type: note
-_Description: Base system successfully installed
-
-Template: base-installer/apt/progress/title
-Type: text
-_Description: Installing extra packages
-
 Template: base-installer/apt/progress/step_config
 Type: text
 _Description: Configuring APT sources...
@@ -163,17 +157,9 @@
 
 Template: base-installer/apt/progress/step_install
 Type: text
-_Description: Installing the extra packages...
+_Description: Installing extra packages...
 
 Template: debian-installer/base-installer/title
 Type: text
 #  Item in the main menu to select this package
 _Description: Install the base system
-
-Template: base-installer/debootstrap/progress/sizedebs
-Type: text
-_Description: Finding package sizes
-
-Template: base-installer/debootstrap/info/checkingsize
-Type: text
-_Description: Checking ${SUBST0}...
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/debian/changelog /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/changelog
--- base-installer-0.044/debian/changelog	2003-12-26 12:32:27.000000000 -0500
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/changelog	2003-12-31 22:21:33.000000000 -0500
@@ -1,3 +1,31 @@
+base-installer (0.046) UNRELEASED; urgency=low
+
+  * Joey Hess
+    - Have debootstrap log to the d-i messages file.
+    - Change error templates to match.
+    - Remove downmainpkgs stuff; only used for slink deboostrapping.
+    - Switch over to a single, milti-stage progress bar for the entire
+      debootstrap run.
+    - Fixed typo that was preventing build with -Os.
+    - Remove the basesuccess note.
+    - Incorporated the apt setup and install steps into the same single
+      progress bar.
+
+ -- Joey Hess <joeyh@debian.org>  Wed, 31 Dec 2003 10:01:38 -0500
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/debian/postinst /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/postinst
--- base-installer-0.044/debian/postinst	2003-12-18 14:19:41.000000000 -0500
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/debian/postinst	2003-12-31 22:19:41.000000000 -0500
@@ -18,6 +18,11 @@
 COMPONENTS=
 DISTRIBUTION=testing
 
+# This single progress bar is used for the entire base-installer run.
+# Steps 0-100 are used by run-debootstrap; the extra steps run after.
+NUM_STEPS=110
+db_progress START 0 $NUM_STEPS base-installer/progress/installing-debian
+
 if [ -f /cdrom/.disk/base_installable ]; then
     PROTOCOL=file
     MIRROR=""
@@ -106,57 +111,50 @@
         ${INCLUDE} ${EXCLUDE} \
         ${DISTRIBUTION} /target \
         "$PROTOCOL://$MIRROR$DIRECTORY" \
-        2> $LOGDIR/debootstrap.err.log \
+        2>> $logfile \
  || debootstrap_failed=$?
-#        3>&1 > $LOGDIR/debootstrap.log \
 
 if [ true = "$COPIED_FSTAB" ] ; then
     mv /target/etc/fstab.orig /target/etc/fstab
 fi
 
 if [ "$debootstrap_failed" ] ; then
+    db_progress stop
     db_input critical base-installer/debootstrap-failed || [ $? -eq 30 ]
     db_go
     exit $debootstrap_failed
 fi
 
-db_progress START 0 3 base-installer/apt/progress/title
-
 db_progress INFO base-installer/apt/progress/step_config
 
-# Why is this here?  Why are the files moved into /target/? [pere 2003-02-07]
+# Move files to where apt expects them to avoid another download or copy.
 APTLISTDIR=/target/var/lib/apt/lists
-if [ -f /cdrom/.disk/base_installable ]; then
-    DEBOOTSTRAPLIST="$APTLISTDIR/debootstrap.invalid_dists_${DISTRIBUTION}"
-else
-    APTDIR=`echo $DIRECTORY | tr "/" "_"`
-    DEBOOTSTRAPLIST="$APTLISTDIR/debootstrap.invalid_dists_${DISTRIBUTION}"
-    APTLIST="$APTLISTDIR/${MIRROR}${APTDIR}_dists_${DISTRIBUTION}"
-
-    mv ${DEBOOTSTRAPLIST}_Release ${APTLIST}_Release
-    mv ${DEBOOTSTRAPLIST}_main_binary-${ARCH}_Packages ${APTLIST}_main_binary-${ARCH}_Packages
-fi
+APTDIR=`echo $DIRECTORY | tr "/" "_"`
+DEBOOTSTRAPLIST="$APTLISTDIR/debootstrap.invalid_dists_${DISTRIBUTION}"
+APTLIST="$APTLISTDIR/${MIRROR}${APTDIR}_dists_${DISTRIBUTION}"
+mv ${DEBOOTSTRAPLIST}_Release ${APTLIST}_Release
+mv ${DEBOOTSTRAPLIST}_main_binary-${ARCH}_Packages ${APTLIST}_main_binary-${ARCH}_Packages
 
 # Copy the files from the CD, to avoid access problems in chroot
-# environment.  This has to happen here, as debootstrap do not
-# understand copy:, while apt-get do.
+# environment. This has to happen here, as debootstrap does not
+# understand copy:, while apt-get does
 if [ file = "$PROTOCOL" ] ; then
     PROTOCOL=copy
 fi
 
-# sources.list use space to separate the components, not comma
+# sources.list uses space to separate the components, not comma
 COMPONENTS=`echo $COMPONENTS | tr , " "`
 APTSOURCE="$PROTOCOL://$MIRROR$DIRECTORY"
 
 [ ! -d /target/etc/apt ] && mkdir -p /target/etc/apt
 echo "deb $APTSOURCE $DISTRIBUTION $COMPONENTS" > /target/etc/apt/sources.list
 
-db_progress STEP 1
+db_progress STEP 1 # the above runs quickly
 db_progress INFO base-installer/apt/progress/step_update
 
 apt-update
 
-db_progress STEP 1
+db_progress STEP 3 # may take a while
 db_progress INFO base-installer/apt/progress/step_install
 
 if [ -f /var/lib/apt-install/queue ] ; then
@@ -170,7 +168,7 @@
     done
 fi
 
-db_progress STEP 1
+db_progress SET $NUM_STEPS
 db_progress STOP
 
 exit 0
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/run-debootstrap.c /home/joey/src/debian-installer/tools/base-installer.progress-bar/run-debootstrap.c
--- base-installer-0.044/run-debootstrap.c	2003-12-08 16:16:09.000000000 -0500
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/run-debootstrap.c	2003-12-31 22:11:44.000000000 -0500
@@ -10,10 +10,13 @@
 #include <cdebconf/debconfclient.h>
 #include <debian-installer.h>
 
+#include "waypoints.h"
+
 #define DEBCONF_BASE          "base-installer/debootstrap/"
 
 volatile int child_exit = 0;
 struct debconfclient *debconf = NULL;
+int current_section = 0;
 
 static void
 sig_child(int sig)
@@ -107,8 +110,6 @@
 /*
  * Copied from boot-floppies/utilities/dbootstrap/extract_base.c
  * and modified to use cdebconf progress bars
- *
- * 
  */
 static int
 exec_debootstrap(char **argv){
@@ -119,8 +120,8 @@
     FILE *ifp;
     pid_t pid;
     int status, rv;
-    char *ptr, *line, *template;
-    int llen, oldphigh = -1;
+    char *ptr, *line, *template, *section_text = NULL;
+    int llen;
     size_t dummy = 0;
 
     pipe(from_db);
@@ -136,7 +137,7 @@
         if (freopen("/dev/null", "r", stdin) == NULL)
             perror("freopen");
 
-        if (freopen("/target/var/log/debootstrap.log", "w", stderr) == NULL)
+        if (freopen("/var/log/messages", "a", stderr) == NULL)
             perror("freopen");
 
         dup2(2, 1);
@@ -239,63 +240,93 @@
                 {
                     int plow = 0, phigh = 0;
                     char what[1024] = "";
+		    char *section_template;
 
                     sscanf(line+3, "%d %d %s", &plow, &phigh, what);
                     if (what[0])
-                        template = find_template("progress", what);
-                    else
-                        template = NULL;
+		    {
+			int i;
+			for (i = 0; waypoints[i].progress_id != NULL; i++)
+			{
+			    if (strcmp(waypoints[i].progress_id, what) == 0)
+			        {
+				    if (current_section == i)
+					    break; /* optimisation */
+			            current_section = i;
+				    /* Get the description of the section
+				     * template for this waypoint. */
+			            section_template = find_template("section", what);
+			            if (section_template)
+				    {
+                                            if (! debconf_metaget(debconf, section_template, "description"))
+					    {
+                                                free(section_text);
+                                                section_text = strdup(debconf->value);
+				            }
+                                            free(section_template);
+			            }
+			            break;
+				}
+			}
+		    }
+		    
                     args = read_arg_lines("PA: ", ifp, &arg_count, &line);
                     if (args == NULL)
                     {
                         child_exit = 1;
                         break;
                     }
-                    if (phigh != oldphigh)
-                    {
-                        oldphigh = phigh;
-                        if (template != NULL)
-                        {
-                            n_subst(template, arg_count, args);
-                            debconf_progress_start(debconf, plow, phigh,
-						   template);
-                        }
-                        else if (strstr(line, "PF:") == line)
-                        {
-                            ptr = n_sprintf(line+4, arg_count, args);
-                            if (ptr == NULL)
-                                return -1;
-                            debconf_subst(debconf, DEBCONF_BASE "fallback-progress",
-					  "PROGRESS", ptr);
-                            debconf_progress_start(debconf, plow, phigh,
-						   DEBCONF_BASE "fallback-progress");
-                            free(ptr);
-                        }
-                        else
-                        {
-                            // err, don't really know what to do here... there
-                            // should always be a fallback...
-                        }
+                    if (strstr(line, "PF:") == line)
+		    {
+                        /* Does not currently need to do anything;
+			 * the implementation of debootstrap could change
+			 * though.. */
                     }
-                    else
-                    {
-                        debconf_progress_set(debconf, plow);
-                        if (plow == phigh)
-                            debconf_progress_stop(debconf);
+		    
+		    if (what[0])
+		    {
+		        /* Calculate progress bar location, starting at
+                         * previous waypoint, and advancing the percent of
+			 * the current section that corresponds to the percent
+			 * of the debootstrap progress indicator. */
+			float section_fraction;
+			int section_span, prev_waypoint, percent;
+			
+			if (current_section > 0)
+			{
+			    prev_waypoint = waypoints[current_section - 1].endpercent;
+			    section_span = waypoints[current_section].endpercent - prev_waypoint;
+			}
+			else {
+			    prev_waypoint = 0;
+			    section_span = 0;
+			}
+			
+			if (phigh > 0)
+				section_fraction = (float) plow / (float) phigh;
+			else
+				section_fraction = 0;
+			
+			percent = prev_waypoint + (section_span * section_fraction);
+			
+			//fprintf(stderr, "waypoint: %s (%i); prev endpercent %i; span: %i; fraction: %.9f (%i / %i); percent: %i\n",
+			//        waypoints[current_section].progress_id,
+			//        current_section, prev_waypoint, section_span, 
+			//        section_fraction, plow, phigh, percent);
+
+			debconf_progress_set(debconf, percent);
                     }
-                    free(template);
+		    
                     break;
                 }
             case 'I':
                 {
                     ptr += 3;
-                    // ptr now contains the identifier of the error.
+                    // ptr now contains the identifier of the info
                     template = find_template("info", ptr);
                     if (strcmp(ptr, "basesuccess") == 0 && template != NULL)
                     {
-                        debconf_fset(debconf, template, "seen", "false");
-                        debconf_input(debconf, "low", template);
-                        debconf_go(debconf);
+                        /* all done */
                         child_exit = 1;
                         break;
                     }
@@ -308,6 +339,8 @@
                     if (template != NULL)
                     {
                         n_subst(template, arg_count, args);
+			debconf_subst(debconf, template,
+			              "SECTION", section_text);
                         debconf_progress_info(debconf, template);
                     }
                     else if (strstr(line, "IF:") == line)
@@ -315,9 +348,11 @@
                         ptr = n_sprintf(line+4, arg_count, args);
                         if (ptr == NULL)
                             return -1;
-                        // fallback error message
+                        // fallback info message
                         debconf_subst(debconf, DEBCONF_BASE "fallback-info",
 				      "INFO", ptr);
+			debconf_subst(debconf, DEBCONF_BASE "fallback-info",
+			              "SECTION", section_text);
                         debconf_progress_info(debconf,
 					      DEBCONF_BASE "fallback-info");
                         free(ptr);
@@ -329,11 +364,10 @@
                     }
                 }
         }
+	
         line = NULL;
     }
 
-    debconf_progress_stop(debconf);
-
     if (waitpid(pid, &status, 0) != -1 && (WIFEXITED(status) != 0))
     {
         rv = WEXITSTATUS(status);
diff --exclude=po --exclude=CVS --new-file -ur base-installer-0.044/waypoints.h /home/joey/src/debian-installer/tools/base-installer.progress-bar/waypoints.h
--- base-installer-0.044/waypoints.h	1969-12-31 19:00:00.000000000 -0500
+++ /home/joey/src/debian-installer/tools/base-installer.progress-bar/waypoints.h	2003-12-31 21:50:04.000000000 -0500
@@ -0,0 +1,20 @@
+/*
+ * debootstrap waypoints for run-debootstrap. See the README for docs.
+ */
+struct waypoint {
+	int endpercent;
+	char *progress_id;
+};
+static struct waypoint waypoints[] = {
+	{ 0,    "START" },	/* dummy entry, required */
+	{ 1,	"DOWNREL" },	/* downloading release files; very quick */
+	{ 10,	"DOWNPKGS" },	/* downloading packages files; time varies
+	                           by bandwidth; low granularity */
+	{ 15,	"SIZEDEBS" },   /* getting packages sizes; high granularity */
+	{ 30,	"DOWNDEBS" },   /* downloading packages; run time varies by
+				   bandwidth; high granularity */
+	{ 50,	"EXTRACTPKGS" },/* extracting the core packages */
+	{ 100,	"INSTBASE" },	/* installing the base system
+				   (currently has very bad granularity) */
+	{ 0,	NULL },		/* last entry, required */
+};

Attachment: signature.asc
Description: Digital signature


Reply to: