On Mon, 23 Jan 2006, Igor Peshansky wrote:
> On Mon, 23 Jan 2006, Brian Dessent wrote:
>
> > Igor Peshansky wrote:
> >
> > > I've looked at this a bit. Here's the weird part: the error says
> > > "Uncaught Exception", but all the throws of that exception appear to be
> > > properly wrapped in try/catch blocks. So a simple "change exception into
> > > an mbox" kind of solution won't work here. More debugging is needed.
> >
> > The reason for the box is that the md5 checking was changed to run in a
> > different thread than originally designed (now in the main thread
> > instead of the download thread IIRC) and that thread does not have any
> > particular catch handler for that exception, only the TOPLEVEL_CATCH
> > which punts with the generic error.
>
> Do you mean packagemeta::ScanDownloadedFiles() calling
> packageversion::scan(), which calls check_for_cached()? Then yes, this
> isn't properly wrapped in a try/catch. I'd like to verify that this is
> the culprit (when someone sends me the corrupt tarball), but I think I see
> a proper fix for this. Will submit a patch shortly.
And here's the patch (attached). The ChangeLog is below.
While we're at it, I found another weird bit of code in setup. Would
someone please enlighten me on how the semantics would change if the
following patch were applied:
Index: package_meta.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_meta.cc,v
retrieving revision 2.50
diff -u -p -r2.50 package_meta.cc
--- package_meta.cc 11 Sep 2005 14:45:54 -0000 2.50
+++ package_meta.cc 23 Jan 2006 21:18:10 -0000
@@ -620,8 +620,8 @@ packagemeta::ScanDownloadedFiles ()
while (i != pkg.versions.end ())
{
/* scan doesn't alter operator == for packageversions */
- const_cast<packageversion &>(*i).scan ();
packageversion foo = *i;
+ foo.scan ();
packageversion pkgsrcver = foo.sourcePackage ();
pkgsrcver.scan ();
The current code looks way too complex, and my alternative looks cleaner,
but there may be an obscure C++ issue I'm missing.
Igor
==============================================================================
2006-01-23 Igor Peshansky <[EMAIL PROTECTED]>
* package_version.cc (packageversion::scan): Catch and handle
validation exception.
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_ [EMAIL PROTECTED] | [EMAIL PROTECTED]
ZZZzz /,`.-'`' -. ;-;;,_ Igor Peshansky, Ph.D. (name changed!)
|,4- ) )-,_. ,\ ( `'-' old name: Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"Index: package_version.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/package_version.cc,v
retrieving revision 2.24
diff -u -p -r2.24 package_version.cc
--- package_version.cc 14 May 2005 12:30:32 -0000 2.24
+++ package_version.cc 23 Jan 2006 21:20:41 -0000
@@ -29,6 +29,7 @@ static const char *cvsid =
#include "resource.h"
#include <algorithm>
#include "download.h"
+#include "Exception.h"
using namespace std;
@@ -322,8 +323,22 @@ packageversion::scan()
* FIXME: This is a bit of a hack. a better way is to abstract
* the availability logic to the package
*/
- if (!check_for_cached (*(source())) && ::source == IDC_SOURCE_CWD)
- source()->sites.clear();
+ try
+ {
+ if (!check_for_cached (*(source())) && ::source == IDC_SOURCE_CWD)
+ source()->sites.clear();
+ }
+ catch (Exception * e)
+ {
+ // We can ignore these, since we're clearing the source list anyway
+ if (e->errNo() == APPERR_CORRUPT_PACKAGE)
+ {
+ source()->sites.clear();
+ return;
+ }
+ // Unexpected exception.
+ throw e;
+ }
}
static bool
