From ae04f0bc2f0243ffc5aeb7358dc1f3292a1faa7f Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Tue, 12 Nov 2024 21:18:01 +1100 Subject: [PATCH] Linker lib flags (#348) * #3 Make it easier to create wrapper around standard compiler. * #3 Added documentation for all tool related classes and their usage. * #3 Added MISC category. * Addressed reviewer's comments. * Updated cli to properly use ToolBox etc, removing hard-coded gnu command linker option. * Fixed mypy failures, including changes to import statement to avoid cyclic imports :(. * #3 Fix circular import. * Added #TODO so that this can be removed once fparser supports sentinels. * Fix typing problems by ignoring fparser. * Replaced more string names for artefacts with enums. * Removed EXECUTABLES from constants. * Moved Artefact class out of ArtefactStore and renamed it to ArtefactSet. * Moved OBJECT_FILES from constants into ArtefactSet. * Moved OBJECT_ARCHIVES from constants to ArtefactSet. * Moved PRAGMAD_C from constants to ArtefactSet. * Turned 'all_source' into an enum. * Allow integer as revision. * Fixed flake8 error. * Removed specific functions to add/get fortran source files etc. * Removed non-existing and unneccessary collections. * Try to fix all run_configs. * Fixed rebase issues. * Added replace functionality to ArtefactStore, updated test_artefacts to cover all lines in that file. * Started to replace artefacts when files are pre-processed. * Removed linker argument from linking step in all examples. * Try to get jules to link. * Fixed build_jules. * Fixed other issues raised in reviews. * Try to get jules to link. * Fixed other issues raised in reviews. * Simplify handling of X90 files by replacing the X90 with x90, meaning only one artefact set is involved when running PSyclone. * Make OBJECT_ARCHIVES also a dict, migrate more code to replace/add files to the default build artefact collections. * Fixed some examples. * Fix flake8 error. * Fixed failing tests. * Support empty comments. * Fix preprocessor to not unnecessary remove and add files that are already in the output directory. * Allow find_soure_files to be called more than once by adding files (not replacing artefact). * Updated lfric_common so that files created by configurator are written in build (not source). * Use c_build_files instead of pragmad_c. * Removed unnecessary str. * Documented the new artefact set handling. * Fixed typo. * Make the PSyclone API configurable. * Fixed formatting of documentation, properly used ArtefactSet names. * Support .f and .F Fortran files. * Removed setter for tool.is_available, which was only used for testing. * #3 Fix documentation and coding style issues from review. * Renamed Categories into Category. * Minor coding style cleanup. * Removed more unnecessary (). * Re-added (invalid) grab_pre_build call. * Fixed typo. * Renamed set_default_vendor to set_default_compiler_suite. * Renamed VendorTool to CompilerSuiteTool. * Also accept a Path as exec_name specification for a tool. * Move the check_available function into the base class. * Fixed some types and documentation. * Fix typing error. * Added explanation for meta-compiler. * Improved error handling and documentation. * Replace mpiifort with mpifort to be a tiny bit more portable. * Use classes to group tests for git/svn/fcm together. * Fixed issue in get_transformation script, and moved script into lfric_common to remove code duplication. * Code improvement as suggested by review. * Fixed run config * Added reference to ticket. * Updated type information. * More typing fixes. * Fixed typing warnings. * As requested by reviewer removed is_working_copy functionality. * Issue a warning (which can be silenced) when a tool in a toolbox is replaced. * Fixed flake8. * Fixed flake8. * Fixed failing test. * Addressed issues raised in review. * Removed now unnecessary operations. * Updated some type information. * Fixed all references to APIs to be consistent with PSyclone 2.5. * Added api to the checksum computation. * Fixed type information. * Added test to verify that changing the api changes the checksum. * Make compiler version a tuple of integers * Update some tests to use tuple versions * Explicitly test handling of bad version format * Fix formatting * Tidying up * Make compiler raise an error for any invalid version string Assume these compilers don't need to be hashed. Saves dealing with empty tuples. * Check compiler version string for compiler name * Fix formatting * Add compiler.get_version_string() method Includes other cleanup from PR comments * Add mpi and openmp settings to BuildConfig, made compiler MPI aware. * Looks like the circular dependency has been fixed. * Revert "Looks like the circular dependency has been fixed." ... while it works with the tests, a real application still triggered it. This reverts commit 150dc379af9df8c38e623fae144a0d5196319f10. * Don't even try to find a C compiler if no C files are to be compiled. * Updated gitignore to ignore (recently renamed) documentation. * Fixed failing test. * Return from compile Fortran early if there are no files to compiles. Fixed coding style. * Add MPI enables wrapper for intel and gnu compiler. * Fixed test. * Automatically add openmp flag to compiler and linker based on BuildConfig. * Removed enforcement of keyword parameters, which is not supported in python 3.7. * Fixed failing test. * Support more than one tool of a given suite by sorting them. * Use different version checkout for each compiler vendor with mixins * Refactoring, remove unittest compiler class * Fix some mypy errors * Use 'Union' type hint to fix build checks * Added option to add flags to a tool. * Introduce proper compiler wrapper, used this to implement properly wrapper MPI compiler. * Fixed typo in types. * Return run_version_command to base Compiler class Provides default version command that can be overridden for other compilers. Also fix some incorrect tests Other tidying * Add a missing type hint * Added (somewhat stupid) 'test' to reach 100% coverage of PSyclone tool. * Simplified MPI support in wrapper. * More compiler wrapper coverage. * Removed duplicated function. * Removed debug print. * Removed permanently changing compiler attributes, which can cause test failures later. * More test for C compiler wrapper. * More work on compiler wrapper tests. * Fixed version and availability handling, added missing tests for 100% coverage. * Fixed typing error. * Try to fix python 3.7. * Tried to fix failing tests. * Remove inheritance from mixins and use protocol * Simplify compiler inheritance Mixins have static methods with unique names, overrides only happen in concrete classes * Updated wrapper and tests to handle error raised in get_version. * Simplified regular expressions (now tests cover detection of version numbers with only a major version). * Test for missing mixin. * Use the parsing mixing from the compiler in a compiler wrapper. * Use setattr instead of assignment to make mypy happy. * Simplify usage of compiler-specific parsing mixins. * Minor code cleanup. * Updated documentation. * Simplify usage of compiler-specific parsing mixins. * Test for missing mixin. * Fixed test. * Added missing openmp_flag property to compiler wrapper. * Don't use isinstance for consistency check, which does not work for CompilerWrappers. * Fixed isinstance test for C compilation which doesn't work with a CompilerWrapper. * Use a linker's compiler to determine MPI support. Removed mpi property from CompilerSuite. * Added more tests for invalid version numbers. * Added more test cases for invalid version number, improved regex to work as expected. * Fixed typo in test. * Fixed flake/mypy errors. * Combine wrapper flags with flags from wrapped compiler. * Made mypy happy. * Fixed test. * Split tests into smaller individual ones, fixed missing asssert in test. * Parameterised compiler version tests to also test wrapper. * Added missing MPI parameter when getting the compiler. * Fixed comments. * Order parameters to be in same order for various compiler classes. * Remove stray character * Added getter for wrapped compiler. * Fixed small error that would prevent nested compiler wrappers from being used. * Added a cast to make mypy happy. * Add simple getter for linker library flags * Add getter for linker flags by library * Fix formatting * Add optional libs argument to link function * Reorder and clean up linker tests * Make sure `Linker.link()` raises for unknown lib * Add missing type * Fix typing error * Add 'libs' argument to link_exe function * Try to add documentation for the linker libs feature * Use correct list type in link_exe hint * Add silent replace option to linker.add_lib_flags * Fixed spelling mistake in option. * Clarified documentation. * Removed unnecessary functions in CompilerWrapper. * Fixed failing test triggered by executing them in specific order (tools then steps) * Fixed line lengths. * Add tests for linker LDFLAG * Add pre- and post- lib flags to link function * Fix syntax in built-in lib flags * Remove netcdf as a built-in linker library Bash-style substitution is not currently handled * Configure pre- and post-lib flags on the Linker object Previously they were passed into the Linker.link() function * Use more realistic linker lib flags * Formatting fix * Made mpi and openmp default to False in the BuildConfig constructor. * Removed white space. * Support compilers that do not support OpenMP. * Added documentation for openmp parameter. --------- Co-authored-by: jasonjunweilyu <161689601+jasonjunweilyu@users.noreply.github.com> Co-authored-by: Luke Hoffmann Co-authored-by: Luke Hoffmann <992315+lukehoffmann@users.noreply.github.com> --- Documentation/source/advanced_config.rst | 8 + source/fab/steps/link.py | 20 +- source/fab/tools/linker.py | 77 ++++++- tests/conftest.py | 1 + .../unit_tests/steps/test_archive_objects.py | 14 +- tests/unit_tests/steps/test_link.py | 8 +- tests/unit_tests/tools/test_linker.py | 210 ++++++++++++++++-- 7 files changed, 302 insertions(+), 36 deletions(-) diff --git a/Documentation/source/advanced_config.rst b/Documentation/source/advanced_config.rst index faaf0b25..d2b54ebf 100644 --- a/Documentation/source/advanced_config.rst +++ b/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 ------------------- diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index 6a14cf64..e67a8cce 100644 --- a/source/fab/steps/link.py +++ b/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. @@ -60,13 +65,15 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None): openmp=config.openmp) 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) @@ -116,4 +123,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) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 69cfcec2..8959b3de 100644 --- a/source/fab/tools/linker.py +++ b/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) diff --git a/tests/conftest.py b/tests/conftest.py index 835cd294..559d4f3b 100644 --- a/tests/conftest.py +++ b/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 diff --git a/tests/unit_tests/steps/test_archive_objects.py b/tests/unit_tests/steps/test_archive_objects.py index f5b2683e..097200ea 100644 --- a/tests/unit_tests/steps/test_archive_objects.py +++ b/tests/unit_tests/steps/test_archive_objects.py @@ -81,20 +81,20 @@ def test_for_library(self): 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, tool_box): '''Test that an incorrect archive tool is detected ''' - config = BuildConfig('proj', ToolBox()) - tool_box = config.tool_box + config = BuildConfig('proj', tool_box) cc = tool_box.get_tool(Category.C_COMPILER, config.mpi, config.openmp) - # And set its category to 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 '' instead of Ar" in str(err.value)) + assert ("Unexpected tool 'mock_c_compiler' of type '' instead of Ar" + in str(err.value)) diff --git a/tests/unit_tests/steps/test_link.py b/tests/unit_tests/steps/test_link.py index a675f54c..a20c4ff4 100644 --- a/tests/unit_tests/steps/test_link.py +++ b/tests/unit_tests/steps/test_link.py @@ -34,6 +34,9 @@ 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', @@ -41,9 +44,10 @@ def test_run(self, tool_box): 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) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 772cd7ec..6984c790 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -9,6 +9,7 @@ from pathlib import Path from unittest import mock +import warnings import pytest @@ -18,8 +19,7 @@ def test_linker(mock_c_compiler, mock_fortran_compiler): '''Test the linker constructor.''' - linker = Linker(name="my_linker", exec_name="my_linker.exe", - suite="suite") + linker = Linker(name="my_linker", exec_name="my_linker.exe", suite="suite") assert linker.category == Category.LINKER assert linker.name == "my_linker" assert linker.exec_name == "my_linker.exe" @@ -52,6 +52,13 @@ def test_linker(mock_c_compiler, mock_fortran_compiler): "creating Linker." in str(err.value)) +def test_linker_gets_ldflags(mock_c_compiler): + """Tests that the linker retrieves env.LDFLAGS""" + with mock.patch.dict("os.environ", {"LDFLAGS": "-lm"}): + linker = Linker(compiler=mock_c_compiler) + assert "-lm" in linker.flags + + def test_linker_check_available(mock_c_compiler): '''Tests the is_available functionality.''' @@ -83,34 +90,168 @@ def test_linker_check_available(mock_c_compiler): assert linker.check_available() is False +# ==================== +# Managing lib flags: +# ==================== +def test_linker_get_lib_flags(mock_linker): + """Linker should provide a map of library names, each leading to a list of + linker flags + """ + # netcdf flags are built in to the mock linker + result = mock_linker.get_lib_flags("netcdf") + assert result == ["-lnetcdff", "-lnetcdf"] + + +def test_linker_get_lib_flags_unknown(mock_c_compiler): + """Linker should raise an error if flags are requested for a library that is + unknown + """ + linker = Linker(compiler=mock_c_compiler) + with pytest.raises(RuntimeError) as err: + linker.get_lib_flags("unknown") + assert "Unknown library name: 'unknown'" in str(err.value) + + +def test_linker_add_lib_flags(mock_c_compiler): + """Linker should provide a way to add a new set of flags for a library""" + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("xios", ["-L", "xios/lib", "-lxios"]) + + # Make sure we can get it back. The order should be maintained. + result = linker.get_lib_flags("xios") + assert result == ["-L", "xios/lib", "-lxios"] + + +def test_linker_add_lib_flags_overwrite_defaults(mock_linker): + """Linker should provide a way to replace the default flags for a library""" + + # Initially we have the default netcdf flags + result = mock_linker.get_lib_flags("netcdf") + assert result == ["-lnetcdff", "-lnetcdf"] + + # Replace them with another set of flags. + warn_message = 'Replacing existing flags for library netcdf' + with pytest.warns(UserWarning, match=warn_message): + mock_linker.add_lib_flags( + "netcdf", ["-L", "netcdf/lib", "-lnetcdf"]) + + # Test that we can see our custom flags + result = mock_linker.get_lib_flags("netcdf") + assert result == ["-L", "netcdf/lib", "-lnetcdf"] + + +def test_linker_add_lib_flags_overwrite_silent(mock_linker): + """Linker should provide the option to replace flags for a library without + generating a warning + """ + + # Initially we have the default netcdf flags + mock_linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + assert mock_linker.get_lib_flags("customlib") == ["-lcustom", "-jcustom"] + + # Replace them with another set of flags. + with warnings.catch_warnings(): + warnings.simplefilter("error") + mock_linker.add_lib_flags("customlib", ["-t", "-b"], + silent_replace=True) + + # Test that we can see our custom flags + result = mock_linker.get_lib_flags("customlib") + assert result == ["-t", "-b"] + + +def test_linker_remove_lib_flags(mock_linker): + """Linker should provide a way to remove the flags for a library""" + mock_linker.remove_lib_flags("netcdf") + + with pytest.raises(RuntimeError) as err: + mock_linker.get_lib_flags("netcdf") + assert "Unknown library name: 'netcdf'" in str(err.value) + + +def test_linker_remove_lib_flags_unknown(mock_linker): + """Linker should silently allow removing flags for unknown library""" + mock_linker.remove_lib_flags("unknown") + + +# ==================== +# Linking: +# ==================== def test_linker_c(mock_c_compiler): - '''Test the link command line when no additional libraries are - specified.''' + '''Test the link command line when no additional libraries are specified.''' linker = Linker(compiler=mock_c_compiler) + # Add a library to the linker, but don't use it in the link step + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + mock_result = mock.Mock(returncode=0) with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: linker.link([Path("a.o")], Path("a.out"), openmp=False) tool_run.assert_called_with( - ["mock_c_compiler.exe", 'a.o', '-o', 'a.out'], capture_output=True, - env=None, cwd=None, check=False) + ["mock_c_compiler.exe", "a.o", "-o", "a.out"], + capture_output=True, env=None, cwd=None, check=False) def test_linker_c_with_libraries(mock_c_compiler): - '''Test the link command line when additional libraries are specified.''' + """Test the link command line when additional libraries are specified.""" + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + + with mock.patch.object(linker, "run") as link_run: + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=True) + # The order of the 'libs' list should be maintained + link_run.assert_called_with( + ["-fopenmp", "a.o", "-lcustom", "-jcustom", "-o", "a.out"]) + + +def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): + """Test the link command line when a library and additional flags are + specified.""" linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + linker.add_post_lib_flags(["-extra-flag"]) + + with mock.patch.object(linker, "run") as link_run: + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) + link_run.assert_called_with( + ["a.o", "-lcustom", "-jcustom", "-extra-flag", "-o", "a.out"]) + + +def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): + """Test the link command line when a library and additional flags are + specified.""" + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + linker.add_pre_lib_flags(["-L", "/common/path/"]) + with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), add_libs=["-L", "/tmp"], - openmp=True) - link_run.assert_called_with(['-fopenmp', 'a.o', '-L', '/tmp', - '-o', 'a.out']) + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) + link_run.assert_called_with( + ["a.o", "-L", "/common/path/", "-lcustom", "-jcustom", "-o", "a.out"]) + + +def test_linker_c_with_unknown_library(mock_c_compiler): + """Test the link command raises an error when unknow libraries are + specified. + """ + linker = Linker(compiler=mock_c_compiler)\ + + with pytest.raises(RuntimeError) as err: + # Try to use "customlib" when we haven't added it to the linker + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=True) + + assert "Unknown library name: 'customlib'" in str(err.value) def test_compiler_linker_add_compiler_flag(mock_c_compiler): '''Test that a flag added to the compiler will be automatically - added to the link line (even if the flags are modified after - creating the linker ... in case that the user specifies additional - flags after creating the linker).''' + added to the link line (even if the flags are modified after creating the + linker ... in case that the user specifies additional flags after creating + the linker).''' linker = Linker(compiler=mock_c_compiler) mock_c_compiler.flags.append("-my-flag") @@ -124,8 +265,8 @@ def test_compiler_linker_add_compiler_flag(mock_c_compiler): def test_linker_add_compiler_flag(): - '''Make sure linker flags work if a linker is created without - a compiler: + '''Make sure ad-hoc linker flags work if a linker is created without a + compiler: ''' linker = Linker("no-compiler", "no-compiler.exe", "suite") linker.flags.append("-some-other-flag") @@ -136,3 +277,40 @@ def test_linker_add_compiler_flag(): tool_run.assert_called_with( ['no-compiler.exe', '-some-other-flag', 'a.o', '-o', 'a.out'], capture_output=True, env=None, cwd=None, check=False) + + +def test_linker_all_flag_types(mock_c_compiler): + """Make sure all possible sources of linker flags are used in the right + order""" + with mock.patch.dict("os.environ", {"LDFLAGS": "-ldflag"}): + linker = Linker(compiler=mock_c_compiler) + + mock_c_compiler.flags.extend(["-compiler-flag1", "-compiler-flag2"]) + linker.flags.extend(["-linker-flag1", "-linker-flag2"]) + linker.add_pre_lib_flags(["-prelibflag1", "-prelibflag2"]) + linker.add_lib_flags("customlib1", ["-lib1flag1", "lib1flag2"]) + linker.add_lib_flags("customlib2", ["-lib2flag1", "lib2flag2"]) + linker.add_post_lib_flags(["-postlibflag1", "-postlibflag2"]) + + mock_result = mock.Mock(returncode=0) + with mock.patch("fab.tools.tool.subprocess.run", + return_value=mock_result) as tool_run: + linker.link([ + Path("a.o")], Path("a.out"), + libs=["customlib2", "customlib1"], + openmp=True) + + tool_run.assert_called_with([ + "mock_c_compiler.exe", + # Note: compiler flags and linker flags will be switched when the Linker + # becomes a CompilerWrapper in a following PR + "-ldflag", "-linker-flag1", "-linker-flag2", + "-compiler-flag1", "-compiler-flag2", + "-fopenmp", + "a.o", + "-prelibflag1", "-prelibflag2", + "-lib2flag1", "lib2flag2", + "-lib1flag1", "lib1flag2", + "-postlibflag1", "-postlibflag2", + "-o", "a.out"], + capture_output=True, env=None, cwd=None, check=False)