Merge pull request #7758 from dcbaker/submit/hdf5-factory

dependencies/hdf5: Convert to a dependency_factory

10 files changed, 206 insertions, 119 deletions

diff --git a/.github/workflows/ci_frameworks.yml b/.github/workflows/ci_frameworks.yml
index 172c12c..1ace2a6 100644
--- a/.github/workflows/ci_frameworks.yml
+++ b/.github/workflows/ci_frameworks.yml
@@ -46,7 +46,7 @@ jobs:
         python-version: '3.x'
     - run: python -m pip install -e .
     - run: brew install pkg-config ninja gcc hdf5
-    - run: meson setup "test cases/frameworks/25 hdf5" build
+    - run: meson setup "test cases/frameworks/25 hdf5" build -Dmethod=config-tool
     - run: meson compile -C build
     - uses: actions/upload-artifact@v1
       if: failure()
diff --git a/docs/markdown/Dependencies.md b/docs/markdown/Dependencies.md
index b89a0aa..efd8728 100644
--- a/docs/markdown/Dependencies.md
+++ b/docs/markdown/Dependencies.md
@@ -364,6 +364,10 @@ language-specific, you must specify the requested language using the
 
 Meson uses pkg-config to find HDF5. The standard low-level HDF5 function and the `HL` high-level HDF5 functions are linked for each language.
 
+`method` may be `auto`, `config-tool` or `pkg-config`.
+
+*New in 0.56.0* the `config-tool` method.
+*New in 0.56.0* the dependencies now return proper dependency types and `get_variable` and similar methods should work as expected.
 
 ## libwmf
 
diff --git a/docs/markdown/snippets/hdf5_dependnecy_improvements.md b/docs/markdown/snippets/hdf5_dependnecy_improvements.md
new file mode 100644
index 0000000..857eb90
--- /dev/null
+++ b/docs/markdown/snippets/hdf5_dependnecy_improvements.md
@@ -0,0 +1,9 @@
+## HDF5 dependency improvements
+
+HDF5 has been improved so that the internal representations have been split.
+This allows selecting pkg-config and config-tool dependencies separately.
+Both work as proper dependencies of their type, so `get_variable` and similar
+now work correctly.
+
+It has also been fixed to use the selected compiler for the build instead of
+the default compiler.
diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py
index 5827a4e..a6cfd5b 100644
--- a/mesonbuild/coredata.py
+++ b/mesonbuild/coredata.py
@@ -390,7 +390,7 @@ class CoreData:
         ) # type: PerMachine[T.defaultdict[str, OptionDictType]]
         self.base_options = {} # type: OptionDictType
         self.cross_files = self.__load_config_files(options, scratch_dir, 'cross')
-        self.compilers = PerMachine(OrderedDict(), OrderedDict())
+        self.compilers = PerMachine(OrderedDict(), OrderedDict())  # type: PerMachine[T.Dict[str, Compiler]]
 
         build_cache = DependencyCache(self.builtins_per_machine, MachineChoice.BUILD)
         host_cache = DependencyCache(self.builtins_per_machine, MachineChoice.BUILD)
diff --git a/mesonbuild/dependencies/__init__.py b/mesonbuild/dependencies/__init__.py
index d75d226..7110c1f 100644
--- a/mesonbuild/dependencies/__init__.py
+++ b/mesonbuild/dependencies/__init__.py
@@ -14,7 +14,7 @@
 
 from .boost import BoostDependency
 from .cuda import CudaDependency
-from .hdf5 import HDF5Dependency
+from .hdf5 import hdf5_factory
 from .base import (  # noqa: F401
     Dependency, DependencyException, DependencyMethods, ExternalProgram, EmptyExternalProgram, NonExistingExternalProgram,
     ExternalDependency, NotFoundDependency, ExternalLibrary, ExtraFrameworkDependency, InternalDependency,
@@ -51,7 +51,7 @@ packages.update({
 
     # per-file
     'coarray': coarray_factory,
-    'hdf5': HDF5Dependency,
+    'hdf5': hdf5_factory,
     'mpi': mpi_factory,
     'scalapack': scalapack_factory,
 
diff --git a/mesonbuild/dependencies/base.py b/mesonbuild/dependencies/base.py
index 99030e8..7b2f2d9 100644
--- a/mesonbuild/dependencies/base.py
+++ b/mesonbuild/dependencies/base.py
@@ -207,7 +207,7 @@ class Dependency:
         """
         raise RuntimeError('Unreachable code in partial_dependency called')
 
-    def _add_sub_dependency(self, deplist: T.List['DependencyType']) -> bool:
+    def _add_sub_dependency(self, deplist: T.Iterable[T.Callable[[], 'Dependency']]) -> bool:
         """Add an internal depdency from a list of possible dependencies.
 
         This method is intended to make it easier to add additional
@@ -302,10 +302,10 @@ class InternalDependency(Dependency):
         return new_dep
 
 class HasNativeKwarg:
-    def __init__(self, kwargs):
+    def __init__(self, kwargs: T.Dict[str, T.Any]):
         self.for_machine = self.get_for_machine_from_kwargs(kwargs)
 
-    def get_for_machine_from_kwargs(self, kwargs):
+    def get_for_machine_from_kwargs(self, kwargs: T.Dict[str, T.Any]) -> MachineChoice:
         return MachineChoice.BUILD if kwargs.get('native', False) else MachineChoice.HOST
 
 class ExternalDependency(Dependency, HasNativeKwarg):
diff --git a/mesonbuild/dependencies/hdf5.py b/mesonbuild/dependencies/hdf5.py
index 6280010..21f4e71 100644
--- a/mesonbuild/dependencies/hdf5.py
+++ b/mesonbuild/dependencies/hdf5.py
@@ -14,122 +14,163 @@
 
 # This file contains the detection logic for miscellaneous external dependencies.
 
-import subprocess
+import functools
+import os
+import re
 import shutil
+import subprocess
 from pathlib import Path
 
-from .. import mlog
-from ..mesonlib import split_args, listify
-from .base import (DependencyException, DependencyMethods, ExternalDependency, ExternalProgram,
-                   PkgConfigDependency)
+from ..mesonlib import OrderedSet, join_args
+from .base import (
+    DependencyException, DependencyMethods, ConfigToolDependency,
+    PkgConfigDependency, factory_methods
+)
 import typing as T
 
 if T.TYPE_CHECKING:
+    from .base import Dependency
+    from ..envconfig import MachineChoice
     from ..environment import Environment
 
-class HDF5Dependency(ExternalDependency):
 
-    def __init__(self, environment: 'Environment', kwargs: T.Dict[str, T.Any]) -> None:
-        language = kwargs.get('language', 'c')
-        super().__init__('hdf5', environment, kwargs, language=language)
-        kwargs['required'] = False
-        kwargs['silent'] = True
-        self.is_found = False
-        methods = listify(self.methods)
+class HDF5PkgConfigDependency(PkgConfigDependency):
+
+    """Handle brokenness in the HDF5 pkg-config files."""
+
+    def __init__(self, name: str, environment: 'Environment', kwargs: T.Dict[str, T.Any], language: T.Optional[str] = None) -> None:
+        language = language or 'c'
+        if language not in {'c', 'cpp', 'fortran'}:
+            raise DependencyException('Language {} is not supported with HDF5.'.format(language))
+
+        super().__init__(name, environment, kwargs, language)
+        if not self.is_found:
+            return
 
-        if language not in ('c', 'cpp', 'fortran'):
+        # some broken pkgconfig don't actually list the full path to the needed includes
+        newinc = []  # type: T.List[str]
+        for arg in self.compile_args:
+            if arg.startswith('-I'):
+                stem = 'static' if kwargs.get('static', False) else 'shared'
+                if (Path(arg[2:]) / stem).is_dir():
+                    newinc.append('-I' + str(Path(arg[2:]) / stem))
+        self.compile_args += newinc
+
+        link_args = []  # type: T.List[str]
+        for larg in self.get_link_args():
+            lpath = Path(larg)
+            # some pkg-config hdf5.pc (e.g. Ubuntu) don't include the commonly-used HL HDF5 libraries,
+            # so let's add them if they exist
+            # additionally, some pkgconfig HDF5 HL files are malformed so let's be sure to find HL anyway
+            if lpath.is_file():
+                hl = []
+                if language == 'cpp':
+                    hl += ['_hl_cpp', '_cpp']
+                elif language == 'fortran':
+                    hl += ['_hl_fortran', 'hl_fortran', '_fortran']
+                hl += ['_hl']  # C HL library, always needed
+
+                suffix = '.' + lpath.name.split('.', 1)[1]  # in case of .dll.a
+                for h in hl:
+                    hlfn = lpath.parent / (lpath.name.split('.', 1)[0] + h + suffix)
+                    if hlfn.is_file():
+                        link_args.append(str(hlfn))
+                # HDF5 C libs are required by other HDF5 languages
+                link_args.append(larg)
+            else:
+                link_args.append(larg)
+
+        self.link_args = link_args
+
+
+class HDF5ConfigToolDependency(ConfigToolDependency):
+
+    """Wrapper around hdf5 binary config tools."""
+
+    version_arg = '-showconfig'
+
+    def __init__(self, name: str, environment: 'Environment', kwargs: T.Dict[str, T.Any], language: T.Optional[str] = None) -> None:
+        language = language or 'c'
+        if language not in {'c', 'cpp', 'fortran'}:
             raise DependencyException('Language {} is not supported with HDF5.'.format(language))
 
-        if set([DependencyMethods.AUTO, DependencyMethods.PKGCONFIG]).intersection(methods):
-            pkgconfig_files = ['hdf5', 'hdf5-serial']
-            PCEXE = shutil.which('pkg-config')
-            if PCEXE:
-                # some distros put hdf5-1.2.3.pc with version number in .pc filename.
-                ret = subprocess.run([PCEXE, '--list-all'], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
-                                     universal_newlines=True)
-                if ret.returncode == 0:
-                    for pkg in ret.stdout.split('\n'):
-                        if pkg.startswith(('hdf5')):
-                            pkgconfig_files.append(pkg.split(' ', 1)[0])
-                    pkgconfig_files = list(set(pkgconfig_files))  # dedupe
-
-            for pkg in pkgconfig_files:
-                pkgdep = PkgConfigDependency(pkg, environment, kwargs, language=self.language)
-                if not pkgdep.found():
-                    continue
-
-                self.compile_args = pkgdep.get_compile_args()
-                # some broken pkgconfig don't actually list the full path to the needed includes
-                newinc = []
-                for arg in self.compile_args:
-                    if arg.startswith('-I'):
-                        stem = 'static' if kwargs.get('static', False) else 'shared'
-                        if (Path(arg[2:]) / stem).is_dir():
-                            newinc.append('-I' + str(Path(arg[2:]) / stem))
-                self.compile_args += newinc
-
-                # derive needed libraries by language
-                pd_link_args = pkgdep.get_link_args()
-                link_args = []
-                for larg in pd_link_args:
-                    lpath = Path(larg)
-                    # some pkg-config hdf5.pc (e.g. Ubuntu) don't include the commonly-used HL HDF5 libraries,
-                    # so let's add them if they exist
-                    # additionally, some pkgconfig HDF5 HL files are malformed so let's be sure to find HL anyway
-                    if lpath.is_file():
-                        hl = []
-                        if language == 'cpp':
-                            hl += ['_hl_cpp', '_cpp']
-                        elif language == 'fortran':
-                            hl += ['_hl_fortran', 'hl_fortran', '_fortran']
-                        hl += ['_hl']  # C HL library, always needed
-
-                        suffix = '.' + lpath.name.split('.', 1)[1]  # in case of .dll.a
-                        for h in hl:
-                            hlfn = lpath.parent / (lpath.name.split('.', 1)[0] + h + suffix)
-                            if hlfn.is_file():
-                                link_args.append(str(hlfn))
-                        # HDF5 C libs are required by other HDF5 languages
-                        link_args.append(larg)
-                    else:
-                        link_args.append(larg)
-
-                self.link_args = link_args
-                self.version = pkgdep.get_version()
-                self.is_found = True
-                self.pcdep = pkgdep
-                return
-
-        if DependencyMethods.AUTO in methods:
-            wrappers = {'c': 'h5cc', 'cpp': 'h5c++', 'fortran': 'h5fc'}
-            comp_args = []
-            link_args = []
-            # have to always do C as well as desired language
-            for lang in set([language, 'c']):
-                prog = ExternalProgram(wrappers[lang], silent=True)
-                if not prog.found():
-                    return
-                shlib_arg = '-noshlib' if kwargs.get('static', False) else '-shlib'
-                cmd = prog.get_command() + [shlib_arg, '-show']
-                p = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, timeout=15)
-                if p.returncode != 0:
-                    mlog.debug('Command', mlog.bold(str(cmd)), 'failed to run:')
-                    mlog.debug(mlog.bold('Standard output\n'), p.stdout)
-                    mlog.debug(mlog.bold('Standard error\n'), p.stderr)
-                    return
-                args = split_args(p.stdout)
-                for arg in args[1:]:
-                    if arg.startswith(('-I', '-f', '-D')) or arg == '-pthread':
-                        comp_args.append(arg)
-                    elif arg.startswith(('-L', '-l', '-Wl')):
-                        link_args.append(arg)
-                    elif Path(arg).is_file():
-                        link_args.append(arg)
-            self.compile_args = comp_args
-            self.link_args = link_args
-            self.is_found = True
+        if language == 'c':
+            cenv = 'CC'
+            tools = ['h5cc']
+        elif language == 'cpp':
+            cenv = 'CXX'
+            tools = ['h5c++']
+        elif language == 'fortran':
+            cenv = 'FC'
+            tools = ['h5fc']
+        else:
+            raise DependencyException('How did you get here?')
+
+        # We need this before we call super()
+        for_machine = self.get_for_machine_from_kwargs(kwargs)
+
+        nkwargs = kwargs.copy()
+        nkwargs['tools'] = tools
+
+        # Override the compiler that the config tools are going to use by
+        # setting the environment variables that they use for the compiler and
+        # linkers.
+        compiler = environment.coredata.compilers[for_machine][language]
+        try:
+            os.environ['HDF5_{}'.format(cenv)] = join_args(compiler.get_exelist())
+            os.environ['HDF5_{}LINKER'.format(cenv)] = join_args(compiler.get_linker_exelist())
+            super().__init__(name, environment, nkwargs, language)
+        finally:
+            del os.environ['HDF5_{}'.format(cenv)]
+            del os.environ['HDF5_{}LINKER'.format(cenv)]
+        if not self.is_found:
             return
 
-    @staticmethod
-    def get_methods() -> T.List[DependencyMethods]:
-        return [DependencyMethods.AUTO, DependencyMethods.PKGCONFIG]
+        args = self.get_config_value(['-show', '-noshlib' if kwargs.get('static', False) else '-shlib'], 'args')
+        for arg in args[1:]:
+            if arg.startswith(('-I', '-f', '-D')) or arg == '-pthread':
+                self.compile_args.append(arg)
+            elif arg.startswith(('-L', '-l', '-Wl')):
+                self.link_args.append(arg)
+            elif Path(arg).is_file():
+                self.link_args.append(arg)
+
+        # If the language is not C we need to add C as a subdependency
+        if language != 'c':
+            nkwargs = kwargs.copy()
+            nkwargs['language'] = 'c'
+            # I'm being too clever for mypy and pylint
+            self.is_found = self._add_sub_dependency(hdf5_factory(environment, for_machine, nkwargs))  # type: ignore  # pylint: disable=no-value-for-parameter
+
+    def _sanitize_version(self, ver: str) -> str:
+        v = re.search(r'\s*HDF5 Version: (\d+\.\d+\.\d+)', ver)
+        return v.group(1)
+
+
+@factory_methods({DependencyMethods.PKGCONFIG, DependencyMethods.CONFIG_TOOL})
+def hdf5_factory(env: 'Environment', for_machine: 'MachineChoice',
+                 kwargs: T.Dict[str, T.Any], methods: T.List[DependencyMethods]) -> T.List[T.Callable[[], 'Dependency']]:
+    language = kwargs.get('language')
+    candidates = []  # type: T.List[T.Callable[[], Dependency]]
+
+    if DependencyMethods.PKGCONFIG in methods:
+        # Use an ordered set so that these remain the first tried pkg-config files
+        pkgconfig_files = OrderedSet(['hdf5', 'hdf5-serial'])
+        # FIXME: This won't honor pkg-config paths, and cross-native files
+        PCEXE = shutil.which('pkg-config')
+        if PCEXE:
+            # some distros put hdf5-1.2.3.pc with version number in .pc filename.
+            ret = subprocess.run([PCEXE, '--list-all'], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
+                                    universal_newlines=True)
+            if ret.returncode == 0:
+                for pkg in ret.stdout.split('\n'):
+                    if pkg.startswith(('hdf5')):
+                        pkgconfig_files.add(pkg.split(' ', 1)[0])
+
+        for pkg in pkgconfig_files:
+            candidates.append(functools.partial(HDF5PkgConfigDependency, pkg, env, kwargs, language))
+
+    if DependencyMethods.CONFIG_TOOL in methods:
+        candidates.append(functools.partial(HDF5ConfigToolDependency, 'hdf5', env, kwargs, language))
+
+    return candidates
diff --git a/test cases/frameworks/25 hdf5/meson.build b/test cases/frameworks/25 hdf5/meson.build
index 5485035..0df2ffd 100644
--- a/test cases/frameworks/25 hdf5/meson.build
+++ b/test cases/frameworks/25 hdf5/meson.build
@@ -2,8 +2,10 @@ project('hdf5_framework', 'c')
 
 # NOTE: all HDF5 languages must have HDF5 C library working.
 
+method = get_option('method')
+
 # --- C tests
-h5c = dependency('hdf5', language : 'c', required : false)
+h5c = dependency('hdf5', language : 'c', required : false, method : method)
 if not h5c.found()
   error('MESON_SKIP_TEST: HDF5 C library not found.')
 endif
@@ -12,16 +14,31 @@ test('HDF5 C', exec, timeout: 30)
 
 # --- C++ tests
 if add_languages('cpp', required: false)
-  h5cpp = dependency('hdf5', language : 'cpp', required : false, disabler: true)
+  h5cpp = dependency('hdf5', language : 'cpp', required : false, disabler: true, method : method)
   execpp = executable('execpp', 'main.cpp', dependencies : h5cpp)
   test('HDF5 C++', execpp, timeout: 30)
 endif
 
-# --- Fortran tests
-if add_languages('fortran', required: false)
-  h5f = dependency('hdf5', language : 'fortran', required : false, disabler: true)
-  exef = executable('exef', 'main.f90', dependencies : h5f)
-  test('HDF5 Fortran', exef, timeout: 30)
+test_fortran = add_languages('fortran', required: false)
+
+if test_fortran
+  cpp = meson.get_compiler('cpp')
+  fc = meson.get_compiler('fortran')
+
+  if host_machine.system() == 'darwin' and cpp.get_id() == 'clang' and fc.get_id() == 'gcc'
+    # Search paths don't work correctly here and -lgfortran doesn't work
+    test_fortran = false
+  elif host_machine.system() == 'windows' and cpp.get_id() != 'gcc' and fc.get_id() == 'gcc'
+    # mixing gfotran with non-gcc doesn't work on windows
+    test_fortran = false
+  endif
+
+  # --- Fortran tests
+  if test_fortran
+    h5f = dependency('hdf5', language : 'fortran', required : false, disabler: true, method : method)
+    exef = executable('exef', 'main.f90', dependencies : h5f)
+    test('HDF5 Fortran', exef, timeout: 30)
+  endif
 endif
 
 # Check we can apply a version constraint
diff --git a/test cases/frameworks/25 hdf5/meson_options.txt b/test cases/frameworks/25 hdf5/meson_options.txt
new file mode 100644
index 0000000..741f58e
--- /dev/null
+++ b/test cases/frameworks/25 hdf5/meson_options.txt
@@ -0,0 +1,6 @@
+option(
+    'method',
+    type : 'combo',
+    choices : ['pkg-config', 'config-tool'],
+    value : 'pkg-config'
+)
diff --git a/test cases/frameworks/25 hdf5/test.json b/test cases/frameworks/25 hdf5/test.json
new file mode 100644
index 0000000..0de1f73
--- /dev/null
+++ b/test cases/frameworks/25 hdf5/test.json
@@ -0,0 +1,10 @@
+{
+  "matrix": {
+    "options": {
+      "method": [
+        { "val": "pkg-config" },
+        { "val": "config-tool" }
+      ]
+    }
+  }
+}