Subject: Re: sysinst source niggles.... run_prog, logging/scripting rework?
To: Jonathan Stone <jonathan@DSG.Stanford.EDU>
From: Chris G. Demetriou <email@example.com>
Date: 07/01/1999 01:43:11
Since I just spent some time whacking on code near some of the things
you metion, I think i might have some useful input... 8-)
Jonathan Stone <jonathan@DSG.Stanford.EDU> writes:
> 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.
One of these things is not like the others... 8-)
It might be nice to have it named run_<something>, e.g. run_va. it's
nice to be able to look for functions named 'run_.*'
> 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'm kinda hesitant to buy into additional significant changes for
1.4.1's sysinst, at least until it gets some real user testing.
In what cases is run_prog_or_die() used (or the 'fatal' run_prog()
flag currently used)? It seems to me that, in general, dying is a
very bad thing, and should be avoided.
For this (and the silent case), it probably makes sense to accumulate
error messages off-screen and, if a failure occurs, display them in a
message window like the run-w/display window.
In the longer term, sysinst's "exit strategy" needs some cleanup, in
my opinion, as does its data/state storage strategy. It'd be nice to
just have the cleanup hook print, among other things, why the program
was dying. However, it's not clear to me that
> 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. [ ... ]
> I'd like to add some functions to do log/script processing
> via a `FILE*/script/log handle, e.g.,
It seems to me that it's reasonable to assume in that code that the
script and log handles are globals. (not clear if you meant that from
your statement above.)
I dunno, it might also be nice to have functions which stuff bits into
the log/script (one thing that jumps out at me is _comments_, so
somebody reading the thing can know what's going on!) if they're
enabled so that you don't need to even check for logging/scripting
being enabled. They could be used by your proposed target_*()
> Comments? Anyone like to review these before a commit?
If you'd like, and are willing to wait a little bit for a response (no
earlier than this weekend, I think), I'd be willing to take a gander
> Or should we
> give up on the sysinst codebase as a dead duck?
While some people have indicated that they're working on Something
Better, it doesn't sound like they're particularly far along (or even
have a rough estimate of completion). Therefore, as far as i'm
concerned, we're stuck with sysinst for the forseeable future.
I have a bunch of work planned for it, if I can make the time. I
think the list is something like the following (in order):
[ in the entire discussion below, when i'm referring to menus i'm
referring to the things sysinst calls menus, which are really more
like selection boxes or something. ]
1 convert the msg code to do automatic layout of text. This
means several things:
a add APIs to provide table formatting, i.e. no
auto-flow of text, and convert table formatting code
to use it.
b turn on the simple layout engine that i committed
a week or two ago. Right now this isn't enabled
because of the lossage with tables.
c nuke unused APIs.
d cleanup markup features for messages, e.g. "force
line break, force paragraph break." Right now this
functionality is already in the layout engine, but
it's... kinda hokey since I wanted it to be mostly-
compatible with the existing message text. Right
now, paragraphs are denoted by \n\n, and line breaks
are denoted by <space>\n. If possible I'd like to
do these in an HTML compatible way, but I'll cross
that bridge when I come to it. In general, this
item can be put off to much later.
2 poke the message compiler so that messages have a couple of
attributes: "preformatted" (don't auto-flow, i.e. preserve
spacing), and "menu selection character." The former would
be used for table headings and table body entries.
3 nuke all native-language (i.e. not command names, etc., but
"user feedback" text) and table format string constants in
the code. (You'd think with a message compiler there
wouldn't be any of the former, but you'd be wrong. I made
the problem a tiny bit worse with my recent changes, but
there were hard-coded strings where it would have been easy
to DTRT already...). Replace them with messages.
4 nuke all public message APIs which operate directly on
strings. Make them all require messages.
5 make the menu system suck much, much less. make all menus
dynamic, and capable of having entries changed, added, and
removed at run time. What this really means is:
a create a decent menu API. Knowing little, I was
planning to grab one of my old copies of Inside
Macintosh and clone some of the basic menu
creation/deletion + item addition/removal APIs.
They worked pretty well for me when I had to use
them 10+ years ago. 8-) Desired features:
* create, destroy menus
* add, remove menu items
* change item text.
* each item can have a predefined keyboard
shortcut, but failing that keyboard
shortcuts will be assigned as it's done
* settable default item.
* menu text items can be described by message
b convert the menu compiler to generate an init
function which generates calls to that API, rather
than hard-coding the tables.
c make the menu compiler try to do basic menu size vs.
screen size sanity checks at startup, rather than as
menus are invoked. No more exiting out in the
middle because there's not enough room for the menu!
d depending on the space cost incurred by 5b, and the
pain involved in generating static entries for the
preconfigured menus, try to generate pregenerated
menu structures rather than just function calls.
This is an optimization, it can be deferred.
[ note that this means that the menu system will
probably be using the message system's internal
APIs. I don't consider this a bug; i consider the
two tightly intertwined. ]
6 move all of the menu message strings into the message files,
and nuke the language dependence in the menu files. It's
just Evil to be mixing code and data like this. A
maintenance nightmare, as I think most of us who've hacked
the code know by now. 8-)
7 nuke all handling of fixed strings in the menu code, even
in the 'replace item text' code. (There you can do
formatting based on messages.) Require that all public APIs
operate on messages.
8 Integrate message window and menu display, or at least put
in some status checking features so that the menus better
accomodate the size of the message in the message window.
Maybe glue them together even more tightly and require that
the message window be the entire stdscr.
Pie in the sky:
* more advanced message formatting. e.g. don't make the
message formatter just printf-like, make it support
additional types, such as messages. For instance, it'd be
nice to say:
message mi_foobar "foo bar: %m"
message mi_foobar_placeholder "XXX"
message mi_foobar_yes "Yes"
message mi_foobar_no "No"
then msg_format(MSG_mi_foobar, MSG_mi_placeholder); to get
"foo bar: XXX". (the reason for using a placeholder here
is so that the initial menu size check could try to
check for enough horizontal space to display the menu. I
prefer that style to hard-coding one of the actual message
values, because e.g. the widest one may change from language
Other code that I know needs work:
* the code that reads the child's output from the pty and then
logs it and/or puts it into the display window is losing,
for a couple of reasons:
* it does: parent_fork()->child1
child1_fork()->child2, child1 exec command, child2
read output and feed it back to parent. This
seems... unnecessarily complex. If there's a
reason for it, it should be thorougly documented.
If not, it should be cleaned up to do only one fork.
* if the command exits before all output has been
passed back to the parent, the parent won't ever
get (and therefore display/log) the last of the
output. This is caused by incorrect loop
termination checks, but (given my experience) I
don't know enough that it's trivial to fix so I
punted on it for the time being. I seem to recall
that running 'stty -a ; false' in a subwindow was
a good way to show the problem. you might not need
the '; false' to trigger it, but if you have it
the window doesn't go away immediately and you can
see the last output pushed to the parent. 8-)
* I've seen several cases now where (it would appear that)
running commands w/o display output has caused sysinst to
hang. This could be because of some child-exit race
condition check, or something. I've never bothered tracking
it down, and i'm not even sure the above description of the
trigger of the problem is accurate, but that's what's seemed
likely to me given the menus i'd accepted right before
sysinst hung. sysinst was killable (via some kind of
signal, I forget which 8-).
Obviously, if anybody's interested in helping with any of these tasks
i'd more than welcome the help.
Chris Demetriou - firstname.lastname@example.org - http://www.netbsd.org/People/Pages/cgd.html
Disclaimer: Not speaking for NetBSD, just expressing my own opinion.