[PATCH 2/4] Consistently resolve requires depth-first to fix non-l flag ordering
Dan Nicholson
dbn.lists at gmail.com
Tue May 29 23:05:44 PDT 2012
recursive_fill_list() is used to order Requires and Requires.private,
but it relied on fill_one_level() to make the list adjustments as it
descended the package tree. There were two issues with this approach:
1. It added all the dependencies from a package immediately rather than
descending through each dependency first. This made it sort of mix
between depth- and breadth-first resolving.
2. It did not add the requested package to the list, forcing the caller
to add it.
This simplifies the code so that it descends all the way to the least
dependent package and prepends them as it unwinds. This ensures the
ordering will be sorted from most dependent to least dependent package.
Ordering of -l flags is corrected by a later sorting, but this fixes
ordering on non-l flags. Add a new test specifically for non-l Libs
flags. For the existing Requires test, add a secondary dependency on
private-dep to make the ordering more explicit.
Freedesktop #34504
---
check/Makefile.am | 7 +++++--
check/check-non-l-flags | 26 ++++++++++++++++++++++++++
check/check-requires-private | 6 +++---
check/non-l-required.pc | 5 +++++
check/non-l.pc | 6 ++++++
check/public-dep.pc | 1 +
pkg.c | 15 +++------------
7 files changed, 49 insertions(+), 17 deletions(-)
create mode 100755 check/check-non-l-flags
create mode 100644 check/non-l-required.pc
create mode 100644 check/non-l.pc
diff --git a/check/Makefile.am b/check/Makefile.am
index fd9cd98..45fffbc 100644
--- a/check/Makefile.am
+++ b/check/Makefile.am
@@ -10,7 +10,8 @@ TESTS = \
check-idirafter \
check-whitespace \
check-cmd-options \
- check-version
+ check-version \
+ check-non-l-flags
EXTRA_DIST = \
$(TESTS) \
@@ -25,4 +26,6 @@ EXTRA_DIST = \
idirafter.pc \
conflicts-test.pc \
whitespace.pc \
- fields-blank.pc
+ fields-blank.pc \
+ non-l.pc \
+ non-l-required.pc
diff --git a/check/check-non-l-flags b/check/check-non-l-flags
new file mode 100755
index 0000000..66bd60f
--- /dev/null
+++ b/check/check-non-l-flags
@@ -0,0 +1,26 @@
+#! /bin/sh
+
+# Make sure we're POSIX
+if [ "$PKG_CONFIG_SHELL_IS_POSIX" != "1" ]; then
+ PKG_CONFIG_SHELL_IS_POSIX=1 PATH=`getconf PATH` exec sh $0 "$@"
+fi
+
+set -e
+
+. ${srcdir}/common
+
+ARGS="--cflags non-l-required non-l"
+RESULT="-I/non-l/include -I/non-l-required/include"
+run_test
+
+ARGS="--cflags --static non-l-required non-l"
+RESULT="-I/non-l/include -I/non-l-required/include"
+run_test
+
+ARGS="--libs non-l-required non-l"
+RESULT="/non-l.a /non-l-required.a -pthread"
+run_test
+
+ARGS="--libs --static non-l-required non-l"
+RESULT="/non-l.a /non-l-required.a -pthread"
+run_test
diff --git a/check/check-requires-private b/check/check-requires-private
index 45115ee..16d66d0 100755
--- a/check/check-requires-private
+++ b/check/check-requires-private
@@ -10,12 +10,12 @@ set -e
# expect cflags from requires-test and public-dep
ARGS="--cflags requires-test"
-RESULT="-I/requires-test/include -I/private-dep/include -I/public-dep/include"
+RESULT="-I/requires-test/include -I/public-dep/include -I/private-dep/include"
run_test
# still expect those cflags for static linking case
ARGS="--static --cflags requires-test"
-RESULT="-I/requires-test/include -I/private-dep/include -I/public-dep/include"
+RESULT="-I/requires-test/include -I/public-dep/include -I/private-dep/include"
run_test
# expect libs for just requires-test and public-dep
@@ -29,7 +29,7 @@ run_test
# expect libs for requires-test, public-dep and private-dep in static case
ARGS="--static --libs requires-test"
-RESULT="-L/requires-test/lib -L/private-dep/lib -L/public-dep/lib -lrequires-test -lprivate-dep -lpublic-dep"
+RESULT="-L/requires-test/lib -L/public-dep/lib -L/private-dep/lib -lrequires-test -lpublic-dep -lprivate-dep"
run_test
diff --git a/check/non-l-required.pc b/check/non-l-required.pc
new file mode 100644
index 0000000..7e398e2
--- /dev/null
+++ b/check/non-l-required.pc
@@ -0,0 +1,5 @@
+Name: Non-l flags required test package
+Description: Test package for checking order of non-L Libs & Cflags
+Version: 1.0.0
+Libs: /non-l-required.a -pthread
+Cflags: -I/non-l-required/include
diff --git a/check/non-l.pc b/check/non-l.pc
new file mode 100644
index 0000000..cf7ea5e
--- /dev/null
+++ b/check/non-l.pc
@@ -0,0 +1,6 @@
+Name: Non-l flags test package
+Description: Test package for checking order of non-L Libs & Cflags
+Version: 1.0.0
+Requires: non-l-required
+Libs: /non-l.a
+Cflags: -I/non-l/include
diff --git a/check/public-dep.pc b/check/public-dep.pc
index 66af831..f6bb42f 100644
--- a/check/public-dep.pc
+++ b/check/public-dep.pc
@@ -1,5 +1,6 @@
Name: Requires test package
Description: Dummy pkgconfig test package for testing Requires/Requires.private
Version: 1.0.0
+Requires.private: private-dep
Libs: -L/public-dep/lib -lpublic-dep
Cflags: -I/public-dep/include
diff --git a/pkg.c b/pkg.c
index 5628686..04f6467 100644
--- a/pkg.c
+++ b/pkg.c
@@ -580,16 +580,10 @@ recursive_fill_list (Package *pkg, GetListFunc func, GSList **listp)
*/
g_assert (func == get_requires || func == get_requires_private);
- fill_one_level (pkg, func, listp);
-
- tmp = (*func) (pkg);
-
- while (tmp != NULL)
- {
- recursive_fill_list (tmp->data, func, listp);
+ for (tmp = (*func) (pkg); tmp != NULL; tmp = g_slist_next (tmp))
+ recursive_fill_list (tmp->data, func, listp);
- tmp = g_slist_next (tmp);
- }
+ *listp = g_slist_prepend (*listp, pkg);
}
static void
@@ -605,7 +599,6 @@ fill_list_single_package (Package *pkg, GetListFunc func,
/* Get list of packages */
packages = NULL;
- packages = g_slist_append (packages, pkg);
recursive_fill_list (pkg,
include_private ? get_requires_private : get_requires,
&packages);
@@ -642,7 +635,6 @@ fill_list (GSList *packages, GetListFunc func,
tmp = packages;
while (tmp != NULL)
{
- expanded = g_slist_append (expanded, tmp->data);
recursive_fill_list (tmp->data,
include_private ? get_requires_private : get_requires,
&expanded);
@@ -766,7 +758,6 @@ verify_package (Package *pkg)
/* Make sure we didn't drag in any conflicts via Requires
* (inefficient algorithm, who cares)
*/
-
recursive_fill_list (pkg, get_requires_private, &requires);
conflicts = get_conflicts (pkg);
--
1.7.7.6
More information about the pkg-config
mailing list