[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.

Ping on this one...

> 	Igor
> ==============================================================================
> 2006-01-23  Igor Peshansky  <pechtcha@cs.nyu.edu>
>
> 	* package_version.cc (packageversion::scan): Catch and handle
> 	validation exception.

And ping on this.  The patch discussion thread starts at
<http://cygwin.com/ml/cygwin-apps/2006-01/msg00204.html>.
	Igor
-- 
				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