This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Setup postinstall logging - take 3?


Sorry for the '?' in the subject - I think I lost track...

On 15 Mar 2003, Robert Collins wrote:

> On Sat, 2003-03-15 at 04:32, Igor Pechtchanski wrote:
>
> > > Howabout that?
> > > Rob
> >
> > Ok, here's the next iteration of this patch.  It still pops a console with
> > nothing in it when running postinstall scripts -- I'm sure there's a way
> > to remove it, but can't find it at the moment.
>
> Don't worry. What would be good would be to make it start minimized.

Ok, I've figured out how to hide the console altogether (see attached
patch, though the CREATE_NO_WINDOW is probably overkill).  Could someone
familiar with the Windows CreateProcess mechanism make sure this is ok?
Also, this needs testing on Win95 again.  Brian?

On another note, now that the window is not shown at all, setup simply
seems to hang while long-running postinstall scripts (e.g., post-texmf.sh,
with run-time of ~2 minutes!) are executed.  Perhaps we should a) run
postinstall scripts from a separate thread, so that setup at least reacts
to Windows events, such as REPAINT, and b) have another dialog saying
"Running postinstall scripts", perhaps even with a progress bar (even if
the bar only reflects progress by scripts executed rather than time -- I'm
not sure if it's even possible to correctly reflect temporal progress).

> > It does, however, correctly redirect the output of postinstall scripts
> > into the LOG_BABBLE file.  Most of the patch is OS-independent, but
> > the output redirection mechanism has been tested on Win95 by Brian
> > Keener (see <http://cygwin.com/ml/cygwin-apps/2003-03/msg00364.html>)
> > and hasn't been changed in this patch.
> >       Igor
> > P.S. Note that this patch conflicts slightly with my other patch
> > (postinstall script ordering).  Nothing a human can't fix, but both at
> > once will not apply cleanly OOTB.  Whichever one of them goes in first,
> > I'll regenerate and resubmit the other one.
>
> Ok, thank you.

The new one conflicts both with the above and with the cleanup patch I
posted today...  Unless some of them are committed, it's going to get
hairy fast, eh, Rob? ;-)

> > ==============================================================================
> > ChangeLog:
> > 2003-03-13  Igor Pechtchanski <pechtcha at cs dot nyu dot edu>
> >
> >       * script.cc (run): Add lname parameter.
> >       Redirect output of subprocess to file, creating the
> >       path if necessary.
> >       (run_script): Add optional to_log boolean parameter.
> >       If to_log, redirect output to temporary file.
> >       (openOutputLog): New helper function.
>
> This should return void and throw an exception on failure...
> OR
> use a class and set a status member.

Ok.  I've created an OutputLog class that encapsulates this behavior.
I've put the Close() into the destructor.  Hopefully this is what you
meant.

> >       (closeOutputLog): New helper function.
> >       (removeOutputLog): New helper function.
> >       * script.h (run_script): Add optional to_log parameter.
> >       * log.h (log_file): New function.
> >       * log.cc (log_file): New Function
> >       (BUFLEN): New #define.
> >       * postinstall.cc (RunFindVisitor::visitFile): Pass
> >       filename to run_script().
          ^^^^^^^^^^^^^^^^^^^^^^^^
Just noticed that the above is outdated.  Fixed in new ChangeLog.

> The define BUFLEN should be a static const member of the class.

Umm, which class?  It's only used in static functions.  I left it as a
#define for now.

> A general nit (just some search n replace for you) - I'd like new
> methods and variables to make sense at first read. So 'lname' isn't
> great. logFilename would be good. Likewise log_file isn't clear that it
> imports a text file into the setup log.

Done.  lname in run() => out_to; lname in run_script() => log_name;
log_file => log_from_file

> Oh, and I loath the MS idiom of including the type in the variable name
> - bInheritHandles being a case in point.

I fully and emphatically agree.  I simply copied the example from the MSDN
web page, and forgot to change it.

> Other than those sugar things, the code looks great. Please update the
> patch and then I'll rubber stamp it.
>
> Rob

Thanks, but I think this needs testing on Win95 again (for the
CreateProcess magic).
	Igor
==============================================================================
ChangeLog:
2003-03-13  Igor Pechtchanski <pechtcha at cs dot nyu dot edu>

	* script.cc (run): Add out_to parameter.
	Redirect output of subprocess to file, creating the
	path if necessary.
	(run_script): Add optional to_log boolean parameter.
	If to_log, redirect output to temporary file.
	(OutputLog): New helper class.
	(removeOutputLog): New helper function.
	* script.h (run_script): Add optional to_log parameter.
	* log.h (log_from_file): New function.
	* log.cc (log_from_file): New Function
	(BUFLEN): New #define.
	* postinstall.cc (RunFindVisitor::visitFile): Instruct
	run_script() to log script output.

-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_		pechtcha at cs dot nyu dot edu
ZZZzz /,`.-'`'    -.  ;-;;,_		igor at watson dot ibm dot com
     |,4-  ) )-,_. ,\ (  `'-'		Igor Pechtchanski
    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

Oh, boy, virtual memory! Now I'm gonna make myself a really *big* RAMdisk!
  -- /usr/games/fortune


Attachment: setup-postinstall-log.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]