Attachment 138201 Details for Bug 105572 – Simplify requires lists

[patch] Simplify requires lists

Simplify-requires-lists.patch (text/plain), 12.00 KB, created by Xavier Claessens on 2018-03-19 15:23:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Xavier Claessens

Created: 2018-03-19 15:23:35 UTC

Size: 12.00 KB

patch

obsolete

>From b8bcd4f7491bb91a0b635abb7061d72689c86fbb Mon Sep 17 00:00:00 2001
>From: Xavier Claessens <xavier.claessens@collabora.com>
>Date: Sun, 18 Mar 2018 11:51:58 -0400
>Subject: [PATCH] Simplify requires lists
>
>- Lists of names are useless, better keep list of RequireVersion to
>always have the context.
>- Since we keep lists of RequireVersion we no longer need an hash table
>to map package name to their context.
>- No longer append requires to requires_private
>
>This allows to factor out log of duplicated code and will simplify
>adding a 3rd category of requires.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=105572
>---
> main.c  |  72 ++++++++++++------------------
> parse.c |   4 +-
> pkg.c   | 153 +++++++++++++++++++++++++++-------------------------------------
> pkg.h   |   8 ++--
> 4 files changed, 97 insertions(+), 140 deletions(-)
>
>diff --git a/main.c b/main.c
>index 9b27d9a..5e95d18 100644
>--- a/main.c
>+++ b/main.c
>@@ -482,6 +482,30 @@ static const GOptionEntry options_table[] = {
>   { NULL, 0, 0, 0, NULL, NULL, NULL }
> };
> 
>+static void
>+print_requires (GList *packages, gboolean private)
>+{
>+  GList *pkgtmp;
>+  for (pkgtmp = packages; pkgtmp != NULL; pkgtmp = g_list_next (pkgtmp))
>+    {
>+      Package *pkg = pkgtmp->data;
>+      GList *reqtmp;
>+
>+      reqtmp = private ? pkg->requires_private : pkg->requires;
>+      for (; reqtmp != NULL; reqtmp = g_list_next (reqtmp))
>+        {
>+          RequiredVersion *req = reqtmp->data;
>+          Package *deppkg = req->package;
>+          if (req->comparison == ALWAYS_MATCH)
>+            printf ("%s\n", deppkg->key);
>+          else
>+            printf ("%s %s %s\n", deppkg->key,
>+              comparison_to_str(req->comparison),
>+              req->version);
>+        }
>+    }
>+}
>+
> int
> main (int argc, char **argv)
> {
>@@ -763,53 +787,13 @@ main (int argc, char **argv)
> 
>   if (want_requires)
>     {
>-      GList *pkgtmp;
>-      for (pkgtmp = packages; pkgtmp != NULL; pkgtmp = g_list_next (pkgtmp))
>-        {
>-          Package *pkg = pkgtmp->data;
>-          GList *reqtmp;
>-
>-          /* process Requires: */
>-          for (reqtmp = pkg->requires; reqtmp != NULL; reqtmp = g_list_next (reqtmp))
>-            {
>-              Package *deppkg = reqtmp->data;
>-              RequiredVersion *req;
>-              req = g_hash_table_lookup(pkg->required_versions, deppkg->key);
>-              if ((req == NULL) || (req->comparison == ALWAYS_MATCH))
>-                printf ("%s\n", deppkg->key);
>-              else
>-                printf ("%s %s %s\n", deppkg->key,
>-                  comparison_to_str(req->comparison),
>-                  req->version);
>-            }
>-        }
>+      /* process Requires: */
>+      print_requires (packages, FALSE);
>     }
>   if (want_requires_private)
>     {
>-      GList *pkgtmp;
>-      for (pkgtmp = packages; pkgtmp != NULL; pkgtmp = g_list_next (pkgtmp))
>-        {
>-          Package *pkg = pkgtmp->data;
>-          GList *reqtmp;
>-          /* process Requires.private: */
>-          for (reqtmp = pkg->requires_private; reqtmp != NULL; reqtmp = g_list_next (reqtmp))
>-            {
>-
>-              Package *deppkg = reqtmp->data;
>-              RequiredVersion *req;
>-
>-              if (g_list_find (pkg->requires, reqtmp->data))
>-                continue;
>-
>-              req = g_hash_table_lookup(pkg->required_versions, deppkg->key);
>-              if ((req == NULL) || (req->comparison == ALWAYS_MATCH))
>-                printf ("%s\n", deppkg->key);
>-              else
>-                printf ("%s %s %s\n", deppkg->key,
>-                  comparison_to_str(req->comparison),
>-                  req->version);
>-            }
>-        }
>+      /* process Requires.private: */
>+      print_requires (packages, TRUE);
>     }
>   
>   /* Print all flags; then print a newline at the end. */
>diff --git a/parse.c b/parse.c
>index 6e9907c..c96305b 100644
>--- a/parse.c
>+++ b/parse.c
>@@ -546,7 +546,7 @@ parse_requires (Package *pkg, const char *str, const char *path)
>     }
> 
>   trimmed = trim_and_sub (pkg, str, path);
>-  pkg->requires_entries = parse_module_list (pkg, trimmed, path);
>+  pkg->requires = parse_module_list (pkg, trimmed, path);
>   g_free (trimmed);
> }
> 
>@@ -565,7 +565,7 @@ parse_requires_private (Package *pkg, const char *str, const char *path)
>     }
> 
>   trimmed = trim_and_sub (pkg, str, path);
>-  pkg->requires_private_entries = parse_module_list (pkg, trimmed, path);
>+  pkg->requires_private = parse_module_list (pkg, trimmed, path);
>   g_free (trimmed);
> }
> 
>diff --git a/pkg.c b/pkg.c
>index f29ecc7..e3e5830 100644
>--- a/pkg.c
>+++ b/pkg.c
>@@ -219,6 +219,24 @@ package_init (gboolean want_list)
>     add_virtual_pkgconfig_package ();
> }
> 
>+static void
>+internal_get_package_foreach (gpointer data, gpointer user_data)
>+{
>+  gboolean warn = GPOINTER_TO_INT (user_data);
>+  RequiredVersion *ver = data;
>+  Package *pkg = ver->owner;
>+
>+  debug_spew ("Searching for '%s' requirement '%s'\n",
>+              pkg->key, ver->name);
>+  ver->package = internal_get_package (ver->name, warn);
>+  if (ver->package == NULL)
>+    {
>+      verbose_error ("Package '%s', required by '%s', not found\n",
>+                     ver->name, pkg->key);
>+      exit (1);
>+    }
>+}
>+
> static Package *
> internal_get_package (const char *name, gboolean warn)
> {
>@@ -226,7 +244,6 @@ internal_get_package (const char *name, gboolean warn)
>   char *key = NULL;
>   char *location = NULL;
>   unsigned int path_position = 0;
>-  GList *iter;
>   GList *dir_iter;
>   
>   pkg = g_hash_table_lookup (packages, name);
>@@ -323,58 +340,12 @@ internal_get_package (const char *name, gboolean warn)
>   g_hash_table_insert (packages, pkg->key, pkg);
> 
>   /* pull in Requires packages */
>-  for (iter = pkg->requires_entries; iter != NULL; iter = g_list_next (iter))
>-    {
>-      Package *req;
>-      RequiredVersion *ver = iter->data;
>-
>-      debug_spew ("Searching for '%s' requirement '%s'\n",
>-                  pkg->key, 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->key);
>-          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_list_prepend (pkg->requires, req);
>-    }
>-
>-  /* pull in Requires.private packages */
>-  for (iter = pkg->requires_private_entries; iter != NULL;
>-       iter = g_list_next (iter))
>-    {
>-      Package *req;
>-      RequiredVersion *ver = iter->data;
>-
>-      debug_spew ("Searching for '%s' private requirement '%s'\n",
>-                  pkg->key, 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->key);
>-          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_list_prepend (pkg->requires_private, req);
>-    }
>-
>-  /* make requires_private include a copy of the public requires too */
>-  pkg->requires_private = g_list_concat (g_list_copy (pkg->requires),
>-                                         pkg->requires_private);
>-
>-  pkg->requires = g_list_reverse (pkg->requires);
>-  pkg->requires_private = g_list_reverse (pkg->requires_private);
>+  g_list_foreach (pkg->requires,
>+                  internal_get_package_foreach,
>+                  GINT_TO_POINTER (warn));
>+  g_list_foreach (pkg->requires_private,
>+                  internal_get_package_foreach,
>+                  GINT_TO_POINTER (warn));
> 
>   verify_package (pkg);
> 
>@@ -513,7 +484,8 @@ static void
> recursive_fill_list (Package *pkg, gboolean include_private,
>                      GHashTable *visited, GList **listp)
> {
>-  GList *tmp;
>+  GList *lists[2];
>+  guint i, nlists;
> 
>   /*
>    * If the package has already been visited, then it is already in 'listp' and
>@@ -534,9 +506,19 @@ recursive_fill_list (Package *pkg, gboolean include_private,
> 
>   /* Start from the end of the required package list to maintain order since
>    * the recursive list is built by prepending. */
>-  tmp = include_private ? pkg->requires_private : pkg->requires;
>-  for (tmp = g_list_last (tmp); tmp != NULL; tmp = g_list_previous (tmp))
>-    recursive_fill_list (tmp->data, include_private, visited, listp);
>+  lists[0] = pkg->requires;
>+  lists[1] = pkg->requires_private;
>+  nlists = include_private ? 2 : 1;
>+  for (i = 0; i < nlists; i++)
>+    {
>+      GList *iter;
>+      for (iter = g_list_last (lists[i]); iter != NULL; iter = g_list_previous (iter))
>+        {
>+          RequiredVersion *ver = iter->data;
>+          if (ver->package != NULL)
>+            recursive_fill_list (ver->package, include_private, visited, listp);
>+        }
>+    }
> 
>   *listp = g_list_prepend (*listp, pkg);
> }
>@@ -640,6 +622,29 @@ static const gchar *msvc_include_envvars[] = {
> };
> #endif
> 
>+static void
>+verify_version_foreach (gpointer data, gpointer user_data)
>+{
>+  RequiredVersion *ver = data;
>+  Package *req = ver->package;
>+  Package *pkg = ver->owner;
>+
>+  if (!version_test (ver->comparison, req->version, ver->version))
>+    {
>+      verbose_error ("Package '%s' requires '%s %s %s' but version of %s is %s\n",
>+                     pkg->key, req->key,
>+                     comparison_to_str (ver->comparison),
>+                     ver->version,
>+                     req->key,
>+                     req->version);
>+      if (req->url)
>+        verbose_error ("You may find new versions of %s at %s\n",
>+                       req->name, req->url);
>+
>+      exit (1);
>+    }
>+}
>+
> static void
> verify_package (Package *pkg)
> {
>@@ -687,38 +692,8 @@ verify_package (Package *pkg)
>     }
>   
>   /* Make sure we have the right version for all requirements */
>-
>-  iter = pkg->requires_private;
>-
>-  while (iter != NULL)
>-    {
>-      Package *req = iter->data;
>-      RequiredVersion *ver = NULL;
>-
>-      if (pkg->required_versions)
>-        ver = g_hash_table_lookup (pkg->required_versions,
>-                                   req->key);
>-
>-      if (ver)
>-        {
>-          if (!version_test (ver->comparison, req->version, ver->version))
>-            {
>-              verbose_error ("Package '%s' requires '%s %s %s' but version of %s is %s\n",
>-                             pkg->key, req->key,
>-                             comparison_to_str (ver->comparison),
>-                             ver->version,
>-                             req->key,
>-                             req->version);
>-              if (req->url)
>-                verbose_error ("You may find new versions of %s at %s\n",
>-                               req->name, req->url);
>-
>-              exit (1);
>-            }
>-        }
>-                                   
>-      iter = g_list_next (iter);
>-    }
>+  g_list_foreach (pkg->requires, verify_version_foreach, NULL);
>+  g_list_foreach (pkg->requires_private, verify_version_foreach, NULL);
> 
>   /* Make sure we didn't drag in any conflicts via Requires
>    * (inefficient algorithm, who cares)
>diff --git a/pkg.h b/pkg.h
>index c6732bd..6373d9c 100644
>--- a/pkg.h
>+++ b/pkg.h
>@@ -61,6 +61,7 @@ struct RequiredVersion_
>   ComparisonType comparison;
>   char *version;
>   Package *owner;
>+  Package *package;
> };
> 
> struct Package_
>@@ -71,14 +72,11 @@ struct Package_
>   char *description;
>   char *url;
>   char *pcfiledir; /* directory it was loaded from */
>-  GList *requires_entries;
>-  GList *requires;
>-  GList *requires_private_entries;
>-  GList *requires_private;
>+  GList *requires; /* list of RequiredVersion */
>+  GList *requires_private; /* list of RequiredVersion */
>   GList *libs;
>   GList *cflags;
>   GHashTable *vars;
>-  GHashTable *required_versions; /* hash from name to RequiredVersion */
>   GList *conflicts; /* list of RequiredVersion */
>   gboolean uninstalled; /* used the -uninstalled file */
>   int path_position; /* used to order packages by position in path of their .pc file, lower number means earlier in path */
>-- 
>2.14.1

Actions: View | Splinter Review

Attachments on bug 105572: 138185 | 138186 | 138201 | 138202 | 138203 | 138204 | 138281 | 138282

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.