From c585ff734a83b97cd945ff96468bcf56eb3ce46c Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 30 Jan 2025 10:18:52 +1100 Subject: [PATCH] Linker wrapper new (#375) * Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Updated comment. * Removed mixing, use a simple regex instead. * Added support for ifx/icx compiler as intel-llvm class. * Added support for nvidia compiler. * Add preliminary support for Cray compiler. * Added Cray compiler wrapper ftn and cc. * Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray). * Changed names again. * Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc. * Fixed incorrect name in comments. * Additional compilers (#349) * 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 * Removed mixing, use a simple regex instead. * Added support for ifx/icx compiler as intel-llvm class. * Added support for nvidia compiler. * Add preliminary support for Cray compiler. * Added Cray compiler wrapper ftn and cc. * Made mpi and openmp default to False in the BuildConfig constructor. * Removed white space. * Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray). * Changed names again. * Support compilers that do not support OpenMP. * Added documentation for openmp parameter. * Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc. * Fixed incorrect name in comments. --------- 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> * Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Added shell tool. * Try to make mypy happy. * Removed debug code. * ToolRepository now only returns default that are available. Updated tests to make tools as available. * Fixed typos and coding style. * Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Updated comment. * Fixed failing tests. * Updated fparser dependency to version 0.2. * Replace old code for handling sentinels with triggering this behaviour in fparser. Require config in constructor of Analyser classes. * Fixed tests for latest changes. * Removed invalid openmp continuation line - since now fparser fails when trying to parse this line. * Added test for disabled openmp parsing. Updated test to work with new test file. * Coding style changes. * Fix flake issues. * Fixed double _. * Make Linker inherit CompilerWrapper * Fix up tests for new Linker inheritence * Fix a flake error * Use linker wrapping to combine flags from the wrapped linker with the linker wrapper. * Minor code cleanup. * Created linker wrapper in ToolRepository. * Try making linker a CompilerSuiteTool instead of a CompilerWrapper. * Updated tests. * Fix support for post-libs. * Fixed mypy. * Removed more accesses to private members. * Added missing type hint. * Make flake8 happy. * Added missing openmp handling in linker. * Addressed issues raised in review. * Forgot that file in previous commit. --------- Co-authored-by: Luke Hoffmann <992315+lukehoffmann@users.noreply.github.com> Co-authored-by: jasonjunweilyu <161689601+jasonjunweilyu@users.noreply.github.com> Co-authored-by: Luke Hoffmann --- source/fab/tools/compiler.py | 7 +- source/fab/tools/compiler_wrapper.py | 4 +- source/fab/tools/linker.py | 203 +++++++++++++----- source/fab/tools/preprocessor.py | 2 +- source/fab/tools/tool_repository.py | 35 ++- tests/conftest.py | 7 +- tests/unit_tests/steps/test_link.py | 38 +++- .../steps/test_link_shared_object.py | 22 +- tests/unit_tests/tools/test_compiler.py | 4 +- tests/unit_tests/tools/test_linker.py | 197 +++++++++++------ 10 files changed, 369 insertions(+), 150 deletions(-) diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index 0b5618de..b937e9d1 100644 --- a/source/fab/tools/compiler.py +++ b/source/fab/tools/compiler.py @@ -64,7 +64,7 @@ def __init__(self, name: str, self._compile_flag = compile_flag if compile_flag else "-c" self._output_flag = output_flag if output_flag else "-o" self._openmp_flag = openmp_flag if openmp_flag else "" - self.flags.extend(os.getenv("FFLAGS", "").split()) + self.add_flags(os.getenv("FFLAGS", "").split()) self._version_regex = version_regex @property @@ -83,6 +83,11 @@ def openmp_flag(self) -> str: '''Returns the flag to enable OpenMP.''' return self._openmp_flag + @property + def output_flag(self) -> str: + '''Returns the flag that specifies the output flag.''' + return self._output_flag + def get_hash(self) -> int: ''':returns: a hash based on the compiler name and version. ''' diff --git a/source/fab/tools/compiler_wrapper.py b/source/fab/tools/compiler_wrapper.py index 09ce5015..9338f848 100644 --- a/source/fab/tools/compiler_wrapper.py +++ b/source/fab/tools/compiler_wrapper.py @@ -4,8 +4,8 @@ # which you should have received as part of this distribution ############################################################################## -"""This file contains the base class for any compiler, and derived -classes for gcc, gfortran, icc, ifort +"""This file contains the base class for any compiler-wrapper, including +the derived classes for mpif90, mpicc, and CrayFtnWrapper and CrayCcWrapper. """ from pathlib import Path diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 8959b3de..2acef01b 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -7,9 +7,11 @@ """This file contains the base class for any Linker. """ +from __future__ import annotations + import os from pathlib import Path -from typing import cast, Dict, List, Optional +from typing import cast, Dict, List, Optional, Union import warnings from fab.tools.category import Category @@ -18,39 +20,51 @@ class Linker(CompilerSuiteTool): - '''This is the base class for any Linker. If a compiler is specified, - its name, executable, and compile suite will be used for the linker (if - not explicitly set in the constructor). - - :param name: the name of the linker. - :param exec_name: the name of the executable. - :param suite: optional, the name of the suite. - :param compiler: optional, a compiler instance - :param output_flag: flag to use to specify the output name. + '''This is the base class for any Linker. It takes either another linker + instance, or a compiler instance as parameter in the constructor. Exactly + one of these must be provided. + + :param compiler: an optional compiler instance + :param linker: an optional linker instance + :param name: name of the linker + + :raises RuntimeError: if both compiler and linker are specified. + :raises RuntimeError: if neither compiler nor linker is specified. ''' - # pylint: disable=too-many-arguments - def __init__(self, name: Optional[str] = None, + def __init__(self, compiler: Optional[Compiler] = None, + linker: Optional[Linker] = None, exec_name: Optional[str] = None, - suite: Optional[str] = None, - compiler: Optional[Compiler] = None, - output_flag: str = "-o"): - if (not name or not exec_name or not suite) and not compiler: - raise RuntimeError("Either specify name, exec name, and suite " - "or a compiler when creating Linker.") - # Make mypy happy, since it can't work out otherwise if these string - # variables might still be None :( - compiler = cast(Compiler, compiler) + name: Optional[str] = None): + + if linker and compiler: + raise RuntimeError("Both compiler and linker is specified in " + "linker constructor.") + if not linker and not compiler: + raise RuntimeError("Neither compiler nor linker is specified in " + "linker constructor.") + self._compiler = compiler + self._linker = linker + + search_linker = self + while search_linker._linker: + search_linker = search_linker._linker + final_compiler = search_linker._compiler if not name: - name = compiler.name + assert final_compiler # make mypy happy + name = f"linker-{final_compiler.name}" + if not exec_name: - exec_name = compiler.exec_name - if not suite: - suite = compiler.suite - self._output_flag = output_flag - super().__init__(name, exec_name, suite, Category.LINKER) - self._compiler = compiler - self.flags.extend(os.getenv("LDFLAGS", "").split()) + # This will search for the name in linker or compiler + exec_name = self.get_exec_name() + + super().__init__( + name=name, + exec_name=exec_name, + suite=self.suite, + category=Category.LINKER) + + self.add_flags(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. self._lib_flags: Dict[str, List[str]] = {} @@ -58,20 +72,55 @@ def __init__(self, name: Optional[str] = None, 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.''' - return self._compiler.mpi - def check_available(self) -> bool: - ''' - :returns: whether the linker is available or not. We do this - by requesting the linker version. + ''':returns: whether this linker is available by asking the wrapped + linker or compiler. ''' if self._compiler: return self._compiler.check_available() + assert self._linker # make mypy happy + return self._linker.check_available() + + def get_exec_name(self) -> str: + ''':returns: the name of the executable by asking the wrapped + linker or compiler.''' + if self._compiler: + return self._compiler.exec_name + assert self._linker # make mypy happy + return self._linker.exec_name + + @property + def suite(self) -> str: + ''':returns: the suite this linker belongs to by getting it from + the wrapper compiler or linker.''' + return cast(CompilerSuiteTool, (self._compiler or self._linker)).suite + + @property + def mpi(self) -> bool: + ''':returns" whether this linker supports MPI or not by checking + with the wrapper compiler or linker.''' + if self._compiler: + return self._compiler.mpi + assert self._linker # make mypy happy + return self._linker.mpi - return super().check_available() + @property + def openmp(self) -> bool: + ''':returns" whether this linker supports OpenMP or not by checking + with the wrapper compiler or linker.''' + if self._compiler: + return self._compiler.openmp + assert self._linker # make mypy happy + return self._linker.openmp + + @property + def output_flag(self) -> str: + ''':returns: the flag that is used to specify the output name. + ''' + if self._compiler: + return self._compiler.output_flag + assert self._linker # make mypy happy + return self._linker.output_flag def get_lib_flags(self, lib: str) -> List[str]: '''Gets the standard flags for a standard library @@ -85,6 +134,10 @@ def get_lib_flags(self, lib: str) -> List[str]: try: return self._lib_flags[lib] except KeyError: + # If a lib is not defined here, but this is a wrapper around + # another linker, return the result from the wrapped linker + if self._linker: + return self._linker.get_lib_flags(lib) raise RuntimeError(f"Unknown library name: '{lib}'") def add_lib_flags(self, lib: str, flags: List[str], @@ -128,6 +181,47 @@ def add_post_lib_flags(self, flags: List[str]): ''' self._post_lib_flags.extend(flags) + def get_pre_link_flags(self) -> List[str]: + '''Returns the list of pre-link flags. It will concatenate the + flags for this instance with all potentially wrapped linkers. + This wrapper's flag will come first - the assumption is that + the pre-link flags are likely paths, so we need a wrapper to + be able to put a search path before the paths from a wrapped + linker. + + :returns: List of pre-link flags of this linker and all + wrapped linkers + ''' + params: List[str] = [] + if self._pre_lib_flags: + params.extend(self._pre_lib_flags) + if self._linker: + # If we are wrapping a linker, get the wrapped linker's + # pre-link flags and append them to the end (so the linker + # wrapper's settings come before the setting from the + # wrapped linker). + params.extend(self._linker.get_pre_link_flags()) + return params + + def get_post_link_flags(self) -> List[str]: + '''Returns the list of post-link flags. It will concatenate the + flags for this instance with all potentially wrapped linkers. + This wrapper's flag will be added to the end. + + :returns: List of post-link flags of this linker and all + wrapped linkers + ''' + params: List[str] = [] + if self._linker: + # If we are wrapping a linker, get the wrapped linker's + # post-link flags and add them first (so this linker + # wrapper's settings come after the setting from the + # wrapped linker). + params.extend(self._linker.get_post_link_flags()) + if self._post_lib_flags: + params.extend(self._post_lib_flags) + return params + def link(self, input_files: List[Path], output_file: Path, openmp: bool, libs: Optional[List[str]] = None) -> str: @@ -141,21 +235,30 @@ def link(self, input_files: List[Path], output_file: Path, :returns: the stdout of the link command ''' - if self._compiler: - # Create a copy: - params = self._compiler.flags[:] - if openmp: - params.append(self._compiler.openmp_flag) - else: - params = [] + + params: List[Union[str, Path]] = [] + + # Find the compiler by following the (potentially + # layered) linker wrapper. + linker = self + while linker._linker: + linker = linker._linker + # Now we must have a compiler + compiler = linker._compiler + assert compiler # make mypy happy + params.extend(compiler.flags) + + if openmp: + params.append(compiler.openmp_flag) + # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) + params.extend(self.get_pre_link_flags()) - 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)]) + + params.extend(self.get_post_link_flags()) + params.extend([self.output_flag, str(output_file)]) + return self.run(params) diff --git a/source/fab/tools/preprocessor.py b/source/fab/tools/preprocessor.py index e620ce2a..dd037874 100644 --- a/source/fab/tools/preprocessor.py +++ b/source/fab/tools/preprocessor.py @@ -63,7 +63,7 @@ class CppFortran(Preprocessor): ''' def __init__(self): super().__init__("cpp", "cpp", Category.FORTRAN_PREPROCESSOR) - self.flags.extend(["-traditional-cpp", "-P"]) + self.add_flags(["-traditional-cpp", "-P"]) # ============================================================================ diff --git a/source/fab/tools/tool_repository.py b/source/fab/tools/tool_repository.py index 965e252b..1bf839f8 100644 --- a/source/fab/tools/tool_repository.py +++ b/source/fab/tools/tool_repository.py @@ -17,8 +17,8 @@ from fab.tools.tool import Tool from fab.tools.category import Category from fab.tools.compiler import Compiler -from fab.tools.compiler_wrapper import (CrayCcWrapper, CrayFtnWrapper, - Mpif90, Mpicc) +from fab.tools.compiler_wrapper import (CompilerWrapper, CrayCcWrapper, + CrayFtnWrapper, Mpif90, Mpicc) from fab.tools.linker import Linker from fab.tools.versioning import Fcm, Git, Subversion from fab.tools import (Ar, Cpp, CppFortran, Craycc, Crayftn, @@ -81,12 +81,12 @@ def __init__(self): # Now create the potential mpif90 and Cray ftn wrapper all_fc = self[Category.FORTRAN_COMPILER][:] for fc in all_fc: - mpif90 = Mpif90(fc) - self.add_tool(mpif90) + if not fc.mpi: + mpif90 = Mpif90(fc) + self.add_tool(mpif90) # I assume cray has (besides cray) only support for Intel and GNU if fc.name in ["gfortran", "ifort"]: crayftn = CrayFtnWrapper(fc) - print("NEW NAME", crayftn, crayftn.name) self.add_tool(crayftn) # Now create the potential mpicc and Cray cc wrapper @@ -114,9 +114,28 @@ def add_tool(self, tool: Tool): # If we have a compiler, add the compiler as linker as well if tool.is_compiler: - tool = cast(Compiler, tool) - linker = Linker(name=f"linker-{tool.name}", compiler=tool) - self[linker.category].append(linker) + compiler = cast(Compiler, tool) + if isinstance(compiler, CompilerWrapper): + # If we have a compiler wrapper, create a new linker using + # the linker based on the wrappped compiler. For example, when + # creating linker-mpif90-gfortran, we want this to be based on + # linker-gfortran (and not on the compiler mpif90-gfortran), + # since the linker-gfortran might have library definitions + # that should be reused. So we first get the existing linker + # (since the compiler exists, a linker for this compiler was + # already created and must exist). + other_linker = self.get_tool( + category=Category.LINKER, + name=f"linker-{compiler.compiler.name}") + other_linker = cast(Linker, other_linker) + linker = Linker(linker=other_linker, + exec_name=compiler.exec_name, + name=f"linker-{compiler.name}") + self[linker.category].append(linker) + else: + linker = Linker(compiler=compiler, + name=f"linker-{compiler.name}") + self[linker.category].append(linker) def get_tool(self, category: Category, name: str) -> Tool: ''':returns: the tool with a given name in the specified category. diff --git a/tests/conftest.py b/tests/conftest.py index 86de6476..36896de7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,7 +11,7 @@ import pytest -from fab.tools import Category, CCompiler, FortranCompiler, Linker, ToolBox +from fab.tools import CCompiler, FortranCompiler, Linker, ToolBox # This avoids pylint warnings about Redefining names from outer scope @@ -44,10 +44,9 @@ def fixture_mock_fortran_compiler(): @pytest.fixture(name="mock_linker") -def fixture_mock_linker(): +def fixture_mock_linker(mock_fortran_compiler): '''Provides a mock linker.''' - mock_linker = Linker("mock_linker", "mock_linker.exe", - Category.FORTRAN_COMPILER) + mock_linker = Linker(mock_fortran_compiler) mock_linker.run = mock.Mock() mock_linker._version = (1, 2, 3) mock_linker.add_lib_flags("netcdf", ["-lnetcdff", "-lnetcdf"]) diff --git a/tests/unit_tests/steps/test_link.py b/tests/unit_tests/steps/test_link.py index a20c4ff4..e9a6750c 100644 --- a/tests/unit_tests/steps/test_link.py +++ b/tests/unit_tests/steps/test_link.py @@ -3,22 +3,29 @@ # For further details please refer to the file COPYRIGHT # which you should have received as part of this distribution # ############################################################################## + +''' +Tests linking an executable. +''' + from pathlib import Path from types import SimpleNamespace from unittest import mock from fab.artefacts import ArtefactSet, ArtefactStore from fab.steps.link import link_exe -from fab.tools import Linker +from fab.tools import FortranCompiler, Linker import pytest class TestLinkExe: + '''Test class for linking an executable. + ''' def test_run(self, tool_box): - # ensure the command is formed correctly, with the flags at the - # end (why?!) - + '''Ensure the command is formed correctly, with the flags at the + end and that environment variable FFLAGS is picked up. + ''' config = SimpleNamespace( project_workspace=Path('workspace'), artefact_store=ArtefactStore(), @@ -29,9 +36,20 @@ def test_run(self, tool_box): config.artefact_store[ArtefactSet.OBJECT_FILES] = \ {'foo': {'foo.o', 'bar.o'}} - with mock.patch('os.getenv', return_value='-L/foo1/lib -L/foo2/lib'): - # We need to create a linker here to pick up the env var: - linker = Linker("mock_link", "mock_link.exe", "mock-vendor") + with mock.patch.dict("os.environ", + {"FFLAGS": "-L/foo1/lib -L/foo2/lib"}): + # We need to create the compiler here in order to pick + # up the environment + mock_compiler = FortranCompiler("mock_fortran_compiler", + "mock_fortran_compiler.exe", + "suite", module_folder_flag="", + version_regex="something", + syntax_only_flag=None, + compile_flag=None, + output_flag=None, openmp_flag=None) + mock_compiler.run = mock.Mock() + + linker = Linker(compiler=mock_compiler) # Mark the linker as available to it can be added to the tool box linker._is_available = True @@ -44,10 +62,12 @@ def test_run(self, tool_box): pytest.warns(UserWarning, match="_metric_send_conn not " "set, cannot send metrics"): - link_exe(config, libs=['mylib'], 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', + ['mock_fortran_compiler.exe', '-L/foo1/lib', '-L/foo2/lib', + 'bar.o', 'foo.o', '-L/my/lib', '-mylib', '-fooflag', '-barflag', '-o', 'workspace/foo'], capture_output=True, env=None, cwd=None, check=False) diff --git a/tests/unit_tests/steps/test_link_shared_object.py b/tests/unit_tests/steps/test_link_shared_object.py index 700a3de3..c76eb957 100644 --- a/tests/unit_tests/steps/test_link_shared_object.py +++ b/tests/unit_tests/steps/test_link_shared_object.py @@ -13,7 +13,7 @@ from fab.artefacts import ArtefactSet, ArtefactStore from fab.steps.link import link_shared_object -from fab.tools import Linker +from fab.tools import FortranCompiler, Linker import pytest @@ -32,9 +32,18 @@ def test_run(tool_box): config.artefact_store[ArtefactSet.OBJECT_FILES] = \ {None: {'foo.o', 'bar.o'}} - with mock.patch('os.getenv', return_value='-L/foo1/lib -L/foo2/lib'): - # We need to create a linker here to pick up the env var: - linker = Linker("mock_link", "mock_link.exe", "vendor") + with mock.patch.dict("os.environ", {"FFLAGS": "-L/foo1/lib -L/foo2/lib"}): + # We need to create the compiler here in order to pick + # up the environment + mock_compiler = FortranCompiler("mock_fortran_compiler", + "mock_fortran_compiler.exe", + "suite", module_folder_flag="", + version_regex="something", + syntax_only_flag=None, + compile_flag=None, output_flag=None, + openmp_flag=None) + mock_compiler.run = mock.Mock() + linker = Linker(mock_compiler) # Mark the linker as available so it can added to the tool box: linker._is_available = True tool_box.add_tool(linker, silent_replace=True) @@ -47,6 +56,7 @@ def test_run(tool_box): flags=['-fooflag', '-barflag']) tool_run.assert_called_with( - ['mock_link.exe', '-L/foo1/lib', '-L/foo2/lib', 'bar.o', 'foo.o', - '-fooflag', '-barflag', '-fPIC', '-shared', '-o', '/tmp/lib_my.so'], + ['mock_fortran_compiler.exe', '-L/foo1/lib', '-L/foo2/lib', 'bar.o', + 'foo.o', '-fooflag', '-barflag', '-fPIC', '-shared', + '-o', '/tmp/lib_my.so'], capture_output=True, env=None, cwd=None, check=False) diff --git a/tests/unit_tests/tools/test_compiler.py b/tests/unit_tests/tools/test_compiler.py index f6c7c158..60c18d78 100644 --- a/tests/unit_tests/tools/test_compiler.py +++ b/tests/unit_tests/tools/test_compiler.py @@ -25,7 +25,7 @@ def test_compiler(): category=Category.C_COMPILER, openmp_flag="-fopenmp") assert cc.category == Category.C_COMPILER assert cc._compile_flag == "-c" - assert cc._output_flag == "-o" + assert cc.output_flag == "-o" # pylint: disable-next=use-implicit-booleaness-not-comparison assert cc.flags == [] assert cc.suite == "gnu" @@ -35,7 +35,7 @@ def test_compiler(): fc = FortranCompiler("gfortran", "gfortran", "gnu", openmp_flag="-fopenmp", version_regex="something", module_folder_flag="-J") assert fc._compile_flag == "-c" - assert fc._output_flag == "-o" + assert fc.output_flag == "-o" assert fc.category == Category.FORTRAN_COMPILER assert fc.suite == "gnu" # pylint: disable-next=use-implicit-booleaness-not-comparison diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 6984c790..052af88d 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -13,43 +13,75 @@ import pytest -from fab.tools import (Category, Linker) +from fab.tools import (Category, Linker, ToolRepository) 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") + assert mock_c_compiler.category == Category.C_COMPILER + assert mock_c_compiler.name == "mock_c_compiler" + + linker = Linker(mock_c_compiler) assert linker.category == Category.LINKER - assert linker.name == "my_linker" - assert linker.exec_name == "my_linker.exe" + assert linker.name == "linker-mock_c_compiler" + assert linker.exec_name == "mock_c_compiler.exe" assert linker.suite == "suite" assert linker.flags == [] + assert linker.output_flag == "-o" - linker = Linker(name="my_linker", compiler=mock_c_compiler) - assert linker.category == Category.LINKER - assert linker.name == "my_linker" - assert linker.exec_name == mock_c_compiler.exec_name - assert linker.suite == mock_c_compiler.suite - assert linker.flags == [] + assert mock_fortran_compiler.category == Category.FORTRAN_COMPILER + assert mock_fortran_compiler.name == "mock_fortran_compiler" - linker = Linker(compiler=mock_c_compiler) + linker = Linker(mock_fortran_compiler) assert linker.category == Category.LINKER - assert linker.name == mock_c_compiler.name - assert linker.exec_name == mock_c_compiler.exec_name - assert linker.suite == mock_c_compiler.suite + assert linker.name == "linker-mock_fortran_compiler" + assert linker.exec_name == "mock_fortran_compiler.exe" + assert linker.suite == "suite" assert linker.flags == [] - linker = Linker(compiler=mock_fortran_compiler) - assert linker.category == Category.LINKER - assert linker.name == mock_fortran_compiler.name - assert linker.exec_name == mock_fortran_compiler.exec_name - assert linker.flags == [] +def test_linker_constructor_error(mock_c_compiler): + '''Test the linker constructor with invalid parameters.''' + + with pytest.raises(RuntimeError) as err: + Linker() + assert ("Neither compiler nor linker is specified in linker constructor." + in str(err.value)) with pytest.raises(RuntimeError) as err: - linker = Linker(name="no-exec-given") - assert ("Either specify name, exec name, and suite or a compiler when " - "creating Linker." in str(err.value)) + Linker(compiler=mock_c_compiler, linker=mock_c_compiler) + assert ("Both compiler and linker is specified in linker constructor." + in str(err.value)) + + +@pytest.mark.parametrize("mpi", [True, False]) +def test_linker_mpi(mock_c_compiler, mpi): + '''Test that linker wrappers handle MPI as expected.''' + + mock_c_compiler._mpi = mpi + linker = Linker(compiler=mock_c_compiler) + assert linker.mpi == mpi + + wrapped_linker = Linker(linker=linker) + assert wrapped_linker.mpi == mpi + + +@pytest.mark.parametrize("openmp", [True, False]) +def test_linker_openmp(mock_c_compiler, openmp): + '''Test that linker wrappers handle openmp as expected. Note that + a compiler detects support for OpenMP by checking if an openmp flag + is defined. + ''' + + if openmp: + mock_c_compiler._openmp_flag = "-some-openmp-flag" + else: + mock_c_compiler._openmp_flag = "" + linker = Linker(compiler=mock_c_compiler) + assert linker.openmp == openmp + + wrapped_linker = Linker(linker=linker) + assert wrapped_linker.openmp == openmp def test_linker_gets_ldflags(mock_c_compiler): @@ -62,31 +94,28 @@ def test_linker_gets_ldflags(mock_c_compiler): def test_linker_check_available(mock_c_compiler): '''Tests the is_available functionality.''' - # First test if a compiler is given. The linker will call the + # First test when a compiler is given. The linker will call the # corresponding function in the compiler: - linker = Linker(compiler=mock_c_compiler) - with mock.patch.object(mock_c_compiler, "check_available", - return_value=True) as comp_run: + linker = Linker(mock_c_compiler) + with mock.patch('fab.tools.compiler.Compiler.get_version', + return_value=(1, 2, 3)): assert linker.check_available() - # It should be called once without any parameter - comp_run.assert_called_once_with() - # Second test, no compiler is given. Mock Tool.run to - # return a success: - linker = Linker("ld", "ld", suite="gnu") - mock_result = mock.Mock(returncode=0) - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result) as tool_run: - linker.check_available() - tool_run.assert_called_once_with( - ["ld", "--version"], capture_output=True, env=None, - cwd=None, check=False) - - # Third test: assume the tool does not exist, check_available - # will return False (and not raise an exception) - linker._is_available = None - with mock.patch("fab.tools.tool.Tool.run", - side_effect=RuntimeError("")) as tool_run: + # Then test the usage of a linker wrapper. The linker will call the + # corresponding function in the wrapper linker: + wrapped_linker = Linker(linker=linker) + with mock.patch('fab.tools.compiler.Compiler.get_version', + return_value=(1, 2, 3)): + assert wrapped_linker.check_available() + + +def test_linker_check_unavailable(mock_c_compiler): + '''Tests the is_available functionality.''' + # assume the tool does not exist, check_available + # will return False (and not raise an exception) + linker = Linker(mock_c_compiler) + with mock.patch('fab.tools.compiler.Compiler.get_version', + side_effect=RuntimeError("")): assert linker.check_available() is False @@ -103,8 +132,8 @@ def test_linker_get_lib_flags(mock_linker): 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 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: @@ -123,7 +152,8 @@ def test_linker_add_lib_flags(mock_c_compiler): def test_linker_add_lib_flags_overwrite_defaults(mock_linker): - """Linker should provide a way to replace the default flags for a library""" + """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") @@ -178,7 +208,9 @@ def test_linker_remove_lib_flags_unknown(mock_linker): # 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"]) @@ -264,29 +296,16 @@ def test_compiler_linker_add_compiler_flag(mock_c_compiler): capture_output=True, env=None, cwd=None, check=False) -def test_linker_add_compiler_flag(): - '''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") - 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( - ['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""" + + # Environment variables for both the linker 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"]) + mock_c_compiler.add_flags(["-compiler-flag1", "-compiler-flag2"]) + linker.add_flags(["-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"]) @@ -302,8 +321,6 @@ def test_linker_all_flag_types(mock_c_compiler): 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", @@ -314,3 +331,49 @@ def test_linker_all_flag_types(mock_c_compiler): "-postlibflag1", "-postlibflag2", "-o", "a.out"], capture_output=True, env=None, cwd=None, check=False) + + +def test_linker_nesting(mock_c_compiler): + """Make sure all possible sources of linker flags are used in the right + order""" + + linker1 = Linker(compiler=mock_c_compiler) + linker1.add_pre_lib_flags(["pre_lib1"]) + linker1.add_lib_flags("lib_a", ["a_from_1"]) + linker1.add_lib_flags("lib_c", ["c_from_1"]) + linker1.add_post_lib_flags(["post_lib1"]) + linker2 = Linker(linker=linker1) + linker2.add_pre_lib_flags(["pre_lib2"]) + linker2.add_lib_flags("lib_b", ["b_from_2"]) + linker2.add_lib_flags("lib_c", ["c_from_2"]) + linker1.add_post_lib_flags(["post_lib2"]) + + mock_result = mock.Mock(returncode=0) + with mock.patch("fab.tools.tool.subprocess.run", + return_value=mock_result) as tool_run: + linker2.link( + [Path("a.o")], Path("a.out"), + libs=["lib_a", "lib_b", "lib_c"], + openmp=True) + tool_run.assert_called_with(["mock_c_compiler.exe", "-fopenmp", + "a.o", "pre_lib2", "pre_lib1", "a_from_1", + "b_from_2", "c_from_2", + "post_lib1", "post_lib2", "-o", "a.out"], + capture_output=True, env=None, cwd=None, + check=False) + + +def test_linker_inheriting(): + '''Make sure that libraries from a wrapper compiler will be + available for a wrapper. + ''' + tr = ToolRepository() + linker_gfortran = tr.get_tool(Category.LINKER, "linker-gfortran") + linker_mpif90 = tr.get_tool(Category.LINKER, "linker-mpif90-gfortran") + + linker_gfortran.add_lib_flags("lib_a", ["a_from_1"]) + assert linker_mpif90.get_lib_flags("lib_a") == ["a_from_1"] + + with pytest.raises(RuntimeError) as err: + linker_mpif90.get_lib_flags("does_not_exist") + assert "Unknown library name: 'does_not_exist'" in str(err.value)