[PATCH 3/4] Delay converting Requires entries to Packages until after parsing

Dan Nicholson dbn.lists at gmail.com
Tue May 29 23:05:45 PDT 2012


When the parser encounters Requires or Requires.private, it immediately
tries to sees if we have a parsed package for that entry. If not it
tries to locate the needed file and parse it out. If there's a circular
dependency, this will eventually error opening too many files.

Instead, just store the requires entries so the parsing completes and
the package is added to the database. After parsing, the entries can be
resolved into Packages and any circular requires entries will find the
first package in the database.

This is a partial fix for Freedesktop #7331.
---
 parse.c |   76 +++-----------------------------------------------------------
 pkg.c   |   63 ++++++++++++++++++++++++++++++++++++++++++++++++----
 pkg.h   |    2 +
 3 files changed, 64 insertions(+), 77 deletions(-)

diff --git a/parse.c b/parse.c
index 6e2a389..38042a4 100644
--- a/parse.c
+++ b/parse.c
@@ -523,10 +523,8 @@ parse_module_list (Package *pkg, const char *str, const char *path)
 static void
 parse_requires (Package *pkg, const char *str, const char *path)
 {
-  GSList *parsed;
-  GSList *iter;
   char *trimmed;
-  
+
   if (pkg->requires)
     {
       verbose_error ("Requires field occurs twice in '%s'\n", path);
@@ -535,45 +533,15 @@ parse_requires (Package *pkg, const char *str, const char *path)
     }
 
   trimmed = trim_and_sub (pkg, str, path);
-  parsed = parse_module_list (pkg, trimmed, path);
+  pkg->requires_entries = parse_module_list (pkg, trimmed, path);
   g_free (trimmed);
-  
-  iter = parsed;
-  while (iter != NULL)
-    {
-      Package *req;
-      RequiredVersion *ver = iter->data;
-      
-      req = get_package (ver->name);
-
-      if (req == NULL)
-        {
-          verbose_error ("Package '%s', required by '%s', not found\n",
-                         ver->name, pkg->name ? pkg->name : path);
-          
-          exit (1);
-        }
-
-      if (pkg->required_versions == NULL)
-        pkg->required_versions = g_hash_table_new (g_str_hash, g_str_equal);
-      
-      g_hash_table_insert (pkg->required_versions, ver->name, ver);
-      
-      pkg->requires = g_slist_prepend (pkg->requires, req);
-
-      iter = g_slist_next (iter);
-    }
-
-  g_slist_free (parsed);
 }
 
 static void
 parse_requires_private (Package *pkg, const char *str, const char *path)
 {
-  GSList *parsed;
-  GSList *iter;
   char *trimmed;
-  
+
   if (pkg->requires_private)
     {
       verbose_error ("Requires.private field occurs twice in '%s'\n", path);
@@ -582,36 +550,8 @@ parse_requires_private (Package *pkg, const char *str, const char *path)
     }
 
   trimmed = trim_and_sub (pkg, str, path);
-  parsed = parse_module_list (pkg, trimmed, path);
+  pkg->requires_private_entries = parse_module_list (pkg, trimmed, path);
   g_free (trimmed);
-  
-  iter = parsed;
-  while (iter != NULL)
-    {
-      Package *req;
-      RequiredVersion *ver = iter->data;
-      
-      req = get_package (ver->name);
-
-      if (req == NULL)
-        {
-          verbose_error ("Package '%s', required by '%s', not found\n",
-                         ver->name, pkg->name ? pkg->name : path);
-          
-          exit (1);
-        }
-
-      if (pkg->required_versions == NULL)
-        pkg->required_versions = g_hash_table_new (g_str_hash, g_str_equal);
-      
-      g_hash_table_insert (pkg->required_versions, ver->name, ver);
-      
-      pkg->requires_private = g_slist_prepend (pkg->requires_private, req);
-
-      iter = g_slist_next (iter);
-    }
-
-  g_slist_free (parsed);
 }
 
 static void
@@ -1144,14 +1084,6 @@ parse_package_file (const char *path, gboolean ignore_requires,
   g_string_free (str, TRUE);
   fclose(f);
 
-  /* make ->requires_private include a copy of the public requires too */
-  pkg->requires_private = g_slist_concat(g_slist_copy (pkg->requires),
-					 pkg->requires_private);
-  
-  pkg->requires = g_slist_reverse (pkg->requires);
-  
-  pkg->requires_private = g_slist_reverse (pkg->requires_private);
-
   pkg->I_cflags = g_slist_reverse (pkg->I_cflags);
   pkg->other_cflags = g_slist_reverse (pkg->other_cflags);
 
diff --git a/pkg.c b/pkg.c
index 04f6467..c640dfa 100644
--- a/pkg.c
+++ b/pkg.c
@@ -264,6 +264,7 @@ internal_get_package (const char *name, gboolean warn)
 {
   Package *pkg = NULL;
   const char *location;
+  GSList *iter;
   
   pkg = g_hash_table_lookup (packages, name);
 
@@ -349,13 +350,65 @@ internal_get_package (const char *name, gboolean warn)
   debug_spew ("Path position of '%s' is %d\n",
               pkg->name, pkg->path_position);
   
-  verify_package (pkg);
-
-  debug_spew ("Adding '%s' to list of known packages, returning as package '%s'\n",
-              pkg->key, name);
-  
+  debug_spew ("Adding '%s' to list of known packages\n", pkg->key);
   g_hash_table_insert (packages, pkg->key, pkg);
 
+  /* pull in Requires packages */
+  for (iter = pkg->requires_entries; iter != NULL; iter = g_slist_next (iter))
+    {
+      Package *req;
+      RequiredVersion *ver = iter->data;
+
+      debug_spew ("Searching for '%s' requirement '%s'\n",
+                  pkg->name, ver->name);
+      req = internal_get_package (ver->name, warn);
+      if (req == NULL)
+        {
+          verbose_error ("Package '%s', required by '%s', not found\n",
+                         ver->name, pkg->name);
+          exit (1);
+        }
+
+      if (pkg->required_versions == NULL)
+        pkg->required_versions = g_hash_table_new (g_str_hash, g_str_equal);
+
+      g_hash_table_insert (pkg->required_versions, ver->name, ver);
+      pkg->requires = g_slist_prepend (pkg->requires, req);
+    }
+
+  /* pull in Requires.private packages */
+  for (iter = pkg->requires_private_entries; iter != NULL;
+       iter = g_slist_next (iter))
+    {
+      Package *req;
+      RequiredVersion *ver = iter->data;
+
+      debug_spew ("Searching for '%s' private requirement '%s'\n",
+                  pkg->name, ver->name);
+      req = internal_get_package (ver->name, warn);
+      if (req == NULL)
+        {
+          verbose_error ("Package '%s', required by '%s', not found\n",
+			 ver->name, pkg->name);
+          exit (1);
+        }
+
+      if (pkg->required_versions == NULL)
+        pkg->required_versions = g_hash_table_new (g_str_hash, g_str_equal);
+
+      g_hash_table_insert (pkg->required_versions, ver->name, ver);
+      pkg->requires_private = g_slist_prepend (pkg->requires_private, req);
+    }
+
+  /* make requires_private include a copy of the public requires too */
+  pkg->requires_private = g_slist_concat(g_slist_copy (pkg->requires),
+                                         pkg->requires_private);
+
+  pkg->requires = g_slist_reverse (pkg->requires);
+  pkg->requires_private = g_slist_reverse (pkg->requires_private);
+
+  verify_package (pkg);
+
   return pkg;
 }
 
diff --git a/pkg.h b/pkg.h
index ef18af0..d8ec085 100644
--- a/pkg.h
+++ b/pkg.h
@@ -52,7 +52,9 @@ struct _Package
   char *description;
   char *url;
   char *pcfiledir; /* directory it was loaded from */
+  GSList *requires_entries;
   GSList *requires;
+  GSList *requires_private_entries;
   GSList *requires_private;
   GSList *l_libs;
   char   *l_libs_merged;
-- 
1.7.7.6



More information about the pkg-config mailing list