[PATCH] Setup postinstall logging - take 3?
Igor Pechtchanski
pechtcha@cs.nyu.edu
Sat Mar 15 19:15:00 GMT 2003
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@cs.nyu.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@cs.nyu.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@cs.nyu.edu
ZZZzz /,`.-'`' -. ;-;;,_ igor@watson.ibm.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
-------------- next part --------------
Index: postinstall.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/postinstall.cc,v
retrieving revision 2.9
diff -u -p -r2.9 postinstall.cc
--- postinstall.cc 19 May 2002 03:07:51 -0000 2.9
+++ postinstall.cc 13 Mar 2003 18:34:51 -0000
@@ -33,7 +33,7 @@ public:
RunFindVisitor (){}
virtual void visitFile(String const &basePath, const WIN32_FIND_DATA *theFile)
{
- run_script ("/etc/postinstall/", theFile->cFileName);
+ run_script ("/etc/postinstall/", theFile->cFileName, TRUE);
}
virtual ~ RunFindVisitor () {}
protected:
Index: log.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/log.cc,v
retrieving revision 2.13
diff -u -p -r2.13 log.cc
--- log.cc 27 Jun 2002 11:01:10 -0000 2.13
+++ log.cc 15 Mar 2003 18:57:54 -0000
@@ -36,6 +36,8 @@ static const char *cvsid =
#include "io_stream.h"
+#define BUFLEN 1000
+
void
log (enum log_level level, String const &message)
{
@@ -45,9 +47,38 @@ log (enum log_level level, String const
void
log (enum log_level level, const char *fmt, ...)
{
- char buf[1000];
+ char buf[BUFLEN];
va_list args;
va_start (args, fmt);
- vsnprintf (buf, 1000, fmt, args);
+ vsnprintf (buf, BUFLEN, fmt, args);
log (level, String(buf));
+}
+
+void
+log_from_file (enum log_level level, String const &filename)
+{
+ char buf[BUFLEN];
+ int num;
+ bool non_empty = false;
+ io_stream *f = io_stream::open(String("cygfile://") + filename, "rt");
+ if (!f)
+ {
+ const char *err = strerror (errno);
+ if (!err)
+ err = "(unknown error)";
+ note (NULL, IDS_ERR_OPEN_READ, filename.cstr_oneuse(), err);
+ return;
+ }
+
+ std::ostream &out = LogSingleton::GetInstance()(level);
+ while ((num = f->read(buf, BUFLEN-1)) != 0)
+ {
+ buf[num] = '\0';
+ out << buf;
+ non_empty = true;
+ }
+ if (non_empty)
+ out << endLog;
+
+ delete f;
}
Index: log.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/log.h,v
retrieving revision 2.5
diff -u -p -r2.5 log.h
--- log.h 4 May 2002 12:15:56 -0000 2.5
+++ log.h 15 Mar 2003 18:57:54 -0000
@@ -23,3 +23,4 @@
void log (enum log_level level, const char *fmt, ...)
__attribute__ ((format (printf, 2, 3)));
void log (enum log_level level, String const &);
+void log_from_file (enum log_level level, String const &filename);
Index: script.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.4
diff -u -p -r2.4 script.cc
--- script.cc 18 Feb 2002 13:53:07 -0000 2.4
+++ script.cc 15 Mar 2003 18:57:54 -0000
@@ -30,6 +30,8 @@ static const char *cvsid =
#include "filemanip.h"
#include "mount.h"
#include "io_stream.h"
+#include "script.h"
+#include "mkdir.h"
static String sh = String();
static const char *cmd = 0;
@@ -78,13 +80,61 @@ init_run_script ()
}
}
+class OutputLog
+{
+public:
+ OutputLog (String const &filename);
+ ~OutputLog ();
+ HANDLE handle () { return _handle; }
+ BOOL isValid () { return _handle != INVALID_HANDLE_VALUE; }
+private:
+ HANDLE _handle;
+};
+
+OutputLog::OutputLog (String const &filename) : _handle(INVALID_HANDLE_VALUE)
+{
+ if (!filename.size())
+ return;
+
+ SECURITY_ATTRIBUTES sa;
+ memset (&sa, 0, sizeof (sa));
+ sa.nLength = sizeof (sa);
+ sa.bInheritHandle = TRUE;
+ sa.lpSecurityDescriptor = NULL;
+
+ if (mkdir_p (0, backslash (cygpath (filename)).cstr_oneuse()))
+ return;
+
+ _handle = CreateFile (backslash (cygpath (filename)).cstr_oneuse(),
+ GENERIC_READ|GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE,
+ &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+}
+
+OutputLog::~OutputLog ()
+{
+ if (_handle != INVALID_HANDLE_VALUE)
+ CloseHandle (_handle);
+}
+
static void
-run (const char *sh, const char *args, const char *file)
+removeOutputLog (String const &filename)
+{
+ if (io_stream::remove (String ("cygfile://") + filename))
+ {
+ log (LOG_PLAIN, String ("error: Unable to remove temporary file '") +
+ filename + "'");
+ }
+}
+
+static void
+run (const char *sh, const char *args, const char *file, String const &out_to)
{
BOOL b;
char cmdline[_MAX_PATH];
STARTUPINFO si;
PROCESS_INFORMATION pi;
+ DWORD flags = CREATE_NEW_CONSOLE;
+ BOOL inheritHandles = FALSE;
sprintf (cmdline, "%s %s %s", sh, args, file);
memset (&pi, 0, sizeof (pi));
@@ -93,34 +143,68 @@ run (const char *sh, const char *args, c
si.lpTitle = (char *) "Cygwin Setup Post-Install Script";
si.dwFlags = STARTF_USEPOSITION;
- b = CreateProcess (0, cmdline, 0, 0, 0,
- CREATE_NEW_CONSOLE, 0, get_root_dir ().cstr_oneuse(), &si, &pi);
+ OutputLog file_out(out_to);
+
+ if (out_to.size())
+ {
+ if (!file_out.isValid ())
+ {
+ log (LOG_PLAIN, String ("Error: Unable to redirect output to '") +
+ out_to + "'; using console");
+ }
+ else
+ {
+ inheritHandles = TRUE;
+ si.dwFlags |= STARTF_USESTDHANDLES;
+ si.hStdOutput = file_out.handle ();
+ si.hStdError = file_out.handle ();
+ si.dwFlags |= STARTF_USESHOWWINDOW;
+ si.wShowWindow = SW_HIDE;
+ flags = CREATE_NO_WINDOW; // Note: might not work on Win9x
+ }
+ }
+
+ b = CreateProcess (0, cmdline, 0, 0, inheritHandles,
+ flags, 0, get_root_dir ().cstr_oneuse(), &si, &pi);
if (b)
WaitForSingleObject (pi.hProcess, INFINITE);
}
void
-run_script (String const &dir, String const &fname)
+run_script (String const &dir, String const &fname, BOOL to_log)
{
char *ext = strrchr (fname.cstr_oneuse(), '.');
if (!ext)
return;
+ String log_name = "";
+ if (to_log)
+ {
+ char tmp_pat[] = "/var/log/setup.log.postinstallXXXXXXX";
+ log_name = String (mktemp(tmp_pat));
+ }
+
if (sh.size() && strcmp (ext, ".sh") == 0)
{
String f2 = dir + fname;
log (LOG_PLAIN, String ("running: ") + sh + " -c " + f2);
- run (sh.cstr_oneuse(), "-c", f2.cstr_oneuse());
+ run (sh.cstr_oneuse(), "-c", f2.cstr_oneuse(), log_name);
}
else if (cmd && strcmp (ext, ".bat") == 0)
{
String f2 = backslash (cygpath (dir + fname));
log (LOG_PLAIN, String ("running: ") + cmd + " /c " + f2);
- run (cmd, "/c", f2.cstr_oneuse());
+ run (cmd, "/c", f2.cstr_oneuse(), log_name);
}
else
return;
+
+ if (to_log)
+ {
+ log_from_file(LOG_BABBLE, log_name);
+ removeOutputLog (log_name);
+ }
/* if file exists then delete it otherwise just ignore no file error */
io_stream::remove (String ("cygfile://") + dir + fname + ".done");
Index: script.h
===================================================================
RCS file: /cvs/cygwin-apps/setup/script.h,v
retrieving revision 2.2
diff -u -p -r2.2 script.h
--- script.h 18 Feb 2002 12:35:23 -0000 2.2
+++ script.h 15 Mar 2003 18:57:54 -0000
@@ -21,7 +21,7 @@
we have a Bourne shell, execute it using sh. Otherwise, if fname
has suffix .bat, execute using cmd */
-void run_script (String const &dir, String const &fname);
+void run_script (String const &dir, String const &fname, BOOL to_log = FALSE);
/* Initialisation stuff for run_script: sh, cmd, CYGWINROOT and PATH */
void init_run_script ();
More information about the Cygwin-apps
mailing list