[PATCH] Setup postinstall logging - take X+1 (regen against HEAD)

Igor Pechtchanski pechtcha@cs.nyu.edu
Tue Mar 18 23:17:00 GMT 2003


On Tue, 18 Mar 2003, Igor Pechtchanski wrote:

> 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@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@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

Same as above, but regenerated against cvs HEAD.
Thanks,
	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
@@ -36,7 +36,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:
@@ -57,7 +57,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