Pending setup patches (issue 6)

Igor Pechtchanski pechtcha@cs.nyu.edu
Mon Mar 10 00:01:00 GMT 2003


On Sun, 9 Mar 2003, Max Bowsher wrote:

> Igor Pechtchanski wrote:
> > On Sun, 9 Mar 2003, Max Bowsher wrote:
> >> But what about:
> >> +    // TODO: free unused map entries
> >
> > Not a show-stopper.  Setup keeps all kinds of memory chunks around.
> > They'll be freed when it exits anyway, it's just cleaner to explicitly
> > manage them.
>
> Fair enough.
>
> >> +    // TODO: detect circular dependences
> >
> > Mmm, I think this one can be done later as an improvement.  Again,
> > not a show stopper, just something that will influence the sort order.
>
> Oh, ok, so at the moment, the order of execution of circular dependencies is
> undefined? It won't get stuck in a loop, or anything terrible?

The dependence propagation step will perform a DFS on the dependence
graph.  No node (FileSpec) will be visited more than once (even with
circular dependences).  As for search order, dependences determine the
partial order of FileSpecs, so a circular dependence breaks irreflexivity
(you'd end up with a FileSpec being less than itself)...

> >> Also, dependences/dependencies: Both are valid words, but the 2nd
> >> seems (to me) more commonly used?
> >
> > Must be my compiler background...  I'll change it if it bothers
> > people.
>
> That would be consistent with elsewhere in setup:
>
> choose.cc:        Dependency *dp = pkg.desired->required;
> package_version.cc:class DependencyProcessor {
> package_version.cc:  DependencyProcessor (trusts const &aTrust, int aDepth=0) : deftrust (aTrust), depth (aDepth) {}
> package_version.cc:select (DependencyProcessor &processor, packagemeta *required, packageversion const &aVersion)
> package_version.cc:processOneDependency(trusts deftrust, size_t depth, PackageSpecification *spec)
> package_version.cc:  DependencyProcessor processor (deftrust, depth);
> package_version.cc:       changed += processOneDependency (deftrust, depth, *i) + 1;
> package_version.cc:       changed += processOneDependency (deftrust, depth, *i) + 1;
>
> Max.

Funny you should mention it, the autotools stuff uses "dependence"...  But
I don't mind changing it (although I may, and probably will, slip up in
regular e-mail).  A new patch is attached (along with the new ChangeLog).
	Igor
==============================================================================
ChangeLog:
2003-03-09  Igor Pechtchanski <pechtcha@cs.nyu.edu>

	* postinstall.cc (RunFindVisitor::executeAll): New
	member function that propagates script dependencies,
	topologically sorts the script list, and then executes
	the scripts (via other calls).
	(RunFindVisitor::visitFile): Change to add file to list
	instead of running it immediately.
	(RunFindVisitor::files): New member variable.
	(RunFindVisitor::checkAndLogMissingDependences): New
	member function.
	(FileDesc): New class that extracts and stores
	dependencies from a script file.
	(DEPEND_STR): New #define.
	(do_postinstall): Add executeAll() call.

-- 
				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	4 Mar 2003 17:54:20 -0000
@@ -26,6 +26,140 @@ static const char *cvsid =
 #include "mount.h"
 #include "script.h"
 #include "FindVisitor.h"
+#include "io_stream.h"
+#include "resource.h"
+#include "msg.h"
+#include "log.h"
+#include <list>
+#include <set>
+#include <map>
+
+#define DEPEND_STR "depends on: "
+#define BUFLEN 500
+
+class FileDesc
+{
+public:
+  FileDesc(String const &basePath);
+  String const &path() { return _path; }
+  std::set<FileDesc*> const & dependencies() { return _dependencies; }
+  bool addDependency(FileDesc *dep) {
+    return _dependencies.insert(dep).second;
+  }
+  bool addDependency(String const &deps) {
+    // TODO: free unused map entries
+    return addDependency(create(deps));
+  }
+  void propagateDependencies() {
+    // TODO: detect circular dependencies
+    if (_mark) return;
+    _mark = true;
+    for (std::set<FileDesc*>::iterator i = _dependencies.begin();
+	 i != _dependencies.end();
+	 ++i)
+      {
+	(*i)->propagateDependencies();
+	for (std::set<FileDesc*>::iterator d = (*i)->_dependencies.begin();
+	     d != (*i)->_dependencies.end();
+	     ++d)
+	  addDependency(*d);
+      }
+  }
+  bool operator == (FileDesc const &f) const {
+    return _path == f._path && _dependencies == f._dependencies;
+  }
+  bool operator > (FileDesc &f) {
+    return (_dependencies.find(&f) != _dependencies.end());
+  }
+  bool operator < (FileDesc &f) {
+    return (f > *this);
+  }
+
+  static FileDesc *create(String const &path) {
+    FileDesc *&fd = fdmap[path];
+    if (fd == NULL) fd = new FileDesc(path);
+    return fd;
+  }
+private:
+  String _path;
+  std::set<FileDesc*> _dependencies;
+  bool _mark;
+  char _buf[BUFLEN];
+  char const *commentString();
+  char *readDependencyLine();
+
+  static std::map<String, FileDesc*, String::caseless> fdmap;
+};
+
+char const *FileDesc::commentString()
+{
+  char const *ext = strrchr (_path.cstr_oneuse(), '.');
+  char const *cmt = NULL;
+  if (strcasecmp(ext, ".sh") == 0)
+    cmt = "#";
+  else if (strcasecmp(ext, ".bat") == 0)
+    cmt = "rem";
+  return cmt;
+}
+
+#define WHITESPACE " \t"
+
+char *FileDesc::readDependencyLine()
+{
+  io_stream *f = io_stream::open(String("file://") + _path, "rt");
+  if (!f)
+    {
+      const char *err = strerror (errno);
+      if (!err)
+	err = "(unknown error)";
+      note (NULL, IDS_ERR_OPEN_READ, _path.cstr_oneuse(), err);
+      return NULL;
+    }
+
+  // Read first line after shbang (if any)
+  f->gets(_buf, BUFLEN);
+  if (*_buf == '#')
+    {
+      char *bang = strtok(_buf+1, WHITESPACE);
+      if (bang != NULL && *bang == '!')
+	f->gets(_buf, BUFLEN);
+      else
+        *(bang+strlen(bang)) = ' ';
+    }
+
+  delete f;
+
+  // Check if a dependency line
+  char const *cmt = commentString();
+
+  //   - has to start with a comment
+  if (strncasecmp(_buf, cmt, strlen(cmt)) != 0)
+    return NULL;
+  //   - has to contain a marker after whitespace
+  char *mstr = strtok(_buf+strlen(cmt), WHITESPACE);
+  if (mstr == NULL)
+    return NULL;
+  *(mstr+strlen(mstr)) = ' ';
+  if (strncasecmp(mstr, DEPEND_STR, strlen(DEPEND_STR)) != 0)
+    return NULL;
+
+  return strchr(mstr, ':')+1;
+}
+
+FileDesc::FileDesc(String const &basePath) : _path(basePath), _mark(false)
+{
+  char *line = readDependencyLine();
+  if (line == NULL)   // Unable to open file or no dependencies
+    return;
+
+  // Parse dependencies
+  for (char *dstr = strtok(line, WHITESPACE);
+       dstr == NULL;
+       dstr = strtok(NULL, WHITESPACE))
+    addDependency(dstr);   // for each dependency
+}
+
+std::map<String, FileDesc*, String::caseless> FileDesc::fdmap;
 
 class RunFindVisitor : public FindVisitor
 {
@@ -33,14 +167,55 @@ public:
   RunFindVisitor (){}
   virtual void visitFile(String const &basePath, const WIN32_FIND_DATA *theFile)
     {
-      run_script ("/etc/postinstall/", theFile->cFileName);
+      files.push_back(FileDesc::create(basePath));
     }
-  virtual ~ RunFindVisitor () {}
+  virtual ~ RunFindVisitor ();
+  virtual void executeAll ();
 protected:
   RunFindVisitor (RunFindVisitor const &);
   RunFindVisitor & operator= (RunFindVisitor const &);
+private:
+  std::list<FileDesc*> files;
+  void checkAndLogMissingDependencies(FileDesc *);
 };
   
+RunFindVisitor::~RunFindVisitor()
+{
+  for (std::list<FileDesc*>::iterator i = files.begin(); i != files.end(); ++i)
+    delete *i;
+}
+
+inline bool lt_fd(FileDesc *f1, FileDesc *f2) { return (*f1) < (*f2); }
+
+void RunFindVisitor::checkAndLogMissingDependencies(FileDesc *f)
+{
+  // Check that all dependencies are executed already
+  for (std::set<FileDesc*>::iterator d = f->dependencies().begin();
+       d != f->dependencies().end();
+       ++d)
+    if (!io_stream::exists (String("file://") + (*d)->path() + ".done"))
+      {
+	char const *msg = "missing";
+	if (!io_stream::exists (String("file://") + (*d)->path()))
+	  msg = "not executed";
+	log (LOG_TIMESTAMP, String("error: dependency ") + (*d)->path() +
+	     " " + msg + " for script " + f->path());
+      }
+}
+
+void RunFindVisitor::executeAll()
+{
+  for (std::list<FileDesc*>::iterator i = files.begin(); i != files.end(); ++i)
+    (*i)->propagateDependencies();
+  // Topological sort
+  files.sort(lt_fd);
+  for (std::list<FileDesc*>::iterator i = files.begin(); i != files.end(); ++i)
+    {
+      checkAndLogMissingDependencies(*i);
+      run_script ("/etc/postinstall/", (*i)->path());
+    }
+}
+
 void
 do_postinstall (HINSTANCE h, HWND owner)
 {
@@ -50,4 +225,5 @@ do_postinstall (HINSTANCE h, HWND owner)
   RunFindVisitor myVisitor;
   String postinst = cygpath ("/etc/postinstall");
   Find (postinst).accept (myVisitor);
+  myVisitor.executeAll();
 }


More information about the Cygwin-apps mailing list