diff --git a/pyproject.toml b/pyproject.toml index ca443aec..e165f7a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ authors = [ license = {file = 'LICENSE.txt'} dynamic = ['version', 'readme'] requires-python = '>=3.7, <4' -dependencies = ['fparser'] +dependencies = ['fparser >= 0.2'] classifiers = [ 'Development Status :: 1 - Planning', 'Environment :: Console', diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index 3f93c588..92997bbf 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -20,8 +20,7 @@ from fab.tools import ToolBox from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import (API, configurator, fparser_workaround_stop_concatenation, - get_transformation_script) +from lfric_common import (API, configurator, get_transformation_script) logger = logging.getLogger('fab') @@ -250,9 +249,6 @@ def file_filtering(config): api=API, ) - # todo: do we need this one in here? - fparser_workaround_stop_concatenation(state) - analyse( state, root_symbol='lfric_atm', diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index 7f075c10..011afa89 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -22,8 +22,7 @@ from fab.tools import ToolBox from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import (API, configurator, fparser_workaround_stop_concatenation, - get_transformation_script) +from lfric_common import (API, configurator, get_transformation_script) logger = logging.getLogger('fab') @@ -75,8 +74,6 @@ api=API, ) - fparser_workaround_stop_concatenation(state) - analyse( state, root_symbol='gungho_model', diff --git a/run_configs/lfric/lfric_common.py b/run_configs/lfric/lfric_common.py index fe8eae03..cbf1ba09 100644 --- a/run_configs/lfric/lfric_common.py +++ b/run_configs/lfric/lfric_common.py @@ -1,10 +1,8 @@ import logging import os -import shutil from typing import Optional from pathlib import Path -from fab.artefacts import ArtefactSet from fab.build_config import BuildConfig from fab.steps import step from fab.steps.find_source_files import find_source_files @@ -84,36 +82,6 @@ def configurator(config, lfric_source: Path, gpl_utils_source: Path, rose_meta_c find_source_files(config, source_root=config_dir) -# ============================================================================ -@step -def fparser_workaround_stop_concatenation(config): - """ - fparser can't handle string concat in a stop statement. This step is - a workaround. - - https://github.com/stfc/fparser/issues/330 - - """ - feign_path = None - for file_path in config.artefact_store[ArtefactSet.FORTRAN_BUILD_FILES]: - if file_path.name == 'feign_config_mod.f90': - feign_path = file_path - break - else: - raise RuntimeError("Could not find 'feign_config_mod.f90'.") - - # rename "broken" version - broken_version = feign_path.with_suffix('.broken') - shutil.move(feign_path, broken_version) - - # make fixed version - bad = "_config: '// &\n 'Unable to close temporary file'" - good = "_config: Unable to close temporary file'" - - open(feign_path, 'wt').write( - open(broken_version, 'rt').read().replace(bad, good)) - - # ============================================================================ def get_transformation_script(fpath: Path, config: BuildConfig) -> Optional[Path]: diff --git a/run_configs/lfric/mesh_tools.py b/run_configs/lfric/mesh_tools.py index fde5b793..5d87c961 100755 --- a/run_configs/lfric/mesh_tools.py +++ b/run_configs/lfric/mesh_tools.py @@ -13,7 +13,7 @@ from fab.steps.psyclone import psyclone, preprocess_x90 from fab.tools import ToolBox -from lfric_common import API, configurator, fparser_workaround_stop_concatenation +from lfric_common import API, configurator from grab_lfric import lfric_source_config, gpl_utils_source_config @@ -60,8 +60,6 @@ api=API, ) - fparser_workaround_stop_concatenation(state) - analyse( state, root_symbol=['cubedsphere_mesh_generator', 'planar_mesh_generator', 'summarise_ugrid'], diff --git a/source/fab/parse/c.py b/source/fab/parse/c.py index 24c26045..02cf2853 100644 --- a/source/fab/parse/c.py +++ b/source/fab/parse/c.py @@ -11,14 +11,14 @@ from pathlib import Path from typing import List, Optional, Union, Tuple -from fab.dep_tree import AnalysedDependent - try: import clang # type: ignore import clang.cindex # type: ignore except ImportError: clang = None +from fab.build_config import BuildConfig +from fab.dep_tree import AnalysedDependent from fab.util import log_or_dot, file_checksum logger = logging.getLogger(__name__) @@ -26,29 +26,33 @@ class AnalysedC(AnalysedDependent): """ - An analysis result for a single C file, containing symbol definitions and dependencies. + An analysis result for a single C file, containing symbol definitions and + dependencies. - Note: We don't need to worry about compile order with pure C projects; we can compile all in one go. - However, with a *Fortran -> C -> Fortran* dependency chain, we do need to ensure that one Fortran file - is compiled before another, so this class must be part of the dependency tree analysis. + Note: We don't need to worry about compile order with pure C projects; we + can compile all in one go. However, with a *Fortran -> C -> Fortran* + dependency chain, we do need to ensure that one Fortran file is + compiled before another, so this class must be part of the + dependency tree analysis. """ - # Note: This subclass adds nothing to it's parent, which provides everything it needs. - # We'd normally remove an irrelevant class like this but we want to keep the door open - # for filtering analysis results by type, rather than suffix. - pass + # Note: This subclass adds nothing to it's parent, which provides + # everything it needs. We'd normally remove an irrelevant class + # like this but we want to keep the door open for filtering + # analysis results by type, rather than suffix. -class CAnalyser(object): +class CAnalyser: """ Identify symbol definitions and dependencies in a C file. """ - def __init__(self): + def __init__(self, config: BuildConfig): # runtime - self._config = None + self._config = config + self._include_region: List[Tuple[int, str]] = [] # todo: simplifiy by passing in the file path instead of the analysed tokens? def _locate_include_regions(self, trans_unit) -> None: @@ -100,8 +104,7 @@ def _check_for_include(self, lineno) -> Optional[str]: include_stack.pop() if include_stack: return include_stack[-1] - else: - return None + return None def run(self, fpath: Path) \ -> Union[Tuple[AnalysedC, Path], Tuple[Exception, None]]: @@ -149,9 +152,11 @@ def run(self, fpath: Path) \ continue logger.debug('Considering node: %s', node.spelling) - if node.kind in {clang.cindex.CursorKind.FUNCTION_DECL, clang.cindex.CursorKind.VAR_DECL}: + if node.kind in {clang.cindex.CursorKind.FUNCTION_DECL, + clang.cindex.CursorKind.VAR_DECL}: self._process_symbol_declaration(analysed_file, node, usr_symbols) - elif node.kind in {clang.cindex.CursorKind.CALL_EXPR, clang.cindex.CursorKind.DECL_REF_EXPR}: + elif node.kind in {clang.cindex.CursorKind.CALL_EXPR, + clang.cindex.CursorKind.DECL_REF_EXPR}: self._process_symbol_dependency(analysed_file, node, usr_symbols) except Exception as err: logger.exception(f'error walking parsed nodes {fpath}') @@ -166,7 +171,8 @@ def _process_symbol_declaration(self, analysed_file, node, usr_symbols): if node.is_definition(): # only global symbols can be used by other files, not static symbols if node.linkage == clang.cindex.LinkageKind.EXTERNAL: - # This should catch function definitions which are exposed to the rest of the application + # This should catch function definitions which are exposed to + # the rest of the application logger.debug(' * Is defined in this file') # todo: ignore if inside user pragmas? analysed_file.add_symbol_def(node.spelling) diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index c5ea7f60..ed2e14d3 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -11,7 +11,6 @@ from pathlib import Path from typing import Union, Optional, Iterable, Dict, Any, Set -from fparser.common.readfortran import FortranStringReader # type: ignore from fparser.two.Fortran2003 import ( # type: ignore Entity_Decl_List, Use_Stmt, Module_Stmt, Program_Stmt, Subroutine_Stmt, Function_Stmt, Language_Binding_Spec, Char_Literal_Constant, Interface_Block, Name, Comment, Module, Call_Stmt, Derived_Type_Def, Derived_Type_Stmt, @@ -21,6 +20,7 @@ from fparser.two.Fortran2008 import ( # type: ignore Type_Declaration_Stmt, Attr_Spec_List) +from fab.build_config import BuildConfig from fab.dep_tree import AnalysedDependent from fab.parse.fortran_common import iter_content, _has_ancestor_type, _typed_child, FortranAnalyserBase from fab.util import file_checksum, string_checksum @@ -167,15 +167,21 @@ class FortranAnalyser(FortranAnalyserBase): A build step which analyses a fortran file using fparser2, creating an :class:`~fab.dep_tree.AnalysedFortran`. """ - def __init__(self, std=None, ignore_mod_deps: Optional[Iterable[str]] = None): + def __init__(self, + config: BuildConfig, + std: Optional[str] = None, + ignore_mod_deps: Optional[Iterable[str]] = None): """ + :param config: The BuildConfig to use. :param std: The Fortran standard. :param ignore_mod_deps: Module names to ignore in use statements. """ - super().__init__(result_class=AnalysedFortran, std=std) + super().__init__(config=config, + result_class=AnalysedFortran, + std=std) self.ignore_mod_deps: Iterable[str] = list(ignore_mod_deps or []) self.depends_on_comment_found = False @@ -295,33 +301,6 @@ def _process_comment(self, analysed_file, obj): # without .o means a fortran symbol else: analysed_file.add_symbol_dep(dep) - if comment[:2] == "!$": - # Check if it is a use statement with an OpenMP sentinel: - # Use fparser's string reader to discard potential comment - # TODO #327: once fparser supports reading the sentinels, - # this can be removed. - # fparser issue: https://github.com/stfc/fparser/issues/443 - reader = FortranStringReader(comment[2:]) - try: - line = reader.next() - except StopIteration: - # No other item, ignore - return - try: - # match returns a 5-tuple, the third one being the module name - module_name = Use_Stmt.match(line.strline)[2] - module_name = module_name.string - except Exception: - # Not a use statement in a sentinel, ignore: - return - - # Register the module name - if module_name in self.ignore_mod_deps: - logger.debug(f"ignoring use of {module_name}") - return - if module_name.lower() not in self._intrinsic_modules: - # found a dependency on fortran - analysed_file.add_module_dep(module_name) def _process_subroutine_or_function(self, analysed_file, fpath, obj): # binding? @@ -353,7 +332,7 @@ def _process_subroutine_or_function(self, analysed_file, fpath, obj): analysed_file.add_symbol_def(name.string) -class FortranParserWorkaround(object): +class FortranParserWorkaround(): """ Use this class to create a workaround when the third-party Fortran parser is unable to process a valid source file. diff --git a/source/fab/parse/fortran_common.py b/source/fab/parse/fortran_common.py index 0ed4f3fe..5942dc8c 100644 --- a/source/fab/parse/fortran_common.py +++ b/source/fab/parse/fortran_common.py @@ -10,13 +10,14 @@ import logging from abc import ABC, abstractmethod from pathlib import Path -from typing import Union, Tuple, Type +from typing import Optional, Tuple, Type, Union from fparser.common.readfortran import FortranFileReader # type: ignore from fparser.two.parser import ParserFactory # type: ignore from fparser.two.utils import FortranSyntaxError # type: ignore from fab import FabException +from fab.build_config import BuildConfig from fab.dep_tree import AnalysedDependent from fab.parse import EmptySourceFile from fab.util import log_or_dot, file_checksum @@ -58,7 +59,8 @@ def _typed_child(parent, child_type: Type, must_exist=False): # Look for a child of a certain type. # Returns the child or None. # Raises ValueError if more than one child of the given type is found. - children = list(filter(lambda child: isinstance(child, child_type), parent.children)) + children = list(filter(lambda child: isinstance(child, child_type), + parent.children)) if len(children) > 1: raise ValueError(f"too many children found of type {child_type}") @@ -66,41 +68,52 @@ def _typed_child(parent, child_type: Type, must_exist=False): return children[0] if must_exist: - raise FabException(f'Could not find child of type {child_type} in {parent}') + raise FabException(f'Could not find child of type {child_type} ' + f'in {parent}') return None class FortranAnalyserBase(ABC): """ - Base class for Fortran parse-tree analysers, e.g FortranAnalyser and X90Analyser. + Base class for Fortran parse-tree analysers, e.g FortranAnalyser and + X90Analyser. """ _intrinsic_modules = ['iso_fortran_env', 'iso_c_binding'] - def __init__(self, result_class, std=None): + def __init__(self, config: BuildConfig, + result_class, + std: Optional[str] = None): """ + :param config: The BuildConfig object. :param result_class: The type (class) of the analysis result. Defined by the subclass. :param std: The Fortran standard. """ + self._config = config self.result_class = result_class self.f2008_parser = ParserFactory().create(std=std or "f2008") - # todo: this, and perhaps other runtime variables like it, might be better set at construction - # if we construct these objects at runtime instead... - # runtime, for child processes to read - self._config = None + @property + def config(self) -> BuildConfig: + '''Returns the BuildConfig to use. + ''' + return self._config def run(self, fpath: Path) \ - -> Union[Tuple[AnalysedDependent, Path], Tuple[EmptySourceFile, None], Tuple[Exception, None]]: + -> Union[Tuple[AnalysedDependent, Path], + Tuple[EmptySourceFile, None], + Tuple[Exception, None]]: """ - Parse the source file and record what we're interested in (subclass specific). + Parse the source file and record what we're interested in (subclass + specific). Reloads previous analysis results if available. - Returns the analysis data and the result file where it was stored/loaded. + Returns the analysis data and the result file where it was + stored/loaded. """ # calculate the prebuild filename @@ -114,9 +127,11 @@ def run(self, fpath: Path) \ # Load the result file into whatever result class we use. loaded_result = self.result_class.load(analysis_fpath) if loaded_result: - # This result might have been created by another user; their prebuild folder copied to ours. - # If so, the fpath in the result will *not* point to the file we eventually want to compile, - # it will point to the user's original file, somewhere else. So replace it with our own path. + # This result might have been created by another user; their + # prebuild folder copied to ours. If so, the fpath in the + # result will *not* point to the file we eventually want to + # compile, it will point to the user's original file, + # somewhere else. So replace it with our own path. loaded_result.fpath = fpath return loaded_result, analysis_fpath @@ -125,43 +140,54 @@ def run(self, fpath: Path) \ # parse the file, get a node tree node_tree = self._parse_file(fpath=fpath) if isinstance(node_tree, Exception): - return Exception(f"error parsing file '{fpath}':\n{node_tree}"), None + return (Exception(f"error parsing file '{fpath}':\n{node_tree}"), + None) if node_tree.content[0] is None: logger.debug(f" empty tree found when parsing {fpath}") - # todo: If we don't save the empty result we'll keep analysing it every time! + # todo: If we don't save the empty result we'll keep analysing + # it every time! return EmptySourceFile(fpath), None # find things in the node tree - analysed_file = self.walk_nodes(fpath=fpath, file_hash=file_hash, node_tree=node_tree) + analysed_file = self.walk_nodes(fpath=fpath, file_hash=file_hash, + node_tree=node_tree) analysed_file.save(analysis_fpath) return analysed_file, analysis_fpath def _get_analysis_fpath(self, fpath, file_hash) -> Path: - return Path(self._config.prebuild_folder / f'{fpath.stem}.{file_hash}.an') + return Path(self.config.prebuild_folder / + f'{fpath.stem}.{file_hash}.an') def _parse_file(self, fpath): """Get a node tree from a fortran file.""" - reader = FortranFileReader(str(fpath), ignore_comments=False) - reader.exit_on_error = False # don't call sys.exit, it messes up the multi-processing + reader = FortranFileReader( + str(fpath), + ignore_comments=False, + include_omp_conditional_lines=self.config.openmp) + # don't call sys.exit, it messes up the multi-processing + reader.exit_on_error = False try: tree = self.f2008_parser(reader) return tree except FortranSyntaxError as err: - # we can't return the FortranSyntaxError, it breaks multiprocessing! + # Don't return the FortranSyntaxError, it breaks multiprocessing! logger.error(f"\nfparser raised a syntax error in {fpath}\n{err}") return Exception(f"syntax error in {fpath}\n{err}") except Exception as err: logger.error(f"\nunhandled error '{type(err)}' in {fpath}\n{err}") - return Exception(f"unhandled error '{type(err)}' in {fpath}\n{err}") + return Exception(f"unhandled error '{type(err)}' in " + f"{fpath}\n{err}") @abstractmethod def walk_nodes(self, fpath, file_hash, node_tree) -> AnalysedDependent: """ - Examine the nodes in the parse tree, recording things we're interested in. + Examine the nodes in the parse tree, recording things we're + interested in. - Return type depends on our subclass, and will be a subclass of AnalysedDependent. + Return type depends on our subclass, and will be a subclass of + AnalysedDependent. """ raise NotImplementedError diff --git a/source/fab/parse/x90.py b/source/fab/parse/x90.py index 902c01fe..09d51718 100644 --- a/source/fab/parse/x90.py +++ b/source/fab/parse/x90.py @@ -9,6 +9,7 @@ from fparser.two.Fortran2003 import Use_Stmt, Call_Stmt, Name, Only_List, Actual_Arg_Spec_List, Part_Ref # type: ignore from fab.parse import AnalysedFile +from fab.build_config import BuildConfig from fab.parse.fortran_common import FortranAnalyserBase, iter_content, logger, _typed_child from fab.util import by_type @@ -64,8 +65,8 @@ class X90Analyser(FortranAnalyserBase): # Makes a parsable fortran version of x90. # todo: Use hashing to reuse previous analysis results. - def __init__(self): - super().__init__(result_class=AnalysedX90) + def __init__(self, config: BuildConfig): + super().__init__(config=config, result_class=AnalysedX90) def walk_nodes(self, fpath, file_hash, node_tree) -> AnalysedX90: # type: ignore diff --git a/source/fab/steps/analyse.py b/source/fab/steps/analyse.py index 1b739e56..49d4b55a 100644 --- a/source/fab/steps/analyse.py +++ b/source/fab/steps/analyse.py @@ -130,8 +130,10 @@ def analyse( unreferenced_deps = list(unreferenced_deps or []) # todo: these seem more like functions - fortran_analyser = FortranAnalyser(std=std, ignore_mod_deps=ignore_mod_deps) - c_analyser = CAnalyser() + fortran_analyser = FortranAnalyser(config=config, + std=std, + ignore_mod_deps=ignore_mod_deps) + c_analyser = CAnalyser(config=config) # Creates the *build_trees* artefact from the files in `self.source_getter`. @@ -144,10 +146,6 @@ def analyse( # - At this point we have a source tree for the entire source. # - (Optionally) Extract a sub tree for every root symbol, if provided. For building executables. - # todo: code smell - refactor (in another PR to keep things small) - fortran_analyser._config = config - c_analyser._config = config - # parse files: List[Path] = source_getter(config.artefact_store) analysed_files = _parse_files(config, files=files, fortran_analyser=fortran_analyser, c_analyser=c_analyser) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 04c1cc27..c9bb3990 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -192,8 +192,7 @@ def _analyse_x90s(config, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: parsable_x90s = run_mp(config, items=x90s, func=make_parsable_x90) # parse - x90_analyser = X90Analyser() - x90_analyser._config = config + x90_analyser = X90Analyser(config=config) with TimerLogger(f"analysing {len(parsable_x90s)} parsable x90 files"): x90_results = run_mp(config, items=parsable_x90s, func=x90_analyser.run) log_or_dot_finish(logger) @@ -209,7 +208,7 @@ def _analyse_x90s(config, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: analysed_x90 = {result.fpath.with_suffix('.x90'): result for result in analysed_x90} # make the hashes from the original x90s, not the parsable versions which have invoke names removed. - for p, r in analysed_x90.items(): + for p in analysed_x90: analysed_x90[p]._file_hash = file_checksum(p).file_hash return analysed_x90 @@ -249,8 +248,7 @@ def _analyse_kernels(config, kernel_roots) -> Dict[str, int]: # We use the normal Fortran analyser, which records psyclone kernel metadata. # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling. # The Analyse step also uses the same fortran analyser. It stores its results so they won't be analysed twice. - fortran_analyser = FortranAnalyser() - fortran_analyser._config = config + fortran_analyser = FortranAnalyser(config=config) with TimerLogger(f"analysing {len(kernel_files)} potential psyclone kernel files"): fortran_results = run_mp(config, items=kernel_files, func=fortran_analyser.run) log_or_dot_finish(logger) diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index ea3f1e77..7448411b 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 @@ -76,13 +76,20 @@ def mpi(self) -> bool: def openmp(self) -> bool: ''':returns: if the compiler supports openmp or not ''' - return self._openmp_flag != "" + # It is important not to use `_openmp_flag` directly, since a compiler + # wrapper overwrites `openmp_flag`. + return self.openmp_flag != "" @property 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 3c5d2997..ae9089c0 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 b0536b69..0fb0f61d 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 Dict, List, Optional, Union import warnings from fab.tools.category import Category @@ -18,39 +20,39 @@ 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 an existing compiler + instance as parameter, and optional another linker. The latter is used + to get linker settings - for example, linker-mpif90-gfortran will use + mpif90-gfortran as compiler (i.e. to test if it is available and get + compilation flags), and linker-gfortran as linker. This way a user + only has to specify linker flags in the most basic class (gfortran), + all other linker wrapper will inherit the settings. + + :param compiler: a 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, - 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) - if not name: - name = 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) + def __init__(self, compiler: Compiler, + linker: Optional[Linker] = None, + name: Optional[str] = None): + self._compiler = compiler - self.flags.extend(os.getenv("LDFLAGS", "").split()) + self._linker = linker + + if not name: + name = f"linker-{compiler.name}" + + super().__init__( + name=name, + exec_name=compiler.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 +60,35 @@ def __init__(self, name: Optional[str] = None, self._pre_lib_flags: List[str] = [] self._post_lib_flags: List[str] = [] + def check_available(self) -> bool: + ''':returns: whether this linker is available by asking the wrapped + linker or compiler. + ''' + return self._compiler.check_available() + + @property + def suite(self) -> str: + ''':returns: the suite this linker belongs to by getting it from + the wrapped compiler.''' + return self._compiler.suite + @property def mpi(self) -> bool: - ''':returns: whether the linker supports MPI or not.''' + ''':returns" whether this linker supports MPI or not by checking + with the wrapped compiler.''' 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. - ''' - if self._compiler: - return self._compiler.check_available() + @property + def openmp(self) -> bool: + ''':returns: whether this linker supports OpenMP or not by checking + with the wrapped compiler.''' + return self._compiler.openmp - return super().check_available() + @property + def output_flag(self) -> str: + ''':returns: the flag that is used to specify the output name. + ''' + return self._compiler.output_flag def get_lib_flags(self, lib: str) -> List[str]: '''Gets the standard flags for a standard library @@ -85,6 +102,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], @@ -118,6 +139,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: @@ -131,21 +193,22 @@ 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]] = [] + + params.extend(self._compiler.flags) + + if openmp: + params.append(self._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/psyclone.py b/source/fab/tools/psyclone.py index 94300bd9..6e4adbbf 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -149,7 +149,7 @@ def process(self, # New version: no API, parameter, but -o for output name: parameters.extend(["-o", transformed_file]) else: - # 2.5.0 or earlier: needs api nemo, output name is -oalg + # 2.5.0 or earlier: needs api nemo, output name is -opsy parameters.extend(["-api", "nemo", "-opsy", transformed_file]) parameters.extend(["-l", "all"]) diff --git a/source/fab/tools/tool.py b/source/fab/tools/tool.py index 78a8de62..a870c657 100644 --- a/source/fab/tools/tool.py +++ b/source/fab/tools/tool.py @@ -63,7 +63,8 @@ def check_available(self) -> bool: :returns: whether the tool is working (True) or not. ''' try: - self.run(self._availability_option) + op = self._availability_option + self.run(op) except (RuntimeError, FileNotFoundError): return False return True diff --git a/source/fab/tools/tool_repository.py b/source/fab/tools/tool_repository.py index 965e252b..a9749757 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 wrapped compiler. For example, when + # creating linker-mpif90-gfortran, we want this to be based on + # linker-gfortran. The compiler mpif90-gfortran will be the + # wrapper compiler. Reason is that e.g. 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(compiler, + linker=other_linker, + 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/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 3c16fd4a..adc35315 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -48,8 +48,8 @@ def test_make_parsable_x90(tmp_path): parsable_x90_path = make_parsable_x90(input_x90_path) - x90_analyser = X90Analyser() with BuildConfig('proj', ToolBox(), fab_workspace=tmp_path) as config: + x90_analyser = X90Analyser(config=config) x90_analyser._config = config # todo: code smell x90_analyser.run(parsable_x90_path) @@ -72,8 +72,8 @@ class TestX90Analyser: def run(self, tmp_path): parsable_x90_path = self.expected_analysis_result.fpath - x90_analyser = X90Analyser() with BuildConfig('proj', ToolBox(), fab_workspace=tmp_path) as config: + x90_analyser = X90Analyser(config=config) x90_analyser._config = config analysed_x90, _ = x90_analyser.run(parsable_x90_path) # type: ignore # don't delete the prebuild diff --git a/tests/unit_tests/parse/c/test_c_analyser.py b/tests/unit_tests/parse/c/test_c_analyser.py index c288baf9..7d457da9 100644 --- a/tests/unit_tests/parse/c/test_c_analyser.py +++ b/tests/unit_tests/parse/c/test_c_analyser.py @@ -17,9 +17,9 @@ def test_simple_result(tmp_path): - c_analyser = CAnalyser() - c_analyser._config = BuildConfig('proj', ToolBox(), mpi=False, - openmp=False, fab_workspace=tmp_path) + config = BuildConfig('proj', ToolBox(), mpi=False, openmp=False, + fab_workspace=tmp_path) + c_analyser = CAnalyser(config) with mock.patch('fab.parse.AnalysedFile.save'): fpath = Path(__file__).parent / "test_c_analyser.c" @@ -72,7 +72,7 @@ def __init__(self, spelling, line): mock_trans_unit = Mock() mock_trans_unit.cursor.get_tokens.return_value = tokens - analyser = CAnalyser() + analyser = CAnalyser(config=None) analyser._locate_include_regions(mock_trans_unit) assert analyser._include_region == expect @@ -81,7 +81,7 @@ def __init__(self, spelling, line): class Test__check_for_include: def test_vanilla(self): - analyser = CAnalyser() + analyser = CAnalyser(config=None) analyser._include_region = [ (10, "sys_include_start"), (20, "sys_include_end"), @@ -113,7 +113,7 @@ def _definition(self, spelling, linkage): node.linkage = linkage node.spelling = spelling - analyser = CAnalyser() + analyser = CAnalyser(config=None) analysed_file = Mock() analyser._process_symbol_declaration(analysed_file=analysed_file, node=node, usr_symbols=None) @@ -134,7 +134,7 @@ def _declaration(self, spelling, include_type): node.is_definition.return_value = False node.spelling = spelling - analyser = CAnalyser() + analyser = CAnalyser(config=None) analyser._check_for_include = Mock(return_value=include_type) usr_symbols = [] @@ -155,7 +155,7 @@ def test_not_usr_symbol(self): analysed_file.add_symbol_dep.assert_not_called() def _dependency(self, spelling, usr_symbols): - analyser = CAnalyser() + analyser = CAnalyser(config=None) analysed_file = Mock() node = Mock(spelling=spelling) @@ -168,7 +168,8 @@ def test_clang_disable(): with mock.patch('fab.parse.c.clang', None): with mock.patch('fab.parse.c.file_checksum') as mock_file_checksum: - result = CAnalyser().run(Path(__file__).parent / "test_c_analyser.c") + c_analyser = CAnalyser(config=None) + result = c_analyser.run(Path(__file__).parent / "test_c_analyser.c") assert isinstance(result[0], ImportWarning) mock_file_checksum.assert_not_called() diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 index 508ba56b..2c530269 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 @@ -19,7 +19,6 @@ END SUBROUTINE internal_sub SUBROUTINE openmp_sentinel !$ USE compute_chunk_size_mod, ONLY: compute_chunk_size ! Note OpenMP sentinel -!$ USE test that is not a sentinel with a use statement inside !GCC$ unroll 6 !DIR$ assume (mod(p, 6) == 0) !$omp do diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.py b/tests/unit_tests/parse/fortran/test_fortran_analyser.py index 75621020..6eb28f3c 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.py +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.py @@ -3,6 +3,11 @@ # For further details please refer to the file COPYRIGHT # which you should have received as part of this distribution # ############################################################################## + +'''Tests the Fortran analyser. +''' + + from pathlib import Path from tempfile import NamedTemporaryFile from unittest import mock @@ -18,9 +23,6 @@ from fab.parse.fortran_common import iter_content from fab.tools import ToolBox -'''Tests the Fortran analyser. -''' - # todo: test function binding @@ -36,7 +38,7 @@ def module_expected(module_fpath) -> AnalysedFortran: test module.''' return AnalysedFortran( fpath=module_fpath, - file_hash=1757501304, + file_hash=3737289404, module_defs={'foo_mod'}, symbol_defs={'external_sub', 'external_func', 'foo_mod'}, module_deps={'bar_mod', 'compute_chunk_size_mod'}, @@ -50,9 +52,10 @@ class TestAnalyser: @pytest.fixture def fortran_analyser(self, tmp_path): - fortran_analyser = FortranAnalyser() - fortran_analyser._config = BuildConfig('proj', ToolBox(), - fab_workspace=tmp_path) + # Enable openmp, so fparser will handle the lines with omp sentinels + config = BuildConfig('proj', ToolBox(), + fab_workspace=tmp_path, openmp=True) + fortran_analyser = FortranAnalyser(config=config) return fortran_analyser def test_empty_file(self, fortran_analyser): @@ -71,6 +74,24 @@ def test_module_file(self, fortran_analyser, module_fpath, assert artefact == (fortran_analyser._config.prebuild_folder / f'test_fortran_analyser.{analysis.file_hash}.an') + def test_module_file_no_openmp(self, fortran_analyser, module_fpath, + module_expected): + '''Disable OpenMP, meaning the dependency on compute_chunk_size_mod + should not be detected anymore. + ''' + fortran_analyser.config._openmp = False + with mock.patch('fab.parse.AnalysedFile.save'): + analysis, artefact = fortran_analyser.run(fpath=module_fpath) + + # Without parsing openmp sentinels, the compute_chunk... symbols + # must not be added: + module_expected.module_deps.remove('compute_chunk_size_mod') + module_expected.symbol_deps.remove('compute_chunk_size_mod') + + assert analysis == module_expected + assert artefact == (fortran_analyser._config.prebuild_folder / + f'test_fortran_analyser.{analysis.file_hash}.an') + def test_program_file(self, fortran_analyser, module_fpath, module_expected): # same as test_module_file() but replacing MODULE with PROGRAM @@ -83,7 +104,7 @@ def test_program_file(self, fortran_analyser, module_fpath, fpath=Path(tmp_file.name)) module_expected.fpath = Path(tmp_file.name) - module_expected._file_hash = 3388519280 + module_expected._file_hash = 325155675 module_expected.program_defs = {'foo_mod'} module_expected.module_defs = set() module_expected.symbol_defs.update({'internal_func', @@ -133,7 +154,7 @@ def test_define_without_bind_name(self, tmp_path): # run our handler fpath = Path('foo') analysed_file = AnalysedFortran(fpath=fpath, file_hash=0) - analyser = FortranAnalyser() + analyser = FortranAnalyser(config=None) analyser._process_variable_binding(analysed_file=analysed_file, obj=var_decl) diff --git a/tests/unit_tests/steps/test_link.py b/tests/unit_tests/steps/test_link.py index 3d5b643a..8669f4d8 100644 --- a/tests/unit_tests/steps/test_link.py +++ b/tests/unit_tests/steps/test_link.py @@ -4,7 +4,8 @@ # which you should have received as part of this distribution # ############################################################################## -'''This module tests the linking step. +''' +Tests linking an executable. ''' from pathlib import Path @@ -13,18 +14,17 @@ from fab.artefacts import ArtefactSet, ArtefactStore from fab.steps.link import link_exe -from fab.tools import Linker, ToolBox +from fab.tools import FortranCompiler, Linker, ToolBox import pytest class TestLinkExe: - '''This class test the linking step. + '''Test class for linking an executable. ''' - def test_run(self, tool_box: ToolBox): - ''' Ensure the command is formed correctly, with the flags at the - end. + '''Ensure the command is formed correctly, with the flags at the + end and that environment variable FFLAGS is picked up. ''' config = SimpleNamespace( @@ -37,9 +37,19 @@ def test_run(self, tool_box: ToolBox): 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) + + linker = Linker(compiler=mock_compiler) # Mark the linker as available to it can be added to the tool box linker._is_available = True @@ -56,7 +66,8 @@ def test_run(self, tool_box: ToolBox): 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', '-lmylib', '-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 eaadbbd9..fbbce38f 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_compiler_wrapper.py b/tests/unit_tests/tools/test_compiler_wrapper.py index bcf8d4a7..78a53249 100644 --- a/tests/unit_tests/tools/test_compiler_wrapper.py +++ b/tests/unit_tests/tools/test_compiler_wrapper.py @@ -257,6 +257,18 @@ def test_compiler_wrapper_flags_independent(): assert mpicc.flags == ["-a", "-b"] assert mpicc.openmp_flag == gcc.openmp_flag + # Test a compiler wrapper correctly queries the wrapper compiler for + # openmp flag: Set the wrapper to have no _openmp_flag (which is + # actually the default, since the wrapper never sets its own flag), but + # gcc does have a flag, so mpicc should report that is supports openmp. + # mpicc.openmp calls openmp of its base class (Compiler), which queries + # if an openmp flag is defined. This query must go to the openmp property, + # since the wrapper overwrites this property to return the wrapped + # compiler's flag (and not the wrapper's flag, which would not be defined) + with mock.patch.object(mpicc, "_openmp_flag", ""): + assert mpicc._openmp_flag == "" + assert mpicc.openmp + # Adding flags to the wrapper should not affect the wrapped compiler: mpicc.add_flags(["-d", "-e"]) assert gcc.flags == ["-a", "-b"] diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 243f0c7a..18f27bbd 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -13,43 +13,62 @@ 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 == [] - 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)) +@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(mock_c_compiler) + assert linker.mpi == mpi + + wrapped_linker = Linker(mock_c_compiler, 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(mock_c_compiler, linker=linker) + assert wrapped_linker.openmp == openmp def test_linker_gets_ldflags(mock_c_compiler): @@ -62,31 +81,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(mock_c_compiler, 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 +119,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 +139,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") @@ -164,7 +181,9 @@ def test_linker_add_lib_flags_overwrite_silent(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"]) @@ -250,29 +269,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"]) @@ -288,8 +294,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", @@ -300,3 +304,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(mock_c_compiler, 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) diff --git a/tests/unit_tests/tools/test_tool_repository.py b/tests/unit_tests/tools/test_tool_repository.py index 0c7d77e5..012487d4 100644 --- a/tests/unit_tests/tools/test_tool_repository.py +++ b/tests/unit_tests/tools/test_tool_repository.py @@ -11,7 +11,7 @@ import pytest from fab.tools import (Ar, Category, FortranCompiler, Gcc, Gfortran, Ifort, - Linker, ToolRepository) + ToolRepository) def test_tool_repository_get_singleton_new(): @@ -62,10 +62,6 @@ def test_tool_repository_get_default(): openmp=False) assert isinstance(gfortran, Gfortran) - gcc_linker = tr.get_default(Category.LINKER, mpi=False, openmp=False) - assert isinstance(gcc_linker, Linker) - assert gcc_linker.name == "linker-gcc" - gcc = tr.get_default(Category.C_COMPILER, mpi=False, openmp=False) assert isinstance(gcc, Gcc)