Increase testing coverage and refactor method names

Making `pkgconfig.call` function accessible, tests can monkey patch it and
provide mock. This improves testing, however raised a need to give
functions better names than `pkgconfig.pkgconfig_kwargs` or `pkgconfig.pc`.

2 files changed, 59 insertions, 57 deletions

diff --git a/cffi/api.py b/cffi/api.py
index 13369a0..eb6f8c2 100644
--- a/cffi/api.py
+++ b/cffi/api.py
@@ -2,7 +2,7 @@ import sys, types
 from .lock import allocate_lock
 from .error import CDefError
 from . import model
-from .pkgconfig import pkgconfig_installed, merge_dicts, pkgconfig_kwargs
+from . import pkgconfig
 
 try:
     callable
@@ -612,17 +612,12 @@ class FFI(object):
         if os.sep in module_name or (os.altsep and os.altsep in module_name):
             raise ValueError("'module_name' must not contain '/': use a dotted "
                              "name to make a 'package.module' location")
-        if "pkgconfig" in kwds:
-            if pkgconfig_installed ():
-                try:
-                    del kwds ["libraries"]
-                except KeyError:
-                    pass
-                merge_dicts (kwds, pkgconfig_kwargs (kwds ["pkgconfig"]))
-                try:
-                    del kwds ["pkgconfig"]
-                except KeyError:
-                    pass
+        if "pkgconfig" in kwds and pkgconfig.is_installed():
+            if "libraries" in kwds:
+                del kwds["libraries"]  # real library names are going to be
+                                        # provided by pkg-config
+            pkgconfig.merge_flags(kwds, pkgconfig.kwargs(kwds["pkgconfig"]))
+            del kwds["pkgconfig"]
         self._assigned_source = (str(module_name), source,
                                  source_extension, kwds)
 
diff --git a/cffi/pkgconfig.py b/cffi/pkgconfig.py
index 2fa3a3e..0601802 100644
--- a/cffi/pkgconfig.py
+++ b/cffi/pkgconfig.py
@@ -1,79 +1,86 @@
 # pkg-config, https://www.freedesktop.org/wiki/Software/pkg-config/ integration for cffi
 import subprocess
 
-def pkgconfig_installed ():
-    """Check if pkg=config is installed or not"""
+def is_installed():
+    """Check if pkg-config is installed or not"""
     try:
-        subprocess.check_output (["pkg-config", "--version"])
+        subprocess.check_output(["pkg-config", "--version"])
         return True
     except subprocess.CalledProcessError:
         return False
 
-def merge_dicts (d1, d2):
-    """Helper function to merge two dicts with lists"""
-    for key, value in d2.items ():
-        if not key in d1:
-            d1 [key] = value
+
+def merge_flags(cfg1, cfg2):
+    """Merge values from cffi config flags cfg2 to cf1
+
+    Example:
+        merge_flags({"libraries": ["one"]}, {"libraries": "two"})
+        {"libraries}" : ["one", "two"]}
+    """
+    for key, value in cfg2.items():
+        if not key in cfg1:
+            cfg1 [key] = value
         else:
-            d1 [key].extend (value)
-    return d1
+            cfg1 [key].extend(value)
+    return cfg1
+
 
-def pkgconfig_kwargs (libs):
-    r"""Return kwargs for FFI.set_source based on pkg-config output
+def call(libname, flag):
+    """Calls pkg-config and returing the output"""
+    a = ["pkg-config", "--print-errors"]
+    a.append(flag)
+    a.append(libname)
+    return subprocess.check_output(a)
+
+
+def flags(libs):
+    r"""Return compiler line flags for FFI.set_source based on pkg-config output
 
     Usage
         ...
-        ffibuilder.set_source ("_foo", libraries = ["foo", "bar"], pkgconfig = ["libfoo", "libbar"])
+        ffibuilder.set_source("_foo", libraries = ["foo", "bar"], pkgconfig = ["libfoo", "libbar"])
 
-    If pkg-config is installed on build machine, then arguments include_dirs,
-    library_dirs and define_macros are extended with an output of pkg-config
-    [command] libfoo and pkgconfig [command] libbar. Argument libraries is
-    replaced by an output of pkgconfig --libs-only-l calls.
+    If `pkg-config` is installed on build machine, then arguments
+    `include_dirs`, `library_dirs`, `libraries`, `define_macros`,
+    `extra_compile_args` and `extra_link_args` are extended with an output of
+    `pkg-config` for `libfoo` and `libbar`.
     """
 
     # make API great again!
-    if isinstance (libs, (str, bytes)):
+    if isinstance(libs, (str, bytes)):
         libs = (libs, )
     
     # drop starting -I -L -l from cflags
-    def dropILl (string):
-        def _dropILl (string):
-            if string.startswith (b"-I") or string.startswith (b"-L") or string.startswith (b"-l"):
+    def dropILl(string):
+        def _dropILl(string):
+            if string.startswith(b"-I") or string.startswith(b"-L") or string.startswith(b"-l"):
                 return string [2:]
-        return [_dropILl (x) for x in string.split ()]
+        return [_dropILl(x) for x in string.split()]
 
     # convert -Dfoo=bar to list of tuples [("foo", "bar")] expected by cffi
-    def macros (string):
-        def _macros (string):
-            return tuple (string [2:].split ('=', 2))
-        return [_macros (x) for x in string.split () if x.startswith ("-D")]
+    def macros(string):
+        def _macros(string):
+            return tuple(string [2:].split(b"=", 2))
+        return [_macros(x) for x in string.split() if x.startswith(b"-D")]
 
-    def drop_macros (string):
-        return [x for x in string.split () if not x.startswith ("-D")]
-
-    # pkg-config call
-    def pc (libname, *args):
-        a = ["pkg-config", "--print-errors"]
-        a.extend (args)
-        a.append (libname)
-        return subprocess.check_output (a)
+    def drop_macros(string):
+        return [x for x in string.split() if not x.startswith(b"-D")]
 
     # return kwargs for given libname
-    def kwargs (libname):
+    def kwargs(libname):
         return {
-                "include_dirs" : dropILl (pc (libname, "--cflags-only-I")),
-                "library_dirs" : dropILl (pc (libname, "--libs-only-L")),
-                "libraries" : dropILl (pc (libname, "--libs-only-l")),
-                "define_macros" : macros (pc (libname, "--cflags-only-other")),
-                "extra_compile_args" : drop_macros (pc (libname, "--cflags-only-other")),
-                "extra_link_args" : pc (libname, "--libs-only-other").split ()
+                "include_dirs" : dropILl(call(libname, "--cflags-only-I")),
+                "library_dirs" : dropILl(call(libname, "--libs-only-L")),
+                "libraries" : dropILl(call(libname, "--libs-only-l")),
+                "define_macros" : macros(call(libname, "--cflags-only-other")),
+                "extra_compile_args" : drop_macros(call(libname, "--cflags-only-other")),
+                "extra_link_args" : call(libname, "--libs-only-other").split()
                 }
 
     # merge all arguments together
     ret = {}
     for libname in libs:
-        foo = kwargs (libname)
-        merge_dicts (ret, foo)
+        foo = kwargs(libname)
+        merge_flags(ret, foo)
 
     return ret
-