Merge pull request #19 from hiker/linker-lib-flags

Linker flags for common libraries

Author: hiker

Documentation/source/advanced_config.rst

@@ -193,6 +193,14 @@ to specify 3rd party libraries at the link stage.
 
     link_exe(state, flags=['-lm', '-lnetcdf'])
 
+Linkers will be pre-configured with flags for common libraries. Where possible,
+a library name should be used to include the required flags for linking.
+
+.. code-block::
+    :linenos:
+
+    link_exe(state, libs=['netcdf'])
+
 Path-specific flags
 -------------------
 

source/fab/steps/link.py

@@ -9,7 +9,7 @@
 """
 import logging
 from string import Template
-from typing import Optional
+from typing import List, Optional, Union
 
 from fab.artefacts import ArtefactSet
 from fab.steps import step
@@ -33,7 +33,10 @@ def __call__(self, artefact_store):
 
 
 @step
-def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None):
+def link_exe(config,
+             libs: Union[List[str], None] = None,
+             flags: Union[List[str], None] = None,
+             source: Optional[ArtefactsGetter] = None):
     """
     Link object files into an executable for every build target.
 
@@ -49,8 +52,10 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None):
         The :class:`fab.build_config.BuildConfig` object where we can read
         settings such as the project workspace folder or the multiprocessing
         flag.
+    :param libs:
+        A list of required library names to pass to the linker.
     :param flags:
-        A list of flags to pass to the linker.
+        A list of additional flags to pass to the linker.
     :param source:
         An optional :class:`~fab.artefacts.ArtefactsGetter`. It defaults to the
         output from compiler steps, which typically is the expected behaviour.
@@ -59,13 +64,15 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None):
     linker = config.tool_box.get_tool(Category.LINKER, config.mpi)
     logger.info(f'Linker is {linker.name}')
 
-    flags = flags or []
+    libs = libs or []
+    if flags:
+        linker.add_post_lib_flags(flags)
     source_getter = source or DefaultLinkerSource()
 
     target_objects = source_getter(config.artefact_store)
     for root, objects in target_objects.items():
         exe_path = config.project_workspace / f'{root}'
-        linker.link(objects, exe_path, openmp=config.openmp, add_libs=flags)
+        linker.link(objects, exe_path, openmp=config.openmp, libs=libs)
         config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path)
 
 
@@ -115,4 +122,5 @@ def link_shared_object(config, output_fpath: str, flags=None,
 
     objects = target_objects[None]
     out_name = Template(output_fpath).substitute(output=config.build_output)
-    linker.link(objects, out_name, openmp=config.openmp, add_libs=flags)
+    linker.add_post_lib_flags(flags)
+    linker.link(objects, out_name, openmp=config.openmp)

source/fab/tools/linker.py

@@ -9,7 +9,8 @@
 
 import os
 from pathlib import Path
-from typing import cast, List, Optional
+from typing import cast, Dict, List, Optional
+import warnings
 
 from fab.tools.category import Category
 from fab.tools.compiler import Compiler
@@ -51,6 +52,12 @@ def __init__(self, name: Optional[str] = None,
         self._compiler = compiler
         self.flags.extend(os.getenv("LDFLAGS", "").split())
 
+        # Maintain a set of flags for common libraries.
+        self._lib_flags: Dict[str, List[str]] = {}
+        # Allow flags to include before or after any library-specific flags.
+        self._pre_lib_flags: List[str] = []
+        self._post_lib_flags: List[str] = []
+
     @property
     def mpi(self) -> bool:
         ''':returns: whether the linker supports MPI or not.'''
@@ -66,16 +73,71 @@ def check_available(self) -> bool:
 
         return super().check_available()
 
+    def get_lib_flags(self, lib: str) -> List[str]:
+        '''Gets the standard flags for a standard library
+
+        :param lib: the library name
+
+        :returns: a list of flags
+
+        :raises RuntimeError: if lib is not recognised
+        '''
+        try:
+            return self._lib_flags[lib]
+        except KeyError:
+            raise RuntimeError(f"Unknown library name: '{lib}'")
+
+    def add_lib_flags(self, lib: str, flags: List[str],
+                      silent_replace: bool = False):
+        '''Add a set of flags for a standard library
+
+        :param lib: the library name
+        :param flags: the flags to use with the library
+        :param silent_replace: if set, no warning will be printed when an
+            existing lib is overwritten.
+        '''
+        if lib in self._lib_flags and not silent_replace:
+            warnings.warn(f"Replacing existing flags for library {lib}: "
+                          f"'{self._lib_flags[lib]}' with "
+                          f"'{flags}'.")
+
+        # Make a copy to avoid modifying the caller's list
+        self._lib_flags[lib] = flags[:]
+
+    def remove_lib_flags(self, lib: str):
+        '''Remove any flags configured for a standard library
+
+        :param lib: the library name
+        '''
+        try:
+            del self._lib_flags[lib]
+        except KeyError:
+            pass
+
+    def add_pre_lib_flags(self, flags: List[str]):
+        '''Add a set of flags to use before any library-specific flags
+
+        :param flags: the flags to include
+        '''
+        self._pre_lib_flags.extend(flags)
+
+    def add_post_lib_flags(self, flags: List[str]):
+        '''Add a set of flags to use after any library-specific flags
+
+        :param flags: the flags to include
+        '''
+        self._post_lib_flags.extend(flags)
+
     def link(self, input_files: List[Path], output_file: Path,
              openmp: bool,
-             add_libs: Optional[List[str]] = None) -> str:
+             libs: Optional[List[str]] = None) -> str:
         '''Executes the linker with the specified input files,
         creating `output_file`.
 
         :param input_files: list of input files to link.
         :param output_file: output file.
         :param openm: whether OpenMP is requested or not.
-        :param add_libs: additional linker flags.
+        :param libs: additional libraries to link with.
 
         :returns: the stdout of the link command
         '''
@@ -88,7 +150,12 @@ def link(self, input_files: List[Path], output_file: Path,
             params = []
         # TODO: why are the .o files sorted? That shouldn't matter
         params.extend(sorted(map(str, input_files)))
-        if add_libs:
-            params += add_libs
+
+        if self._pre_lib_flags:
+            params.extend(self._pre_lib_flags)
+        for lib in (libs or []):
+            params.extend(self.get_lib_flags(lib))
+        if self._post_lib_flags:
+            params.extend(self._post_lib_flags)
         params.extend([self._output_flag, str(output_file)])
         return self.run(params)

tests/conftest.py

@@ -48,6 +48,7 @@ def fixture_mock_linker():
                          Category.FORTRAN_COMPILER)
     mock_linker.run = mock.Mock()
     mock_linker._version = (1, 2, 3)
+    mock_linker.add_lib_flags("netcdf", ["-lnetcdff", "-lnetcdf"])
     return mock_linker
 
 

tests/unit_tests/steps/test_archive_objects.py

@@ -51,7 +51,8 @@ def test_for_exes(self):
 
         # ensure the correct artefacts were created
         assert config.artefact_store[ArtefactSet.OBJECT_ARCHIVES] == {
-            target: set([str(config.build_output / f'{target}.a')]) for target in targets}
+            target: set([str(config.build_output / f'{target}.a')])
+            for target in targets}
 
     def test_for_library(self):
         '''As used when building an object archive or archiving before linking
@@ -65,32 +66,36 @@ def test_for_library(self):
         mock_result = mock.Mock(returncode=0, return_value=123)
         with mock.patch('fab.tools.tool.subprocess.run',
                         return_value=mock_result) as mock_run_command, \
-                pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
-            archive_objects(config=config, output_fpath=config.build_output / 'mylib.a')
+                pytest.warns(UserWarning, match="_metric_send_conn not set, "
+                                                "cannot send metrics"):
+            archive_objects(config=config,
+                            output_fpath=config.build_output / 'mylib.a')
 
         # ensure the correct command line calls were made
         mock_run_command.assert_called_once_with([
-            'ar', 'cr', str(config.build_output / 'mylib.a'), 'util1.o', 'util2.o'],
+            'ar', 'cr', str(config.build_output / 'mylib.a'), 'util1.o',
+            'util2.o'],
             capture_output=True, env=None, cwd=None, check=False)
 
         # ensure the correct artefacts were created
         assert config.artefact_store[ArtefactSet.OBJECT_ARCHIVES] == {
             None: set([str(config.build_output / 'mylib.a')])}
 
-    def test_incorrect_tool(self):
+    def test_incorrect_tool(self, mock_c_compiler):
         '''Test that an incorrect archive tool is detected
         '''
 
         config = BuildConfig('proj', ToolBox(), mpi=False, openmp=False)
         tool_box = config.tool_box
-        cc = tool_box.get_tool(Category.C_COMPILER, config.mpi)
-        # And set its category to C_COMPILER
+        cc = mock_c_compiler
+        # And set its category to be AR
         cc._category = Category.AR
-        # So overwrite the C compiler with the re-categories Fortran compiler
+        # Now add this 'ar' tool to the tool box
         tool_box.add_tool(cc)
 
         with pytest.raises(RuntimeError) as err:
             archive_objects(config=config,
                             output_fpath=config.build_output / 'mylib.a')
-        assert ("Unexpected tool 'gcc' of type '<class "
-                "'fab.tools.compiler.Gcc'>' instead of Ar" in str(err.value))
+        assert ("Unexpected tool 'mock_c_compiler' of type '<class "
+                "'fab.tools.compiler.CCompiler'>' instead of Ar"
+                in str(err.value))

tests/unit_tests/steps/test_link.py

@@ -34,16 +34,20 @@ def test_run(self, tool_box):
             linker = Linker("mock_link", "mock_link.exe", "mock-vendor")
             # Mark the linker as available to it can be added to the tool box
             linker._is_available = True
+
+            # Add a custom library to the linker
+            linker.add_lib_flags('mylib', ['-L/my/lib', '-mylib'])
             tool_box.add_tool(linker, silent_replace=True)
             mock_result = mock.Mock(returncode=0, stdout="abc\ndef".encode())
             with mock.patch('fab.tools.tool.subprocess.run',
                             return_value=mock_result) as tool_run, \
                     pytest.warns(UserWarning,
                                  match="_metric_send_conn not "
                                        "set, cannot send metrics"):
-                link_exe(config, flags=['-fooflag', '-barflag'])
+                link_exe(config, libs=['mylib'], flags=['-fooflag', '-barflag'])
 
         tool_run.assert_called_with(
             ['mock_link.exe', '-L/foo1/lib', '-L/foo2/lib', 'bar.o', 'foo.o',
-             '-fooflag', '-barflag', '-o', 'workspace/foo'],
+             '-L/my/lib', '-mylib', '-fooflag', '-barflag',
+             '-o', 'workspace/foo'],
             capture_output=True, env=None, cwd=None, check=False)