This is the mail archive of the cygwin-apps 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] |
On Fri, Jul 23, 2010 at 06:45:47PM +0100, Jon TURNEY wrote:> scripts after the first one fails.Here's a small patch for setup.exe which causes setup to indicate if a postinstall script didn't run successfully.
This should help avoid the situation where the postinstall scripts fail to run and the user has a broken installation, but they don't notice until they try to run something which relies on postinstall scripts, e.g [1] and I'm sure there are other examples.
[1] http://cygwin.com/ml/cygwin-xfree/2010-07/msg00084.html
ChangeLog:
* postinstall.cc : Note if any postinstall script fails and tell the user with an error messagebox * script.cc (run): Don't rename script as .done if it didn't run successfully
Thanks for doing this. I like the idea but I have some stylistic comments.
Index: postinstall.cc =================================================================== RCS file: /cvs/cygwin-apps/setup/postinstall.cc,v retrieving revision 2.23 diff -u -p -r2.23 postinstall.cc --- postinstall.cc 11 May 2009 10:49:15 -0000 2.23 +++ postinstall.cc 23 Jul 2010 17:34:30 -0000 @@ -64,7 +64,7 @@ private: class RunScript : public unary_function<Script const&, int> { public: - RunScript(const std::string& name, int num) : _num(num), _cnt(0) + RunScript(const std::string& name, int num) : _num(num), _cnt(0), _no_errors(TRUE) { Progress.SetText2 (name.c_str()); Progress.SetBar1 (_cnt, _num); @@ -80,16 +80,26 @@ public: retval = aScript.run(); ++_cnt; Progress.SetBar1 (_cnt, _num); + + if ((retval != 0)&& (retval != -ERROR_INVALID_DATA)) + _no_errors = FALSE; + return retval; } + bool success(void) + { + return _no_errors; + } private: int _num; int _cnt; + bool _no_errors; };
-static void +static bool do_postinstall_thread (HINSTANCE h, HWND owner) { + bool success = TRUE; Progress.SetText1 ("Running..."); Progress.SetText2 (""); Progress.SetText3 (""); @@ -114,8 +124,8 @@ do_postinstall_thread (HINSTANCE h, HWND { packagemeta& pkg = **i;
- for_each (pkg.installed.scripts().begin(), pkg.installed.scripts().end(), - RunScript(pkg.name, pkg.installed.scripts().size())); + success&= for_each (pkg.installed.scripts().begin(), pkg.installed.scripts().end(), + RunScript(pkg.name, pkg.installed.scripts().size())).success();
I'd prefer a simple if here like: if (success) success = ...
But, OTOH, if I'm reading this correctly, you stop doing the install
if (success) success = f();
for each package for each file in /etc/postinstall belonging to that package
I think it may be better to record the failing scripts, keep going, and then report the package names which failed rather than forcing the user to look around.
Attachment:
postinstall_failure_error2.patch
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |