Factor out pkg-config functionality to a separate module.

Functional changes:

* Consistently check that return code from pkg-config is zero.
* Use shell word splitting rules to process pkg-config output to match
  behaviour obtained by running `cc program.cc $(pkg-config --cflags ...)`.
  Fixes issue #171 .
* Use user preferred encoding to process output from pkg-config on
  Python 3. Python 2 behaviour defaults to using ascii encoding as before.

edit creiter: still ignore pkg-config errors by default for now as we
depend on it when glib is a subproject.

8 files changed, 197 insertions, 36 deletions

diff --git a/Makefile-giscanner.am b/Makefile-giscanner.am
index 23149950..8d7d8734 100644
--- a/Makefile-giscanner.am
+++ b/Makefile-giscanner.am
@@ -40,17 +40,18 @@ pkgpyexec_PYTHON = 			\
 	giscanner/docmain.py		\
 	giscanner/docwriter.py		\
 	giscanner/dumper.py		\
-	giscanner/introspectablepass.py	\
+	giscanner/gdumpparser.py 	\
 	giscanner/girparser.py		\
 	giscanner/girwriter.py		\
-	giscanner/gdumpparser.py 	\
+	giscanner/introspectablepass.py	\
 	giscanner/libtoolimporter.py	\
 	giscanner/maintransformer.py	\
 	giscanner/message.py		\
 	giscanner/msvccompiler.py	\
-	giscanner/shlibs.py		\
+	giscanner/pkgconfig.py		\
 	giscanner/scannermain.py	\
 	giscanner/sectionparser.py	\
+	giscanner/shlibs.py		\
 	giscanner/sourcescanner.py	\
 	giscanner/testcodegen.py	\
 	giscanner/transformer.py	\
diff --git a/giscanner/dumper.py b/giscanner/dumper.py
index bb97bc81..494c7ff7 100644
--- a/giscanner/dumper.py
+++ b/giscanner/dumper.py
@@ -32,7 +32,7 @@ import tempfile
 from distutils.errors import LinkError
 
 from .gdumpparser import IntrospectionBinary
-from . import utils
+from . import pkgconfig, utils
 from .ccompiler import CCompiler
 
 # bugzilla.gnome.org/558436
@@ -94,9 +94,8 @@ class DumpCompiler(object):
         # Acquire the compiler (and linker) commands via the CCompiler class in ccompiler.py
         self._compiler = CCompiler()
 
-        self._pkgconfig_cmd = os.environ.get('PKG_CONFIG', 'pkg-config')
         self._uninst_srcdir = os.environ.get('UNINSTALLED_INTROSPECTION_SRCDIR')
-        self._packages = ['gio-2.0 gmodule-2.0']
+        self._packages = ['gio-2.0', 'gmodule-2.0']
         self._packages.extend(options.packages)
         if self._compiler.check_is_msvc():
             self._linker_cmd = ['link.exe']
@@ -189,21 +188,9 @@ class DumpCompiler(object):
                             self._options.namespace_version, suffix)
         return os.path.join(tmpdir, tmpl)
 
-    def _run_pkgconfig(self, flag):
-        # Enable the --msvc-syntax pkg-config flag when
-        # the Microsoft compiler is used
-        if self._compiler.check_is_msvc():
-            cmd = [self._pkgconfig_cmd, '--msvc-syntax', flag]
-        else:
-            cmd = [self._pkgconfig_cmd, flag]
-        proc = subprocess.Popen(
-            cmd + self._packages,
-            stdout=subprocess.PIPE)
-        out, err = proc.communicate()
-        return out.decode('ascii').split()
-
     def _compile(self, *sources):
-        cflags = self._run_pkgconfig('--cflags')
+        cflags = pkgconfig.cflags(self._packages,
+                                  msvc_syntax=self._compiler.check_is_msvc())
         cflags.extend(self._options.cflags)
         return self._compiler.compile(cflags,
                                       self._options.cpp_includes,
@@ -255,7 +242,8 @@ class DumpCompiler(object):
 
         args.extend(sources)
 
-        pkg_config_libs = self._run_pkgconfig('--libs')
+        pkg_config_libs = pkgconfig.libs(self._packages,
+                                         msvc_syntax=self._compiler.check_is_msvc())
 
         if not self._options.external_library:
             self._compiler.get_internal_link_flags(args,
diff --git a/giscanner/meson.build b/giscanner/meson.build
index b4ec3796..7b4b0d9d 100644
--- a/giscanner/meson.build
+++ b/giscanner/meson.build
@@ -17,6 +17,7 @@ giscanner_files = [
   'maintransformer.py',
   'message.py',
   'msvccompiler.py',
+  'pkgconfig.py',
   'shlibs.py',
   'scannermain.py',
   'sectionparser.py',
diff --git a/giscanner/pkgconfig.py b/giscanner/pkgconfig.py
new file mode 100644
index 00000000..6f0b2d57
--- /dev/null
+++ b/giscanner/pkgconfig.py
@@ -0,0 +1,58 @@
+# GObject-Introspection - a framework for introspecting GObject libraries
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+#
+
+import os
+import shlex
+import subprocess
+
+
+class PkgConfigError(Exception):
+    pass
+
+
+def check_output(flags, ignore_errors, command=None):
+    if command is None:
+        command = [os.environ.get('PKG_CONFIG', 'pkg-config')]
+    argv = command[:]
+    argv.extend(flags)
+    try:
+        return subprocess.check_output(argv, universal_newlines=True, stderr=subprocess.STDOUT)
+    except subprocess.CalledProcessError as e:
+        output = e.output or ''
+        if ignore_errors:
+            print(output)
+            return ''
+        raise PkgConfigError('pkg-config: %s\n%s' % (e, output))
+    except OSError as e:
+        raise PkgConfigError('pkg-config: error executing command %s: %s' % (argv, e))
+
+
+def cflags(packages, msvc_syntax=False, ignore_errors=True, command=None):
+    flags = ['--msvc-syntax'] if msvc_syntax else []
+    flags.append('--cflags')
+    flags.extend(packages)
+    out = check_output(flags, ignore_errors, command)
+    return shlex.split(out)
+
+
+def libs(packages, msvc_syntax=False, ignore_errors=True, command=None):
+    flags = ['--msvc-syntax'] if msvc_syntax else []
+    flags.append('--libs')
+    flags.extend(packages)
+    out = check_output(flags, ignore_errors, command)
+    return shlex.split(out)
diff --git a/giscanner/scannermain.py b/giscanner/scannermain.py
index 2f1d9095..e0af993a 100755..100644
--- a/giscanner/scannermain.py
+++ b/giscanner/scannermain.py
@@ -37,7 +37,7 @@ import platform
 import shlex
 
 import giscanner
-from giscanner import message
+from giscanner import message, pkgconfig
 from giscanner.annotationparser import GtkDocCommentBlockParser
 from giscanner.ast import Include, Namespace
 from giscanner.dumper import compile_introspection_binary
@@ -288,7 +288,7 @@ def test_codegen(optstring,
 
 
 def process_options(output, allowed_flags):
-    for option in output.split():
+    for option in output:
         for flag in allowed_flags:
             if not option.startswith(flag):
                 continue
@@ -297,19 +297,11 @@ def process_options(output, allowed_flags):
 
 
 def process_packages(options, packages):
-    args = [os.environ.get('PKG_CONFIG', 'pkg-config'), '--cflags']
-    args.extend(packages)
-    output = subprocess.Popen(args,
-                              stdout=subprocess.PIPE).communicate()[0]
-    if output is None:
-        # the error output should have already appeared on our stderr,
-        # so we just exit
-        return 1
-    output = output.decode('ascii')
+    flags = pkgconfig.cflags(packages)
     # Some pkg-config files on Windows have options we don't understand,
     # so we explicitly filter to only the ones we need.
     options_whitelist = ['-I', '-D', '-U', '-l', '-L']
-    filtered_output = list(process_options(output, options_whitelist))
+    filtered_output = list(process_options(flags, options_whitelist))
     parser = _get_option_parser()
     pkg_options, unused = parser.parse_args(filtered_output)
     options.cpp_includes.extend([os.path.realpath(f) for f in pkg_options.cpp_includes])
@@ -539,9 +531,10 @@ def scanner_main(args):
     packages = set(options.packages)
     packages.update(transformer.get_pkgconfig_packages())
     if packages:
-        exit_code = process_packages(options, packages)
-        if exit_code:
-            return exit_code
+        try:
+            process_packages(options, packages)
+        except pkgconfig.PkgConfigError as e:
+            _error(str(e))
 
     ss = create_source_scanner(options, args)
 
diff --git a/tests/scanner/Makefile.am b/tests/scanner/Makefile.am
index 30651ee1..91c4a3ee 100644
--- a/tests/scanner/Makefile.am
+++ b/tests/scanner/Makefile.am
@@ -230,6 +230,7 @@ endif
 
 PYTESTS = \
 	test_shlibs.py \
+	test_pkgconfig.py \
 	test_sourcescanner.py \
 	test_transformer.py \
 	test_xmlwriter.py
diff --git a/tests/scanner/meson.build b/tests/scanner/meson.build
index a2ca035e..ea7e9e5f 100644
--- a/tests/scanner/meson.build
+++ b/tests/scanner/meson.build
@@ -12,6 +12,7 @@ if cc.get_id() != 'msvc'
     'test_sourcescanner.py',
     'test_transformer.py',
     'test_xmlwriter.py',
+    'test_pkgconfig.py',
     ]
 endif
 
diff --git a/tests/scanner/test_pkgconfig.py b/tests/scanner/test_pkgconfig.py
new file mode 100644
index 00000000..cca0bf87
--- /dev/null
+++ b/tests/scanner/test_pkgconfig.py
@@ -0,0 +1,118 @@
+# -*- coding: UTF-8 -*-
+#
+# GObject-Introspection - a framework for introspecting GObject libraries
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+#
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+import contextlib
+import os
+import sys
+import tempfile
+import tempfile
+import textwrap
+import time
+import unittest
+
+from giscanner import pkgconfig
+
+
+@contextlib.contextmanager
+def pkg_config_script(code):
+    """Points PKG_CONFIG environment variable to an executable file with given python code.
+    Common leading whitespace is removed from code for convenience."""
+
+    with tempfile.NamedTemporaryFile(mode='wb', suffix='.py', delete=False) as file:
+        file.write('#!{}\n'.format(sys.executable).encode("utf-8"))
+        file.write(textwrap.dedent(code).encode("utf-8"))
+        os.chmod(file.name, 0o777)
+
+    try:
+        yield [sys.executable, file.name]
+    finally:
+        os.unlink(file.name)
+
+
+class PkgConfigTest(unittest.TestCase):
+
+    def test_non_zero_exit_code(self):
+        """Checks that non-zero exit code from pkg-config results in exception."""
+        s = """
+        import sys
+        sys.exit(1)
+        """
+        with self.assertRaises(pkgconfig.PkgConfigError):
+            with pkg_config_script(s) as command:
+                pkgconfig.cflags(['foo'], command=command, ignore_errors=False)
+
+    def test_cflags(self):
+        """Checks arguments passed to pkg-config when asking for --cflags."""
+        s = """
+        import sys
+        args = sys.argv[1:]
+        assert len(args) == 4
+        assert args[0] == '--cflags'
+        assert args[1] == 'foo-1.0'
+        assert args[2] == 'bar-2.0'
+        assert args[3] == 'baz'
+        """
+        with pkg_config_script(s) as command:
+            pkgconfig.cflags(['foo-1.0', 'bar-2.0', 'baz'], command=command)
+
+    def test_libs(self):
+        """Checks arguments passed to pkg-config when asking for --libs."""
+        s = """
+        import sys
+        args = sys.argv[1:]
+        assert len(args) == 3
+        assert args[0] == '--libs'
+        assert args[1] == 'a'
+        assert args[2] == 'b-42.0'
+        """
+        with pkg_config_script(s) as command:
+            pkgconfig.libs(['a', 'b-42.0'], command=command)
+
+    @unittest.skipIf(
+        sys.version_info < (3, 0) or os.name == "nt",
+        "Python 2 defaults to ascii encoding in text file I/O and nothing is done to change that")
+    def test_non_ascii_output(self):
+        with pkg_config_script("""print("-L'zażółć gęślą jaźń'")""") as command:
+            flags = pkgconfig.cflags(['test-1.0'], command=command)
+            self.assertEqual(flags, ["-Lzażółć gęślą jaźń"])
+
+        with pkg_config_script("""print('-Lé')""") as command:
+            flags = pkgconfig.cflags(['test-1.0'], command=command)
+            self.assertEqual(flags, ['-Lé'])
+
+    def test_shell_word_splitting_rules(self):
+        # Regression test for issue #171.
+        with pkg_config_script("""print('-L"/usr/lib64" -lgit2')""") as command:
+            flags = pkgconfig.cflags(['foo-2.0'], command=command)
+            self.assertEqual(flags, ['-L/usr/lib64', '-lgit2'])
+
+        # Macro define for a C string literal.
+        with pkg_config_script('''print("""-DLOG='"HELLO"'""")''') as command:
+            flags = pkgconfig.cflags(['bar-3.0'], command=command)
+            self.assertEqual(flags, ['-DLOG="HELLO"'])
+
+
+if __name__ == '__main__':
+    unittest.main()