gobject-introspection: continue searching include path on parse error or version conflict of .gir files

[Mongoose <mongoose synx us to>]

I was trying out the latest GIT and putting it off in /var/opt/gnome to
avoid conflict with my stable libraries, but I noticed something about
g-object-introspection that messed that up. In /usr/share/gir-1.0 I have
for instance a file called "GModule-2.0.gir" and within it is the stable
version for the object, version="1.0". When installing gir-repositories
in /var/opt/gnome though I ended up with a file in
/var/opt/gnome/share/gir-1.0 named "GModule-2.0.gir" with testing
version, version="1.2".

The trouble is, when g-ir-scanner or g-ir-compiler search through the
.gir files for an appropriate one, they return the first file they find
by name and end the search, before checking if that file contains too
early a version. So that means if I need version = "1.2" I'm in trouble
because when it finds the first Gmodule-2.0.gir file it assumes that
file is the correct one, and then errors out when it's not. What would
be better is to catch that kind of error and continue searching for
other Gmodule-2.0.gir files, which may have higher versions, until you
find one that meets the minimum required version. You could name the
files Gmodule-2.0-1.0.gir but that's kind of cluttery and most people
will only have one .gir file for each type. It's just convenient for
developers and hackers and such to have support for it checking all
possible .gir files, and won't result in a lot of extra overhead.

So I made a patch to g-object-introspection in GIT that adds support for
continuing to search through .gir files upon error. I tested it when
compiling the GIT of gdk-pixbuf and it worked great. I don't uh... have
git write access though, so I'll just attach the patch here. It's pretty
small, and should describe the sort of feature I'm looking for a lot
better than I am doing just writing in here. Substituting yield for
return worked pretty good in the python, and for the C I just applied a
bit of CPS to get the repeated search done via a callback.

diff --git a/girepository/girparser.c b/girepository/girparser.c
index cff2305..52e158a 100644
--- a/girepository/girparser.c
+++ b/girepository/girparser.c
@@ -256,13 +256,15 @@ static GMarkupParser firstpass_parser =
   NULL,
 };
 
-static char *
+static gboolean
 locate_gir (GIrParser  *parser,
-	    const char *girname)
+			const char *girname,
+			gboolean (*handle)(const char*))
 {
   const gchar *const *datadirs;
   const gchar *const *dir;
   char *path = NULL;
+  gboolean result = FALSE;
 
   datadirs = g_get_system_data_dirs ();
 
@@ -271,26 +273,33 @@ locate_gir (GIrParser  *parser,
       for (dir = (const gchar *const *)parser->includes; *dir; dir++)
 	{
 	  path = g_build_filename (*dir, girname, NULL);
-	  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
-	    return path;
+	  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+		  result = handle(path);
+	  }
 	  g_free (path);
 	  path = NULL;
+	  if(result==TRUE) return TRUE;
 	}
     }
   for (dir = datadirs; *dir; dir++)
     {
       path = g_build_filename (*dir, GIR_SUFFIX, girname, NULL);
-      if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
-	return path;
+      if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+		  result = handle(path);
+	  }
+
       g_free (path);
       path = NULL;
+	  if(result==TRUE) return TRUE;
     }
 
   path = g_build_filename (GIR_DIR, girname, NULL);
-  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
-    return path;
+  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+	  result = handle(path);
+  }
   g_free (path);
-  return NULL;
+  result |= handle(NULL);
+  return result;
 }
 
 #define MISSING_ATTRIBUTE(ctx,error,element,attribute)			        \
@@ -2604,65 +2613,70 @@ parse_include (GMarkupParseContext *context,
   GError *error = NULL;
   gchar *buffer;
   gsize length;
-  gchar *girpath, *girname;
+  gchar *girname;
   GIrModule *module;
   GList *l;
 
   for (l = ctx->parser->parsed_modules; l; l = l->next)
-    {
-      GIrModule *m = l->data;
-
-      if (strcmp (m->name, name) == 0)
-	{
-	  if (strcmp (m->version, version) == 0)
-	    {
-	      ctx->include_modules = g_list_prepend (ctx->include_modules, m);
+	  {
+		  GIrModule *m = l->data;
+
+		  if (strcmp (m->name, name) == 0)
+			  {
+				  if (strcmp (m->version, version) == 0)
+					  {
+						  ctx->include_modules = g_list_prepend (ctx->include_modules, m);
+
+						  return TRUE;
+					  }
+				  else
+					  {
+						  g_printerr ("Module '%s' imported with conflicting versions '%s' and '%s'\n",
+									  name, m->version, version);
+						  return FALSE;
+					  }
+			  }
+	  }
 
-	      return TRUE;
-	    }
-	  else
-	    {
-	      g_printerr ("Module '%s' imported with conflicting versions '%s' and '%s'\n",
-			  name, m->version, version);
-	      return FALSE;
-	    }
-	}
-    }
+  gboolean handle_girpath(const char* girpath) {
+	  if (girpath == NULL)
+		  {
+			  g_printerr ("Could not find GIR file '%s'; check XDG_DATA_DIRS or use --includedir\n",
+						  girname);
+			  return FALSE;
+		  }
+
+	  g_debug ("Parsing include %s\n", girpath);
+
+	  if (!g_file_get_contents (girpath, &buffer, &length, &error))
+		  {
+			  g_printerr ("%s: %s\n", girpath, error->message);
+			  g_clear_error (&error);
+			  return FALSE;
+		  }
+
+	  // set module of wrapper function here:
+	  module = g_ir_parser_parse_string (ctx->parser, name, girpath, buffer, length, &error);
+	  if (error != NULL)
+		  {
+			  int line_number, char_number;
+			  g_markup_parse_context_get_position (context, &line_number, &char_number);
+			  g_printerr ("%s:%d:%d: error: %s\n", girpath, line_number, char_number, error->message);
+			  g_clear_error (&error);
+			  return FALSE;
+		  }
+
+	  return TRUE;
+  }
 
   girname = g_strdup_printf ("%s-%s.gir", name, version);
-  girpath = locate_gir (ctx->parser, girname);
-
-  if (girpath == NULL)
-    {
-      g_printerr ("Could not find GIR file '%s'; check XDG_DATA_DIRS or use --includedir\n",
-		   girname);
-      g_free (girname);
-      return FALSE;
-    }
+  gboolean ok = locate_gir (ctx->parser, girname, handle_girpath);
   g_free (girname);
+  g_free (buffer);
 
-  g_debug ("Parsing include %s\n", girpath);
+  if(ok==FALSE) return FALSE;
 
-  if (!g_file_get_contents (girpath, &buffer, &length, &error))
-    {
-      g_printerr ("%s: %s\n", girpath, error->message);
-      g_clear_error (&error);
-      g_free (girpath);
-      return FALSE;
-    }
-
-  module = g_ir_parser_parse_string (ctx->parser, name, girpath, buffer, length, &error);
-  g_free (buffer);
-  if (error != NULL)
-    {
-      int line_number, char_number;
-      g_markup_parse_context_get_position (context, &line_number, &char_number);
-      g_printerr ("%s:%d:%d: error: %s\n", girpath, line_number, char_number, error->message);
-      g_clear_error (&error);
-      g_free (girpath);
-      return FALSE;
-    }
-  g_free (girpath);
+  // module has been set by callback if ok != FALSE
 
   ctx->include_modules = g_list_append (ctx->include_modules,
 					module);
@@ -2966,8 +2980,9 @@ start_element_handler (GMarkupParseContext *context,
 	    g_set_error (error,
 			 G_MARKUP_ERROR,
 			 G_MARKUP_ERROR_INVALID_CONTENT,
-			 "Unsupported version '%s'",
-			 version);
+			 "Unsupported version '%s' != '%s'",
+					 version,
+					 SUPPORTED_GIR_VERSION);
 	  else
 	    state_switch (ctx, STATE_REPOSITORY);
 
diff --git a/giscanner/girparser.py b/giscanner/girparser.py
index ab2b29d..b7c2ab7 100644
--- a/giscanner/girparser.py
+++ b/giscanner/girparser.py
@@ -116,9 +116,9 @@ class GIRParser(object):
         assert root.tag == _corens('repository')
         version = root.attrib['version']
         if version != COMPATIBLE_GIR_VERSION:
-            raise SystemExit("%s: Incompatible version %s (supported: %s)" \
-                             % (self._get_current_file(),
-                                version, COMPATIBLE_GIR_VERSION))
+            raise RuntimeError("%s: Incompatible version %s (supported: %s)" \
+                                   % (self._get_current_file(),
+                                      version, COMPATIBLE_GIR_VERSION))
 
         for node in root.getchildren():
             if node.tag == _corens('include'):
diff --git a/giscanner/transformer.py b/giscanner/transformer.py
index 1acc67a..4126e39 100644
--- a/giscanner/transformer.py
+++ b/giscanner/transformer.py
@@ -42,6 +42,8 @@ class TransformerException(Exception):
 _xdg_data_dirs = [x for x in os.environ.get('XDG_DATA_DIRS', '').split(':') \
                       + [DATADIR, '/usr/share'] if x]
 
+class IncludeFailed(Exception): pass
+
 class Transformer(object):
     namespace = property(lambda self: self._namespace)
 
@@ -109,9 +111,15 @@ class Transformer(object):
     def register_include(self, include):
         if include in self._include_names:
             return
-        filename = self._find_include(include)
-        self._parse_include(filename)
-        self._include_names.add(include)
+        savedException = None
+        for filename in self._find_include(include):
+            try:
+                self._parse_include(filename)
+                self._include_names.add(include)
+                return
+            except Exception,e: 
+                savedException = e
+        raise IncludeFailed("Couldn't find include %r (%r) (search path: %r)\n" % (include,savedException,self.searchdirs))
 
     def register_include_uninstalled(self, include_path):
         basename = os.path.basename(include_path)
@@ -150,19 +158,16 @@ None."""
     # Private
 
     def _find_include(self, include):
-        searchdirs = self._includepaths[:]
+        self.searchdirs = self._includepaths[:]
         for path in _xdg_data_dirs:
-            searchdirs.append(os.path.join(path, GIR_SUFFIX))
-        searchdirs.append(GIR_DIR)
+            self.searchdirs.append(os.path.join(path, GIR_SUFFIX))
+        self.searchdirs.append(GIR_DIR)
 
         girname = '%s-%s.gir' % (include.name, include.version)
-        for d in searchdirs:
+        for d in self.searchdirs:
             path = os.path.join(d, girname)
             if os.path.exists(path):
-                return path
-        sys.stderr.write("Couldn't find include %r (search path: %r)\n"\
-                         % (girname, searchdirs))
-        sys.exit(1)
+                yield path
 
     def _parse_include(self, filename, uninstalled=False):
         parser = self._cachestore.load(filename)