[PATCH] setup: Handle the package validation exception

[Igor Peshansky]

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  <pechtcha@cs.nyu.edu>

	* package_version.cc (packageversion::scan): Catch and handle
	validation exception.

-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_	    pechtcha@cs.nyu.edu | igor@watson.ibm.com
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"
-------------- next part --------------
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