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: Setup showstopper


Dave Korn wrote:
Hey all,

 As long as we're talking about setup, let me raise the one I've just run
into:

When there's an error validating the checksum of a package (to be precise,
when validateCachedPackage() returns an error indication to
check_for_cached() in download.cc), setup raises an exception and aborts,
because there's no handler to catch it. For example:


Fatal Error: Uncaught Exception
Thread: DialogProc
Type: 9Exception
Message: Package validation failure for
file:///...path..to.../release/X11/fontconfig/libfontconfig-devel/libfontcon
fig-devel-2.2.2-1.tar.bz2
AppErrNo: 1

 This isn't very helpful, as it will only fail again in the same way next
time it gets to try and validate that package.

In other words, a validation error becomes permanent and fatal and there's
no way to ever recover/repair/upgrade/revert your cygwin installation
without manually going into the local package dir, finding the relevant
file, and getting rid of it. Which is not newbie-friendly. (It can of
course be worked around with --no-md5, but no newbie is going to type "setup
--help" and guess where the hell the output has gone!).

Known issue, though currently, I'm not sure what the best UI for this situation is.


1)  Wouldn't the best thing be to just delete (or rename-aside, or even
ignore) the apparently-corrupt package?

Yes. I think probably "rename-aside, plus notify the user" is probably the best way to go.


2)  check_for_cached is supposed to return 0 for failure.  Why is it
throwing an exception?

There is a difference between "not cached" and "corrupt".


3) The functions download_one and do_download_thread, which call
check_for_cached, do in fact wrap the call in try...except handlers, but as
soon as they catch the exception they pass it straight to fatal (...); why
should it be regarded as a fatal error?

All exceptions are fatal until someone has designed a better way to handle them.


4) The only function to call check_for_cached as an extern is
packageversion::scan in package_version.h; this function fails to wrap it in
any kind of exception handler at all, which is presumably because someone
looked at the check_for_cached function, saw the comment about "Return 0 for
failure", and didn't look through the source and see that it might also
raise an exception.

The exceptions were added after that package_version.h code was written. Setup's code is in flux between design patterns.


So, what do you reckon? Should I make check_for_cached just quietly
return zero if the package validates wrong? That would (I think) have the
effect of making setup redownload the package and overwrite the old corrupt
one, wouldn't it?

I think at very least we should tell the user.


Or should I leave it to the callers to decide what to do, but perhaps find
a better error return mechanism (zero for 'not found', -ve for 'error', +ve
for 'success' perhaps?) that's more logical and maintainable?

I don't think that is logical or maintainable.


Either an enum of possibilities, or a simple bool, with exceptions for exceptional cases.

BTW, I am working on this too. There are additional complications... like why the local package scan is being run in "Download" and "Local" modes, but not in "Install" mode.

Max.


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