Merge branch 'add_shell_tool_clean' into update_to_fparser_0_2

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',

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',

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]:

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'],

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.
         '''

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

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,60 +20,75 @@
 
 
 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]] = {}
         # Allow flags to include before or after any library-specific flags.
         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)

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"])
 
 
 # ============================================================================

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