diff --git a/Documentation/source/advanced_config.rst b/Documentation/source/advanced_config.rst index fa2ced66..cfa63d64 100644 --- a/Documentation/source/advanced_config.rst +++ b/Documentation/source/advanced_config.rst @@ -32,7 +32,7 @@ In not-representative tests, importing tool using ``api`` takes around The list of functions and classes provided: -.. literalinclude:: ../../source/fab/api/__init__.py +.. literalinclude:: ../../source/fab/api.py :start-after: __all__ = [ :end-before: ] @@ -404,6 +404,144 @@ instance itself, as shown further above, is the better approach. Path-specific flags ------------------- +A common use case in complex applications is the need to specify +file-specific compiler flags. For example, a file might need to enable +additional optimisations, or disable other optimisations. + +Fab uses a set of classes to manage flags consistently across +all tools. + +AlwaysFlags + This object represents a list of flags that are always used, i.e. + they are independent of the filename being processed. It provides + templating capabilities, i.e. a set of keywords starting with `$` + will be replaced with a value. The following keywords are defined + if a config file is specified: + + - ``$source`` for */source* + - ``$output`` for */build_output* + - ``$relative`` for ** + + These values are taken from the build configuration (if specified). + + Example: + + .. code-block:: + :linenos: + :caption: Creating an AlwaysFlags instance + + from fab.api import AlwaysFlags + + # Assuming that build_config specifies /some/path/fab + # as project directory for Fab: + af = AlwaysFlags(["-g", "-O2", "-I$source/um/include/other"]) + assert (af.get_flags(build_config) == + ["-g", "-O2", "-I/some/path/fab/source/um/include/other"]) + + +MatchFlags (based on ``AlwaysFlags``) + This represents a set of flags that are only applied if a wildcard + search matches the source file to be processed. Note that a wildcard + search must in general match the full path (unless the pattern starts + with a ``*``). The templates explained in the ``AlwaysFlags`` can be + used to create absolute paths in the pattern as well. + + Example: + + .. code-block:: + :linenos: + :caption: Creating a MatchFlags instance + + from fab.api import MatchFlags + + mf = MatchFlags("/some/dir", "-g") + + assert mf.get_flags(file_path=Path("/some/dir")) == ["-g"] + assert mf.get_flags(file_path=Path("/other/some/dir")) == [] + + # Starting a pattern with * will match a: + mf = MatchFlags("*/some/dir", "-g") + assert mf.get_flags(file_path=Path("/other/some/dir")) == ["-g"] + + +ContainFlags (based on ``AlwaysFlags``) + Flags that are only applied if the file path contains the specified + string. The difference to ``MatchFlags`` is that ``ContainFlags`` do not + need the full path to be specified. + + Example: + + .. code-block:: + :linenos: + :caption: Creating a ContainFlags instance + + from fab.api import ContainFlags + + cf = ContainFlags(pattern="/some/dir", "-g") + + assert cf.get_flags(file_path=Path("/not/in/root/some/dir")) == ["-g"] + assert cf.get_flags(file_path=Path("/some/other")) == [] + +FlagList: + This class represents the main class to manage a set of flags. + Each of its members is either an ``AlwaysFlags``, ``MatchFlags`` + or ``ContainFlags``. When the list is evaluated, each individual + flag instance is evaluated and will depending on the processed + file add flags or not. + + This class provides convenient constructor which will convert + a string or list of strings to an ``AlwaysFlags`` instance. + It also provides backwards compatibility to the previous + ``AddFlag`` object (see :ref:`add_flag`): + + Example: + + .. code-block:: + :linenos: + :caption: Creating a FlagList + + from fab.api import ContainFlags, FlagList + from fab.build_config import AddFlags + + cf = ContainFlags(pattern="/some/dir", "-g") + add_flags = AddFlags(match="/some/pattern", flags=["-g", "-O0"]) + # This will trigger conversion of add_flags to MathFlags + flag_list = FlagList(add_flags=[add_flags]) + +ProfileFlags: + Tools have a ``ProfileFlags`` as member, which they use to manage + all their flags. Depending on the selected profile, this ``ProfileFlags`` + members returns the ``FlagList`` to use. As explained earlier, each of + the members of FlagList can be any of the previous flag types to give + flexibility for a user to specify the compilation flags. See + :ref:`Advanced Flags` for details about profiling modes. + + Example: + + .. code-block:: + :linenos: + :caption: Setting up compiler for the UM + + tr = ToolRepository() + ifx = tr.get_tool(Category.FORTRAN_COMPILER, "ifx") + + # These will be converted to an AlwaysFlag: + ifx.add_flags(["-stand", "f08"], "base") + + # ifx 2025.2 has no omp_version in omp_lib, so we + # have to disable OpenMP: + if (2025, 2) <= ifx.get_version() < (2025, 3): + ifx.add_flags(ContainFlags("/um_shell.", ["-qno-openmp"]), "base") + + # Prevent loss of comparison across decompositions when OpenMP on + ifx.add_flags(ContainFlags("/ni_imp_ctl.", "-qno-openmp"), "base") + + + +.. _add_flag: + +Path-specific flags - old style +------------------------------- For preprocessing and compilation, we sometimes need to specify flags *per-file*. These steps accept both common flags and *path specific* flags. diff --git a/source/fab/api.py b/source/fab/api.py index e447f0b2..a5b00575 100644 --- a/source/fab/api.py +++ b/source/fab/api.py @@ -30,10 +30,12 @@ from fab.tools.category import Category from fab.tools.compiler import Compiler, Ifort from fab.tools.compiler_wrapper import CompilerWrapper +from fab.tools.flags import AlwaysFlags, ContainFlags, FlagList, MatchFlags from fab.tools.linker import Linker from fab.tools.tool import Tool from fab.tools.tool_box import ToolBox from fab.tools.tool_repository import ToolRepository +from fab.tools.shell import Shell from fab.util import common_arg_parser from fab.util import file_checksum, log_or_dot, TimerLogger from fab.util import get_fab_workspace @@ -41,6 +43,7 @@ __all__ = [ "AddFlags", + "AlwaysFlags", "analyse", "archive_objects", "ArtefactSet", @@ -53,10 +56,12 @@ "CompilerWrapper", "compile_c", "compile_fortran", + "ContainFlags", "c_pragma_injector", "Exclude", "fcm_export", "file_checksum", + "FlagList", "get_fab_workspace", "git_checkout", "grab_folder", @@ -69,12 +74,14 @@ "link_exe", "link_shared_object", "log_or_dot", + "MatchFlags", "preprocess_c", "preprocess_fortran", "preprocess_x90", "psyclone", "root_inc_files", "run_mp", + "Shell", "step", "SuffixFilter", "TimerLogger", diff --git a/source/fab/build_config.py b/source/fab/build_config.py index 9caeef87..ffb42ba2 100644 --- a/source/fab/build_config.py +++ b/source/fab/build_config.py @@ -91,9 +91,9 @@ def __init__(self, project_label: str, self._openmp = openmp if profile is None: # An empty string is used for non-profiled flags - self._profile = "" + self.set_profile("") else: - self._profile = profile + self.set_profile(profile) self.two_stage = two_stage self.verbose = verbose compiler = tool_box.get_tool(Category.FORTRAN_COMPILER, mpi=mpi, @@ -223,6 +223,13 @@ def profile(self) -> str: ''':returns: the name of the compiler profile to use.''' return self._profile + def set_profile(self, profile: str) -> None: + """Sets the compilation profile. + + :param profile: the name of the profile. + """ + self._profile = profile.lower() + def add_current_prebuilds(self, artefacts: Iterable[Path]): """ Mark the given file paths as being current prebuilds, not to be @@ -288,10 +295,16 @@ def _finalise_metrics(self, start_time, steps_timer): # todo: better name? perhaps PathFlags? +# todo: This is going to be replaced with tools.flags.MatchFlags + class AddFlags(): """ Add command-line flags when our path filter matches. - Generally used inside a :class:`~fab.build_config.FlagsConfig`. + This class is deprecated, use the new + :class:`~fab.tools.flags.FlagList` and + :class:`~fab.tools.flags.MatchFlag` instead. + The compile and preprocess steps will convert AddFlags into + MatchFlags """ def __init__(self, match: str, flags: List[str]): @@ -347,48 +360,3 @@ def run(self, fpath: Path, input_flags: List[str], config): # add our flags input_flags += add_flags - - -class FlagsConfig(): - """ - Return command-line flags for a given path. - - Simply allows appending flags but may evolve to also replace and - remove flags. - - """ - def __init__(self, common_flags: Optional[List[str]] = None, - path_flags: Optional[List[AddFlags]] = None): - """ - :param common_flags: - List of flags to apply to all files. E.g `['-O2']`. - :param path_flags: - List of :class:`~fab.build_config.AddFlags` objects which apply - flags to selected paths. - - """ - self.common_flags = common_flags or [] - self.path_flags = path_flags or [] - - # todo: there's templating both in this method and the run method it calls. - # make sure it's all properly documented and rationalised. - def flags_for_path(self, path: Path, config): - """ - Get all the flags for a given file, in a reproducible order. - - :param path: - The file path for which we want command-line flags. - :param config: - The config contains the source root and project workspace. - - """ - # We COULD make the user pass these template params to the constructor - # but we have a design requirement to minimise the config burden on - # the user, so we take care of it for them here instead. - params = {'source': config.source_root, 'output': config.build_output} - flags = [Template(i).substitute(params) for i in self.common_flags] - - for flags_modifier in self.path_flags: - flags_modifier.run(path, flags, config=config) - - return flags diff --git a/source/fab/fab_base/fab_base.py b/source/fab/fab_base/fab_base.py index def219cd..0e4c5a18 100755 --- a/source/fab/fab_base/fab_base.py +++ b/source/fab/fab_base/fab_base.py @@ -673,19 +673,15 @@ def compile_c_step( all C files. Optionally, common flags, path-specific flags and alternative source can also be passed to Fab for compilation. """ - site_path_flags: List[AddFlags] = [] - if self._site_config: - site_path_flags = self._site_config.get_path_flags(self._config) if not common_flags: common_flags = [] - assert isinstance(common_flags, list) if not path_flags: path_flags = [] compile_c(self.config, common_flags=(common_flags + self.c_compiler_flags_commandline), - path_flags=path_flags + site_path_flags) + path_flags=path_flags) def compile_fortran_step( self, @@ -701,9 +697,6 @@ def compile_fortran_step( :param path_flags: optional list of path-specific flags to be passed to Fab compile_fortran, default is None. """ - site_path_flags: List[AddFlags] = [] - if self._site_config: - site_path_flags = self._site_config.get_path_flags(self._config) if not common_flags: common_flags = [] if not path_flags: @@ -711,7 +704,7 @@ def compile_fortran_step( compile_fortran(self.config, common_flags=(common_flags + self.fortran_compiler_flags_commandline), - path_flags=path_flags + site_path_flags) + path_flags=path_flags) def link_step(self) -> None: """ diff --git a/source/fab/fab_base/site_specific/default/config.py b/source/fab/fab_base/site_specific/default/config.py index e7af469f..96fce337 100644 --- a/source/fab/fab_base/site_specific/default/config.py +++ b/source/fab/fab_base/site_specific/default/config.py @@ -6,9 +6,8 @@ ''' import argparse -from typing import cast, Dict, List -from fab.api import AddFlags, BuildConfig, Category, Compiler, ToolRepository +from fab.api import BuildConfig, Category, ToolRepository from fab.fab_base.site_specific.default.setup_cray import setup_cray from fab.fab_base.site_specific.default.setup_gnu import setup_gnu @@ -29,11 +28,6 @@ class Config: def __init__(self) -> None: self._args: argparse.Namespace - # Stores for each compiler suite a mapping of profiles to the list of - # path-specific flags to use. - # _path_flags[suite][profile] - self._path_flags: Dict[str, Dict[str, List[AddFlags]]] = {} - @property def args(self) -> argparse.Namespace: ''' @@ -41,7 +35,7 @@ def args(self) -> argparse.Namespace: ''' return self._args - def get_valid_profiles(self) -> List[str]: + def get_valid_profiles(self) -> list[str]: ''' Determines the list of all allowed compiler profiles. The first entry in this list is the default profile to be used. This method @@ -100,16 +94,6 @@ def update_toolbox(self, build_config: BuildConfig) -> None: self.setup_nvidia(build_config) self.setup_cray(build_config) - def get_path_flags(self, build_config: BuildConfig) -> List[AddFlags]: - ''' - Returns the path-specific flags to be used. - TODO #313: Ideally we have only one kind of flag, but as a quick - work around we provide this method. - ''' - compiler = build_config.tool_box.get_tool(Category.FORTRAN_COMPILER) - compiler = cast(Compiler, compiler) - return self._path_flags[compiler.suite].get(build_config.profile, []) - def setup_cray(self, build_config: BuildConfig) -> None: ''' This method sets up the Cray compiler and linker flags. @@ -119,7 +103,7 @@ def setup_cray(self, build_config: BuildConfig) -> None: :param build_config: the Fab build configuration instance ''' - self._path_flags["cray"] = setup_cray(build_config, self.args) + setup_cray(build_config, self.args) def setup_gnu(self, build_config: BuildConfig) -> None: ''' @@ -130,7 +114,7 @@ def setup_gnu(self, build_config: BuildConfig) -> None: :param build_config: the Fab build configuration instance ''' - self._path_flags["gnu"] = setup_gnu(build_config, self.args) + setup_gnu(build_config, self.args) def setup_intel_classic(self, build_config: BuildConfig) -> None: ''' @@ -141,8 +125,7 @@ def setup_intel_classic(self, build_config: BuildConfig) -> None: :param build_config: the Fab build configuration instance ''' - self._path_flags["intel_classic"] = setup_intel_classic(build_config, - self.args) + setup_intel_classic(build_config, self.args) def setup_intel_llvm(self, build_config: BuildConfig) -> None: ''' @@ -153,8 +136,7 @@ def setup_intel_llvm(self, build_config: BuildConfig) -> None: :param build_config: the Fab build configuration instance ''' - self._path_flags["intel-llvm"] = setup_intel_llvm(build_config, - self.args) + setup_intel_llvm(build_config, self.args) def setup_nvidia(self, build_config: BuildConfig) -> None: ''' @@ -165,4 +147,4 @@ def setup_nvidia(self, build_config: BuildConfig) -> None: :param build_config: the Fab build configuration instance ''' - self._path_flags["nvidia"] = setup_nvidia(build_config, self.args) + setup_nvidia(build_config, self.args) diff --git a/source/fab/fab_base/site_specific/default/setup_cray.py b/source/fab/fab_base/site_specific/default/setup_cray.py index 1ded1d35..1f25144d 100644 --- a/source/fab/fab_base/site_specific/default/setup_cray.py +++ b/source/fab/fab_base/site_specific/default/setup_cray.py @@ -8,14 +8,13 @@ ''' import argparse -from typing import cast, Dict, List +from typing import cast -from fab.api import (AddFlags, BuildConfig, Category, Compiler, Linker, - ToolRepository) +from fab.api import BuildConfig, Category, Compiler, Linker, ToolRepository def setup_cray(build_config: BuildConfig, - args: argparse.Namespace) -> Dict[str, List[AddFlags]]: + args: argparse.Namespace) -> None: # pylint: disable=unused-argument, too-many-branches ''' Defines the default flags for ftn. @@ -30,7 +29,7 @@ def setup_cray(build_config: BuildConfig, ftn = cast(Compiler, ftn) if not ftn.is_available: - return {} + return # The base flags # ============== @@ -101,5 +100,3 @@ def setup_cray(build_config: BuildConfig, # You can use: # ftn = tr.get_tool(Category.FORTRAN_COMPILER, "crayftn-gfortran") # ftn.add_flags("-fallow-argument-mismatch") - - return {} diff --git a/source/fab/fab_base/site_specific/default/setup_gnu.py b/source/fab/fab_base/site_specific/default/setup_gnu.py index 511a117d..4113f496 100644 --- a/source/fab/fab_base/site_specific/default/setup_gnu.py +++ b/source/fab/fab_base/site_specific/default/setup_gnu.py @@ -8,13 +8,13 @@ ''' import argparse -from typing import cast, Dict, List +from typing import cast -from fab.api import AddFlags, BuildConfig, Category, Linker, ToolRepository +from fab.api import BuildConfig, Category, Compiler, Linker, ToolRepository def setup_gnu(build_config: BuildConfig, - args: argparse.Namespace) -> Dict[str, List[AddFlags]]: + args: argparse.Namespace) -> None: # pylint: disable=unused-argument ''' Defines the default flags for all GNU compilers and linkers. @@ -30,7 +30,9 @@ def setup_gnu(build_config: BuildConfig, if not gfortran.is_available: gfortran = tr.get_tool(Category.FORTRAN_COMPILER, "mpif90-gfortran") if not gfortran.is_available: - return {} + return + + gfortran = cast(Compiler, gfortran) # The base flags # ============== @@ -67,5 +69,3 @@ def setup_gnu(build_config: BuildConfig, # Add more flags to be always used, e.g.: # linker.add_post_lib_flags(["-lstdc++"], "base") - - return {} diff --git a/source/fab/fab_base/site_specific/default/setup_intel_classic.py b/source/fab/fab_base/site_specific/default/setup_intel_classic.py index 3796f5d2..7b9c4997 100644 --- a/source/fab/fab_base/site_specific/default/setup_intel_classic.py +++ b/source/fab/fab_base/site_specific/default/setup_intel_classic.py @@ -8,21 +8,20 @@ ''' import argparse -from typing import cast, Dict, List +from typing import cast -from fab.api import (AddFlags, BuildConfig, Category, Compiler, Linker, - ToolRepository) +from fab.api import BuildConfig, Category, Compiler, Linker, ToolRepository def setup_intel_classic(build_config: BuildConfig, - args: argparse.Namespace) -> Dict[str, List[AddFlags]]: + args: argparse.Namespace) -> None: # pylint: disable=unused-argument, too-many-locals ''' Defines the default flags for all Intel classic compilers and linkers. :param build_config: the Fab build config instance from which required parameters can be taken. - :param argparse.Namespace args: all command line options + :param args: all command line options ''' tr = ToolRepository() @@ -38,7 +37,7 @@ def setup_intel_classic(build_config: BuildConfig, if not ifort.is_available: # Since some flags depends on version, the code below requires # that the intel compiler actually works. - return {} + return # The base flags # ============== @@ -75,5 +74,3 @@ def setup_intel_classic(build_config: BuildConfig, # Add more flags to be always used, e.g.: # linker.add_post_lib_flags(["-lstdc++"], "base") - - return {} diff --git a/source/fab/fab_base/site_specific/default/setup_intel_llvm.py b/source/fab/fab_base/site_specific/default/setup_intel_llvm.py index b55da708..74da1a81 100644 --- a/source/fab/fab_base/site_specific/default/setup_intel_llvm.py +++ b/source/fab/fab_base/site_specific/default/setup_intel_llvm.py @@ -8,21 +8,20 @@ ''' import argparse -from typing import cast, Dict, List +from typing import cast -from fab.api import (AddFlags, BuildConfig, Category, Compiler, Linker, - ToolRepository) +from fab.api import BuildConfig, Category, Compiler, Linker, ToolRepository def setup_intel_llvm(build_config: BuildConfig, - args: argparse.Namespace) -> Dict[str, List[AddFlags]]: + args: argparse.Namespace) -> None: # pylint: disable=unused-argument, too-many-locals ''' Defines the default flags for all Intel llvm compilers. :param build_config: the Fab build config instance from which required parameters can be taken. - :param argparse.Namespace args: all command line options + :param args: all command line options ''' tr = ToolRepository() @@ -33,7 +32,7 @@ def setup_intel_llvm(build_config: BuildConfig, ifx = tr.get_tool(Category.FORTRAN_COMPILER, "mpif90-ifx") ifx = cast(Compiler, ifx) if not ifx.is_available: - return {} + return # The base flags # ============== @@ -64,5 +63,3 @@ def setup_intel_llvm(build_config: BuildConfig, # Add more flags to be always used, e.g.: # linker.add_post_lib_flags(["-lstdc++"], "base") - - return {} diff --git a/source/fab/fab_base/site_specific/default/setup_nvidia.py b/source/fab/fab_base/site_specific/default/setup_nvidia.py index 8ff358ee..ac6e5907 100644 --- a/source/fab/fab_base/site_specific/default/setup_nvidia.py +++ b/source/fab/fab_base/site_specific/default/setup_nvidia.py @@ -8,14 +8,13 @@ ''' import argparse -from typing import cast, Dict, List +from typing import cast -from fab.api import (AddFlags, BuildConfig, Category, Compiler, Linker, - ToolRepository) +from fab.api import BuildConfig, Category, Compiler, Linker, ToolRepository def setup_nvidia(build_config: BuildConfig, - args: argparse.Namespace) -> Dict[str, List[AddFlags]]: + args: argparse.Namespace) -> None: # pylint: disable=unused-argument ''' Defines the default flags for nvfortran. @@ -33,7 +32,7 @@ def setup_nvidia(build_config: BuildConfig, nvfortran = tr.get_tool(Category.FORTRAN_COMPILER, "mpif90-nvfortran") nvfortran = cast(Compiler, nvfortran) if not nvfortran.is_available: - return {} + return # The base flags # ============== @@ -66,5 +65,3 @@ def setup_nvidia(build_config: BuildConfig, # Always link with C++ libs # linker.add_post_lib_flags(["-c++libs"], "base") - - return {} diff --git a/source/fab/steps/compile_c.py b/source/fab/steps/compile_c.py index ad8423c9..5a75c623 100644 --- a/source/fab/steps/compile_c.py +++ b/source/fab/steps/compile_c.py @@ -14,13 +14,13 @@ from fab import FabException from fab.artefacts import (ArtefactsGetter, ArtefactSet, ArtefactStore, FilterBuildTrees) -from fab.build_config import BuildConfig, FlagsConfig +from fab.build_config import AddFlags, BuildConfig 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.category import Category from fab.tools.compiler import Compiler -from fab.tools.flags import Flags +from fab.tools.flags import FlagList from fab.util import CompiledFile, log_or_dot, Timer, by_type logger = logging.getLogger(__name__) @@ -33,12 +33,13 @@ class MpCommonArgs: '''A simple class to pass arguments to subprocesses.''' config: BuildConfig - flags: FlagsConfig + flag_list: FlagList @step -def compile_c(config, common_flags: Optional[List[str]] = None, - path_flags: Optional[List] = None, +def compile_c(config: BuildConfig, + common_flags: Optional[List[str]] = None, + path_flags: Optional[List[AddFlags]] = None, source: Optional[ArtefactsGetter] = None): """ Compiles all C files in all build trees, creating or extending a set of @@ -69,7 +70,8 @@ def compile_c(config, common_flags: Optional[List[str]] = None, common_flags = common_flags or [] - flags = FlagsConfig(common_flags=common_flags, path_flags=path_flags) + flag_list = FlagList(common_flags, add_flags=path_flags) + source_getter = source or DEFAULT_SOURCE_GETTER # gather all the source to compile, for all build trees, into one big lump @@ -85,7 +87,7 @@ def compile_c(config, common_flags: Optional[List[str]] = None, openmp=config.openmp) logger.info(f'C compiler is {compiler}') - mp_payload = MpCommonArgs(config=config, flags=flags) + mp_payload = MpCommonArgs(config=config, flag_list=flag_list) mp_items = [(fpath, mp_payload) for fpath in to_compile] # compile everything in one go @@ -131,10 +133,9 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]): # to cast it to be a Compiler. compiler = cast(Compiler, compiler) with Timer() as timer: - flags = Flags(mp_payload.flags.flags_for_path(path=analysed_file.fpath, - config=config)) + flag_list = mp_payload.flag_list obj_combo_hash = _get_obj_combo_hash(config, compiler, - analysed_file, flags) + analysed_file, flag_list) obj_file_prebuild = (config.prebuild_folder / f'{analysed_file.fpath.stem}.' @@ -147,6 +148,7 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]): else: obj_file_prebuild.parent.mkdir(parents=True, exist_ok=True) log_or_dot(logger, f'CompileC compiling {analysed_file.fpath}') + flags = flag_list.get_flags(config, analysed_file.fpath) try: compiler.compile_file(analysed_file.fpath, obj_file_prebuild, config=config, @@ -164,13 +166,13 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]): def _get_obj_combo_hash(config: BuildConfig, - compiler: Compiler, analysed_file, flags: Flags): + compiler: Compiler, analysed_file, flags: FlagList): # get a combo hash of things which matter to the object file we define try: obj_combo_hash = sum([ analysed_file.file_hash, - flags.checksum(), - compiler.get_hash(config.profile), + flags.checksum(config, analysed_file.fpath), + compiler.get_hash(config, analysed_file.fpath), ]) except TypeError as err: raise ValueError("could not generate combo hash for " diff --git a/source/fab/steps/compile_fortran.py b/source/fab/steps/compile_fortran.py index d2771838..32bd07f0 100644 --- a/source/fab/steps/compile_fortran.py +++ b/source/fab/steps/compile_fortran.py @@ -17,13 +17,13 @@ from fab.artefacts import (ArtefactsGetter, ArtefactSet, ArtefactStore, FilterBuildTrees) -from fab.build_config import BuildConfig, FlagsConfig +from fab.build_config import BuildConfig 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.category import Category -from fab.tools.compiler import Compiler -from fab.tools.flags import Flags +from fab.tools.compiler import Compiler, FortranCompiler +from fab.tools.flags import FlagList from fab.util import (CompiledFile, log_or_dot_finish, log_or_dot, Timer, by_type, file_checksum) @@ -37,7 +37,7 @@ class MpCommonArgs: """Arguments to be passed into the multiprocessing function, alongside the filenames.""" config: BuildConfig - flags: FlagsConfig + flag_list: FlagList mod_hashes: Dict[str, int] syntax_only: bool @@ -87,15 +87,15 @@ def compile_fortran(config: BuildConfig, if len(uncompiled) == 0: return - compiler, flags_config = handle_compiler_args(config, common_flags, - path_flags) + compiler, flag_list = handle_compiler_args(config, common_flags, + path_flags) # Set module output folder: compiler.set_module_output_path(config.build_output) syntax_only = compiler.has_syntax_only and config.two_stage # build the arguments passed to the multiprocessing function mp_common_args = MpCommonArgs( - config=config, flags=flags_config, + config=config, flag_list=flag_list, mod_hashes=mod_hashes, syntax_only=syntax_only) if syntax_only: @@ -129,8 +129,9 @@ def compile_fortran(config: BuildConfig, store_artefacts(compiled, build_lists, config.artefact_store) -def handle_compiler_args(config: BuildConfig, common_flags=None, - path_flags=None): +def handle_compiler_args(config: BuildConfig, + common_flags=None, + path_flags=None) -> Tuple[FortranCompiler, FlagList]: # Command line tools are sometimes specified with flags attached. compiler = config.tool_box.get_tool(Category.FORTRAN_COMPILER) @@ -139,16 +140,15 @@ def handle_compiler_args(config: BuildConfig, common_flags=None, f"'{compiler.category}' instead of FortranCompiler") # The ToolBox returns a Tool. In order to make mypy happy, we need to # cast this to become a Compiler. - compiler = cast(Compiler, compiler) + compiler = cast(FortranCompiler, compiler) logger.info( f'Fortran compiler is {compiler} {compiler.get_version_string()}') # Collate the flags from 1) flags env and 2) parameters. common_flags = common_flags or [] - flags_config = FlagsConfig(common_flags=common_flags, - path_flags=path_flags) - return compiler, flags_config + flag_list = FlagList(common_flags, add_flags=path_flags) + return compiler, flag_list def compile_pass(config, compiled: Dict[Path, CompiledFile], @@ -270,16 +270,16 @@ def process_file(arg: Tuple[AnalysedFortran, MpCommonArgs]) \ f"category '{compiler.category}' instead of " f"FortranCompiler") # The ToolBox returns a Tool, but we need to tell mypy that - # this is a Compiler - compiler = cast(Compiler, compiler) - flags = Flags(mp_common_args.flags.flags_for_path( - path=analysed_file.fpath, config=config)) + # this is a FortranCompiler + compiler = cast(FortranCompiler, compiler) + flag_list = mp_common_args.flag_list mod_combo_hash = _get_mod_combo_hash(config, analysed_file, compiler=compiler) obj_combo_hash = _get_obj_combo_hash(config, analysed_file, mp_common_args=mp_common_args, - compiler=compiler, flags=flags) + compiler=compiler, + flag_list=flag_list) # calculate the incremental/prebuild artefact filenames obj_file_prebuild = ( @@ -296,8 +296,9 @@ def process_file(arg: Tuple[AnalysedFortran, MpCommonArgs]) \ [obj_file_prebuild] + mod_file_prebuilds)) if not all(prebuilds_exist): # compile + logger.debug(f'CompileFortran compiling {analysed_file.fpath}') + flags = flag_list.get_flags(config, analysed_file.fpath) try: - logger.debug(f'CompileFortran compiling {analysed_file.fpath}') compile_file(analysed_file.fpath, flags, output_fpath=obj_file_prebuild, mp_common_args=mp_common_args) @@ -345,8 +346,10 @@ def process_file(arg: Tuple[AnalysedFortran, MpCommonArgs]) \ def _get_obj_combo_hash(config: BuildConfig, - analysed_file, mp_common_args: MpCommonArgs, - compiler: Compiler, flags: Flags): + analysed_file: AnalysedFortran, + mp_common_args: MpCommonArgs, + compiler: Compiler, + flag_list: FlagList): # get a combo hash of things which matter to the object file we define # todo: don't just silently use 0 for a missing dep hash mod_deps_hashes = { @@ -355,9 +358,9 @@ def _get_obj_combo_hash(config: BuildConfig, try: obj_combo_hash = sum([ analysed_file.file_hash, - flags.checksum(), + flag_list.checksum(config, analysed_file.fpath), sum(mod_deps_hashes.values()), - compiler.get_hash(config.profile), + compiler.get_hash(config, analysed_file.fpath), ]) except TypeError as err: raise ValueError("Could not generate combo hash " @@ -370,7 +373,7 @@ def _get_mod_combo_hash(config, analysed_file, compiler: Compiler): try: mod_combo_hash = sum([ analysed_file.file_hash, - compiler.get_hash(config.profile), + compiler.get_hash(config, analysed_file.fpath), ]) except TypeError as err: raise ValueError("Could not generate combo " @@ -378,7 +381,10 @@ def _get_mod_combo_hash(config, analysed_file, compiler: Compiler): return mod_combo_hash -def compile_file(analysed_file, flags, output_fpath, mp_common_args): +def compile_file(input_fpath: Path, + flags: List[str], + output_fpath: Path, + mp_common_args: MpCommonArgs) -> None: """ Call the compiler. @@ -390,11 +396,12 @@ def compile_file(analysed_file, flags, output_fpath, mp_common_args): """ output_fpath.parent.mkdir(parents=True, exist_ok=True) - # tool config = mp_common_args.config compiler = config.tool_box.get_tool(Category.FORTRAN_COMPILER) + compiler = cast(FortranCompiler, compiler) - compiler.compile_file(input_file=analysed_file, output_file=output_fpath, + compiler.compile_file(input_file=input_fpath, + output_file=output_fpath, config=config, add_flags=flags, syntax_only=mp_common_args.syntax_only) diff --git a/source/fab/steps/preprocess.py b/source/fab/steps/preprocess.py index 0e1927a9..b4bf92de 100644 --- a/source/fab/steps/preprocess.py +++ b/source/fab/steps/preprocess.py @@ -15,11 +15,12 @@ from fab.artefacts import (ArtefactSet, ArtefactsGetter, SuffixFilter, CollectionGetter) -from fab.build_config import BuildConfig, FlagsConfig +from fab.build_config import BuildConfig from fab.metrics import send_metric from fab.steps import check_for_errors, run_mp, step from fab.tools.category import Category from fab.tools.preprocessor import Cpp, CppFortran, Preprocessor +from fab.tools.flags import FlagList from fab.util import (log_or_dot_finish, input_to_output_fpath, log_or_dot, suffix_filter, Timer, by_type) @@ -32,7 +33,7 @@ class MpCommonArgs(): config: BuildConfig output_suffix: str preprocessor: Preprocessor - flags: FlagsConfig + flag_list: FlagList name: str @@ -60,15 +61,15 @@ def pre_processor(config: BuildConfig, preprocessor: Preprocessor, :param output_suffix: Suffix for output files. :param common_flags: - Used to construct a :class:`~fab.config.FlagsConfig` object. + Path-independent flags for the preprocessor to use. :param path_flags: - Used to construct a :class:`~fab.build_config.FlagsConfig` object. + Path-dependent flags for the preprocessor to use. :param name: Human friendly name for logger output, with sensible default. """ common_flags = common_flags or [] - flags = FlagsConfig(common_flags=common_flags, path_flags=path_flags) + flag_list = FlagList(common_flags, add_flags=path_flags) logger.info(f"preprocessor is '{preprocessor.name}'.") @@ -79,7 +80,7 @@ def pre_processor(config: BuildConfig, preprocessor: Preprocessor, config=config, output_suffix=output_suffix, preprocessor=preprocessor, - flags=flags, + flag_list=flag_list, name=name, ) @@ -113,12 +114,12 @@ def process_artefact(arg: Tuple[Path, MpCommonArgs]): else: output_fpath.parent.mkdir(parents=True, exist_ok=True) - params = args.flags.flags_for_path(path=input_fpath, config=args.config) - + flags = args.flag_list.get_flags(args.config, input_fpath) log_or_dot(logger, f"PreProcessor running with parameters: " - f"'{' '.join(params)}'.'") + f"'{' '.join(flags)}'.'") try: - args.preprocessor.preprocess(input_fpath, output_fpath, params) + args.preprocessor.preprocess(input_fpath, output_fpath, + flags) except Exception as err: raise Exception(f"error preprocessing {input_fpath}:\n" f"{err}") from err diff --git a/source/fab/tools/__init__.py b/source/fab/tools/__init__.py index e69de29b..dc61f11f 100644 --- a/source/fab/tools/__init__.py +++ b/source/fab/tools/__init__.py @@ -0,0 +1,5 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index 9051ad6e..a2e84e15 100644 --- a/source/fab/tools/compiler.py +++ b/source/fab/tools/compiler.py @@ -12,12 +12,12 @@ from pathlib import Path import warnings from typing import cast, List, Optional, Tuple, Union -import zlib -from fab.build_config import BuildConfig +from fab.build_config import BuildConfig from fab.tools.category import Category -from fab.tools.flags import Flags -from fab.tools.tool import CompilerSuiteTool +from fab.tools.flags import AlwaysFlags +from fab.tools.compiler_suite_tool import CompilerSuiteTool +from fab.util import string_checksum class Compiler(CompilerSuiteTool): @@ -103,19 +103,23 @@ def output_flag(self) -> str: """ return self._output_flag - def get_hash(self, profile: Optional[str] = None) -> int: + def get_hash(self, + config: "BuildConfig", + file_path: Path + ) -> int: """ + Computes a hash code using the name and version of the compiler, + and the compilation flag used by the compiler for the specified + file. + + :param config: The build configuration to use. + :param file_path: Path of the file to compile. :returns: hash of compiler name and version. """ - return (zlib.crc32(self.name.encode()) + - zlib.crc32(str(self.get_flags(profile)).encode()) + - zlib.crc32(self.get_version_string().encode())) - - def get_flags(self, profile: Optional[str] = None) -> List[str]: - '''Determines the flags to be used. - - :returns: the flags to be used with this tool.''' - return self._flags[profile] + all_params = (self.name + + self.get_version_string() + + str(self.get_flags(config, file_path))) + return string_checksum(all_params) def get_all_commandline_options( self, @@ -142,6 +146,11 @@ def get_all_commandline_options( if config.openmp: params.append(self.openmp_flag) + + # Explicitly add all compilation flags here, where the input + # path can be provided to properly resolve path-specific flags. + params += self.get_flags(config, input_file) + if add_flags: if self.openmp_flag in add_flags: warnings.warn( @@ -153,6 +162,20 @@ def get_all_commandline_options( params.extend([input_file.name, self._output_flag, str(output_file)]) return params + def get_flags(self, config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """ + The flags to use when compiling the specified flag. All + AbstractFlags (e.g. MatchFlags, ...) will be resolved. + + :param config: The build configuration to use. + :param file_path: the path to the file to be compiled. + + :returns: the flags actually used when building the specified + path. + """ + return self.flags.get_flags(config, file_path) + def compile_file(self, input_file: Path, output_file: Path, config: "BuildConfig", @@ -173,7 +196,7 @@ def compile_file(self, input_file: Path, params = self.get_all_commandline_options(config, input_file, output_file, add_flags) - return self.run(profile=config.profile, cwd=input_file.parent, + return self.run(cwd=input_file.parent, additional_parameters=params) def check_available(self) -> bool: @@ -396,13 +419,14 @@ def get_all_commandline_options( :returns: all command line options for Fortran compilation. ''' if add_flags: - add_flags = Flags(add_flags) + # Create an AlwaysFlags to use its remove_flag method + af = AlwaysFlags(add_flags) if 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) - add_flags.remove_flag(self._compile_flag, has_parameter=False) + af.remove_flag(self._module_folder_flag, has_parameter=True) + af.remove_flag(self._compile_flag, has_parameter=False) + add_flags = af.get_flags(config, input_file) # Get the flags from the base class params = super().get_all_commandline_options(config, input_file, @@ -448,8 +472,7 @@ def compile_file(self, input_file: Path, output_file, add_flags, syntax_only) - self.run(profile=config.profile, cwd=input_file.parent, - additional_parameters=params) + self.run(cwd=input_file.parent, additional_parameters=params) # ============================================================================ diff --git a/source/fab/tools/compiler_suite_tool.py b/source/fab/tools/compiler_suite_tool.py new file mode 100644 index 00000000..87a735ed --- /dev/null +++ b/source/fab/tools/compiler_suite_tool.py @@ -0,0 +1,50 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +"""This file contains the base class for all tools, i.e. compiler, +preprocessor, linker, archiver, Psyclone, rsync, versioning tools. + +Each tool belongs to one category (e.g. FORTRAN_COMPILER). This category +is used when adding a tool to a ToolRepository or ToolBox. +It provides basic support for running a binary, and keeping track if +a tool is actually available. +""" + +from pathlib import Path +from typing import List, Optional, Union + +from fab.tools.category import Category +from fab.tools.tool_with_flags import ToolWithFlags + + +class CompilerSuiteTool(ToolWithFlags): + '''A tool that is part of a compiler suite (typically compiler + and linker). + + :param name: name of the tool. + :param exec_name: name of the executable to start. + :param suite: name of the compiler suite. + :param category: the Category to which this tool belongs. + :param availability_option: a command line option for the tool to test + if the tool is available on the current system. Defaults to + `--version`. + ''' + def __init__( + self, + name: str, + exec_name: Union[str, Path], + suite: str, + category: Category, + availability_option: Optional[Union[str, + List[str]]] = None) -> None: + super().__init__(name, exec_name, category, + availability_option=availability_option) + self._suite = suite + + @property + def suite(self) -> str: + ''':returns: the compiler suite of this tool.''' + return self._suite diff --git a/source/fab/tools/compiler_wrapper.py b/source/fab/tools/compiler_wrapper.py index cae8df7c..41d07e7b 100644 --- a/source/fab/tools/compiler_wrapper.py +++ b/source/fab/tools/compiler_wrapper.py @@ -14,7 +14,7 @@ from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.compiler import Compiler, FortranCompiler -from fab.tools.flags import Flags +from fab.tools.flags import FlagList class CompilerWrapper(Compiler): @@ -70,14 +70,15 @@ def has_syntax_only(self) -> bool: raise RuntimeError(f"Compiler '{self._compiler.name}' has " f"no has_syntax_only.") - def get_flags(self, profile: Optional[str] = None) -> List[str]: + def get_flags(self, config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: ''':returns: the ProfileFlags for the given profile, combined from the wrapped compiler and this wrapper. :param profile: the profile to use. ''' - return (self._compiler.get_flags(profile) + - super().get_flags(profile)) + return (self._compiler.get_flags(config, file_path) + + super().get_flags(config, file_path)) def set_module_output_path(self, path: Path): '''Sets the output path for modules. @@ -99,7 +100,7 @@ def get_all_commandline_options( input_file: Path, output_file: Path, add_flags: Union[None, List[str]] = None, - syntax_only: Optional[bool] = False) -> List[str]: + syntax_only: Optional[bool] = None) -> List[str]: '''This function returns all command line options for a compiler wrapper. The syntax_only flag is only accepted, if the wrapped compiler is a Fortran compiler. Otherwise, @@ -120,7 +121,9 @@ def get_all_commandline_options( ''' # We need to distinguish between Fortran and non-Fortran compiler, # since only a Fortran compiler supports the syntax-only flag. - new_flags = Flags(add_flags) + new_flags = FlagList(self.flags.get_flags(config, input_file)) + if add_flags: + new_flags.add_flags(add_flags) if self._compiler.category is Category.FORTRAN_COMPILER: # Mypy complains that self._compiler does not take the syntax @@ -134,16 +137,18 @@ def get_all_commandline_options( # with Fab's module handling. new_flags.remove_flag(self._compiler._module_folder_flag, has_parameter=True) + resolved_flags = new_flags.get_flags(file_path=input_file) flags = self._compiler.get_all_commandline_options( - config, input_file, output_file, add_flags=add_flags, + config, input_file, output_file, add_flags=resolved_flags, syntax_only=syntax_only) else: # It's not valid to specify syntax_only for a non-Fortran compiler if syntax_only is not None: raise RuntimeError(f"Syntax-only cannot be used with compiler " f"'{self.name}'.") + resolved_flags = new_flags.get_flags(file_path=input_file) flags = self._compiler.get_all_commandline_options( - config, input_file, output_file, add_flags=add_flags) + config, input_file, output_file, add_flags=resolved_flags) return flags @@ -168,7 +173,7 @@ def compile_file(self, input_file: Path, config, input_file, output_file, add_flags=add_flags, syntax_only=syntax_only) - self.run(profile=config.profile, cwd=input_file.parent, + self.run(cwd=input_file.parent, additional_parameters=flags) diff --git a/source/fab/tools/flags.py b/source/fab/tools/flags.py index 23b04b3c..5ec01d9f 100644 --- a/source/fab/tools/flags.py +++ b/source/fab/tools/flags.py @@ -4,51 +4,173 @@ # which you should have received as part of this distribution ############################################################################## -'''This file contains a simple Flag class to manage tool flags. -It will need to be combined with build_config.FlagsConfig in a follow up -PR. ''' +This file contains the flag classes used to manage command line flags +for tools, especially path-specific flags for compiler. + +AbstractFlags: + The base class for a set of flags. + +AlwaysFlags(AbstractFlags): + Flags that will always apply, independent of the path of the + file to be compiled. It provides Python Template functionality: + - `$source` for */source* + - `$output` for */build_output* + - `$relative` for ** + +MatchFlags(AlwaysFlags) + Flags that are only applied if a wildcard search matches the + source file. This will likely require to make sure that the full + path is specified (or the pattern starts with `*`). + +ContainFlags(AlwaysFlags) + Flags that are only applied if the file path contains the specified + string. The difference to MatchFlags is that ContainFlags do not + need the full path to be specified. + +FlagList: + Manages a list of flags, each of which is an instance of an AbstractFlag. + +ProfileFlags: + Manages a set of flags for specific profiles, including inheritance. + +Each tool uses a ProfileFlags instance. At runtime, the compilation steps +will use the selected profile to get the Flags instance to use. +The function `get_flags` will resolve the list of AbstractFlags by +converting them from left to right into a list of strings. For example, +[AlwaysFlag("-g"), ContainFlags("-O3", pattern="special_file")] +will be convert to `["-g", "-O3"]` if the file contains the string +`special_file`, and otherwise it will be `["-g"]`. +''' + +from abc import ABC, abstractmethod +from fnmatch import fnmatch import logging +from pathlib import Path +from string import Template from typing import Dict, List, Optional, Union import warnings from fab.util import string_checksum +from fab.build_config import AddFlags, BuildConfig -class Flags(list): - '''This class represents a list of parameters for a tool. It is a - list with some additional functionality. +logger = logging.getLogger(__name__) - TODO #22: This class and build_config.FlagsConfig should be combined. - :param list_of_flags: List of parameters to initialise this object with. +class AbstractFlags(ABC): + ''' + An abstract class to act as base class for all flag classes. ''' - def __init__(self, list_of_flags: Optional[List[str]] = None): - self._logger = logging.getLogger(__name__) - super().__init__() - if list_of_flags: - self.extend(list_of_flags) + @abstractmethod + def __init__(self) -> None: + """ + Dummy constructor. + """ - def checksum(self) -> str: + @abstractmethod + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: """ - :returns: a checksum of the flags. + This function returns the list of flags to be used for the given + filename. + + :param config: the config object (required for paths in templated + strings) + :param file_path: the file path of the file. This might not be used + in all implementations. + :returns: the list of flags to use. """ - return string_checksum(str(self)) - def add_flags(self, new_flags: Union[str, List[str]]): - '''Adds the specified flags to the list of flags. + @abstractmethod + def remove_flag(self, remove_flag: str, has_parameter: bool = False): + '''Removes all occurrences of `remove_flag` in flags. + If has_parameter is defined, the next entry in flags will also be + removed, and if this object contains this flag+parameter without space + (e.g. `-J/tmp`), it will be correctly removed. Note that only the + flag itself must be specified, you cannot remove a flag only if a + specific parameter is given (i.e. `remove_flag="-J/tmp"` will not + work if this object contains `[...,"-J", "/tmp"]`). - :param new_flags: A single string or list of strings which are the - flags to be added. + :param remove_flag: the flag to remove + :param has_parameter: if the flag to remove takes a parameter ''' - if isinstance(new_flags, str): - self.append(new_flags) + +class AlwaysFlags(AbstractFlags): + """ + This class represents a list of flags that is always to be used, + independent of the path of the source file. It also provides + template functionality. This class also acts as convenient base + class for all applications that add path-independent flags. + + :param flags: a string or list of strings with command line flags. + """ + def __init__(self, flags: Optional[Union[str, List[str]]] = None) -> None: + + if isinstance(flags, str): + self._flags = [flags] + elif flags: + # Make a copy in case the user modifies the list later + self._flags = flags[:] else: - self.extend(new_flags) + self._flags = [] + + @staticmethod + def replace_template(string_list: List[str], + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """This function replaces all `$relative`, `$source`, and `$output` + in the string or list of string with the values taken from + the config object and the file path. + It is implemented as an abstract method (instead of acting on + self._flags) since templating is also supported in patterns, to + the same code here can be re-used). + + :param string_list: list of strings, which will get all template + arguments replaced. + :param config: the build configuration object from which paths + are taken. + :param file_path: the file path of an argument, used for `$source`. + + :returns: a new list with strings where all the template parameters + have been removed. + """ + params = {} + if config: + params['source'] = config.source_root + params['output'] = config.build_output + else: + params['source'] = Path("/") + params['output'] = Path("/") + if file_path: + params['relative'] = file_path.parent + else: + params['relative'] = Path(".") + + # Use templating to render any relative paths in our flags + return [Template(i).substitute(params) for i in string_list] + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """ + This function returns the list of flags to be used for the given + filename. This class will not take the file path into account, + but it does support templated expressions `$relative`, `$source`, + and `$output`. + + :param config: the config object (used for paths in templates) + :param file_path: the file path of the file, not used in this + class. + + :returns: the list of flags to use. + """ + return AlwaysFlags.replace_template(self._flags, config, file_path) def remove_flag(self, remove_flag: str, has_parameter: bool = False): '''Removes all occurrences of `remove_flag` in flags. @@ -63,24 +185,23 @@ def remove_flag(self, remove_flag: str, has_parameter: bool = False): :param has_parameter: if the flag to remove takes a parameter ''' - # TODO #313: Check if we can use an OrderedDict and get O(1) - # behaviour here (since ordering of flags can be important) i = 0 flag_len = len(remove_flag) - while i < len(self): - flag = self[i] + while i < len(self._flags): + flag = self._flags[i] # First check for the flag stand-alone, i.e. if it has a parameter, # it will be the next entry: [... "-J", "/tmp"]: if flag == remove_flag: - if has_parameter and i + 1 == len(self): + if has_parameter and i + 1 == len(self._flags): # We have a flag which takes a parameter, but there is no # parameter. Issue a warning: - self._logger.warning(f"Flags '{' '. join(self)}' contain " - f"'{remove_flag}' but no parameter.") - del self[i] + logger.warning(f"Flags '{' '. join(self._flags)}'" + f" contain '{remove_flag}' but no " + f"parameter.") + del self._flags[i] else: # Delete the argument and if required its parameter - del self[i:i+(2 if has_parameter else 1)] + del self._flags[i:i+(2 if has_parameter else 1)] warnings.warn(f"Removing managed flag '{remove_flag}'.") continue # Now check if it has flag and parameter as one argument (-J/tmp) @@ -88,32 +209,255 @@ def remove_flag(self, remove_flag: str, has_parameter: bool = False): if has_parameter and flag[:flag_len] == remove_flag: # No space between flag and parameter, remove this one flag warnings.warn(f"Removing managed flag '{remove_flag}'.") - del self[i] + del self._flags[i] continue i += 1 +class MatchFlags(AlwaysFlags): + """ + This class implements path-specific flags using a file system + wild card expressions (e.g. supporting `*` and `?`). It does support + templated expressions `$relative`, `$source`, and `$output` in the + pattern as well. + + :param pattern: the wildcard pattern which is used when matching. + :param flags: a string or list of strings with command line flags. + """ + def __init__(self, + pattern: str, + flags: Union[str, List[str]]) -> None: + super().__init__(flags) + self._pattern = pattern + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """ + This function returns the list of flags to be used for the given + filename if the specified file path matches the pattern specified. + + :param config: the config object (used for paths in templates) + :param file_path: the file path of the file that must match the + specified wildcard pattern. + + :returns: the list of flags to use. + """ + # Resolve the pattern template: + pattern = self.replace_template([self._pattern], config, file_path)[0] + # If there is no wildcard match, return an empty list + if not fnmatch(str(file_path), pattern): + return [] + + return super().get_flags(config, file_path) + + +class ContainFlags(AlwaysFlags): + """ + This class implements path-specific flags using a simple + substring search. + + :param flags: a string or list of strings with command line flags. + :param pattern: the substring which is used when matching. + """ + + def __init__(self, + pattern: str, + flags: Union[str, List[str]]) -> None: + super().__init__(flags) + self._pattern = pattern + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """ + This function returns the list of flags to be used for the given + filename if the specified file path contains the pattern as + a substrings (i.e. anywhere in the file path). + + :param config: the config object (used for paths in templates) + :param file_path: the file path of the file that must match the + specified wildcard pattern. + + :returns: the list of flags to use. + """ + + # Resolve the pattern template: + pattern = self.replace_template([self._pattern], config, file_path)[0] + # If pattern is not in the file path, return an empty list + if pattern not in str(file_path): + return [] + + return super().get_flags(config, file_path) + + +class FlagList(List[AbstractFlags]): + '''This class represents a list of parameters for a tool. It is a + list with some additional functionality. + + :param list_of_flags: List of parameters to initialise this object with. + :param add_flags: List of old-style AddFlags, which will be converted + to the new MatchFlags. + ''' + + def __init__( + self, + list_of_flags: Optional[Union[AbstractFlags, str, + List[str]]] = None, + add_flags: Optional[Union[AddFlags, + List[AddFlags]]] = None) -> None: + self._logger = logging.getLogger(__name__) + super().__init__() + if isinstance(list_of_flags, (str, list)): + self.append(AlwaysFlags(list_of_flags)) + elif list_of_flags: + self.append(list_of_flags) + if add_flags: + if isinstance(add_flags, AddFlags): + add_flags = [add_flags] + # Convert old-style AddFlags to the new MatchFlags: + for add_flag in add_flags: + self.add_flags(MatchFlags(add_flag.match, + add_flag.flags)) + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + """ + :returns: the flags to be used for the compilation profile and + file path specified. + """ + + if not file_path: + # If no path, provide a dummy path + file_path = Path() + + all_flags_resolved: List[str] = [] + + for flags in self: + all_flags_resolved.extend(flags.get_flags(config, file_path)) + + return all_flags_resolved + + def checksum(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> int: + """ + :param config: the config object (used for templating) + :param file_path: the file path of the source file, used for + path-specific flags. + + :returns: a checksum of the flags. + """ + + if not file_path: + # If no path, provide a dummy path + file_path = Path() + + resolve_flags: List[str] = self.get_flags(config, file_path) + return string_checksum(str(resolve_flags)) + + def add_flags(self, + new_flags: Union[AbstractFlags, str, List[str]]) -> None: + '''Adds the specified flags to the list of flags. + + :param new_flags: New flags to be added. Can be either an class + derived from AbstractFlags, a single string or list of strings. + ''' + + if isinstance(new_flags, AbstractFlags): + self.append(new_flags) + else: + self.append(AlwaysFlags(new_flags)) + + def remove_flag(self, remove_flag: str, has_parameter: bool = False): + '''Removes all occurrences of `remove_flag` in flags. + If `has_parameter` is defined, the next entry in flags will also be + removed, and if this object contains this flag+parameter without space + (e.g. `-J/tmp`), it will be correctly removed. Note that only the + flag itself must be specified, you cannot remove a flag only if a + specific parameter is given (i.e. `remove_flag="-J/tmp"` will not + work if this object contains `[...,"-J", "/tmp"]`). + + This function will just call all individual flag objects to remove + the flag. Note that a flag and its parameter cannot be split into + two Flag instances (i.e. [AlwaysFlag("-J"), AlwaysFlag("/tmp")] + will not work). + + :param remove_flag: the flag to remove + :param has_parameter: if the flag to remove takes a parameter + ''' + + for flags in self: + flags.remove_flag(remove_flag, has_parameter) + + class ProfileFlags: '''A list of flags that support a 'profile' to be used. If no profile is specified, it will use "" (empty string) as 'profile'. All functions take an optional profile parameter, so this class can also be used for tools that do not need a profile. + + :param flags: optional flags to be added to this profile. + :param profile: optional profile to use if flags are specified, + defaults to "". ''' - def __init__(self: "ProfileFlags"): + def __init__(self: "ProfileFlags", + flags: Optional[Union[AbstractFlags, str, List[str]]] = None, + profile: str = "") -> None: # Stores the flags for each profile mode. The key is the (lower case) - # name of the profile mode, and it contains a list of flags - self._profiles: Dict[str, Flags] = {"": Flags()} + # name of the profile mode, and it contains a list of flags. + # Initialise the dict with the default (empty) profile + self._profiles: Dict[str, FlagList] = {"": FlagList()} # This dictionary stores an optional inheritance, where one mode # 'inherits' the flags from a different mode (recursively) self._inherit_from: Dict[str, str] = {} - def __getitem__(self, profile: Optional[str] = None) -> List[str]: + if flags: + if profile != "": + self.define_profile(profile) + self.add_flags(flags, profile) + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + ''' + This method returns the flags used for the specified file, + i.e. it will support path-specific flags. The BuildConfig + is added as parameter to get the profile, but also to + allow flags to use templated expressions `$relative` and + `$output` (the values are taken from the config object). + + :param config: the build config object. It stores the selected + compilation profile, and paths that can be used in templated + expressions. + :param file_path: path to the source file to compile. + ''' + if not file_path: + # If no path, provide a dummy path + file_path = Path() + if config: + profile = config.profile + else: + profile = "" + + all_flags = self[profile] + + resolved_flags = [] + for flags in all_flags: + resolved_flags.extend(flags.get_flags(config, file_path)) + + return resolved_flags + + def __getitem__(self, + profile: Optional[str] = None) -> List[AbstractFlags]: '''Returns the flags for the requested profile. If profile is not specified, the empty profile ("") will be used. It will also take inheritance into account, so add flags (recursively) from inherited - profiles. + profiles. But this function will not resolve the flags, i.e. replace + the AbstractFlags instances with a list of strings. :param profile: the optional profile to use. @@ -156,7 +500,7 @@ def define_profile(self, ''' if name in self._profiles: raise KeyError(f"Profile '{name}' is already defined.") - self._profiles[name.lower()] = Flags() + self._profiles[name.lower()] = FlagList() if inherit_from is not None: if inherit_from not in self._profiles: @@ -165,8 +509,8 @@ def define_profile(self, self._inherit_from[name.lower()] = inherit_from.lower() def add_flags(self, - new_flags: Union[str, List[str]], - profile: Optional[str] = None): + new_flags: Union[AbstractFlags, str, List[str]], + profile: Optional[str] = None) -> None: '''Adds the specified flags to the list of flags. :param new_flags: A single string or list of strings which are the @@ -211,17 +555,28 @@ def remove_flag(self, self._profiles[profile].remove_flag(remove_flag, has_parameter) - def checksum(self, profile: Optional[str] = None) -> str: + def checksum(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> int: """ - :returns: a checksum of the flags. + :param config: the config object (used for templating) + :param file_path: the file path of the source file, used for + path-specific flags. + :returns: a checksum of the flags. """ - if not profile: - profile = "" + + if not file_path: + # If no path, provide a dummy path + file_path = Path() + if config: + profile = config.profile else: - profile = profile.lower() + profile = "" if profile not in self._profiles: - raise KeyError(f"checksum: Profile '{profile}' is not defined.") + raise KeyError(f"checksum: Profile '{profile}' is " + f"not defined.") - return self._profiles[profile].checksum() + resolve_flags: List[str] = self.get_flags(config, file_path) + return string_checksum(str(resolve_flags)) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 394bd3d3..68d1099f 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -10,14 +10,14 @@ from __future__ import annotations from pathlib import Path -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional import warnings from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.compiler import Compiler from fab.tools.flags import ProfileFlags -from fab.tools.tool import CompilerSuiteTool +from fab.tools.compiler_suite_tool import CompilerSuiteTool class Linker(CompilerSuiteTool): @@ -114,7 +114,7 @@ def define_profile(self, self._pre_lib_flags.define_profile(name, inherit_from) self._post_lib_flags.define_profile(name, inherit_from) - def get_profile_flags(self, profile: str) -> List[str]: + def get_profile_flags(self, config: "BuildConfig") -> List[str]: ''' :returns: the ProfileFlags for the given profile, combined from the wrapped compiler and this wrapper. @@ -122,10 +122,13 @@ def get_profile_flags(self, profile: str) -> List[str]: :param profile: the profile to use. ''' if self._linker: - flags = self._linker.get_profile_flags(profile)[:] + flags = self._linker.get_profile_flags(config)[:] else: flags = [] - return flags + self._compiler.get_flags(profile) + # A compiler can have path specific flags. Since we are only + # interested in linking here, path-specific flags are not + # needed, so we provide a dummy file-path. + return flags + self._compiler.get_flags(config, Path("")) def get_lib_flags(self, lib: str) -> List[str]: '''Gets the standard flags for a standard library @@ -191,7 +194,7 @@ def get_pre_link_flags(self, config: "BuildConfig") -> List[str]: ''' params: List[str] = [] if self._pre_lib_flags: - params.extend(self._pre_lib_flags[config.profile]) + params.extend(self._pre_lib_flags.get_flags(config)) 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 @@ -216,7 +219,7 @@ def get_post_link_flags(self, config: "BuildConfig") -> List[str]: # wrapped linker). params.extend(self._linker.get_post_link_flags(config)) if self._post_lib_flags: - params.extend(self._post_lib_flags[config.profile]) + params.extend(self._post_lib_flags.get_flags(config)) return params def link(self, input_files: List[Path], output_file: Path, @@ -235,9 +238,9 @@ def link(self, input_files: List[Path], output_file: Path, :returns: the stdout of the link command ''' - params: List[Union[str, Path]] = [] + params: List[str] = self.get_flags(config) - params.extend(self._compiler.get_flags(config.profile)) + params.extend(self._compiler.get_flags(config, Path())) if config.openmp: params.append(self._compiler.openmp_flag) diff --git a/source/fab/tools/preprocessor.py b/source/fab/tools/preprocessor.py index d93c2688..2e75d3f1 100644 --- a/source/fab/tools/preprocessor.py +++ b/source/fab/tools/preprocessor.py @@ -10,13 +10,13 @@ """ from pathlib import Path -from typing import List, Optional, Union +from typing import List, Optional, Sequence, Union from fab.tools.category import Category -from fab.tools.tool import Tool +from fab.tools.tool_with_flags import ToolWithFlags -class Preprocessor(Tool): +class Preprocessor(ToolWithFlags): '''This is the base class for any preprocessor. :param name: the name of the preprocessor. @@ -32,7 +32,7 @@ def __init__(self, name: str, exec_name: Union[str, Path], self._version = None def preprocess(self, input_file: Path, output_file: Path, - add_flags: Union[None, List[Union[Path, str]]] = None): + add_flags: Optional[Sequence[Union[Path, str]]] = None): '''Calls the preprocessor to process the specified input file, creating the requested output file. @@ -43,7 +43,7 @@ def preprocess(self, input_file: Path, output_file: Path, params: List[Union[str, Path]] = [] if add_flags: # Make a copy to avoid modifying the caller's list - params = add_flags[:] + params = list(add_flags) # Input and output files come as the last two parameters params.extend([input_file, output_file]) @@ -66,6 +66,22 @@ def __init__(self): super().__init__("cpp", "cpp", Category.FORTRAN_PREPROCESSOR) self.add_flags(["-traditional-cpp", "-P"]) + def preprocess(self, input_file: Path, output_file: Path, + add_flags: Optional[Sequence[Union[Path, str]]] = None): + '''Calls the preprocessor to process the specified input file, + creating the requested output file. + + :param input_file: input file. + :param output_file: the output filename. + :param add_flags: List with additional flags to be used. + ''' + params: List[Union[str, Path]] = ["-traditional-cpp", "-P"] + + if add_flags: + params.extend(add_flags) + + super().preprocess(input_file, output_file, params) + # ============================================================================ class Fpp(Preprocessor): diff --git a/source/fab/tools/tool.py b/source/fab/tools/tool.py index a12773cc..9272b988 100644 --- a/source/fab/tools/tool.py +++ b/source/fab/tools/tool.py @@ -19,7 +19,6 @@ from typing import Dict, List, Optional, Sequence, Union from fab.tools.category import Category -from fab.tools.flags import ProfileFlags class Tool: @@ -40,7 +39,6 @@ def __init__(self, name: str, exec_name: Union[str, Path], self._logger = logging.getLogger(__name__) self._name = name self._exec_path = Path(exec_name) - self._flags = ProfileFlags() self._category = category if availability_option: self._availability_option = availability_option @@ -121,31 +119,6 @@ def category(self) -> Category: ''':returns: the category of this tool.''' return self._category - def get_flags(self, profile: Optional[str] = None): - ''':returns: the flags to be used with this tool.''' - return self._flags[profile] - - def add_flags(self, new_flags: Union[str, List[str]], - profile: Optional[str] = None): - '''Adds the specified flags to the list of flags. - - :param new_flags: A single string or list of strings which are the - flags to be added. - ''' - self._flags.add_flags(new_flags, profile) - - def define_profile(self, - name: str, - inherit_from: Optional[str] = None): - '''Defines a new profile name, and allows to specify if this new - profile inherit settings from an existing profile. - - :param name: Name of the profile to define. - :param inherit_from: Optional name of a profile to inherit - settings from. - ''' - self._flags.define_profile(name, inherit_from) - @property def logger(self) -> logging.Logger: ''':returns: a logger object for convenience.''' @@ -159,7 +132,6 @@ def __str__(self): def run(self, additional_parameters: Optional[ Union[str, Sequence[Union[Path, str]]]] = None, - profile: Optional[str] = None, env: Optional[Dict[str, str]] = None, cwd: Optional[Union[Path, str]] = None, capture_output=True) -> str: @@ -173,6 +145,9 @@ def run(self, :param env: Optional env for the command. By default it will use the current session's environment. + :param cwd: + Optional working directory for the command. By default it will + use the current working directory. :param capture_output: If True, capture and return stdout. If False, the command will print its output directly to the console. @@ -180,7 +155,7 @@ def run(self, :raises RuntimeError: if the code is not available. :raises RuntimeError: if the return code of the executable is not 0. """ - command = [str(self.exec_path)] + self.get_flags(profile) + command = [str(self.exec_path)] if additional_parameters: if isinstance(additional_parameters, str): command.append(additional_parameters) @@ -213,28 +188,3 @@ def run(self, if capture_output: return res.stdout.decode() return "" - - -class CompilerSuiteTool(Tool): - '''A tool that is part of a compiler suite (typically compiler - and linker). - - :param name: name of the tool. - :param exec_name: name of the executable to start. - :param suite: name of the compiler suite. - :param category: the Category to which this tool belongs. - :param availability_option: a command line option for the tool to test - if the tool is available on the current system. Defaults to - `--version`. - ''' - def __init__(self, name: str, exec_name: Union[str, Path], suite: str, - category: Category, - availability_option: Optional[Union[str, List[str]]] = None): - super().__init__(name, exec_name, category, - availability_option=availability_option) - self._suite = suite - - @property - def suite(self) -> str: - ''':returns: the compiler suite of this tool.''' - return self._suite diff --git a/source/fab/tools/tool_with_flags.py b/source/fab/tools/tool_with_flags.py new file mode 100644 index 00000000..673716d5 --- /dev/null +++ b/source/fab/tools/tool_with_flags.py @@ -0,0 +1,79 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +"""This file contains the base class for a tool with flags. +It is the base class for compiler, linker, and pre-processor. + +""" + +from pathlib import Path +from typing import List, Optional, TYPE_CHECKING, Union + +from fab.tools.category import Category +from fab.tools.flags import AbstractFlags, ProfileFlags +from fab.tools.tool import Tool + +if TYPE_CHECKING: + from fab.build_config import BuildConfig + + +class ToolWithFlags(Tool): + '''This is the base class for all tools that provide flags. + Note that the run method of the Tool base class is not overwritten + to provide the flags to the base class, that needs to be done + by the individual derived tools. + + :param name: name of the tool. + :param exec_name: name or full path of the executable to start. + :param category: the Category to which this tool belongs. + :param availability_option: a command line option for the tool to test + if the tool is available on the current system. Defaults to + `--version`. + ''' + + def __init__( + self, + name: str, + exec_name: Union[str, Path], + category: Category = Category.MISC, + availability_option: Optional[Union[str, + List[str]]] = None) -> None: + + super().__init__(name, exec_name, category, availability_option) + self._flags = ProfileFlags() + + @property + def flags(self) -> ProfileFlags: + ''':returns: the profile flags for this tool.''' + return self._flags + + def get_flags(self, + config: Optional["BuildConfig"] = None, + file_path: Optional[Path] = None) -> List[str]: + ''':returns: the flags to be used with this tool.''' + return self.flags.get_flags(config, file_path) + + def add_flags(self, + new_flags: Union[AbstractFlags, str, List[str]], + profile: Optional[str] = None): + '''Adds the specified flags to the list of flags. + + :param new_flags: A single string or list of strings which are the + flags to be added. + ''' + self._flags.add_flags(new_flags, profile) + + def define_profile(self, + name: str, + inherit_from: Optional[str] = None): + '''Defines a new profile name, and allows to specify if this new + profile inherit settings from an existing profile. + + :param name: Name of the profile to define. + :param inherit_from: Optional name of a profile to inherit + settings from. + ''' + self._flags.define_profile(name, inherit_from) diff --git a/source/fab/tools/versioning.py b/source/fab/tools/versioning.py index 410cc250..7e602f4c 100644 --- a/source/fab/tools/versioning.py +++ b/source/fab/tools/versioning.py @@ -45,7 +45,7 @@ def __init__(self): def current_commit(self, folder: Optional[Union[Path, str]] = None) -> str: ''':returns: the hash of the current commit. - :param folder: the folder for which to determine the current commitf + :param folder: the folder for which to determine the current commit (defaults to .). ''' folder = folder or '.' diff --git a/source/fab/util.py b/source/fab/util.py index fb9f60ad..4cedef1e 100644 --- a/source/fab/util.py +++ b/source/fab/util.py @@ -63,7 +63,7 @@ def file_checksum(fpath): return HashedFile(fpath, zlib.crc32(infile.read())) -def string_checksum(s: str): +def string_checksum(s: str) -> int: """ Return a checksum of the given string. diff --git a/tests/unit_tests/fab_base/site_specific/default/config.py b/tests/unit_tests/fab_base/site_specific/default/config.py index b5906ffd..f48e5a4d 100644 --- a/tests/unit_tests/fab_base/site_specific/default/config.py +++ b/tests/unit_tests/fab_base/site_specific/default/config.py @@ -8,7 +8,7 @@ import argparse from typing import List -from fab.build_config import AddFlags, BuildConfig +from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.tool_repository import ToolRepository @@ -84,11 +84,3 @@ def handle_command_line_options(self, args: argparse.Namespace) -> None: # Keep a copy of the args, so they can be used when # initialising compilers self._args = args - - def get_path_flags(self, build_config: BuildConfig) -> List[AddFlags]: - ''' - Returns the path-specific flags to be used. - TODO #313: Ideally we have only one kind of flag, but as a quick - work around we provide this method. - ''' - return [] diff --git a/tests/unit_tests/steps/test_compile_c.py b/tests/unit_tests/steps/test_compile_c.py index 685f4319..9729bd00 100644 --- a/tests/unit_tests/steps/test_compile_c.py +++ b/tests/unit_tests/steps/test_compile_c.py @@ -17,7 +17,7 @@ from fab.parse.c import AnalysedC from fab.steps.compile_c import _get_obj_combo_hash, _compile_file, compile_c from fab.tools.category import Category -from fab.tools.flags import Flags +from fab.tools.flags import ProfileFlags from fab.tools.tool_box import ToolBox @@ -81,7 +81,7 @@ def test_vanilla(self, content, fake_process.register([ 'scc', '-c', '-I', 'foo/include', '-Dhello', 'foo.c', - '-o', str(config.prebuild_folder / 'foo.18f203cab.o') + '-o', str(config.prebuild_folder / 'foo.17ecdc5e7.o') ]) with warns(UserWarning, match="_metric_send_conn not set, " "cannot send metrics"): @@ -92,7 +92,7 @@ def test_vanilla(self, content, # ensure it created the correct artefact collection assert config.artefact_store[ArtefactSet.OBJECT_FILES] == { - None: {config.prebuild_folder / 'foo.18f203cab.o', } + None: {config.prebuild_folder / 'foo.17ecdc5e7.o', } } def test_exception_handling(self, content, @@ -105,7 +105,7 @@ def test_exception_handling(self, content, fake_process.register(['scc', '--version'], stdout='1.2.3') fake_process.register([ 'scc', '-c', 'foo.c', - '-o', str(config.build_output / '_prebuild/foo.101865856.o') + '-o', str(config.build_output / '_prebuild/foo.f133e192.o') ], returncode=1) with raises(RuntimeError): compile_c(config=config) @@ -117,7 +117,9 @@ class TestGetObjComboHash: @fixture(scope='function') def flags(self): '''Returns the flag for these tests.''' - return Flags(['-Denv_flag', '-I', 'foo/include', '-Dhello']) + pf = ProfileFlags() + pf.add_flags(['-Denv_flag', '-I', 'foo/include', '-Dhello']) + return pf def test_vanilla(self, content, flags, fake_process: FakeProcess) -> None: """ @@ -131,7 +133,7 @@ def test_vanilla(self, content, flags, fake_process: FakeProcess) -> None: # ToDo: Messing with "private" members. # result = _get_obj_combo_hash(config, compiler, analysed_file, flags) - assert result == 5289295574 + assert result == 5015455762 def test_change_file(self, content, flags, fake_process: FakeProcess) -> None: @@ -147,7 +149,7 @@ def test_change_file(self, content, flags, # analysed_file._file_hash += 1 result = _get_obj_combo_hash(config, compiler, analysed_file, flags) - assert result == 5289295575 + assert result == 5015455763 def test_change_flags(self, content, flags, fake_process: FakeProcess) -> None: @@ -158,7 +160,7 @@ def test_change_flags(self, content, flags, fake_process.register(['scc', '--version'], stdout='1.2.3') compiler = config.tool_box.get_tool(Category.C_COMPILER) - flags = Flags(['-Dfoo'] + flags) + flags = ProfileFlags(['-Dfoo'] + flags.get_flags(), config.profile) result = _get_obj_combo_hash(config, compiler, analysed_file, flags) assert result != 5066163117 diff --git a/tests/unit_tests/steps/test_compile_fortran.py b/tests/unit_tests/steps/test_compile_fortran.py index 9e86cb79..6fc6e98a 100644 --- a/tests/unit_tests/steps/test_compile_fortran.py +++ b/tests/unit_tests/steps/test_compile_fortran.py @@ -7,7 +7,7 @@ from pytest_subprocess.fake_process import FakeProcess from fab.artefacts import ArtefactSet, ArtefactStore -from fab.build_config import BuildConfig, FlagsConfig +from fab.build_config import BuildConfig from fab.parse.fortran import AnalysedFortran from fab.steps.compile_fortran import ( compile_pass, get_compile_next, @@ -15,6 +15,7 @@ store_artefacts ) from fab.tools.category import Category +from fab.tools.flags import FlagList from fab.tools.tool_box import ToolBox from fab.util import CompiledFile @@ -97,7 +98,7 @@ def test_vanilla(self, analysed_files, stub_tool_box: ToolBox, config = BuildConfig('proj', stub_tool_box, fab_workspace=tmp_path) mp_common_args = MpCommonArgs(config, - FlagsConfig(), + FlagList(), {}, syntax_only=True) uncompiled_result = compile_pass(config=config, @@ -172,8 +173,7 @@ def test_vanilla(self): @fixture(scope='function') def content(stub_tool_box, fs: FakeFilesystem): flags = ['flag1', 'flag2'] - flags_config = Mock() - flags_config.flags_for_path.return_value = flags + flag_list = FlagList(flags) analysed_file = AnalysedFortran(fpath=Path('foofile'), file_hash=34567) analysed_file.add_module_dep('mod_dep_1') @@ -183,7 +183,7 @@ def content(stub_tool_box, fs: FakeFilesystem): mp_common_args = MpCommonArgs( config=BuildConfig('proj', stub_tool_box, fab_workspace=Path('/fab')), - flags=flags_config, + flag_list=flag_list, mod_hashes={'mod_dep_1': 12345, 'mod_dep_2': 23456}, syntax_only=False, ) @@ -212,13 +212,13 @@ def test_without_prebuild(self, content, res, artefacts = process_file((analysed_file, mp_common_args)) expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o' + '/fab/proj/build_output/_prebuild/foofile.106dc4755.o' ) assert res == CompiledFile(input_fpath=analysed_file.fpath, output_fpath=expect_object_fpath) assert [call.args for call in record.calls] == [ ['sfc', '-c', 'flag1', 'flag2', 'foofile', - '-o', '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o'] + '-o', '/fab/proj/build_output/_prebuild/foofile.106dc4755.o'] ] # check the correct artefacts were generated. @@ -228,16 +228,16 @@ def test_without_prebuild(self, content, pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.1ff6e93b2.o', - pb / 'mod_def_2.188dd00a8.mod', - pb / 'mod_def_1.188dd00a8.mod' + pb / 'foofile.106dc4755.o', + pb / 'mod_def_2.904ab44b.mod', + pb / 'mod_def_1.904ab44b.mod' } assert Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_1.904ab44b.mod' ).read_text() == "First module" assert Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_2.904ab44b.mod' ).read_text() == "Second module" def test_with_prebuild(self, content, @@ -251,17 +251,20 @@ def test_with_prebuild(self, content, fake_process.register(['sfc', '--version'], stdout='1.2.3') record = fake_process.register(['sfc', fake_process.any()]) + hash_foofile = "106dc4755" + hash_mod = "904ab44b" + Path('/fab/proj/build_output/_prebuild').mkdir(parents=True) Path('/fab/proj/build_output/mod_def_1.mod').write_text("First module") Path('/fab/proj/build_output/mod_def_2.mod').write_text("Second module") Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a8.mod' + f'/fab/proj/build_output/_prebuild/mod_def_1.{hash_mod}.mod' ).write_text("First module") Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' + f'/fab/proj/build_output/_prebuild/mod_def_2.{hash_mod}.mod' ).write_text("Second module") Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o' + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o' ).write_text("Object file") with warns(UserWarning, @@ -269,7 +272,7 @@ def test_with_prebuild(self, content, res, artefacts = process_file((analysed_file, mp_common_args)) expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o' + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o' ) # check the correct artefacts were generated. @@ -279,9 +282,9 @@ def test_with_prebuild(self, content, pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.1ff6e93b2.o', - pb / 'mod_def_2.188dd00a8.mod', - pb / 'mod_def_1.188dd00a8.mod' + pb / f'foofile.{hash_foofile}.o', + pb / f'mod_def_2.{hash_mod}.mod', + pb / f'mod_def_1.{hash_mod}.mod' } assert [call.args for call in record.calls] == [] @@ -289,10 +292,10 @@ def test_with_prebuild(self, content, output_fpath=expect_object_fpath) assert Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a8.mod' + f'/fab/proj/build_output/_prebuild/mod_def_1.{hash_mod}.mod' ).read_text() == "First module" assert Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' + f'/fab/proj/build_output/_prebuild/mod_def_2.{hash_mod}.mod' ).read_text() == "Second module" def test_file_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) -> None: @@ -320,14 +323,16 @@ def test_file_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) match="_metric_send_conn not set, cannot send metrics"): res, artefacts = process_file((analysed_file, mp_common_args)) + hash_foofile = "106dc4756" + hash_mod = "904ab44c" expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b3.o' + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o' ) assert res == CompiledFile(input_fpath=analysed_file.fpath, output_fpath=expect_object_fpath) assert [call.args for call in record.calls] == [ - ['sfc', '-c', 'flag1', 'flag2', 'foofile', - '-o', '/fab/proj/build_output/_prebuild/foofile.1ff6e93b3.o'] + ['sfc', '-c', 'flag1', 'flag2', 'foofile', '-o', + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o'] ] # check the correct artefacts were generated. @@ -337,27 +342,31 @@ def test_file_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.1ff6e93b3.o', - pb / 'mod_def_2.188dd00a9.mod', - pb / 'mod_def_1.188dd00a9.mod' + pb / f'foofile.{hash_foofile}.o', + pb / f'mod_def_2.{hash_mod}.mod', + pb / f'mod_def_1.{hash_mod}.mod' } assert Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a9.mod' + f'/fab/proj/build_output/_prebuild/mod_def_1.{hash_mod}.mod' ).read_text() == "First module" assert Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a9.mod' + f'/fab/proj/build_output/_prebuild/mod_def_2.{hash_mod}.mod' ).read_text() == "Second module" - def test_flags_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) -> None: + def test_flags_hash(self, + content, + fs: FakeFilesystem, + fake_process: FakeProcess) -> None: """ Tests changing compiler arguments changes generated object and module hashes. Not source modules. """ - mp_common_args, flags, analysed_file = content + mp_common_args, _, analysed_file = content + # Change the flags in mp_common_args flags = ['flag1', 'flag3'] - mp_common_args.flags.flags_for_path.return_value = flags + mp_common_args.flag_list[0]._flags = flags fake_process.register(['sfc', '--version'], stdout='1.2.3') record = fake_process.register(['sfc', fake_process.any()]) @@ -376,14 +385,16 @@ def test_flags_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess match="_metric_send_conn not set, cannot send metrics"): res, artefacts = process_file((analysed_file, mp_common_args)) + hash_foofile = "1079ead2a" + hash_mod = "904ab44b" expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.20030f987.o' + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o' ) assert res == CompiledFile(input_fpath=analysed_file.fpath, output_fpath=expect_object_fpath) assert [call.args for call in record.calls] == [ - ['sfc', '-c', 'flag1', 'flag3', 'foofile', - '-o', '/fab/proj/build_output/_prebuild/foofile.20030f987.o'] + ['sfc', '-c', 'flag1', 'flag3', 'foofile', '-o', + f'/fab/proj/build_output/_prebuild/foofile.{hash_foofile}.o'] ] # check the correct artefacts were generated. @@ -394,9 +405,9 @@ def test_flags_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.20030f987.o', - pb / 'mod_def_2.188dd00a8.mod', - pb / 'mod_def_1.188dd00a8.mod' + pb / f'foofile.{hash_foofile}.o', + pb / f'mod_def_2.{hash_mod}.mod', + pb / f'mod_def_1.{hash_mod}.mod' } assert Path( @@ -412,7 +423,7 @@ def test_deps_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) The generated object hash should change but the generated module hashes should not. """ - mp_common_args, flags, analysed_file = content + mp_common_args, _, analysed_file = content mp_common_args.mod_hashes['mod_dep_1'] += 1 @@ -434,13 +445,17 @@ def test_deps_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) res, artefacts = process_file((analysed_file, mp_common_args)) expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b3.o' + '/fab/proj/build_output/_prebuild/foofile.106dc4756.o' ) + print("XX", res) + print("YY", analysed_file.fpath, expect_object_fpath) + # XX CompiledFile(foofile, /fab/proj/build_output/_prebuild/foofile.106dc4756.o) + # YY foofile /fab/proj/build_output/_prebuild/foofile.1ff6e93b3.o assert res == CompiledFile(input_fpath=analysed_file.fpath, output_fpath=expect_object_fpath) assert [call.args for call in record.calls] == [ ['sfc', '-c', 'flag1', 'flag2', 'foofile', - '-o', '/fab/proj/build_output/_prebuild/foofile.1ff6e93b3.o'] + '-o', '/fab/proj/build_output/_prebuild/foofile.106dc4756.o'] ] # check the correct artefacts were created. @@ -451,23 +466,23 @@ def test_deps_hash(self, content, fs: FakeFilesystem, fake_process: FakeProcess) pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.1ff6e93b3.o', - pb / 'mod_def_2.188dd00a8.mod', - pb / 'mod_def_1.188dd00a8.mod' + pb / 'foofile.106dc4756.o', + pb / 'mod_def_2.904ab44b.mod', + pb / 'mod_def_1.904ab44b.mod' } assert Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_1.904ab44b.mod' ).read_text() == "First module" assert Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_2.904ab44b.mod' ).read_text() == "Second module" def test_mod_missing(self, content, fs: FakeFilesystem, fake_process: FakeProcess) -> None: """ Tests compilation on missing module. """ - mp_common_args, flags, analysed_file = content + mp_common_args, _, analysed_file = content fake_process.register(['sfc', '--version'], stdout='1.2.3') record = fake_process.register(['sfc', fake_process.any()]) @@ -479,7 +494,7 @@ def test_mod_missing(self, content, fs: FakeFilesystem, fake_process: FakeProces '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' ).write_text("Second module") Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o' + '/fab/proj/build_output/_prebuild/foofile.106dc4755.o' ).write_text("Object file") with warns(UserWarning, @@ -487,13 +502,13 @@ def test_mod_missing(self, content, fs: FakeFilesystem, fake_process: FakeProces res, artefacts = process_file((analysed_file, mp_common_args)) expect_object_fpath = Path( - '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o' + '/fab/proj/build_output/_prebuild/foofile.106dc4755.o' ) assert res == CompiledFile(input_fpath=analysed_file.fpath, output_fpath=expect_object_fpath) assert [call.args for call in record.calls] == [ ['sfc', '-c', 'flag1', 'flag2', 'foofile', - '-o', '/fab/proj/build_output/_prebuild/foofile.1ff6e93b2.o'] + '-o', '/fab/proj/build_output/_prebuild/foofile.106dc4755.o'] ] # check the correct artefacts were created. @@ -504,21 +519,21 @@ def test_mod_missing(self, content, fs: FakeFilesystem, fake_process: FakeProces pb = mp_common_args.config.prebuild_folder assert artefacts is not None assert set(artefacts) == { - pb / 'foofile.1ff6e93b2.o', - pb / 'mod_def_2.188dd00a8.mod', - pb / 'mod_def_1.188dd00a8.mod' + pb / 'foofile.106dc4755.o', + pb / 'mod_def_2.904ab44b.mod', + pb / 'mod_def_1.904ab44b.mod' } assert Path( - '/fab/proj/build_output/_prebuild/mod_def_1.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_1.904ab44b.mod' ).read_text() == "First module" assert Path( - '/fab/proj/build_output/_prebuild/mod_def_2.188dd00a8.mod' + '/fab/proj/build_output/_prebuild/mod_def_2.904ab44b.mod' ).read_text() == "Second module" @mark.parametrize(['version', 'mod_hash', 'obj_hash'], [ - ('1.2.3', '188dd00a8', '1ff6e93b2'), - ('9.8.7', '1b26306b2', '228f499bc') + ('1.2.3', '904ab44b', '106dc4755'), + ('9.8.7', 'ee1d5a65', '164aeed6f') ]) def test_obj_missing(self, content, version, mod_hash, obj_hash, fs: FakeFilesystem, fake_process: FakeProcess) -> None: @@ -526,7 +541,7 @@ def test_obj_missing(self, content, version, mod_hash, obj_hash, Tests compilation of missing object. Also tests that different compiler version numbers lead to different hashes. """ - mp_common_args, flags, analysed_file = content + mp_common_args, _, analysed_file = content fake_process.register(['sfc', '--version'], stdout=version) record = fake_process.register(['sfc', fake_process.any()]) diff --git a/tests/unit_tests/steps/test_preprocess.py b/tests/unit_tests/steps/test_preprocess.py index b6016d65..107b65e5 100644 --- a/tests/unit_tests/steps/test_preprocess.py +++ b/tests/unit_tests/steps/test_preprocess.py @@ -20,61 +20,57 @@ from fab.tools.tool_repository import ToolRepository -class Test_preprocess_fortran: +def test_big_little(tmp_path: Path, + stub_tool_repository: ToolRepository, + fake_process: FakeProcess) -> None: + """ + Tests big F90 files are preprocessed and little f90 files are copied. + """ + version_command = ['cpp', '--version'] + fake_process.register(version_command, stdout='1.2.3') + process_command = ['cpp', '-traditional-cpp', '-P', + str(tmp_path / 'proj/source/big.F90'), + str(tmp_path / 'proj/build_output/big.f90')] + fake_process.register(process_command) + config = BuildConfig('proj', ToolBox(), fab_workspace=tmp_path, + multiprocessing=False) + config.source_root.mkdir(parents=True) + big_f90 = Path(config.source_root / 'big.F90') + big_f90.write_text("Big F90 file.", encoding="utf-8") + little_f90 = Path(config.source_root / 'little.f90') + little_f90.write_text("Little f90 file.", encoding="utf-8") - def test_big_little(self, tmp_path: Path, - stub_tool_repository: ToolRepository, - fake_process: FakeProcess) -> None: - """ - Tests big F90 files are preprocessed and little f90 files are copied. - """ - version_command = ['cpp', '-traditional-cpp', '-P', '--version'] - fake_process.register(version_command, stdout='1.2.3') - process_command = ['cpp', '-traditional-cpp', '-P', - str(tmp_path / 'proj/source/big.F90'), - str(tmp_path / 'proj/build_output/big.f90')] - fake_process.register(process_command) + def source_getter(_: ArtefactStore) -> list[Path]: + return [big_f90, little_f90] - config = BuildConfig('proj', ToolBox(), fab_workspace=tmp_path, - multiprocessing=False) - config.source_root.mkdir(parents=True) - big_f90 = Path(config.source_root / 'big.F90') - big_f90.write_text("Big F90 file.") - little_f90 = Path(config.source_root / 'little.f90') - little_f90.write_text("Little f90 file.") + with warns(UserWarning, + match="_metric_send_conn not set, cannot send metrics"): + preprocess_fortran(config=config, source=source_getter) + assert (config.build_output / 'little.f90').read_text() \ + == "Little f90 file." + assert fake_process.call_count(process_command) == 1 - def source_getter(artefact_store: ArtefactStore) -> list[Path]: - return [big_f90, little_f90] - with warns(UserWarning, - match="_metric_send_conn not set, cannot send metrics"): - preprocess_fortran(config=config, source=source_getter) +def test_wrong_exe(stub_tool_repository: ToolRepository, + tmp_path: Path, + fake_process: FakeProcess) -> None: + """ + Tests detection of wrong executable. - assert (config.build_output / 'little.f90').read_text() \ - == "Little f90 file." - assert fake_process.call_count(process_command) == 1 + ToDo: Can this ever happen? Don't like messing with "private" state. + """ + fake_process.register(['cpp', '--version']) + tool_box = ToolBox() + # Take the C preprocessor + cpp = tool_box.get_tool(Category.C_PREPROCESSOR) + # And set its category to FORTRAN_PREPROCESSOR + cpp._category = Category.FORTRAN_PREPROCESSOR + # Now overwrite the Fortran preprocessor with the re-categorised + # C preprocessor: + tool_box.add_tool(cpp, silent_replace=True) - def test_wrong_exe(self, stub_tool_repository: ToolRepository, - tmp_path: Path, - fake_process: FakeProcess) -> None: - """ - Tests detection of wrong executable. - - ToDo: Can this ever happen? Don't like messing with "private" state. - """ - fake_process.register(['cpp', '-traditional-cpp', '-P', '--version']) - fake_process.register(['cpp', '--version']) - tool_box = ToolBox() - # Take the C preprocessor - cpp = tool_box.get_tool(Category.C_PREPROCESSOR) - # And set its category to FORTRAN_PREPROCESSOR - cpp._category = Category.FORTRAN_PREPROCESSOR - # Now overwrite the Fortran preprocessor with the re-categorised - # C preprocessor: - tool_box.add_tool(cpp, silent_replace=True) - - config = BuildConfig('proj', tool_box, fab_workspace=tmp_path) - with raises(RuntimeError) as err: - preprocess_fortran(config=config) - assert str(err.value) == "Unexpected tool 'cpp' of type '' instead of CppFortran" + config = BuildConfig('proj', tool_box, fab_workspace=tmp_path) + with raises(RuntimeError) as err: + preprocess_fortran(config=config) + assert str(err.value) == "Unexpected tool 'cpp' of type '' instead of CppFortran" diff --git a/tests/unit_tests/test_build_config.py b/tests/unit_tests/test_build_config.py index 0ba1a64d..86501192 100644 --- a/tests/unit_tests/test_build_config.py +++ b/tests/unit_tests/test_build_config.py @@ -87,3 +87,16 @@ def test_fab_workspace_with_env( some_dir = Path('/some_dir') config = BuildConfig('proj', ToolBox(), fab_workspace=some_dir) assert config.project_workspace == some_dir / 'proj' + + +def test_set_profile(stub_configuration): + """ + Tests that setting compilation profiles work as expected. + """ + + stub_configuration.set_profile("fast-debug") + assert stub_configuration.profile == "fast-debug" + + # All profiles names are in lower case + stub_configuration.set_profile("fast-DEBUG") + assert stub_configuration.profile == "fast-debug" diff --git a/tests/unit_tests/tools/test_ar.py b/tests/unit_tests/tools/test_ar.py index 3e9dc5f3..0133b3be 100644 --- a/tests/unit_tests/tools/test_ar.py +++ b/tests/unit_tests/tools/test_ar.py @@ -10,10 +10,9 @@ from pytest_subprocess.fake_process import FakeProcess -from tests.conftest import ExtendedRecorder, call_list - -from fab.tools.category import Category from fab.tools.ar import Ar +from fab.tools.category import Category +from tests.conftest import ExtendedRecorder, call_list def test_constructor() -> None: @@ -24,7 +23,6 @@ def test_constructor() -> None: assert ar.category == Category.AR assert ar.name == "ar" assert ar.exec_name == "ar" - assert ar.get_flags() == [] def test_check_available(subproc_record: ExtendedRecorder) -> None: diff --git a/tests/unit_tests/tools/test_compiler.py b/tests/unit_tests/tools/test_compiler.py index 7ca10ca4..4eec8d01 100644 --- a/tests/unit_tests/tools/test_compiler.py +++ b/tests/unit_tests/tools/test_compiler.py @@ -9,6 +9,7 @@ from pathlib import Path from textwrap import dedent from unittest import mock +from zlib import crc32 from pytest import mark, raises, warns from pytest_subprocess.fake_process import FakeProcess @@ -21,6 +22,7 @@ Icc, Ifort, Icx, Ifx, Nvc, Nvfortran) +from fab.tools.flags import ContainFlags from tests.conftest import arg_list, call_list @@ -108,43 +110,74 @@ def test_compiler_check_available_runtime_error(): assert not cc.check_available() -def test_compiler_hash(): +def test_compiler_hash(stub_configuration): '''Test the hash functionality.''' cc = Gcc() with mock.patch.object(cc, "_version", (5, 6, 7)): - hash1 = cc.get_hash() - assert hash1 == 2991650113 + hash1 = cc.get_hash(stub_configuration, Path('.')) + assert hash1 == 804998173 # A change in the version number must change the hash: with mock.patch.object(cc, "_version", (8, 9)): - hash2 = cc.get_hash() + hash2 = cc.get_hash(stub_configuration, Path('.')) assert hash2 != hash1 # A change in the name must change the hash, again: cc._name = "new_name" - hash3 = cc.get_hash() + hash3 = cc.get_hash(stub_configuration, Path('.')) assert hash3 not in (hash1, hash2) -def test_compiler_hash_compiler_error(): +def test_compiler_path_specific_flags(stub_configuration, + stub_fortran_compiler): + """ + Tests that path-specific flags are used as expected. + """ + fc = stub_fortran_compiler + # Make sure we can get a version number for the stub compiler: + fc._version = (1, 2) + print(fc.name, fc.get_version()) + + contain_flag = ContainFlags(pattern="myfile", + flags=["-myflag"]) + fc.add_flags("-always-flag") + fc.add_flags(contain_flag) + + flags = fc.get_flags(stub_configuration, Path(".")) + assert flags == ["-always-flag"] + flags = fc.get_flags(stub_configuration, Path("/somewhere/myfile.F90")) + assert flags == ["-always-flag", "-myflag"] + + compiler_info = "some Fortran compiler1.2['-always-flag']" + hash_without = fc.get_hash(stub_configuration, Path('.')) + assert hash_without == crc32(compiler_info.encode()) + + compiler_info = "some Fortran compiler1.2['-always-flag', '-myflag']" + hash_with = fc.get_hash(stub_configuration, Path('/somewhere/myfile.F90')) + assert hash_with == crc32(compiler_info.encode()) + # Just to be certain they are indeed different + assert hash_with != hash_without + + +def test_compiler_hash_compiler_error(stub_configuration): '''Test the hash functionality when version info is missing.''' cc = Gcc() # raise an error when trying to get compiler version with mock.patch.object(cc, 'run', side_effect=RuntimeError()): with raises(RuntimeError) as err: - cc.get_hash() + cc.get_hash(stub_configuration, Path('.')) assert "Error asking for version of compiler" in str(err.value) -def test_compiler_hash_invalid_version(): +def test_compiler_hash_invalid_version(stub_configuration): '''Test the hash functionality when version info is missing.''' cc = Gcc() # returns an invalid compiler version string with mock.patch.object(cc, "run", mock.Mock(return_value='foo v1')): with raises(RuntimeError) as err: - cc.get_hash() + cc.get_hash(stub_configuration, Path('.')) assert ("Unexpected version output format for compiler 'gcc'" in str(err.value)) diff --git a/tests/unit_tests/tools/test_compiler_suite_tool.py b/tests/unit_tests/tools/test_compiler_suite_tool.py new file mode 100644 index 00000000..617b7ec1 --- /dev/null +++ b/tests/unit_tests/tools/test_compiler_suite_tool.py @@ -0,0 +1,29 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## +""" +Tests tooling base classes. +""" +import logging +from pathlib import Path + +from fab.tools.category import Category +from fab.tools.compiler_suite_tool import CompilerSuiteTool + + +def test_compiler_suite_toolconstructor() -> None: + """ + Tests construction from argument list. + """ + tool = CompilerSuiteTool(name="gfortran", exec_name="gfortran", + suite="gnu", category=Category.FORTRAN_COMPILER) + assert tool.suite == "gnu" + assert str(tool) == "CompilerSuiteTool - gfortran: gfortran" + assert tool.exec_name == "gfortran" + assert tool.exec_path == Path("gfortran") + assert tool.name == "gfortran" + assert tool.category == Category.FORTRAN_COMPILER + assert isinstance(tool.logger, logging.Logger) + assert tool.is_compiler diff --git a/tests/unit_tests/tools/test_compiler_wrapper.py b/tests/unit_tests/tools/test_compiler_wrapper.py index 0109d82c..1c1a0b99 100644 --- a/tests/unit_tests/tools/test_compiler_wrapper.py +++ b/tests/unit_tests/tools/test_compiler_wrapper.py @@ -11,8 +11,6 @@ from pytest import raises, warns from pytest_subprocess.fake_process import FakeProcess -from tests.conftest import ExtendedRecorder, call_list, not_found_callback - from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.compiler import CCompiler, FortranCompiler @@ -20,6 +18,8 @@ CrayCcWrapper, CrayFtnWrapper, Mpicc, Mpif90) +from tests.conftest import ExtendedRecorder, call_list, not_found_callback + def test_compiler_getter(stub_c_compiler: CCompiler) -> None: """ @@ -85,7 +85,8 @@ def test_compiler_is_available_no_version(stub_c_compiler: CCompiler, assert not mpicc.is_available -def test_compiler_hash(fake_process: FakeProcess) -> None: +def test_compiler_hash(stub_configuration, + fake_process: FakeProcess) -> None: """ Test the hash functionality. """ @@ -94,8 +95,8 @@ def test_compiler_hash(fake_process: FakeProcess) -> None: cc1 = CCompiler('test C compiler', 'tcc', 'test', version_regex=r'([\d.]+)') mpicc1 = Mpicc(cc1) - hash1 = mpicc1.get_hash() - assert hash1 == 5953380633 + hash1 = mpicc1.get_hash(stub_configuration, Path('.')) + assert hash1 == 2609406574 # A change in the version number must change the hash: fake_process.register(['tcc', '--version'], stdout='8.9') @@ -103,7 +104,7 @@ def test_compiler_hash(fake_process: FakeProcess) -> None: cc2 = CCompiler('test C compiler', 'tcc', 'test', version_regex=r'([\d.]+)') mpicc2 = Mpicc(cc2) - hash2 = mpicc2.get_hash() + hash2 = mpicc2.get_hash(stub_configuration, Path('.')) assert hash2 != hash1 # A change in the name with the original version number @@ -113,7 +114,7 @@ def test_compiler_hash(fake_process: FakeProcess) -> None: cc3 = CCompiler('New test C compiler', 'tcc', 'test', version_regex=r'([\d.]+)') mpicc3 = Mpicc(cc3) - hash3 = mpicc3.get_hash() + hash3 = mpicc3.get_hash(stub_configuration, Path('.')) assert hash3 not in (hash1, hash2) # A change in the name with the modified version number @@ -123,7 +124,7 @@ def test_compiler_hash(fake_process: FakeProcess) -> None: cc4 = CCompiler('New test C compiler', 'tcc', 'test', version_regex=r'([\d.]+)') mpicc4 = Mpicc(cc4) - hash4 = mpicc4.get_hash() + hash4 = mpicc4.get_hash(stub_configuration, Path('.')) assert hash4 not in (hash1, hash2, hash3) @@ -263,23 +264,43 @@ def test_flags_independent(stub_c_compiler: CCompiler, assert stub_c_compiler.get_flags() == [] assert wrapper.get_flags() == [] - stub_c_compiler.add_flags(["-a", "-b"]) + stub_c_compiler.add_flags(['-a', '-b']) assert stub_c_compiler.get_flags() == ["-a", "-b"] - assert wrapper.get_flags() == ['-a', '-b'] + resolved_flags = stub_c_compiler.get_all_commandline_options( + stub_configuration, Path('/in'), Path("/out")) + assert resolved_flags == ['-c', '-a', '-b', 'in', '-o', '/out'] + + assert wrapper.get_flags() == ["-a", "-b"] + + # We need to test `get_all_commandline_options` to check the correct + # behaviour of flags, which can resolve path-specific flags. + resolved_flags = wrapper.get_all_commandline_options(stub_configuration, + Path('/in'), + Path('/out')) + + assert resolved_flags == ['-c', '-a', '-b', 'in', '-o', '/out'] assert wrapper.openmp_flag == stub_c_compiler.openmp_flag # Adding flags to the wrapper should not affect the wrapped compiler: - wrapper.add_flags(["-d", "-e"]) - assert stub_c_compiler.get_flags() == ['-a', '-b'] + wrapper.add_flags(['-d', '-e']) + resolved_flags = stub_c_compiler.get_all_commandline_options( + stub_configuration, Path('/in'), Path("/out")) + assert resolved_flags == ['-c', '-a', '-b', 'in', '-o', '/out'] + # And the compiler wrapper should report the wrapped compiler's flag # followed by the wrapper flag (i.e. the wrapper flag can therefore # overwrite the wrapped compiler's flags) - assert wrapper.get_flags() == ['-a', '-b', "-d", "-e"] + assert wrapper.get_flags() == ["-a", "-b", "-d", "-e"] + resolved_flags = wrapper.get_all_commandline_options(stub_configuration, + Path("/in"), + Path("/out")) + assert resolved_flags == ['-c', '-a', '-b', '-d', '-e', + 'in', '-o', '/out'] wrapper.compile_file(Path("a.f90"), Path('a.o'), add_flags=['-f'], config=stub_configuration) assert subproc_record.invocations() == [ - ['mpicc', "-a", "-b", "-d", "-e", "-c", "-f", "a.f90", "-o", 'a.o'] + ['mpicc', '-c', '-a', '-b', '-d', '-e', '-f', 'a.f90', '-o', 'a.o'] ] assert subproc_record.extras()[0]['cwd'] == '.' @@ -290,26 +311,26 @@ def test_compiler_wrapper_flags_with_add_arg(stub_c_compiler: CCompiler, '''Tests that flags set in the base compiler will be accessed in the wrapper if also additional flags are specified.''' mpicc = Mpicc(stub_c_compiler) - stub_c_compiler.define_profile("default", inherit_from="") - mpicc.define_profile("default", inherit_from="") + stub_c_compiler.define_profile('default', inherit_from="") + mpicc.define_profile('default', inherit_from="") # Due to inheritance, this will give "-a -b" for gcc - stub_c_compiler.add_flags(["-a"]) - stub_c_compiler.add_flags(["-b"], "default") - # Due to inheritance, this will give "-d -e" for mpicc - mpicc.add_flags(["-d"]) - mpicc.add_flags(["-e"], "default") + stub_c_compiler.add_flags(['-a']) + stub_c_compiler.add_flags(['-b'], 'default') + # Due to inheritance, this will give '-d -e' for mpicc + mpicc.add_flags(['-d']) + mpicc.add_flags(['-e'], 'default') # Check that the flags are assembled in the right order in the # actual compiler call: first the wrapped compiler flag, then # the wrapper flag, then additional flags stub_configuration._openmp = True - stub_configuration._profile = "default" + stub_configuration._profile = 'default' - mpicc.compile_file(Path("a.f90"), Path("a.o"), add_flags=["-f"], + mpicc.compile_file(Path('a.f90'), Path('a.o'), add_flags=['-f'], config=stub_configuration) assert subproc_record.invocations() == [ - ['mpicc', "-a", "-b", "-d", "-e", "-c", "-omp", "-f", - "a.f90", "-o", 'a.o'] + ['mpicc', '-c', '-omp', '-a', '-b', '-d', '-e', '-f', + 'a.f90', '-o', 'a.o'] ] @@ -322,12 +343,12 @@ def test_args_without_add_arg(stub_c_compiler: CCompiler, """ wrapper = CompilerWrapper('wrapper', 'wrp', compiler=stub_c_compiler) - stub_c_compiler.add_flags(["-a", "-b"]) - wrapper.add_flags(["-d", "-e"]) + stub_c_compiler.add_flags(['-a', '-b']) + wrapper.add_flags(['-d', '-e']) - wrapper.compile_file(Path("a.f90"), Path('a.o'), config=stub_configuration) + wrapper.compile_file(Path('a.f90'), Path('a.o'), config=stub_configuration) assert subproc_record.invocations() == [ - ['wrp', "-a", "-b", "-d", "-e", "-c", "a.f90", "-o", 'a.o'] + ['wrp', '-c', '-a', '-b', '-d', '-e', 'a.f90', '-o', 'a.o'] ] assert subproc_record.extras()[0]['cwd'] == '.' diff --git a/tests/unit_tests/tools/test_flags.py b/tests/unit_tests/tools/test_flags.py index b123e0cd..87ddcf86 100644 --- a/tests/unit_tests/tools/test_flags.py +++ b/tests/unit_tests/tools/test_flags.py @@ -7,70 +7,159 @@ '''Tests the compiler implementation. ''' +from pathlib import Path import pytest -from fab.tools.flags import Flags, ProfileFlags +from fab.build_config import AddFlags +from fab.tools.flags import (AlwaysFlags, ContainFlags, FlagList, MatchFlags, + ProfileFlags) from fab.util import string_checksum -def test_flags_constructor(): +def test_always_flags(stub_configuration): + """ + Tests the various AbstractFlags constructors. + """ + + # Constructor: + af = AlwaysFlags() + assert af.get_flags() == [] + af = AlwaysFlags("-g") + assert af.get_flags() == ["-g"] + af = AlwaysFlags(["-g", "-O2"]) + assert af.get_flags() == ["-g", "-O2"] + + # Templating + af = AlwaysFlags(["$source", "$output"]) + assert (af.get_flags(stub_configuration) == + [str(stub_configuration.source_root), + str(stub_configuration.build_output)]) + af = AlwaysFlags(["$source", "$output", "$relative"]) + file_path = Path("/my/file") + assert (af.get_flags(stub_configuration, file_path) == + [str(stub_configuration.source_root), + str(stub_configuration.build_output), + "/my"]) + + +def test_always_flags_remove_flags(): + '''Test remove_flags functionality.''' + flags = AlwaysFlags() + flags.remove_flag("-c", False) + # pylint: disable-next=use-implicit-booleaness-not-comparison + assert flags.get_flags() == [] + + all_flags = ['a.f90', '-c', '-o', 'a.o', '-fsyntax-only', "-J", "/tmp"] + flags = AlwaysFlags(all_flags) + assert flags.get_flags() == all_flags + with pytest.warns(UserWarning, match="Removing managed flag"): + flags.remove_flag("-c") + del all_flags[1] + assert flags.get_flags() == all_flags + with pytest.warns(UserWarning, match="Removing managed flag"): + flags.remove_flag("-J", has_parameter=True) + del all_flags[-2:] + assert flags.get_flags() == all_flags + + for flags_in, expected in [(["-J", "b"], []), + (["-Jb"], []), + (["a", "-J", "c"], ["a"]), + (["a", "-Jc"], ["a"]), + (["a", "-J"], ["a"]), + ]: + flags = AlwaysFlags(flags_in) + with pytest.warns(UserWarning, match="Removing managed flag"): + flags.remove_flag("-J", has_parameter=True) + assert flags.get_flags() == expected + + +def test_match_flags() -> None: + """ + Tests matching using wildcards. + """ + mf = MatchFlags("/*", "-g") + assert mf.get_flags(file_path=Path(".")) == [] + mf = MatchFlags("/*", ["-g", "$relative"]) + file_path = Path("/my/dir") + assert mf.get_flags(file_path=file_path) == ["-g", "/my"] + + +def test_contain_flags() -> None: + """ + Tests matching using substrings. + """ + cf = ContainFlags(pattern="yes", flags="-g") + assert cf.get_flags(file_path=Path(".")) == [] + cf = ContainFlags("/", ["-g", "$relative"]) + file_path = Path("/my/dir") + assert cf.get_flags(file_path=file_path) == ["-g", "/my"] + + +def test_flag_list_constructor(): '''Tests the constructor of Flags.''' - f1 = Flags() + f1 = FlagList() assert isinstance(f1, list) # pylint: disable-next=use-implicit-booleaness-not-comparison assert f1 == [] - f2 = Flags(["a"]) + f2 = FlagList(["a"]) assert isinstance(f2, list) - assert f2 == ["a"] + assert f2.get_flags() == ["a"] def test_flags_adding(): '''Tests adding flags.''' - f1 = Flags() + f1 = FlagList() # pylint: disable-next=use-implicit-booleaness-not-comparison - assert f1 == [] + assert f1.get_flags() == [] f1.add_flags("-a") - assert f1 == ["-a"] + assert f1.get_flags() == ["-a"] f1.add_flags(["-b", "-c"]) - assert f1 == ["-a", "-b", "-c"] + assert len(f1) == 2 + assert f1.get_flags() == ["-a", "-b", "-c"] + assert len(f1) == 2 + assert f1[0].get_flags() == ["-a"] + assert f1[1].get_flags() == ["-b", "-c"] + + # Check functionality when adding a flag object: + af1 = AlwaysFlags("-g") + f1 = FlagList(af1) + assert f1 == [af1] + assert f1.get_flags() == ["-g"] + + af2 = AlwaysFlags(["-O2", "-warn"]) + f1.add_flags(af2) + assert f1 == [af1, af2] + assert f1.get_flags() == ["-g", "-O2", "-warn"] def test_remove_flags(): - '''Test remove_flags functionality.''' - flags = Flags() + '''Test remove_flags functionality. This is a subset of the remove + tests for AlwaysFlags, just to ensure that the calls are getting + forwarded from Flags to the AlwaysFlags implementation. + ''' + flags = FlagList() flags.remove_flag("-c", False) # pylint: disable-next=use-implicit-booleaness-not-comparison assert flags == [] all_flags = ['a.f90', '-c', '-o', 'a.o', '-fsyntax-only', "-J", "/tmp"] - flags = Flags(all_flags) - assert flags == all_flags + flags = FlagList(all_flags) + assert flags.get_flags() == all_flags with pytest.warns(UserWarning, match="Removing managed flag"): flags.remove_flag("-c") del all_flags[1] - assert flags == all_flags + assert flags.get_flags() == all_flags with pytest.warns(UserWarning, match="Removing managed flag"): flags.remove_flag("-J", has_parameter=True) del all_flags[-2:] - assert flags == all_flags - - for flags_in, expected in [(["-J", "b"], []), - (["-Jb"], []), - (["a", "-J", "c"], ["a"]), - (["a", "-Jc"], ["a"]), - (["a", "-J"], ["a"]), - ]: - flags = Flags(flags_in) - with pytest.warns(UserWarning, match="Removing managed flag"): - flags.remove_flag("-J", has_parameter=True) - assert flags == expected + assert flags.get_flags() == all_flags def test_flags_checksum(): '''Tests computation of the checksum.''' list_of_flags = ['one', 'two', 'three', 'four'] - flags = Flags(list_of_flags) + flags = FlagList(list_of_flags) assert flags.checksum() == string_checksum(str(list_of_flags)) @@ -80,9 +169,17 @@ def test_profile_flags_with_profile(): pf.define_profile("base") assert pf["base"] == [] pf.add_flags("-base", "base") - assert pf["base"] == ["-base"] + + assert len(pf["base"]) == 1 + assert isinstance(pf["base"][0], AlwaysFlags) + assert pf["base"][0].get_flags() == ["-base"] + pf.add_flags(["-base2", "-base3"], "base") - assert pf["base"] == ["-base", "-base2", "-base3"] + assert len(pf["base"]) == 2 + assert isinstance(pf["base"][0], AlwaysFlags) + assert isinstance(pf["base"][1], AlwaysFlags) + assert pf["base"][0].get_flags() == ["-base"] + assert pf["base"][1].get_flags() == ["-base2", "-base3"] # Check that we get an exception if we specify a profile # that does not exist @@ -91,14 +188,33 @@ def test_profile_flags_with_profile(): assert "Profile 'does_not_exist' is not defined" in str(err.value) +def test_profile_flags_constructor_args(): + '''Tests various constructor argument combinations.''' + pf = ProfileFlags("-g") + assert len(pf[""]) == 1 + assert isinstance(pf[""][0], AlwaysFlags) + assert pf[""][0].get_flags() == ["-g"] + + pf = ProfileFlags("-g", profile="prof") + assert pf[""] == [] + assert len(pf["prof"]) == 1 + assert isinstance(pf["prof"][0], AlwaysFlags) + assert pf["prof"][0].get_flags() == ["-g"] + + def test_profile_flags_without_profile(): '''Tests adding flags.''' pf = ProfileFlags() assert pf[""] == [] + assert pf[None] == [] pf.add_flags("-base") - assert pf[""] == ["-base"] + assert len(pf[""]) == 1 + assert isinstance(pf[""][0], AlwaysFlags) + assert pf[""][0].get_flags() == ["-base"] pf.add_flags(["-base2", "-base3"]) - assert pf[""] == ["-base", "-base2", "-base3"] + assert len(pf[""]) == 2 + assert pf[""][0].get_flags() == ["-base"] + assert pf[""][1].get_flags() == ["-base2", "-base3"] # Check that we get an exception if we specify a profile with pytest.raises(KeyError) as err: @@ -117,7 +233,7 @@ def test_profile_flags_without_profile(): assert pf._inherit_from["from_default"] == "" -def test_profile_flags_inheriting(): +def test_profile_flags_inheriting(stub_configuration): '''Tests adding flags.''' pf = ProfileFlags() pf.define_profile("base") @@ -126,21 +242,25 @@ def test_profile_flags_inheriting(): assert "base" not in pf._inherit_from pf.add_flags("-base", "base") - assert pf["base"] == ["-base"] + stub_configuration.set_profile("base") + assert pf.get_flags(stub_configuration) == ["-base"] pf.define_profile("derived", "base") - assert pf["derived"] == ["-base"] + stub_configuration.set_profile("derived") + assert pf.get_flags(stub_configuration) == ["-base"] assert pf._inherit_from["derived"] == "base" pf.add_flags("-derived", "derived") - assert pf["derived"] == ["-base", "-derived"] + assert pf.get_flags(stub_configuration) == ["-base", "-derived"] pf.define_profile("derived2", "derived") - assert pf["derived2"] == ["-base", "-derived"] + stub_configuration.set_profile("derived2") + assert pf.get_flags(stub_configuration) == ["-base", "-derived"] pf.add_flags("-derived2", "derived2") - assert pf["derived2"] == ["-base", "-derived", "-derived2"] + assert pf.get_flags(stub_configuration) == ["-base", "-derived", + "-derived2"] -def test_profile_flags_removing(): +def test_profile_flags_removing(stub_configuration): '''Tests adding flags.''' pf = ProfileFlags() pf.define_profile("base") @@ -149,34 +269,49 @@ def test_profile_flags_removing(): warn_message = "Removing managed flag '-base1'." with pytest.warns(UserWarning, match=warn_message): pf.remove_flag("-base1", "base") - assert pf["base"] == ["-base2"] + stub_configuration.set_profile("base") + assert pf.get_flags(stub_configuration, Path()) == ["-base2"] # Try removing a flag that's not there. This should not # cause any issues. pf.remove_flag("-does-not-exist") - assert pf["base"] == ["-base2"] + assert pf.get_flags(stub_configuration, Path()) == ["-base2"] pf.add_flags(["-base1", "-base2"]) warn_message = "Removing managed flag '-base1'." with pytest.warns(UserWarning, match=warn_message): pf.remove_flag("-base1") - assert pf[""] == ["-base2"] + stub_configuration.set_profile("") + assert pf.get_flags(stub_configuration) == ["-base2"] -def test_profile_flags_checksum(): +def test_profile_flags_checksum(stub_configuration): '''Tests computation of the checksum.''' pf = ProfileFlags() pf.define_profile("base") list_of_flags = ['one', 'two', 'three', 'four'] pf.add_flags(list_of_flags, "base") - assert pf.checksum("base") == string_checksum(str(list_of_flags)) + stub_configuration._profile = "base" + assert (pf.checksum(stub_configuration, Path()) == + string_checksum(str(list_of_flags))) - list_of_flags_new = ['one', 'two', 'three', 'four', "five"] + # These flags get added to the "" profile, NOT base: + list_of_flags_new = ["five", "six"] pf.add_flags(list_of_flags_new) - assert pf.checksum() == string_checksum(str(list_of_flags_new)) + stub_configuration.set_profile("") + assert (pf.checksum(stub_configuration, Path()) == + string_checksum(str(list_of_flags_new))) + + # Test handling when no config is provided: + assert (pf.checksum(file_path=Path()) == + string_checksum(str(list_of_flags_new))) + # Test handling when no file_path is provided: + assert (pf.checksum(stub_configuration) == + string_checksum(str(list_of_flags_new))) -def test_profile_flags_errors_invalid_profile_name(): + +def test_profile_flags_errors_invalid_profile_name(stub_configuration): '''Tests that given undefined profile names will raise KeyError in call functions. ''' @@ -196,7 +331,28 @@ def test_profile_flags_errors_invalid_profile_name(): assert ("remove_flag: Profile 'does not exist' is not defined." in str(err.value)) + stub_configuration._profile = "does_not_exist" with pytest.raises(KeyError) as err: - pf.checksum("does not exist") - assert ("checksum: Profile 'does not exist' is not defined." + pf.checksum(stub_configuration, Path("/some/path")) + assert ("checksum: Profile 'does_not_exist' is not defined." in str(err.value)) + + +def test_old_addflags(): + """ + Tests that old-style AddFlags are converted to MatchFlags. + """ + add_flags = AddFlags(match="/some/pattern", flags=["-g", "-O0"]) + flag_list = FlagList(add_flags=[add_flags]) + match_flag = flag_list[0] + assert isinstance(match_flag, MatchFlags) + assert match_flag._pattern == "/some/pattern" + assert match_flag._flags == ["-g", "-O0"] + + # Provide a single AddFlags instead of a list: + flag_list = FlagList(["-x"], + add_flags=AddFlags("pattern", ["-y"])) + match_flag = flag_list[1] + assert isinstance(match_flag, MatchFlags) + assert match_flag._pattern == "pattern" + assert match_flag._flags == ["-y"] diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 998c6d8a..5bf4fca7 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -21,7 +21,8 @@ from fab.tools.linker import Linker -def test_c_linker(stub_c_compiler: CCompiler) -> None: +def test_c_linker(stub_c_compiler: CCompiler, + stub_configuration: BuildConfig) -> None: """ Tests construction from C compiler """ @@ -29,18 +30,20 @@ def test_c_linker(stub_c_compiler: CCompiler) -> None: assert linker.category == Category.LINKER assert linker.name == "linker-some C compiler" assert linker.exec_name == "scc" + assert linker.compiler is stub_c_compiler assert linker.suite == "stub" - assert linker.get_flags() == [] + assert linker.get_flags(stub_configuration) == [] assert linker.output_flag == "-o" -def test_fortran_linker(stub_fortran_compiler: FortranCompiler) -> None: +def test_fortran_linker(stub_fortran_compiler: FortranCompiler, + stub_configuration: BuildConfig) -> None: linker = Linker(stub_fortran_compiler) assert linker.category == Category.LINKER assert linker.name == "linker-some Fortran compiler" assert linker.exec_name == "sfc" assert linker.suite == "stub" - assert linker.get_flags() == [] + assert linker.get_flags(stub_configuration) == [] @mark.parametrize("mpi", [True, False]) @@ -182,21 +185,20 @@ def test_linker_add_lib_flags_overwrite_silent(stub_linker: Linker) -> None: assert result == ["-t", "-b"] -class TestLinkerLinking: - def test_c(self, stub_c_compiler: CCompiler, - stub_configuration: BuildConfig, - subproc_record: ExtendedRecorder) -> None: - """ - Tests linkwhen no additional libraries are specified. - """ - linker = Linker(compiler=stub_c_compiler) - # Add a library to the linker, but don't use it in the link step - linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) +def test_c(stub_c_compiler: CCompiler, + stub_configuration: BuildConfig, + subproc_record: ExtendedRecorder) -> None: + """ + Tests linking when no additional libraries are specified. + """ + linker = Linker(compiler=stub_c_compiler) + # Add a library to the linker, but don't use it in the link step + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) - linker.link([Path("a.o")], Path("a.out"), config=stub_configuration) - assert subproc_record.invocations() == [ - ['scc', "a.o", "-o", "a.out"] - ] + linker.link([Path("a.o")], Path("a.out"), config=stub_configuration) + assert subproc_record.invocations() == [ + ['scc', "a.o", "-o", "a.out"] + ] def test_c_with_libraries(stub_c_compiler: CCompiler, @@ -219,9 +221,10 @@ def test_c_with_libraries(stub_c_compiler: CCompiler, ] -def test_c_with_libraries_and_post_flags(stub_c_compiler: CCompiler, - stub_configuration: BuildConfig, - subproc_record: ExtendedRecorder) -> None: +def test_c_with_libraries_and_post_flags( + stub_c_compiler: CCompiler, + stub_configuration: BuildConfig, + subproc_record: ExtendedRecorder) -> None: """ Tests link command line when a library and additional flags are specified. """ @@ -236,9 +239,10 @@ def test_c_with_libraries_and_post_flags(stub_c_compiler: CCompiler, ] -def test_c_with_libraries_and_pre_flags(stub_c_compiler: CCompiler, - stub_configuration: BuildConfig, - subproc_record: ExtendedRecorder) -> None: +def test_c_with_libraries_and_pre_flags( + stub_c_compiler: CCompiler, + stub_configuration: BuildConfig, + subproc_record: ExtendedRecorder) -> None: """ Tests link command line when a library and additional flags are specified. """ @@ -367,7 +371,8 @@ def test_linker_inheriting() -> None: assert str(err.value) == "Unknown library name: 'does_not_exist'" -def test_linker_profile_flags_inheriting(stub_c_compiler): +def test_linker_profile_flags_inheriting(stub_c_compiler, + stub_configuration): """ Tests nested compiler and nested linker with inherited profiling flags. """ @@ -386,7 +391,8 @@ def test_linker_profile_flags_inheriting(stub_c_compiler): count += 2 # One set f0-f3 from the compiler wrapper, one from the wrapped linker - assert (linker_wrapper.get_profile_flags("derived") == + stub_configuration.set_profile("derived") + assert (linker_wrapper.get_profile_flags(stub_configuration) == ["-f0", "-f1", "-f2", "-f3", "-f0", "-f1", "-f2", "-f3"]) diff --git a/tests/unit_tests/tools/test_preprocessor.py b/tests/unit_tests/tools/test_preprocessor.py index fb91a8b6..6470600c 100644 --- a/tests/unit_tests/tools/test_preprocessor.py +++ b/tests/unit_tests/tools/test_preprocessor.py @@ -9,16 +9,14 @@ from logging import Logger from pathlib import Path -from tests.conftest import ExtendedRecorder - from pytest import mark from pytest_subprocess.fake_process import FakeProcess -from tests.conftest import call_list - from fab.tools.category import Category from fab.tools.preprocessor import Cpp, CppFortran, Fpp, Preprocessor +from tests.conftest import call_list, ExtendedRecorder + def test_constructor() -> None: """ @@ -66,7 +64,7 @@ def test_is_not_available(self, fake_process: FakeProcess) -> None: """ Tests CPP in "traditional" mode. """ - command = ['cpp', '-traditional-cpp', '-P', '--version'] + command = ['cpp', '--version'] fake_process.register(command, returncode=1) cppf = CppFortran() @@ -79,5 +77,6 @@ def test_preprocess(self, subproc_record: ExtendedRecorder) -> None: cppf.preprocess(Path("a.in"), Path("a.out"), ["-DDO_SOMETHING"]) assert subproc_record.invocations() == [ ["cpp", "-traditional-cpp", "-P", "a.in", "a.out"], - ["cpp", "-traditional-cpp", "-P", "-DDO_SOMETHING", "a.in", "a.out"] + ["cpp", "-traditional-cpp", "-P", "-DDO_SOMETHING", + "a.in", "a.out"] ] diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py index f9c133d1..7db26520 100644 --- a/tests/unit_tests/tools/test_psyclone.py +++ b/tests/unit_tests/tools/test_psyclone.py @@ -28,8 +28,6 @@ def test_constructor(): assert psyclone.category == Category.PSYCLONE assert psyclone.name == "psyclone" assert psyclone.exec_name == "psyclone" - # pylint: disable=use-implicit-booleaness-not-comparison - assert psyclone.get_flags() == [] @mark.parametrize("version", ["2.4.0", "2.5.0", "3.0.0", "3.1.0"]) diff --git a/tests/unit_tests/tools/test_rsync.py b/tests/unit_tests/tools/test_rsync.py index b6c9db4b..bc2bd4ec 100644 --- a/tests/unit_tests/tools/test_rsync.py +++ b/tests/unit_tests/tools/test_rsync.py @@ -24,7 +24,6 @@ def test_constructor(): assert rsync.category == Category.RSYNC assert rsync.name == "rsync" assert rsync.exec_name == "rsync" - assert rsync.get_flags() == [] def test_check_available(fake_process: FakeProcess) -> None: diff --git a/tests/unit_tests/tools/test_tool.py b/tests/unit_tests/tools/test_tool.py index bbfa9738..4fd6515f 100644 --- a/tests/unit_tests/tools/test_tool.py +++ b/tests/unit_tests/tools/test_tool.py @@ -15,8 +15,8 @@ from tests.conftest import ExtendedRecorder, call_list, not_found_callback from fab.tools.category import Category -from fab.tools.flags import ProfileFlags -from fab.tools.tool import CompilerSuiteTool, Tool +from fab.tools.tool import Tool +from fab.tools.compiler_suite_tool import CompilerSuiteTool def test_constructor() -> None: @@ -127,42 +127,6 @@ def test_run_missing(fake_process: FakeProcess) -> None: assert "this is stderr" in str(err.value) -def test_tool_flags_no_profile() -> None: - """ - Test that flags without using a profile work as expected. - """ - tool = Tool("some tool", "stool", Category.MISC) - assert tool.get_flags() == [] - tool.add_flags("-a") - assert tool.get_flags() == ["-a"] - tool.add_flags(["-b", "-c"]) - assert tool.get_flags() == ["-a", "-b", "-c"] - - -def test_tool_profiles() -> None: - '''Test that profiles work as expected. These tests use internal - implementation details of ProfileFlags, but we need to test that the - exposed flag-related API works as expected - - ''' - tool = Tool("gfortran", "gfortran", Category.FORTRAN_COMPILER) - # Make sure by default we get ProfileFlags - assert isinstance(tool._flags, ProfileFlags) - assert tool.get_flags() == [] - - # Define a profile with no inheritance - tool.define_profile("mode1") - assert tool.get_flags("mode1") == [] - tool.add_flags("-flag1", "mode1") - assert tool.get_flags("mode1") == ["-flag1"] - - # Define a profile with inheritance - tool.define_profile("mode2", "mode1") - assert tool.get_flags("mode2") == ["-flag1"] - tool.add_flags("-flag2", "mode2") - assert tool.get_flags("mode2") == ["-flag1", "-flag2"] - - class TestToolRun: """ Tests tool run method. diff --git a/tests/unit_tests/tools/test_tool_repository.py b/tests/unit_tests/tools/test_tool_repository.py index 5fb42b5a..942a5b37 100644 --- a/tests/unit_tests/tools/test_tool_repository.py +++ b/tests/unit_tests/tools/test_tool_repository.py @@ -321,6 +321,9 @@ def test_tool_repository_full_path(fake_process: FakeProcess) -> None: stdout='GNU Fortran (gcc) 1.2.3') tr = ToolRepository() gfortran = tr.get_tool(Category.FORTRAN_COMPILER, "/usr/bin/gfortran") + gfortran = cast(Compiler, gfortran) + gfortran._is_available = True + gfortran._version = (1, 2) assert isinstance(gfortran, Gfortran) assert gfortran.name == "gfortran" assert gfortran.exec_name == "gfortran" diff --git a/tests/unit_tests/tools/test_tool_with_flags.py b/tests/unit_tests/tools/test_tool_with_flags.py new file mode 100644 index 00000000..51c4c62e --- /dev/null +++ b/tests/unit_tests/tools/test_tool_with_flags.py @@ -0,0 +1,66 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +""" +Tests ToolsWithFlags +""" + +from pathlib import Path + +from fab.tools.category import Category +from fab.tools.flags import ProfileFlags +from fab.tools.tool_with_flags import ToolWithFlags + + +def test_tool_with_flags_constructor(stub_configuration) -> None: + """ + Tests construction from argument list. + """ + tool = ToolWithFlags("gnu", "gfortran", Category.FORTRAN_COMPILER) + assert isinstance(tool._flags, ProfileFlags) + # pylint: disable=use-implicit-booleaness-not-comparison + assert tool.get_flags(stub_configuration, Path()) == [] + + +def test_tool_with_flags_no_profile(stub_configuration) -> None: + """ + Test that flags without using a profile work as expected. + """ + tool = ToolWithFlags("some tool", "stool", Category.MISC) + # pylint: disable=use-implicit-booleaness-not-comparison + assert tool.get_flags(stub_configuration, Path()) == [] + tool.add_flags("-a") + assert tool.get_flags(stub_configuration, Path()) == ["-a"] + tool.add_flags(["-b", "-c"]) + assert tool.get_flags(stub_configuration, Path()) == ["-a", "-b", "-c"] + + +def test_tool_with_flags_profiles(stub_configuration) -> None: + """ + Test that profiles work as expected. These tests use internal + implementation details of ProfileFlags, but we need to test that the + exposed flag-related API works as expected + """ + + # pylint: disable=use-implicit-booleaness-not-comparison + tool = ToolWithFlags("gfortran", "gfortran", Category.FORTRAN_COMPILER) + # Make sure by default we get ProfileFlags + assert isinstance(tool._flags, ProfileFlags) + assert tool.get_flags(stub_configuration) == [] + + # Define a profile with no inheritance + stub_configuration.set_profile("mode1") + tool.define_profile("mode1") + assert tool.get_flags(stub_configuration) == [] + tool.add_flags("-flag1", "mode1") + assert tool.get_flags(stub_configuration) == ["-flag1"] + + # Define a profile with inheritance + tool.define_profile("mode2", "mode1") + stub_configuration.set_profile("mode2") + assert tool.get_flags(stub_configuration) == ["-flag1"] + tool.add_flags("-flag2", "mode2") + assert tool.get_flags(stub_configuration) == ["-flag1", "-flag2"] diff --git a/tests/unit_tests/tools/test_versioning.py b/tests/unit_tests/tools/test_versioning.py index bf9f41e2..200be869 100644 --- a/tests/unit_tests/tools/test_versioning.py +++ b/tests/unit_tests/tools/test_versioning.py @@ -31,7 +31,6 @@ def test_git_constructor(self): '''Test the git constructor.''' git = Git() assert git.category == Category.GIT - assert git.get_flags() == [] def test_git_check_available(self, fake_process: FakeProcess) -> None: """ @@ -236,7 +235,6 @@ def test_svn_constructor(self): """ svn = Subversion() assert svn.category == Category.SUBVERSION - assert svn.get_flags() == [] assert svn.name == "Subversion" assert svn.exec_name == "svn" @@ -395,7 +393,6 @@ def test_extract_from_http(self, repo: Tuple[Path, Path], tmp_path: Path): TODO: This is hard to test without a full Apache installation. For the moment we forgo the test on the basis that it's too hard. """ - pass # ============================================================================ @@ -409,6 +406,5 @@ def test_fcm_constructor(self): """ fcm = Fcm() assert fcm.category == Category.FCM - assert fcm.get_flags() == [] assert fcm.name == "FCM" assert fcm.exec_name == "fcm"