NetBSD Problem Report #38571

From apb@cequrux.com  Sat May  3 21:15:52 2008
Return-Path: <apb@cequrux.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id D319763B8BC
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  3 May 2008 21:15:52 +0000 (UTC)
Message-Id: <20080503211537.379E28A496D@apb-laptoy.apb.alt.za>
Date: Sat,  3 May 2008 23:15:37 +0200 (SAST)
From: apb@cequrux.com
Reply-To: apb@cequrux.com
To: gnats-bugs@gnats.NetBSD.org
Subject: sysinst runs "postinstall fix"
X-Send-Pr-Version: 3.95

>Number:         38571
>Category:       install
>Synopsis:       sysinst runs "postinstall fix"
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    install-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 03 21:20:00 +0000 2008
>Last-Modified:  Sun Sep 16 01:05:02 +0000 2012
>Originator:     Alan Barrett
>Release:        NetBSD 4.99.60
>Organization:
Not much
>Environment:
System: NetBSD 4.99.60 i386
>Description:
If you use sysinst to upgrade to a new vesion of NetBSD, then sysinst
will "/usr/sbin/postinstall -s /.sysinst -d / fix".  This will overwrite
any local changes to files in several parts of /etc, leading to data
loss if there are inadequate backups.

>How-To-Repeat:
Code inspection

>Fix:
Run "postinstall check" instead of "postinstall fix"; display the results,
and obtain permission from the user before running "postinstall fix".
Alternatively, offer to run "etcupdate" instead of "postinstall fix".

>Audit-Trail:
From: Julian Djamil Fagir <gnrp@komkon2.de>
To: gnats-bugs@gnats.netbsd.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 02:30:29 +0100

 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline

 Hi,

 the attached patch fixes that issue for me. 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.

 Please carefully look at that patch, I only have one (virtual) machine to
 test sysinst, and I'm not that sure about the curses part (though it works
 for me).

 Regards, Julian

 PS: In advance: I'm sorry if my mailer breaks encoding again by making it
 quoted-printable.
 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=defs.h.diff

 --- defs.h
 +++ defs.h
 @@ -400,10 +400,11 @@
  void	mnt_net_config(void);

  /* From run.c */
  int	collect(int, char **, const char *, ...) __printflike(3, 4);
  int	run_program(int, const char *, ...) __printflike(2, 3);
 +int	launch_command(int, const char *);
  void	do_logging(void);
  int	do_system(const char *);

  /* from upgrade.c */
  void	do_upgrade(void);


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=menus.mi.diff

 --- menus.mi
 +++ menus.mi
 @@ -374,10 +374,16 @@
  		  yesno = 0;
  #endif
  		}; 
  	option MSG_other, exit, action
  		{ yesno = 0; };
 +
 +menu runpostinst, title MSG_Run_Postinst, y=-5;
 +	option MSG_Postinst,       exit, action { *(int *)arg = 0; };
 +	option MSG_Etcupdate,      exit, action { *(int *)arg = 1; };
 +	option MSG_Skip_postinst,  exit, action { *(int *)arg = 2; };
 +	option MSG_Abandon,        exit, action { *(int *)arg = 3; };

  menu rootsh, title MSG_Root_shell;
  	option "/bin/sh",  exit, action {*(const char **)arg = "/bin/sh";}; 
  	option "/bin/ksh", exit, action {*(const char **)arg = "/bin/ksh";};
  	option "/bin/csh", exit, action {*(const char **)arg = "/bin/csh";};


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename=msg.mi.de.diff

 --- msg.mi.de
 +++ msg.mi.de
 @@ -106,10 +106,22 @@
  Zu diesem Zeitpunkt sollten Sie bereits eine vollst=E4ndige
  Datensicherung durchgef=FChrt haben! Wollen Sie NetBSD wirklich aktualisie=
 ren?
  (Dies ist die letzte Warnung, bevor Ihre Festplatte(n) modifiziert werden.)
  }
 =20
 +message Run_Postinst
 +{Dateien in /etc updaten}
 +
 +message Postinst
 +{postinst ausf=FChren (existierende Konfiguration =FCberschreiben)}
 +
 +message Etcupdate
 +{etcupdate ausf=FChren (zu jeder ge=E4nderten Datei nachfragen)}
 +
 +message Skip_postinst
 +{Keine Dateien in /etc updaten}
 +
  message reinstallusure
  {Im folgenden werden die NetBSD Distributionssets (Kernel + Basissystem)
  auf eine vorbereitete Festplatte ausgepackt. Diese Prozedur l=E4dt und ent=
 packt
  die Sets auf eine im Vorfeld partitionierte und bootf=E4hige Festplatte au=
 s.
  Es werden weder Festplatten gelabelt, Bootbl=F6cke aktualisiert noch beste=
 hende
 @@ -122,10 +134,13 @@
  Wollen Sie die NetBSD Distributionssets wirklich erneut installieren?
  (Dies ist die letzte Warnung, bevor die Inhalte Ihres Dateisystems
  =FCberschrieben werden!)
  }
 =20
 +message mount_failed
 +{Versuch, %s zu mounten ist fehlgeschlagen. Fortfahren?
 +}
 =20
  message nodisk
  {Ich kann keine f=FCr NetBSD nutzbaren Festplatten finden.
  Zur=FCck zum Hauptmen=FC...
  }
 @@ -709,13 +724,13 @@
  {Erstelle Ger=E4tedateien in /dev ...
  }
 =20
  message badfs
  {Das Dateisystem auf /dev/%s%c scheint kein BSD-Dateisystem zu sein,
 -die Pr=FCfung des Dateisystems (fsck) ist fehlgeschlagen.
 +die Pr=FCfung des Dateisystems (fsck) ist fehlgeschlagen (Fehler %d).
 =20
 -Die Aktualisierung wird abgebrochen. (Fehlernummer %d.)
 +Die Aktualisierung trotzdem fortsetzen?
  }
 =20
  message rootmissing
  {Das Zielverzeichnis %s existiert nicht.
  }


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=msg.mi.en.diff

 --- msg.mi.en
 +++ msg.mi.en
 @@ -102,10 +102,22 @@
  before this procedure!  Do you really want to upgrade NetBSD?
  (This is your last warning before this procedure starts modifying your
  disks.)
  }

 +message Run_Postinst
 +{Update files in /etc}
 +
 +message Postinst
 +{Execute postinst (overwrite existing configuration)}
 +
 +message Etcupdate
 +{Execute etcupdate (ask for every overwrite)}
 +
 +message Skip_postinst
 +{Don't update any file on /etc}
 +
  message reinstallusure
  {Ok, let's unpack the NetBSD distribution sets to a bootable hard disk.
  This procedure just fetches and unpacks sets onto an pre-partitioned
  bootable disk. It does not label disks, upgrade bootblocks, or save
  any existing configuration info.  (Quit and choose `install' or
 @@ -115,10 +127,13 @@
  Do you really want to reinstall NetBSD distribution sets?
  (This is your last warning before this procedure starts modifying your
  disks.)
  }

 +message mount_failed
 +{Mounting %s failed. Continue?
 +}

  message nodisk
  {I can not find any hard disks for use by NetBSD.  You will be
  returned to the original menu.
  }
 @@ -678,11 +693,11 @@
  {Making device nodes ...
  }

  message badfs
  {It appears that /dev/%s%c is not a BSD file system or the fsck was
 -not successful.  The upgrade has been aborted.  (Error number %d.)
 +not successful (Error number %d.). Try mounting it anyway?
  }

  message rootmissing
  { target root is missing %s.
  }


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename=msg.mi.es.diff

 --- msg.mi.es
 +++ msg.mi.es
 @@ -108,10 +108,22 @@
  completa antes de este procedimiento!  =BFRealmente desea actualizar NetBS=
 D?
  (=C9ste es su =FAltimo aviso antes de que el programa empiece a modificar
  sus discos.)
  }
 =20
 +message Run_Postinst
 +{Update files in /etc}
 +
 +message Postinst
 +{Execute postinst (overwrite existing configuration)}
 +
 +message Etcupdate
 +{Execute etcupdate (ask for every overwrite)}
 +
 +message Skip_postinst
 +{Don't update any file on /etc}
 +
  message reinstallusure
  {Se va a desempaquetar los conjuntos de distribuci=F3n de NetBSD
  a un disco duro marcado como arrancable.  Este procedimiento solamente
  descarga y desempaqueta los conjuntos en un disco arrancable preparticiona=
 do.
  No pone nombre a los discos, ni actualiza los bloques de arranque, ni guar=
 da
 @@ -122,10 +134,13 @@
  =BFRealmente quiere reinstalar los conjuntos de la distribuci=F3n NetBSD?
  (=C9ste es su =FAltimo aviso antes de que el programa empiece a modificar
  sus discos.)
  }
 =20
 +message mount_failed
 +{Mounting %s failed. Continue?
 +}
 =20
  message nodisk
  {No se ha podido encontrar ning=FAn disco duro para ser usado por NetBSD.
  Se le volver=E1 a llevar al men=FA original.
  }
 @@ -699,11 +714,11 @@
  {Creando nodos de dispositivo ...
  }
 =20
  message badfs
  {Parece que /dev/%s%c no es un sistema de archivos BSD o el fsck no ha sido
 -correcto.  La actualizaci=F3n ha sido interrumpida.  (Error n=FAmero %d.)
 +correcto (Error n=FAmero %d.) =BFContinuar?
  }
 =20
  message rootmissing
  { el directorio ra=EDz objetivo no existe %s.
  }


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename=msg.mi.fr.diff

 --- msg.mi.fr
 +++ msg.mi.fr
 @@ -107,10 +107,22 @@
  Voulez-vous vraiment mettre =E0 jour NetBSD ?
  (Ceci est le dernier avertissement avant que cette
  proc=E9dure ne modifie votre disque.)
  }
 =20
 +message Run_Postinst
 +{Update files in /etc}
 +
 +message Postinst
 +{Execute postinst (overwrite existing configuration)}
 +
 +message Etcupdate
 +{Execute etcupdate (ask for every overwrite)}
 +
 +message Skip_postinst
 +{Don't update any file on /etc}
 +
  message reinstallusure
  {D=E9compressons maintenant les composants de NetBSD sur un disque dur
  d=E9marrable.
  Cette proc=E9dure va rapatrier et d=E9compresser les composants
  sur un disque d=E9marrable d=E9j=E0 partitionnn=E9.
 @@ -122,10 +134,14 @@
  ou une mise =E0 jour avant de d=E9marrer cette proc=E9dure !
 =20
  Voulez-vous r=E9ellement r=E9installer les composants NetBSD ?
  (Ceci est le dernier avertissement avant que cette proc=E9dure ne commence=
  =E0
  modifier vos disques.)
 +}
 +
 +message mount_failed
 +{Mounting %s failed. Continue?
  }
 =20
  message nodisk
  {Nous ne trouvons aucun disque utilisable par NetBSD. Vous allez retourner
  au menu pr=E9c=E9dent.
 @@ -740,13 +756,13 @@
  }
 =20
  message badfs
  {
  /dev/%s%c ne semble pas =EAtre un syst=E8me de fichiers BSD ou bien
 -la v=E9rification de son int=E9grit=E9 par fsck a =E9chou=E9.
 +la v=E9rification de son int=E9grit=E9 par fsck a =E9chou=E9, code d'erreu=
 r %d.
 =20
 -Interruption de la proc=E9dure de mise =E0 jour, code d'erreur %d.
 +Souhaitez-vous continuer mise =E0 jour?
  }
 =20
  message rootmissing
  {
  Impossible de trouver la racine du disque cible %s.


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=msg.mi.pl.diff

 --- msg.mi.pl
 +++ msg.mi.pl
 @@ -106,10 +106,22 @@
  przed rozpoczeciem!  Czy napewno chcesz zaktualizowac NetBSD?
  (Jest to ostatnie ostrzezenie zanim zacznie sie modyfikacja danych na
  twoich dyskach.)
  }

 +message Run_Postinst
 +{Update files in /etc}
 +
 +message Postinst
 +{Execute postinst (overwrite existing configuration)}
 +
 +message Etcupdate
 +{Execute etcupdate (ask for every overwrite)}
 +
 +message Skip_postinst
 +{Don't update any file on /etc}
 +
  message reinstallusure
  {Ok, rozpakujmy pakiety dystrybucyjne NetBSD na bootowalny twardy dysk.
  Ta procedura tylko sciaga i rozpakowuje pakiety na pre-partycjonowany
  bootowalny dysk. Nie nazywa dyskow, aktualizuje bootblokow, lub zapisuje
  istniejacej konfiguracji.   (Wyjdz i wybierz `instaluj' lub
 @@ -119,10 +131,13 @@
  Czy napewno chcesz przeinstalowac pakiety dystrybucjne NetBSD?
  (Jest to ostatnie ostrzezenie zanim zacznie sie modyfikacja danych na
  twoich dyskach.)
  }

 +message mount_failed
 +{Mounting %s failed. Continue?
 +}

  message nodisk
  {Nie moge znalezc zadnych twardych dyskow do uzycia z NetBSD. Zostaniesz
  przeniesiony do menu glownego.
  }
 @@ -677,10 +692,14 @@

  message badfs
  {Wyglada na to, ze /dev/%s%c nie jest systemem plikow BSD albo nie powiodlo sie
  jego sprawdzenie. Aktualizacja zostala przerwana. (Blad numer %d.)
  }
 +/* XXX: Translate:
 + * -not successful.  The upgrade has been aborted.  (Error number %d.)
 + * +not successful (Error number %d.). Try mounting it anyway?
 + */

  message rootmissing
  { docelowy / jest zagubiony %s.
  }



 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=run.c.diff

 --- run.c
 +++ run.c
 @@ -564,10 +564,86 @@
  	if (WIFSIGNALED(status)) {
  		*errstr = msg_string(MSG_Command_ended_on_signal);
  		return WTERMSIG(status);
  	}
  	return 0;
 +}
 +
 +/*
 + * launch a program inside a subwindow, and report its return status when done
 + */
 +int
 +launch_command(int flags, const char *cmd)
 +{
 +	int status;
 +	pid_t child, pid;
 +	char **args;
 +
 +	args = make_argv(cmd);
 +
 +	/* Make curses save tty settings */
 +	def_prog_mode();
 +
 +	if (logfp)
 +		fflush(logfp);
 +	if (script)
 +		fflush(script);
 +
 +	child = fork();
 +	if (child == -1) { /* error */
 +		refresh();
 +		return -1;
 +	} else if (child == 0) { /* child */
 +		/* silently stop curses */
 +		endwin();
 +
 +		if (logfp) {
 +			fprintf(logfp, "executing: %s\n", cmd);
 +			fclose(logfp);
 +			logfp = NULL;
 +		}
 +		if (script) {
 +			fprintf(script, "%s\n", cmd);
 +			fclose(script);
 +			script = NULL;
 +		}
 +		if (strcmp(args[0], "cd") == 0 && strcmp(args[2], "&&") == 0) {
 +			target_chdir_or_die(args[1]);
 +			args += 3;
 +		}
 +		if (flags & RUN_XFER_DIR)
 +			target_chdir_or_die(xfer_dir);
 +		/*
 +		 * If target_prefix == "", the chroot will fail, but
 +		 * that's ok, since we don't need it then.
 +		 */
 +		if (flags & RUN_CHROOT && *target_prefix()
 +		    && chroot(target_prefix()) != 0)
 +			warn("chroot(%s) for %s", target_prefix(), *args);
 +		else {
 +			execvp(*args, args);
 +			warn("execvp %s", *args);
 +		}
 +		_exit(EXIT_FAILURE);
 +		// break; /* end of child */
 +	} else { /* parent */
 +		pid = waitpid(child, &status, 0);
 +	}
 +
 +	if (logfp)
 +		fflush(logfp);
 +
 +	reset_prog_mode();
 +	free_argv(args);
 +
 +	if (WIFEXITED(status))
 +		return WEXITSTATUS(status);
 +	else if (WIFSIGNALED(status))
 +		return WTERMSIG(status);
 +
 +	/* Never reached. */
 +	return -1;
  }

  /*
   * generic program runner.
   * flags:


 --MP_/joB4JmgFVA+c7H_120kwHN4
 Content-Type: text/x-patch
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment; filename=util.c.diff

 --- util.c
 +++ util.c
 @@ -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,38 @@
  			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");
 +		if (runpostinstall == 1) {
 +			process_menu(MENU_runpostinst, &runpostinstall);
 +			/* runpostinstall:
 +			 * 0: postinstall fix
 +			 * 1: etcupdate
 +			 * 2: continue
 +			 * 3: 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:
 +					break;
 +				case 3:
 +					return -1;
 +					break;
 +			}
 +		}
  	}

  	/* Configure the system */
  	if (set_status[SET_BASE] & SET_INSTALLED)
  		run_makedev();


 --MP_/joB4JmgFVA+c7H_120kwHN4--

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 10:10:35 +0100

 On Wed, Feb 29, 2012 at 02:25:07AM +0000, Julian Djamil Fagir wrote:
 >  +message Postinst
 >  +{Execute postinst (overwrite existing configuration)}

 This is not quite what it actually does, I'm not sure the user will be
 always able to understand the implications and what the alternatives
 are, so from a usability POV it is a bad question to ask.

 Martin

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

 --Sig_/xgjYv.xg/z53cusaRPacLLE
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: quoted-printable

 Hi

 >  >  +message Postinst
 >  >  +{Execute postinst (overwrite existing configuration)}
 > =20
 >  This is not quite what it actually does, I'm not sure the user will be
 >  always able to understand the implications and what the alternatives
 >  are, so from a usability POV it is a bad question to ask.
 but what is a better one-line description of `postinstall fix`?
 Btw, it should be named postinstall anyway (just now occurred to me).

 Regards, Julian

 --Sig_/xgjYv.xg/z53cusaRPacLLE
 Content-Type: application/pgp-signature; name=signature.asc
 Content-Disposition: attachment; filename=signature.asc

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAk9OAqAACgkQc7h7cu1Hpp5y9gCgsxtLFNv0UotpJv3lduHg6nrc
 GhwAnRTLvxHRlz1ghrlFJVFHVg1u6B7l
 =nh4q
 -----END PGP SIGNATURE-----

 --Sig_/xgjYv.xg/z53cusaRPacLLE--

From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 06:20:39 -0500

 On Wed, 29 Feb 2012 10:50:04 +0000 (UTC)
 Julian Djamil Fagir <gnrp@komkon2.de> wrote:

 >  but what is a better one-line description of `postinstall fix`?
 >  Btw, it should be named postinstall anyway (just now occurred to me).

 Is that the actual execution of postinstall?  If so, in my experience
 it generally doesn't break /etc/, but it'll do things like delete
 obsolete files, report if some users or groups must be added (as
 postinstall cannot add those automatically yet), etc.  It'll also
 occasionally warn that some directories or files should be moved by the
 administrator.

 "Execute the post-installation/post-upgrade script"?

 However, that also won't really fix /etc/ like adding new configuration
 files or applying changes to them, for which etcupate is used.

 In the current state of postinstall, the output has to also be
 preserved so that the user can view it, perform any manual fixes
 needed, and rerun the check/fix phases to verify that all is clean.
 Of course, it'd probably be nice if the fix option could really
 autonomously perform any fix including merging user and group
 additions...
 -- 
 Matt

From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 13:35:03 +0200

 On Wed, 29 Feb 2012, Julian Djamil Fagir wrote:
 >  the attached patch fixes that issue for me.

 Thsnk you for working on this.

 >  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.

 >  +		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.

 >  +			process_menu(MENU_runpostinst, &runpostinstall);
 >  +			/* runpostinstall:
 >  +			 * 0: postinstall fix
 >  +			 * 1: etcupdate
 >  +			 * 2: continue
 >  +			 * 3: abandon installation
 >  +			 */

 Will the output from postinstall still be visible when the user 
 has to choose a menu option?

 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, ...)

 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".

 --apb (Alan Barrett)

From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 13:41:01 +0200

 On Wed, 29 Feb 2012, Martin Husemann wrote:
 >  On Wed, Feb 29, 2012 at 02:25:07AM +0000, Julian Djamil Fagir wrote:
 >  >  +message Postinst
 >  >  +{Execute postinst (overwrite existing configuration)}
 >
 > This is not quite what it actually does,

 Yes, the message could be improved.  Perhaps "Run 'postinstall fix' to
 fix the problems reported above".

 > I'm not sure the user will be always able to understand the 
 > implications and what the alternatives are, so from a usability 
 > POV it is a bad question to ask.

 "postinstall fix" is dangerous, in my opinion, because it 
 overwrites changes that the user may have made to /etc/rc.d/*, 
 /etc/daily and friends, and probably some other configuration 
 files.  Asking the question may be bad for usability, but simply 
 overwriting the user's changes is also bad.

 --apb (Alan Barrett)

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

 Hi,

 >  >  but what is a better one-line description of `postinstall fix`?
 >  >  Btw, it should be named postinstall anyway (just now occurred to me).
 >  
 >  Is that the actual execution of postinstall?  If so, in my experience
 >  it generally doesn't break /etc/, but it'll do things like delete
 >  obsolete files, report if some users or groups must be added (as
 >  postinstall cannot add those automatically yet), etc.  It'll also
 >  occasionally warn that some directories or files should be moved by the
 >  administrator.
 >  
 >  "Execute the post-installation/post-upgrade script"?
 but that's not a clear distinction from etcupdate(8) imho.

 >  However, that also won't really fix /etc/ like adding new configuration
 >  files or applying changes to them, for which etcupate is used.
 >  
 >  In the current state of postinstall, the output has to also be
 >  preserved so that the user can view it, perform any manual fixes
 >  needed, and rerun the check/fix phases to verify that all is clean.
 >  Of course, it'd probably be nice if the fix option could really
 >  autonomously perform any fix including merging user and group
 >  additions...
 Currently, there's a postinstall(8) run when `postinstall check` fails after
 an upgrade. According to its manpage, it replaces existing files with the new
 one and does not attempt to merge them.
 Doesn't the documentation match the actual program?

 Regards, Julian

From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 07:12:07 -0500

 On Wed, 29 Feb 2012 11:45:04 +0000 (UTC)
 Julian Djamil Fagir <gnrp@komkon2.de> wrote:

 >  Currently, there's a postinstall(8) run when `postinstall check` fails after
 >  an upgrade. According to its manpage, it replaces existing files with the new
 >  one and does not attempt to merge them.
 >  Doesn't the documentation match the actual program?

 Probably at least for files in rc.d like Alan noted, I guess.  I only
 made additions to rc.d rather editing existing files, so have never
 noticed a problem using it after source upgrades.
 -- 
 Matt

From: Julian Djamil Fagir <gnrp@komkon2.de>
To: gnats-bugs@NetBSD.org
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--

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 18:22:52 +0000

 On Wed, Feb 29, 2012 at 02:25:07AM +0000, Julian Djamil Fagir wrote:
 > The following reply was made to PR install/38571; it has been noted by GNATS.
 > 
 > From: Julian Djamil Fagir <gnrp@komkon2.de>
 > To: gnats-bugs@gnats.netbsd.org
 > Cc: 
 > Subject: Re: install/38571: sysinst runs "postinstall fix"
 > Date: Wed, 29 Feb 2012 02:30:29 +0100
 > 
 >  --MP_/joB4JmgFVA+c7H_120kwHN4
 >  Content-Type: text/plain; charset=US-ASCII
 >  Content-Transfer-Encoding: 7bit
 >  Content-Disposition: inline
 >  
 >  Hi,
 >  
 >  the attached patch fixes that issue for me. 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.

 Why is that needed, IIRC there is already stuff to run a program and
 display its output in the curses window.

 (Or am I just thinking of the code that displays output to /dev/console.)

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Alan Barrett <apb@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Wed, 29 Feb 2012 20:42:16 +0200

 On Wed, 29 Feb 2012, David Laight wrote:
 >> the attached patch fixes that issue for me. 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.
 >
 > Why is that needed, IIRC there is already stuff to run a program 
 > and display its output in the curses window.

 Yes, we have that, but does it work for interactive programs? 
 AFAIK we use it only for programs that spew output without reading 
 anything from the keyboard.

 --apb (Alan Barrett)

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

 Hi,

 >  >> the attached patch fixes that issue for me. 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.
 >  >
 >  > Why is that needed, IIRC there is already stuff to run a program 
 >  > and display its output in the curses window.
 >  
 >  Yes, we have that, but does it work for interactive programs? 
 >  AFAIK we use it only for programs that spew output without reading 
 >  anything from the keyboard.
 yep, and I derived the code for this function from the existing one.
 I don't remember exactly anymore, but there were cases where the top three
 lines occupying the view of the command were breaking things.
 I think it was when looking at files or diffs, then you never could see the
 top three lines when scrolling a page, but wouldn't bet on it.

 Regards, Julian

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: install-manager@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org, apb@cequrux.com
Subject: re: install/38571: sysinst runs "postinstall fix"
Date: Thu, 01 Mar 2012 10:54:56 +1100

 i'm curious what local configuration postinstall breaks.  apb, you had
 the original report can you expand on this?

 to me, that sounds like a bug itself that needs to be fixed.

 wrt the message, "update configuration" sounds better than "overwrite"
 to me.  (overwrite is a bug, to me.)


 .mrg.

From: Julian Djamil Fagir <gnrp@komkon2.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Sun, 16 Sep 2012 02:02:47 +0200

 --Sig_/_jnx9sQ+mjDX8nI_MQzq/pt
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: quoted-printable

 Hi,

 >  i'm curious what local configuration postinstall breaks.  apb, you had
 >  the original report can you expand on this?
 > =20
 >  to me, that sounds like a bug itself that needs to be fixed.

 a quick check (only looking at code, no real test) revealed that the
 following files at least will be replaced with those from the sets:

  * /etc/defaults/{rc.conf,daily.conf,monthly.conf,weekly.conf,
    security.conf,pf.boot.conf}
  * /dev/{MAKEDEV,MAKEDEV.local}
  * /etc/mtree/{NetBSD.dist,special}
  * /etc/namedb/root.cache
  * /etc/{daily,weekly,monthly,security}
  * /etc/pf.os
  * /etc/{rc,rc.subr,rc.shutdown}
  * /etc/rc.d/{DAEMON,DISKS,LOGIN,NETWORKING,SERVERS,accounting,altqd,
    amd,apmd,bluetooth,bootconf.sh,bootparams,ccd,cgd,cleartmp,cron,
    dhclient,dhcpcd,dhcpd,dhcrelay,dmesg,downinterfaces,envsys,fsck,
    fsck_root,ftp_proxy,ftpd,gpio,hostapd,httpd,identd,ifwatchd,inetd,
    ipfilter,ipfs,ipmon,ipnat,ipsec,irdaattach,iscsi_target,isdnd,
    isibootd,kdc,ldconfig,ldpd,local,lpd,lvm,makemandb,mdnsd,mixerctl,
    mopd,motd,mountall,mountcritlocal,mountcritremote,mountd,moused,
    mrouted,named,ndbootd,network,newsyslog,nfsd,nfslocking,npf,ntpd,
    ntpdate,perusertmp,pf,pf_boot,pflogd,postfix,powerd,ppp,pwcheck,
    quota,racoon,rpcbind,raidframe,raidframeparity,random_seed,rarpd,
    rbootd,rndctl,root,route6d,routed,rtadvd,rtclocaltime,rtsold,rwho,
    savecore,screenblank,securelevel,sshd,staticroute,swap1,swap2,
    sysctl,sysdb,syslogd,timed,tpctl,ttys,veriexec,virecover,wdogctl,
    wpa_supplicant,wscons,wsmoused,ypbind,yppasswdd,ypserv}
  * /etc/mailer.conf

 Regards, Julian

 --Sig_/_jnx9sQ+mjDX8nI_MQzq/pt
 Content-Type: application/pgp-signature; name=signature.asc
 Content-Disposition: attachment; filename=signature.asc

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAlBVFygACgkQc7h7cu1Hpp6CigCbB165UlLksOHEDSKH6HQPjngJ
 XesAnRvnpgjXc9GzddWo/uf5pxAiJhFw
 =9u7l
 -----END PGP SIGNATURE-----

 --Sig_/_jnx9sQ+mjDX8nI_MQzq/pt--

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: install/38571: sysinst runs "postinstall fix"
Date: Sun, 16 Sep 2012 01:02:04 +0000

 On Sun, Sep 16, 2012 at 12:05:04AM +0000, Julian Djamil Fagir wrote:
  >   * /etc/defaults/{rc.conf,daily.conf,monthly.conf,weekly.conf,
  >     security.conf,pf.boot.conf}
  >   * /dev/{MAKEDEV,MAKEDEV.local}
  >   * /etc/mtree/{NetBSD.dist,special}
  >   * /etc/namedb/root.cache
  >   * /etc/{daily,weekly,monthly,security}
  >   * /etc/pf.os
  >   * /etc/{rc,rc.subr,rc.shutdown}
  >   * /etc/rc.d/*
  >   * /etc/mailer.conf

 Right, the only files in here that are supposed to be edited locally
 are MAKEDEV.local and mailer.conf; postinstall shouldn't be trashing
 those. (I believe all it will do to mailer.conf is migrate from base
 sendmail to base postfix. No idea about MAKEDEV.local.)

 The rest of that stuff is no more editable than, say, /sbin/mount is,
 and these files are only in /etc for historical reasons. Arguably they
 should just be updated automatically during installworld and not left
 to postinstall at all.

 -- 
 David A. Holland
 dholland@netbsd.org

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.