[PATCH] Setup postinstall logging - take X+1

Igor Pechtchanski pechtcha@cs.nyu.edu
Tue Mar 18 16:35:00 GMT 2003


On Sun, 16 Mar 2003, Robert Collins wrote:

> Igor Pechtchanski wrote:
> > Sorry for the '?' in the subject - I think I lost track...

Starting a new count here... :-)

> > On 15 Mar 2003, Robert Collins wrote:
>
> >>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).
>
> That would be nice. A trivial step to take is to minimize, not hide, the
> window. Then the user will see flicky things happening in their tool
> bar, which may reduce the panic.

Ok, done.  The windows won't be "flicky", though (a long-running script
window will still look hung), and they won't show anything.  Good enough
for now, I guess.

> > 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? ;-)
>
> Nay. You'll want to merge in the HEAD script.cc changes though.

Yep.

> >>>      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.
>
> Yep, looking good.
>
> >>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.
>
> Err just realised. log.h and log.cc are deprecated. Thats why they have
> the foo_bar naming style and no classes. It's probably best as an
> operator for OutputLog (given that it's a layer on top of the log
> infrastructure) - i.e.
> LogSingleton::GetInstance()(LOG_PLAIN) << file_out;
> And removing the logfile then becomes part of OutputLog::~OutputLog. Hey
> this is starting to be too easy :}.

Done.  Also upgraded to call the current interface instead of the
deprecated one.

> > Thanks, but I think this needs testing on Win95 again (for the
> > CreateProcess magic).
>
> Ok.
>
> Can you make the window minimized, not hidden? Otherwise we should put
> the requisite UI changes in place first.
> Rob

Did that.  However, I've "#if 0"'d the correct code, and left a TODO
there -- I'd like to keep that in place until the UI changes are in.
An orthogonal change would be to run the scripts from another thread.
I'll look into that (separately).  This patch is ready to go in as-is,
though (the non-"#if 0"'d portion *should* work on Win95, according to
MSDN).
	Igor
==============================================================================
ChangeLog:
2003-03-18  Igor Pechtchanski <pechtcha@cs.nyu.edu>

	* script.cc (run): Add file_out parameter.
	Redirect output of subprocess to file, creating the
	path if necessary.  Minimize the script window.
	(run_script): Add optional to_log boolean parameter.
	If to_log, redirect output to temporary file and then
	import it into LOG_BABBLE.
	(OutputLog): New helper class.
	(operator<<): New operation on OutputLog.
	* script.h (run_script): Add optional to_log parameter.
	* postinstall.cc (RunFindVisitor::visitFile): Instruct
	run_script() to log script output.
	(do_postinstall): Ditto.

-- 
				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: script.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/script.cc,v
retrieving revision 2.6
diff -u -p -r2.6 script.cc
--- script.cc	17 Mar 2003 22:23:33 -0000	2.6
+++ script.cc	18 Mar 2003 15:50:02 -0000
@@ -26,11 +26,12 @@ static const char *cvsid =
 #include <stdlib.h>
 #include <unistd.h>
 #include <stdio.h>
-#include "log.h"
+#include "LogSingleton.h"
 #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;
@@ -79,12 +80,92 @@ init_run_script ()
     }
 }
 
+class OutputLog
+{
+public:
+  OutputLog (String const &filename);
+  ~OutputLog ();
+  HANDLE handle () { return _handle; }
+  BOOL isValid () { return _handle != INVALID_HANDLE_VALUE; }
+  BOOL isEmpty () { return GetFileSize (_handle, NULL) == 0; }
+  friend std::ostream &operator<< (std::ostream &, OutputLog &);
+private:
+  enum { BUFLEN = 1000 };
+  HANDLE _handle;
+  String _filename;
+  void out_to(std::ostream &);
+};
+
+OutputLog::OutputLog (String const &filename)
+  : _handle(INVALID_HANDLE_VALUE), _filename(filename)
+{
+  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);
+
+  if (_handle == INVALID_HANDLE_VALUE)
+    {
+      log(LOG_PLAIN) << "error: Unable to redirect output to '" << _filename
+		     << "'; using console" << endLog;
+    }
+}
+
+OutputLog::~OutputLog ()
+{
+  if (_handle != INVALID_HANDLE_VALUE)
+    CloseHandle (_handle);
+  if (_filename.size() &&
+      !DeleteFile(backslash (cygpath (_filename)).cstr_oneuse()))
+    {
+      log(LOG_PLAIN) << "error: Unable to remove temporary file '" << _filename
+		     << "'" << endLog;
+    }
+}
+
+std::ostream &
+operator<< (std::ostream &out, OutputLog &log)
+{
+  log.out_to(out);
+  return out;
+}
+
+void
+OutputLog::out_to(std::ostream &out)
+{
+  char buf[BUFLEN];
+  DWORD num;
+  FlushFileBuffers (_handle);
+  SetFilePointer(_handle, 0, NULL, FILE_BEGIN);
+  
+  while (ReadFile(_handle, buf, BUFLEN-1, &num, NULL) && num != 0)
+    {
+      buf[num] = '\0';
+      out << buf;
+    }
+
+  SetFilePointer(_handle, 0, NULL, FILE_END);
+}
+
 static void
-run (const char *sh, const char *args, const char *file)
+run (const char *sh, const char *args, const char *file, OutputLog &file_out)
 {
   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,8 +174,26 @@ run (const char *sh, const char *args, c
   si.lpTitle = (char *) "Cygwin Setup Post-Install Script";
   si.dwFlags = STARTF_USEPOSITION;
 
-  BOOL createSucceeded = CreateProcess (0, cmdline, 0, 0, 0,
-		     CREATE_NEW_CONSOLE, 0, get_root_dir ().cstr_oneuse(), &si, &pi);
+  if (file_out.isValid ())
+    {
+      inheritHandles = TRUE;
+      si.dwFlags |= STARTF_USESTDHANDLES;
+      si.hStdInput = GetStdHandle (STD_INPUT_HANDLE);
+      si.hStdOutput = file_out.handle ();
+      si.hStdError = file_out.handle ();
+      si.dwFlags |= STARTF_USESHOWWINDOW;
+#if 0
+      si.wShowWindow = SW_HIDE;
+      flags = CREATE_NO_WINDOW;  // Note: might not work on Win9x
+#else
+      // TODO: introduce a script progress tracker and use the above
+      si.wShowWindow = SW_MINIMIZE;
+#endif
+    }
+
+  BOOL createSucceeded = CreateProcess (0, cmdline, 0, 0, inheritHandles,
+					flags, 0, get_root_dir ().cstr_oneuse(),
+					&si, &pi);
 
   if (createSucceeded)
     WaitForSingleObject (pi.hProcess, INFINITE);
@@ -103,26 +202,37 @@ run (const char *sh, const char *args, c
 }
 
 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));
+    }
+  OutputLog file_out(log_name);
+
   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());
+      log(LOG_PLAIN) << "running: " << sh << " -c " << f2 << endLog;
+      run (sh.cstr_oneuse(), "-c", f2.cstr_oneuse(), file_out);
     }
   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());
+      log(LOG_PLAIN) << "running: " << cmd << " /c " << f2 << endLog;
+      run (cmd, "/c", f2.cstr_oneuse(), file_out);
     }
   else
     return;
+
+  if (to_log && !file_out.isEmpty ())
+    log(LOG_BABBLE) << file_out << endLog;
 
   /* 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.3
diff -u -p -r2.3 script.h
--- script.h	17 Mar 2003 22:23:33 -0000	2.3
+++ script.h	18 Mar 2003 15:50:02 -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 ();
Index: postinstall.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/postinstall.cc,v
retrieving revision 2.10
diff -u -p -r2.10 postinstall.cc
--- postinstall.cc	17 Mar 2003 22:23:33 -0000	2.10
+++ postinstall.cc	18 Mar 2003 15:50:02 -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:
@@ -56,7 +56,7 @@ do_postinstall (HINSTANCE h, HWND owner)
       packagemeta & pkg = **i;
       if (pkg.installed)
 	for (std::vector<Script>::iterator script=pkg.installed.scripts().begin(); script != pkg.installed.scripts().end(); ++script) 
-	  run_script ("/etc/postinstall/", script->baseName());
+	  run_script ("/etc/postinstall/", script->baseName(), TRUE);
       ++i;
     }
   RunFindVisitor myVisitor;


More information about the Cygwin-apps mailing list