This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Postinstall script ordering in setup - take 3


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 at cs dot nyu dot 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.

> 	* 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?

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;
}

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.


> 	(DEPEND_STR): New #define.

BUFLEN should be a static const member of the class.

> 	(do_postinstall): Add executeAll() call.

Thanks again for all the work you've put into this.

I've actually got some time today (gasp!), so I'm going to review the
current install script ordering - I'd like to apply your work to the
package dependency script ordering, which is currently not implemented.

Rob

-- 
GPG key available at: <http://users.bigpond.net.au/robertc/keys.txt>.

Attachment: signature.asc
Description: This is a digitally signed message part


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]