Subject: sysinst source niggles.... run_prog, logging/scripting rework?
To: None <tech-install@netbsd.org>
From: Jonathan Stone <jonathan@DSG.Stanford.EDU>
List: tech-install
Date: 06/30/1999 23:34:33
I'm considering reverting run_prog() back to a previous design
goal. Many calls to run_prog() currently look like

     run-prog({0|1}, {0|1}, NULL,
		     "command string %s %s",
		     "args", "more args");

which increases the burden on anyone trying to audit the run_prog()
calls, to make sure that all errors that should be hanlded are
properly handled.

I've patched my local tree to use named variants for the 
common cases to run_prog as follows:

        run_prog(0, 0, NULL, ...) -> run_silent(...);
        run_prog(1, 0, NULL, ...) -> run_prog_or_die(...);
        run_prog(0, 1, NULL, ...) -> run_prog_in_subwin(...);

joined the command lines to make them more readable.  run_prog() is
replacd with a va_run_prog(), taking a va_alist as last arg, and all
the run_* variants coded as wrappers to va_run_prog.


Any feedback/objections to committing that?  One thing it did
highlight is that run_prog_or_die() simply calls exit. It needs to
display a message saying what failed; not doing so is a bug,
and we should really fix that for 1.4.1.

I've done a few more random cleanups: e.g., moving the script/log
processing inside do_system() rather than dupliating it at each call
to do_system(), and adding a target_rm() function rather than, e.g.,

      run_silent("rm -f %s", target_expand("/.profile"));

since target_expand() was supposed to be private to target.c.


I've also looked at the logging and scriting functionality.  Both are
great ideas but the implementation leaves something to be
eseired. Doing the logging and scripting inline adds extra levels of
nested if()s.  Which in turn clutters the code. It's much harder to
follow just what (e.g.) the networking code is doing. Besides which,
having two copies of string constants around is a maintenance worry.

I'd like to  add some functions to do log/script processing
via a `FILE*/script/log handle, e.g.,

  target_fcreat()    -- do target_fopen(path, "w"),
			AND do script/log processing of the create/trucate..

  target_fappend()   -- do target_fopen(path, "a");
			AND does log processing of the append.

  target target_fprintf()  -- writes a string to file handle,
			  AND appends the string written to script/log output.

  target_fclose() -- does end-of-file processing for nonzero
		     files.

which would allow moving most of the logging/script processing inside
the target() functions.

Comments?  Anyone like to review these before a commit?  Or should we
give up on the sysinst codebase as a dead duck?