Re: install/38571: sysinst runs "postinstall fix"

[Julian Djamil Fagir <gnrp%komkon2.de@localhost>]

The following reply was made to PR install/38571; it has been noted by GNATS.

From: Julian Djamil Fagir <gnrp%komkon2.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 14:05:59 +0100

 --MP_/5XsSkzMyzyEn1mIEIo_l7K4
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline
 
 Hi,
 
 >  >  On the one hand, it introduces a
 >  >  new function that exits curses, executes a command and afterwards
 >  > returns. This function is used to execute etcupdate(8) in case the user
 >  > wants it.
 >  
 >  That's fine in principle.  I haven't checked the implementation.
 the implementation is what I'm most concerned about. ;-)
 
 >  >  +                runpostinstall = run_program(RUN_DISPLAY | RUN_CHROOT
 >  > | RUN_ERROR_OK
 >  >  +                                | RUN_PROGRESS,
 >  >  +                                "/usr/sbin/postinstall -s /.sysinst
 >  > -d / check");
 >  >  +                if (runpostinstall == 1) {
 >  
 >  If run_program returns the program's exit status, then any non-zero
 >  value means that something needs to be fixed.
 Fixed.
 
 >  Will the output from postinstall still be visible when the user 
 >  has to choose a menu option?
 No, it's not. It's difficult to do that, anyway, as it isn't as ordered as it
 should be for piping it somewhere.
 menuc doesn't really have methods to display the output of a program and a
 window afterwards.
 
 >  Also, I think we could have "re-run postinstall check" and "run 
 >  a shell to fix problems manually" as additional options in thie 
 >  menu, and loop here until the user is happy.  (Maybe they want to 
 >  run postinstall check, fix something in a shell, postinstall check 
 >  again, etcupdate, postinstall check again, fix something else in a 
 >  shell, ...)
 What about the attached patch?
 It loops though the process, until `postinstall check` is ok or the user
 explicitly stops.
 
 >  I usually want to run "postinstall fix SOME_KEYWORDS" and not
 >  "postinstall fix OTHER_KEYWORDS".  With enough work, we could
 >  dynamically build a menu using a list of keywords parsed from the
 >  output of "postinstall check", and descriptions parsed from the
 >  output of "postinstall list".
 This is difficult and a heck of work. As sysinst should be replaced anyway, I
 wonder if it's really worth the time implementing this.
 
 
 Regards, Julian
 --MP_/5XsSkzMyzyEn1mIEIo_l7K4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=util.c.diff
 
 --- util.c
 +++ util.c
 @@ -571,11 +571,11 @@
        if (silent)
                msg_display("");
        else {
                umount_mnt2();
                msg_display(MSG_cd_path_not_found);
 -              msg_display_add("\r\n\r\n");
 +              process_menu(MENU_ok, NULL);
        }
  
        /* ask for paths on the CD */
        process_menu(MENU_cdromsource, NULL);
  
 @@ -1061,10 +1061,12 @@
         * otherwise /var/db/obsolete will only have current information
         * from the base, comp, and etc sets.
         */
        if (update && (set_status[SET_ETC] & SET_INSTALLED)) {
                int oldsendmail;
 +              int runpostinstall;
 +
                oldsendmail = run_program(RUN_DISPLAY | RUN_CHROOT |
                                          RUN_ERROR_OK | RUN_PROGRESS,
                                          "/usr/sbin/postinstall -s /.sysinst 
-d / check mailerconf");
                if (oldsendmail == 1) {
                        msg_display(MSG_oldsendmail);
 @@ -1072,12 +1074,49 @@
                        if (yesno) {
                                run_program(RUN_DISPLAY | RUN_CHROOT,
                                            "/usr/sbin/postinstall -s /.sysinst 
-d / fix mailerconf");
                        }
                }
 -              run_program(RUN_DISPLAY | RUN_CHROOT,
 -                      "/usr/sbin/postinstall -s /.sysinst -d / fix");
 +
 +              runpostinstall = run_program(RUN_DISPLAY | RUN_CHROOT | 
RUN_ERROR_OK
 +                              | RUN_PROGRESS,
 +                              "/usr/sbin/postinstall -s /.sysinst -d / 
check");
 +              while (runpostinstall != 0) {
 +                      process_menu(MENU_runpostinst, &runpostinstall);
 +                      /* runpostinstall:
 +                       * 0: postinstall fix
 +                       * 1: etcupdate
 +                       * 2: Run shell
 +                       * 3: continue
 +                       * 4: abandon installation
 +                       */
 +                      switch(runpostinstall) {
 +                              case 0:
 +                                      run_program(RUN_DISPLAY | RUN_CHROOT,
 +                                              "/usr/sbin/postinstall -s 
/.sysinst -d / fix");
 +                                      break;
 +                              case 1:
 +                                      launch_command(RUN_CHROOT,
 +                                              "/usr/sbin/etcupdate -s 
/.sysinst");
 +                                      break;
 +                              case 2:
 +                                      launch_command(RUN_CHROOT,
 +                                              "/bin/sh");
 +                                      break;
 +                              case 3:
 +                                      runpostinstall = 0;
 +                                      break;
 +                              case 4:
 +                                      return -1;
 +                                      break;
 +                      }
 +
 +                      if (runpostinstall != 0)
 +                              run_program(RUN_DISPLAY | RUN_CHROOT | 
RUN_ERROR_OK
 +                                              | RUN_PROGRESS,
 +                                              "/usr/sbin/postinstall -s 
/.sysinst -d / check");
 +              }
        }
  
        /* Configure the system */
        if (set_status[SET_BASE] & SET_INSTALLED)
                run_makedev();
 
 
 --MP_/5XsSkzMyzyEn1mIEIo_l7K4--