Compiler wrapper (#347)

* #3 Converted svn and fcm to tools.

* #3 Fixed missing whitespace.

* Modified the documentation for writing a config with PSyclone

* Add config as a parameter for run_psyclone for the transformation_script to use;Updated the related functions and tests;
Changed the logic of the transformation_script examples

* #3 Replaced ar with tool object.

* #3 Added tests for ar.py.

* #3 Removed debug output.

* #3 Converted PSyclone to be a tool.

* #3 Removed debug print, fixed python 3.7 typing information.

* #3 Updated comments.

* Modified the get_optimisation_script function examples and updated the doc formatting

* #3 Add Rsync tool.

* #3 Removed now unused function.

* #3 Added test for rsync.

* #3 Fixed all mypy warnings about functions not checked.

* #3 Replace all mock-tests to use subprocess so the name of the executable is tested as well.

* #3 Remove duplicated flags.

* #3 Fixed changed order of linking.

* #3 Removed run_command function.

* #3 Fixed 3.8 typing error.

* #3 Fixed unused imports.

* #3 Move flags checksum into Flags, and remove now unused tools.py file.

* #3 Renamed newtools to tools.

* #3 Made custom function for all git functions called (instead of just calling run).

* #3 Updated and fixed comments.

* #3 Fixed errors in comments.

* Fixed minor errors in documentation.

* #3 Make it easier to create wrapper around standard compiler.

* #3 Added documentation for all tool related classes and their usage.

* #3 Added MISC category.

* Addressed reviewer's comments.

* Updated cli to properly use ToolBox etc, removing hard-coded gnu command linker option.

* Fixed mypy failures, including changes to import statement to avoid cyclic imports :(.

* #3 Fix circular import.

* Added #TODO so that this can be removed once fparser supports sentinels.

* Fix typing problems by ignoring fparser.

* Replaced more string names for artefacts with enums.

* Removed EXECUTABLES from constants.

* Moved Artefact class out of ArtefactStore and renamed it to ArtefactSet.

* Moved OBJECT_FILES from constants into ArtefactSet.

* Moved OBJECT_ARCHIVES from constants to ArtefactSet.

* Moved PRAGMAD_C from constants to ArtefactSet.

* Turned 'all_source' into an enum.

* Allow integer as revision.

* Fixed flake8 error.

* Removed specific functions to add/get fortran source files etc.

* Removed non-existing and unneccessary collections.

* Try to fix all run_configs.

* Fixed rebase issues.

* Added replace functionality to ArtefactStore, updated test_artefacts to cover all lines in that file.

* Started to replace artefacts when files are pre-processed.

* Removed linker argument from linking step in all examples.

* Try to get jules to link.

* Fixed build_jules.

* Fixed other issues raised in reviews.

* Try to get jules to link.

* Fixed other issues raised in reviews.

* Simplify handling of X90 files by replacing the X90 with x90, meaning only one artefact set is involved when running PSyclone.

* Make OBJECT_ARCHIVES also a dict, migrate more code to replace/add files to the default build artefact collections.

* Fixed some examples.

* Fix flake8 error.

* Fixed failing tests.

* Support empty comments.

* Fix preprocessor to not unnecessary remove and add files that are already in the output directory.

* Allow find_soure_files to be called more than once by adding files (not replacing artefact).

* Updated lfric_common so that files created by configurator are written in build (not source).

* Use c_build_files instead of pragmad_c.

* Removed unnecessary str.

* Documented the new artefact set handling.

* Fixed typo.

* Make the PSyclone API configurable.

* Fixed formatting of documentation, properly used ArtefactSet names.

* Support .f and .F Fortran files.

* Removed setter for tool.is_available, which was only used for testing.

* #3 Fix documentation and coding style issues from review.

* Renamed Categories into Category.

* Minor coding style cleanup.

* Removed more unnecessary ().

* Re-added (invalid) grab_pre_build call.

* Fixed typo.

* Renamed set_default_vendor to set_default_compiler_suite.

* Renamed VendorTool to CompilerSuiteTool.

* Also accept a Path as exec_name specification for a tool.

* Move the check_available function into the base class.

* Fixed some types and documentation.

* Fix typing error.

* Added explanation for meta-compiler.

* Improved error handling and documentation.

* Replace mpiifort with mpifort to be a tiny bit more portable.

* Use classes to group tests for git/svn/fcm together.

* Fixed issue in get_transformation script, and moved script into lfric_common to remove code duplication.

* Code improvement as suggested by review.

* Fixed run config

* Added reference to ticket.

* Updated type information.

* More typing fixes.

* Fixed typing warnings.

* As requested by reviewer removed is_working_copy functionality.

* Issue a warning (which can be silenced) when a tool in a toolbox is replaced.

* Fixed flake8.

* Fixed flake8.

* Fixed failing test.

* Addressed issues raised in review.

* Removed now unnecessary operations.

* Updated some type information.

* Fixed all references to APIs to be consistent with PSyclone 2.5.

* Added api to the checksum computation.

* Fixed type information.

* Added test to verify that changing the api changes the checksum.

* Make compiler version a tuple of integers

* Update some tests to use tuple versions

* Explicitly test handling of bad version format

* Fix formatting

* Tidying up

* Make compiler raise an error for any invalid version string

Assume these compilers don't need to be hashed.
Saves dealing with empty tuples.

* Check compiler version string for compiler name

* Fix formatting

* Add compiler.get_version_string() method

Includes other cleanup from PR comments

* Add mpi and openmp settings to BuildConfig, made compiler MPI aware.

* Looks like the circular dependency has been fixed.

* Revert "Looks like the circular dependency has been fixed." ...
while it works with the tests, a real application still triggered it.

This reverts commit 150dc37.

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

* Fixed spelling mistake in option.

* Clarified documentation.

* Removed unnecessary functions in CompilerWrapper.

* Made mpi and openmp default to False in the BuildConfig constructor.

* Removed white space.

* Support compilers that do not support OpenMP.

* Added documentation for openmp parameter.

---------

Co-authored-by: Junwei Lyu <[email protected]>
Co-authored-by: Junwei Lyu <[email protected]>
Co-authored-by: jasonjunweilyu <[email protected]>
Co-authored-by: Luke Hoffmann <[email protected]>
Co-authored-by: Luke Hoffmann <[email protected]>

Documentation/source/site-specific-config.rst

@@ -161,6 +161,64 @@ On the other hand, if no MPI is requested, an MPI-enabled compiler
 might be returned, which does not affect the final result, since
 an MPI compiler just adds include- and library-paths.
 
+
+Compiler Wrapper
+================
+Fab supports the concept of a compiler wrapper, which is typically
+a script that calls the actual compiler. An example for a wrapper is
+`mpif90`, which might call a GNU or Intel based compiler (with additional
+parameter for accessing the MPI specific include and library paths.).
+An example to create a `mpicc` wrapper (note that this wrapper is already
+part of Fab, there is no need to explicitly add this yourself):
+
+.. code-block::
+    :linenos:
+    :caption: Defining an mpicc compiler wrapper
+
+    class Mpicc(CompilerWrapper):
+        def __init__(self, compiler: Compiler):
+            super().__init__(name=f"mpicc-{compiler.name}",
+                             exec_name="mpicc",
+                             compiler=compiler, mpi=True)
+
+The tool system allows several different tools to use the same name
+for the executable, as long as the Fab name is different, i.e. the
+`mpicc-{compiler.name}`. The tool
+repository will automatically add compiler wrapper for `mpicc` and
+`mpif90` for any compiler that is added by Fab. If you want to add
+a new compiler, which can also be invoked using `mpicc`, you need
+to add a compiler wrapper as follows:
+
+.. code-block::
+    :linenos:
+    :caption: Adding a mpicc wrapper to the tool repository
+
+    my_new_compiler = MyNewCompiler()
+    ToolRepository().add_tool(my_new_compiler)
+    my_new_mpicc = Mpicc(MyNewCompiler)
+    ToolRepository().add_tool(my_new_mpicc)
+
+When creating a completely new compiler and compiler wrapper
+as in the example above, it is strongly recommended to add
+the new compiler instance to the tool repository as well. This will
+allow the wrapper and the wrapped compiler to share flags. For example,
+a user script can query the ToolRepository to get the original compiler
+and modify its flags. These modification will then automatically be
+applied to the wrapper as well:
+
+.. code-block::
+    :linenos:
+    :caption: Sharing flags between compiler and compiler wrapper
+
+    tr = ToolRepository()
+    my_compiler = tr.get_tool(Category.C_COMPILER, "my_compiler")
+    my_mpicc = tr.get_tool(Category.C_COMPILER, "mpicc-my_compiler")
+
+    my_compiler.add_flags(["-a", "-b"])
+
+    assert my_mpicc.flags == ["-a", "-b"]
+
+
 TODO
 ====
 At this stage compiler flags are still set in the corresponding Fab
@@ -169,3 +227,8 @@ definition in the compiler objects.
 This will allow a site to define their own set of default flags to
 be used with a certain compiler by replacing or updating a compiler
 instance in the Tool Repository
+
+Also, a lot of content in this chapter is not actually about site-specific
+configuration. This should likely be renamed or split (once we
+have details about the using site-specific configuration, which might be
+once the Baf base script is added to Fab).

source/fab/steps/compile_c.py

@@ -10,7 +10,7 @@
 import logging
 import os
 from dataclasses import dataclass
-from typing import List, Dict, Optional, Tuple
+from typing import cast, Dict, List, Optional, Tuple
 
 from fab import FabException
 from fab.artefacts import (ArtefactsGetter, ArtefactSet, ArtefactStore,
@@ -19,7 +19,7 @@
 from fab.metrics import send_metric
 from fab.parse.c import AnalysedC
 from fab.steps import check_for_errors, run_mp, step
-from fab.tools import Category, CCompiler, Flags
+from fab.tools import Category, Compiler, Flags
 from fab.util import CompiledFile, log_or_dot, Timer, by_type
 
 logger = logging.getLogger(__name__)
@@ -124,9 +124,10 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]):
     analysed_file, mp_payload = arg
     config = mp_payload.config
     compiler = config.tool_box[Category.C_COMPILER]
-    if not isinstance(compiler, CCompiler):
-        raise RuntimeError(f"Unexpected tool '{compiler.name}' of type "
-                           f"'{type(compiler)}' instead of CCompiler")
+    if compiler.category != Category.C_COMPILER:
+        raise RuntimeError(f"Unexpected tool '{compiler.name}' of category "
+                           f"'{compiler.category}' instead of CCompiler")
+    compiler = cast(Compiler, compiler)
     with Timer() as timer:
         flags = Flags(mp_payload.flags.flags_for_path(path=analysed_file.fpath,
                                                       config=config))

source/fab/steps/compile_fortran.py

@@ -14,15 +14,15 @@
 from dataclasses import dataclass
 from itertools import chain
 from pathlib import Path
-from typing import List, Set, Dict, Tuple, Optional, Union
+from typing import cast, Dict, List, Optional, Set, Tuple, Union
 
 from fab.artefacts import (ArtefactsGetter, ArtefactSet, ArtefactStore,
                            FilterBuildTrees)
 from fab.build_config import BuildConfig, FlagsConfig
 from fab.metrics import send_metric
 from fab.parse.fortran import AnalysedFortran
 from fab.steps import check_for_errors, run_mp, step
-from fab.tools import Category, Compiler, Flags, FortranCompiler
+from fab.tools import Category, Compiler, Flags
 from fab.util import (CompiledFile, log_or_dot_finish, log_or_dot, Timer,
                       by_type, file_checksum)
 
@@ -133,9 +133,10 @@ def handle_compiler_args(config: BuildConfig, common_flags=None,
 
     # Command line tools are sometimes specified with flags attached.
     compiler = config.tool_box[Category.FORTRAN_COMPILER]
-    if not isinstance(compiler, FortranCompiler):
-        raise RuntimeError(f"Unexpected tool '{compiler.name}' of type "
-                           f"'{type(compiler)}' instead of FortranCompiler")
+    if compiler.category != Category.FORTRAN_COMPILER:
+        raise RuntimeError(f"Unexpected tool '{compiler.name}' of category "
+                           f"'{compiler.category}' instead of FortranCompiler")
+    compiler = cast(Compiler, compiler)
     logger.info(
         f'Fortran compiler is {compiler} {compiler.get_version_string()}')
 
@@ -263,10 +264,11 @@ def process_file(arg: Tuple[AnalysedFortran, MpCommonArgs]) \
         config = mp_common_args.config
         compiler = config.tool_box.get_tool(Category.FORTRAN_COMPILER,
                                             config.mpi)
-        if not isinstance(compiler, FortranCompiler):
-            raise RuntimeError(f"Unexpected tool '{compiler.name}' of type "
-                               f"'{type(compiler)}' instead of "
+        if compiler.category != Category.FORTRAN_COMPILER:
+            raise RuntimeError(f"Unexpected tool '{compiler.name}' of "
+                               f"category '{compiler.category}' instead of "
                                f"FortranCompiler")
+        compiler = cast(Compiler, compiler)
         flags = Flags(mp_common_args.flags.flags_for_path(
             path=analysed_file.fpath, config=config))
 

source/fab/tools/__init__.py

@@ -11,8 +11,8 @@
 from fab.tools.category import Category
 from fab.tools.compiler import (CCompiler, Compiler, FortranCompiler, Gcc,
                                 Gfortran, GnuVersionHandling, Icc, Ifort,
-                                IntelVersionHandling, MpiGcc, MpiGfortran,
-                                MpiIcc, MpiIfort)
+                                IntelVersionHandling)
+from fab.tools.compiler_wrapper import CompilerWrapper, Mpicc, Mpif90
 from fab.tools.flags import Flags
 from fab.tools.linker import Linker
 from fab.tools.psyclone import Psyclone
@@ -29,6 +29,7 @@
            "CCompiler",
            "Compiler",
            "CompilerSuiteTool",
+           "CompilerWrapper",
            "Cpp",
            "CppFortran",
            "Fcm",
@@ -43,10 +44,8 @@
            "Ifort",
            "IntelVersionHandling",
            "Linker",
-           "MpiGcc",
-           "MpiGfortran",
-           "MpiIcc",
-           "MpiIfort",
+           "Mpif90",
+           "Mpicc",
            "Preprocessor",
            "Psyclone",
            "Rsync",