[PATCH] Postinstall script ordering in setup - take 4
Igor Pechtchanski
pechtcha@cs.nyu.edu
Sat Mar 15 20:28:00 GMT 2003
On 15 Mar 2003, Robert Collins wrote:
> On Sat, 2003-03-15 at 05:06, Igor Pechtchanski wrote:
>
> > Ok. Here's that alternative, more reasonably implemented, and *tested*
> > this time :-) (at least as far as reading files, sorting them, and
> > executing scripts is concerned). The dependence-extraction mechanism
> > still needs to be verified, though. Comments and suggestions for
> > improvement are welcome.
> > Igor
> > ==============================================================================
> > ChangeLog:
> > 2003-03-03 Igor Pechtchanski <pechtcha@cs.nyu.edu>
>
> I'd like you do keep the class declaration free of code. It's fine to
> have it in the .cc file if it's really trivial an private, but even then
> can you put the implementations outside the class declaration. (With one
> exception: one-liners.
Done.
> > * 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 script to
> > list instead of running it immediately.
> > (RunFindVisitor::files): New member variable.
> > (RunFindVisitor::checkAndLogMissingDependencies): New
> > member function.
> > (FileDesc): New class that extracts and stores
> > dependencies from a script file.
>
> The logic in here looks almost identical to that in
> packageversion::set_requirements. I don't want to add near-duplicate
> code if we can avoid it. Perhaps extracting the core for both to a
> convenience class?
Sure. Not sure how to do it, though - the logic in
packageversion::set_requirements is a bit confusing (I think it does much
more than my code).
> Your operator > and < appear to have a problem.
>
> foo: bar
> gam: bar
>
> foo > gam = false
> gam < foo = false
> gam == foo = false.
>
> I'd expect stl associative containers to choke on that.
>
> I suggest you do
> {
> /* we are greater than any dependency of ours */
> if (_dependencies.find(&f) != _dependencies.end())
> return true;
> return _path > f._path;
> }
See the note on partial order in
<http://cygwin.com/ml/cygwin-apps/2003-03/msg00440.html>. FWIW, this
works fine if there are no dependences (i.e., the order is always
undefined).
> this
> + const char *err = strerror (errno);
> + if (!err)
> + err = "(unknown error)";
>
> could be usefull extracted so that we can do
> String const errorString ( StringError(errno));
> and always get back a string.
Agreed. I put it in as a static function for now - where should it go?
> > (DEPEND_STR): New #define.
>
> BUFLEN should be a static const member of the class.
Do you mean DEPEND_STR or BUFLEN? In either case, BUFLEN is now inside
the FileDesc namespace (as an enum), and DEPEND_STR is a static const
member.
> > (do_postinstall): Add executeAll() call.
>
> Thanks again for all the work you've put into this.
> [snip]
> Rob
New iteration attached.
Igor
==============================================================================
ChangeLog:
2003-03-15 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 script to
list instead of running it immediately.
(RunFindVisitor::files): New member variable.
(RunFindVisitor::checkAndLogMissingDependencies): New
member function.
(FileDesc): New class that extracts and stores
dependencies from a script file.
(StringError): New static helper function.
(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 15 Mar 2003 20:20:45 -0000
@@ -26,6 +26,147 @@ 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>
+
+class FileDesc
+{
+public:
+ FileDesc(String const &filename);
+ 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) { return addDependency(create(deps)); }
+ void propagateDependencies();
+ bool operator == (FileDesc const &f) const {
+ return _path == f._path && _dependencies == f._dependencies;
+ }
+ bool operator < (FileDesc &f) {
+ return (f._dependencies.find(this) != f._dependencies.end());
+ }
+ static FileDesc *create(String const &path);
+private:
+ enum { BUFLEN = 500 };
+ String _path;
+ std::set<FileDesc*> _dependencies;
+ bool _mark;
+ char _buf[BUFLEN];
+ char const *commentString() const;
+ char *readDependencyLine();
+
+ static char const * const DEPEND_STR;
+ static std::map<String, FileDesc*, String::caseless> fdmap;
+};
+
+void FileDesc::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);
+ }
+}
+
+FileDesc *FileDesc::create(String const &path)
+{
+ // TODO: free unused map entries
+ FileDesc *&fd = fdmap[path];
+ if (fd == NULL) fd = new FileDesc(path);
+ return fd;
+}
+
+char const *FileDesc::commentString() const
+{
+ 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;
+}
+
+static String const StringError(int errnum)
+{
+ char const *err = strerror (errnum);
+ if (err == NULL)
+ err = "(unknown error)";
+ return String(err);
+}
+
+#define WHITESPACE " \t"
+
+char *FileDesc::readDependencyLine()
+{
+ io_stream *f = io_stream::open(String("cygfile://") + _path, "rt");
+ if (!f)
+ {
+ log (LOG_TIMESTAMP, String("error: unable to open script ") +
+ _path + " for reading: " + StringError(errno));
+ 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 be a known type of script
+ if (cmt == NULL)
+ return NULL;
+ // - 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 &filename) : _path(filename), _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;
+char const * const FileDesc::DEPEND_STR = "depends on: ";
class RunFindVisitor : public FindVisitor
{
@@ -33,14 +174,62 @@ public:
RunFindVisitor (){}
virtual void visitFile(String const &basePath, const WIN32_FIND_DATA *theFile)
{
- run_script ("/etc/postinstall/", theFile->cFileName);
+ files.push_back(FileDesc::create(String ("/etc/postinstall/") +
+ theFile->cFileName));
}
- 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("cygfile://") + (*d)->path() + ".done"))
+ {
+ char const *msg = "missing";
+ if (!io_stream::exists (String("cygfile://") + (*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);
+
+ String all_files = "";
+ for (std::list<FileDesc*>::iterator i = files.begin(); i != files.end(); ++i)
+ all_files += String(" ") + (*i)->path();
+ log (LOG_TIMESTAMP, String("Script order:") + all_files);
+
+ for (std::list<FileDesc*>::iterator i = files.begin(); i != files.end(); ++i)
+ {
+ checkAndLogMissingDependencies(*i);
+ run_script ("", (*i)->path());
+ }
+}
+
void
do_postinstall (HINSTANCE h, HWND owner)
{
@@ -50,4 +239,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