MetOffice · hiker · Aug 21, 2024 · Aug 22, 2024 · Aug 22, 2024 · Aug 22, 2024
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1,14 +1,6 @@
 name: Build
 
-on:
-  push:
-    branches:
-      - main
-      - v*.*
-  pull_request:
-    branches:
-      - main
-      - v*.*
+on: [push, pull_request]
 
 jobs:
   build:
@@ -17,7 +9,7 @@ jobs:
     strategy:
       max-parallel: 4
       matrix:
-        python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
+        python-version: ['3.9', '3.10']
 
     steps:
       # Should this step use a cache?

diff --git a/Documentation/source/site-specific-config.rst b/Documentation/source/site-specific-config.rst
@@ -60,24 +60,6 @@ New tools can easily be created, look at
 created by providing a different set of parameters in the
 constructor.
 
-This also allows compiler wrappers to be easily defined. For example,
-if you want to use `mpif90` as compiler, which is a MPI-specific
-wrapper for `ifort`, you can create this class as follows:
-
-.. code-block::
-    :linenos:
-    :caption: Compiler wrapper
-
-    from fab.tools import Ifort
-
-    class MpiF90(Ifort):
-        '''A simple compiler wrapper'''
-        def __init__(self):
-            super().__init__(name="mpif90-intel",
-                             exec_name="mpif90")
-
-.. note:: In `ticket 312 <https://github.com/metomi/fab/issues/312>`_ a better
-        implementation of compiler wrapper will be implemented.
 
 Tool Repository
 ===============

diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py
@@ -93,6 +93,11 @@ def openmp_flag(self) -> str:
         """
         return self._openmp_flag
 
+    @property
+    def compile_flag(self) -> str:
+        ''':returns: the flag to indicate compilation only (not linking).'''
+        return self._compile_flag
+
     @property
     def output_flag(self) -> str:
         """
@@ -116,6 +121,42 @@ def get_flags(self, profile: Optional[str] = None) -> List[str]:
         fflags = os.getenv("FFLAGS", "").split()
         return self._flags[profile] + fflags
 
+    def get_all_commandline_options(
+            self,
+            config: "BuildConfig",
+            input_file: Path,
+            output_file: Path,
+            add_flags:  Union[None, List[str]] = None) -> List[str]:
+        '''This function returns all command line options for a compiler
+        (but not the executable name). It is used by a compiler wrapper
+        to pass the right flags to the wrapper.
+        This base implementation adds the input and output filename (including
+        the -o flag), the flag to only compile (and not link), and if
+        required openmp.
+
+        :param input_file: the name of the input file.
+        :param output_file: the name of the output file.
+        :param config: The BuildConfig, from which compiler profile and OpenMP
+            status are taken.
+        :param add_flags: additional flags for the compiler.
+
+        :returns: all command line options for compilation.
+        '''
+        params: List[str] = [self._compile_flag]
+
+        if config.openmp:
+            params.append(self.openmp_flag)
+        if add_flags:
+            if self.openmp_flag in add_flags:
+                warnings.warn(
+                    f"OpenMP flag '{self.openmp_flag}' explicitly provided. "
+                    f"OpenMP should be enabled in the BuildConfiguration "
+                    f"instead.")
+            params += add_flags
+
+        params.extend([input_file.name, self._output_flag, str(output_file)])
+        return params
+
     def compile_file(self, input_file: Path,
                      output_file: Path,
                      config: "BuildConfig",
@@ -134,21 +175,8 @@ def compile_file(self, input_file: Path,
         :param add_flags: additional compiler flags.
         '''
 
-        params: List[Union[Path, str]] = [self._compile_flag]
-
-        if config.openmp:
-            params.append(self.openmp_flag)
-        if add_flags:
-            if self.openmp_flag in add_flags:
-                warnings.warn(
-                    f"OpenMP flag '{self.openmp_flag}' explicitly provided. "
-                    f"OpenMP should be enabled in the BuildConfiguration "
-                    f"instead.")
-            params += add_flags
-
-        params.extend([input_file.name,
-                      self._output_flag, str(output_file)])
-
+        params = self.get_all_commandline_options(config, input_file,
+                                                  output_file, add_flags)
         return self.run(profile=config.profile, cwd=input_file.parent,
                        additional_parameters=params)
 
@@ -345,12 +373,19 @@ def set_module_output_path(self, path: Path):
         '''
         self._module_output_path = str(path)
 
-    def compile_file(self, input_file: Path,
-                     output_file: Path,
-                     config: "BuildConfig",
-                     add_flags: Union[None, List[str]] = None,
-                     syntax_only: Optional[bool] = False):
-        '''Compiles a file.
+    def get_all_commandline_options(
+            self,
+            config: "BuildConfig",
+            input_file: Path,
+            output_file: Path,
+            add_flags:  Union[None, List[str]] = None,
+            syntax_only: Optional[bool] = False) -> List[str]:
+        '''This function returns all command line options for a Fortran
+        compiler (but not the executable name). It is used by a compiler
+        wrapper to pass the right flags to the wrapper.
+        This Fortran-specific implementation adds the module- and
+        syntax-only flags (as required) to the standard compiler
+        flags.
 
         :param input_file: the name of the input file.
         :param output_file: the name of the output file.
@@ -359,26 +394,53 @@ def compile_file(self, input_file: Path,
         :param add_flags: additional flags for the compiler.
         :param syntax_only: if set, the compiler will only do
             a syntax check
-        '''
 
-        params: List[str] = []
+        :returns: all command line options for Fortran compilation.
+        '''
         if add_flags:
-            new_flags = Flags(add_flags)
+            add_flags = Flags(add_flags)
             if self._module_folder_flag:
-                new_flags.remove_flag(self._module_folder_flag,
+                # Remove any module flag the user has specified, since
+                # this will interfere with Fab's module handling.
+                add_flags.remove_flag(self._module_folder_flag,
                                       has_parameter=True)
-            new_flags.remove_flag(self._compile_flag, has_parameter=False)
-            params += new_flags
+            add_flags.remove_flag(self._compile_flag, has_parameter=False)
 
+        # Get the flags from the base class
+        params = super().get_all_commandline_options(config, input_file,
+                                                     output_file, add_flags)
         if syntax_only and self._syntax_only_flag:
             params.append(self._syntax_only_flag)
 
         # Append module output path
         if self._module_folder_flag and self._module_output_path:
             params.append(self._module_folder_flag)
             params.append(self._module_output_path)
-        super().compile_file(input_file, output_file, config=config,
-                             add_flags=params)
+
+        return params
+
+    def compile_file(self, input_file: Path,
+                     output_file: Path,
+                     config: "BuildConfig",
+                     add_flags: Union[None, List[str]] = None,
+                     syntax_only: Optional[bool] = False):
+        '''Compiles a file. This basically re-implements `compile_file` of
+        the base class, but passes the syntax_only flag in
+
+        :param input_file: the name of the input file.
+        :param output_file: the name of the output file.
+        :param config: The BuildConfig, from which compiler profile and OpenMP
+            status are taken.
+        :param add_flags: additional flags for the compiler.
+        :param syntax_only: if set, the compiler will only do
+            a syntax check
+        '''
+        params = self.get_all_commandline_options(config, input_file,
+                                                  output_file, add_flags,
+                                                  syntax_only)
+
+        self.run(profile=config.profile, cwd=input_file.parent,
+                 additional_parameters=params)
 
 
 # ============================================================================
@@ -438,8 +500,6 @@ class Icc(CCompiler):
     def __init__(self, name: str = "icc", exec_name: str = "icc"):
         super().__init__(name, exec_name, suite="intel-classic",
                          openmp_flag="-qopenmp",
-                         availability_option='-V',
-                         version_argument='-V',
                          version_regex=r"icc \(ICC\) (\d[\d\.]+\d) ")
 
 
@@ -457,7 +517,6 @@ def __init__(self, name: str = "ifort", exec_name: str = "ifort"):
                          module_folder_flag="-module",
                          openmp_flag="-qopenmp",
                          syntax_only_flag="-syntax-only",
-                         version_argument='-V',
                          version_regex=r"ifort \(IFORT\) (\d[\d\.]+\d) ")
 
 
@@ -497,9 +556,8 @@ def __init__(self, name: str = "ifx", exec_name: str = "ifx"):
 # nvidia
 # ============================================================================
 class Nvc(CCompiler):
-    '''Class for Nvidia's nvc compiler. Nvc has a '-' in the
-    version number. In order to get this, we overwrite run_version_command
-    and replace any '-' with a '.'
+    '''Class for Nvidia's nvc compiler. Note that the '-' in the Nvidia
+    version number is ignored, e.g. 23.5-0 would return '23.5'.
 
     :param name: name of this compiler.
     :param exec_name: name of the executable.
@@ -514,9 +572,8 @@ def __init__(self, name: str = "nvc", exec_name: str = "nvc"):
 
 # ============================================================================
 class Nvfortran(FortranCompiler):
-    '''Class for Nvidia's nvfortran compiler. Nvfortran has a '-' in the
-    version number. In order to get this, we overwrite run_version_command
-    and replace any '-' with a '.'
+    '''Class for Nvidia's nvfortran compiler. Note that the '-' in the Nvidia
+    version number is ignored, e.g. 23.5-0 would return '23.5'.
 
     :param name: name of this compiler.
     :param exec_name: name of the executable.