diff --git a/Documentation/source/site-specific-config.rst b/Documentation/source/site-specific-config.rst index b7481c08..9faaeb04 100644 --- a/Documentation/source/site-specific-config.rst +++ b/Documentation/source/site-specific-config.rst @@ -20,7 +20,12 @@ contains all the tools that will be used during the build process, but it can only store one tool per category. If a certain tool should not be defined in the toolbox, the default from the `ToolRepository` will be used. This is useful for many standard tools like `git`, `rsync` -etc that de-facto will never be changed. +etc that de-facto will never be changed. Fab will check if a tool +is actually available on the system before adding it to a ToolBox. +This is typically done by trying to run the tool with some testing +parameters, for example requesting its version number. If this fails, +the tool is considered not to be available and will not be used (unless +the user explicitly puts a tool into the ToolBox). .. note:: If you need to use for example different compilers for different files, you would implement this as a `meta-compiler`: diff --git a/source/fab/tools/__init__.py b/source/fab/tools/__init__.py index bc7430b7..f6830d85 100644 --- a/source/fab/tools/__init__.py +++ b/source/fab/tools/__init__.py @@ -19,6 +19,7 @@ from fab.tools.psyclone import Psyclone from fab.tools.rsync import Rsync from fab.tools.preprocessor import Cpp, CppFortran, Fpp, Preprocessor +from fab.tools.shell import Shell from fab.tools.tool import Tool, CompilerSuiteTool # Order here is important to avoid a circular import from fab.tools.tool_repository import ToolRepository @@ -56,6 +57,7 @@ "Preprocessor", "Psyclone", "Rsync", + "Shell", "Subversion", "Tool", "ToolBox", diff --git a/source/fab/tools/category.py b/source/fab/tools/category.py index 6eab9b9d..a64781f1 100644 --- a/source/fab/tools/category.py +++ b/source/fab/tools/category.py @@ -25,6 +25,7 @@ class Category(Enum): SUBVERSION = auto() AR = auto() RSYNC = auto() + SHELL = auto() MISC = auto() def __str__(self): diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index 3f8c31e1..0b5618de 100644 --- a/source/fab/tools/compiler.py +++ b/source/fab/tools/compiler.py @@ -56,7 +56,7 @@ def __init__(self, name: str, compile_flag: Optional[str] = None, output_flag: Optional[str] = None, openmp_flag: Optional[str] = None, - availability_option: Optional[str] = None): + availability_option: Optional[Union[str, List[str]]] = None): super().__init__(name, exec_name, suite, category=category, availability_option=availability_option) self._version: Union[Tuple[int, ...], None] = None diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index cbf12a9f..066b05be 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -180,7 +180,7 @@ def process(self, # New version: no API, parameter, but -o for output name: parameters.extend(["-o", transformed_file]) else: - # 2.5.0 or earlier: needs api nemo, output name is -oalg + # 2.5.0 or earlier: needs api nemo, output name is -opsy parameters.extend(["-api", "nemo", "-opsy", transformed_file]) parameters.extend(["-l", "all"]) diff --git a/source/fab/tools/shell.py b/source/fab/tools/shell.py new file mode 100644 index 00000000..162649fb --- /dev/null +++ b/source/fab/tools/shell.py @@ -0,0 +1,46 @@ +############################################################################## +# (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 a base class for shells. This can be used to execute +other scripts. +""" + +from pathlib import Path +from typing import List, Union + +from fab.tools.category import Category +from fab.tools.tool import Tool + + +class Shell(Tool): + '''A simple wrapper that runs a shell script. There seems to be no + consistent way to simply check if a shell is working - not only support + a version command (e.g. sh and dash don't). Instead, availability + is tested by running a simple 'echo' command. + + :name: the path to the script to run. + ''' + def __init__(self, name: str): + super().__init__(name=name, exec_name=name, + availability_option=["-c", "echo hello"], + category=Category.SHELL) + + def exec(self, command: Union[str, List[Union[Path, str]]]) -> str: + '''Executes the specified command. + + :param command: the command and potential parameters to execute. + + :returns: stdout of the result. + ''' + # Make mypy happy: + params: List[Union[str, Path]] + if isinstance(command, str): + params = ["-c", command] + else: + params = ["-c"] + params.extend(command) + return super().run(additional_parameters=params, + capture_output=True) diff --git a/source/fab/tools/tool.py b/source/fab/tools/tool.py index cb8a7a06..78a8de62 100644 --- a/source/fab/tools/tool.py +++ b/source/fab/tools/tool.py @@ -16,7 +16,7 @@ import logging from pathlib import Path import subprocess -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional, Sequence, Union from fab.tools.category import Category from fab.tools.flags import Flags @@ -36,7 +36,7 @@ class Tool: def __init__(self, name: str, exec_name: Union[str, Path], category: Category = Category.MISC, - availability_option: Optional[str] = None): + availability_option: Optional[Union[str, List[str]]] = None): self._logger = logging.getLogger(__name__) self._name = name self._exec_name = str(exec_name) @@ -107,7 +107,7 @@ def name(self) -> str: return self._name @property - def availability_option(self) -> str: + def availability_option(self) -> Union[str, List[str]]: ''':returns: the option to use to check if the tool is available.''' return self._availability_option @@ -139,7 +139,7 @@ def __str__(self): def run(self, additional_parameters: Optional[ - Union[str, List[Union[Path, str]]]] = None, + Union[str, Sequence[Union[Path, str]]]] = None, env: Optional[Dict[str, str]] = None, cwd: Optional[Union[Path, str]] = None, capture_output=True) -> str: @@ -210,7 +210,7 @@ class CompilerSuiteTool(Tool): ''' def __init__(self, name: str, exec_name: Union[str, Path], suite: str, category: Category, - availability_option: Optional[str] = None): + availability_option: Optional[Union[str, List[str]]] = None): super().__init__(name, exec_name, category, availability_option=availability_option) self._suite = suite diff --git a/source/fab/tools/tool_repository.py b/source/fab/tools/tool_repository.py index d699574a..965e252b 100644 --- a/source/fab/tools/tool_repository.py +++ b/source/fab/tools/tool_repository.py @@ -23,7 +23,7 @@ from fab.tools.versioning import Fcm, Git, Subversion from fab.tools import (Ar, Cpp, CppFortran, Craycc, Crayftn, Gcc, Gfortran, Icc, Icx, Ifort, Ifx, - Nvc, Nvfortran, Psyclone, Rsync) + Nvc, Nvfortran, Psyclone, Rsync, Shell) class ToolRepository(dict): @@ -63,7 +63,7 @@ def __init__(self): # Add the FAB default tools: # TODO: sort the defaults so that they actually work (since not all # tools FAB knows about are available). For now, disable Fpp (by not - # adding it). IF someone actually uses it it can added. + # adding it). If someone actually uses it it can added. for cls in [Craycc, Crayftn, Gcc, Gfortran, Icc, Icx, Ifort, Ifx, @@ -72,6 +72,12 @@ def __init__(self): Ar, Fcm, Git, Psyclone, Rsync, Subversion]: self.add_tool(cls()) + # Add the common shells. While Fab itself does not need this, + # it is a very convenient tool for user configuration (e.g. to + # query nc-config etc) + for shell_name in ["sh", "bash", "ksh", "dash"]: + self.add_tool(Shell(shell_name)) + # Now create the potential mpif90 and Cray ftn wrapper all_fc = self[Category.FORTRAN_COMPILER][:] for fc in all_fc: @@ -95,9 +101,10 @@ def __init__(self): def add_tool(self, tool: Tool): '''Creates an instance of the specified class and adds it - to the tool repository. + to the tool repository. If the tool is a compiler, it automatically + adds the compiler as a linker as well (named "linker-{tool.name}"). - :param cls: the tool to instantiate. + :param tool: the tool to add. ''' # We do not test if a tool is actually available. The ToolRepository @@ -155,17 +162,20 @@ def set_default_compiler_suite(self, suite: str): def get_default(self, category: Category, mpi: Optional[bool] = None, openmp: Optional[bool] = None): - '''Returns the default tool for a given category. For most tools - that will be the first entry in the list of tools. The exception - are compilers and linker: in this case it must be specified if - MPI support is required or not. And the default return will be + '''Returns the default tool for a given category that is available. + For most tools that will be the first entry in the list of tools. The + exception are compilers and linker: in this case it must be specified + if MPI support is required or not. And the default return will be the first tool that either supports MPI or not. :param category: the category for which to return the default tool. :param mpi: if a compiler or linker is required that supports MPI. - :param open: if a compiler or linker is required that supports OpenMP. + :param openmp: if a compiler or linker is required that supports + OpenMP. :raises KeyError: if the category does not exist. + :raises RuntimeError: if no tool in the requested category is + available on the system. :raises RuntimeError: if no compiler/linker is found with the requested level of MPI support (yes or no). ''' @@ -176,7 +186,12 @@ def get_default(self, category: Category, # If not a compiler or linker, return the first tool if not category.is_compiler and category != Category.LINKER: - return self[category][0] + for tool in self[category]: + if tool.is_available: + return tool + tool_names = ",".join(i.name for i in self[category]) + raise RuntimeError(f"Can't find available '{category}' tool. " + f"Tools are '{tool_names}'.") if not isinstance(mpi, bool): raise RuntimeError(f"Invalid or missing mpi specification " @@ -191,8 +206,8 @@ def get_default(self, category: Category, # ignore it. if openmp and not tool.openmp: continue - # If the tool supports/does not support MPI, return it. - if mpi == tool.mpi: + # If the tool supports/does not support MPI, return the first one + if tool.is_available and mpi == tool.mpi: return tool # Don't bother returning an MPI enabled tool if no-MPI is requested - diff --git a/tests/unit_tests/steps/test_archive_objects.py b/tests/unit_tests/steps/test_archive_objects.py index 097200ea..1cc9e2cf 100644 --- a/tests/unit_tests/steps/test_archive_objects.py +++ b/tests/unit_tests/steps/test_archive_objects.py @@ -84,7 +84,6 @@ def test_for_library(self): def test_incorrect_tool(self, tool_box): '''Test that an incorrect archive tool is detected ''' - config = BuildConfig('proj', tool_box) cc = tool_box.get_tool(Category.C_COMPILER, config.mpi, config.openmp) # And set its category to be AR diff --git a/tests/unit_tests/steps/test_grab.py b/tests/unit_tests/steps/test_grab.py index 348dc293..dc222a22 100644 --- a/tests/unit_tests/steps/test_grab.py +++ b/tests/unit_tests/steps/test_grab.py @@ -3,6 +3,10 @@ # For further details please refer to the file COPYRIGHT # which you should have received as part of this distribution ############################################################################## + +'''Test various grab implementation - folders and fcm. +''' + from pathlib import Path from types import SimpleNamespace from unittest import mock @@ -15,14 +19,21 @@ class TestGrabFolder: + '''Test grab folder functionality.''' def test_trailing_slash(self): - with pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"): - self._common(grab_src='/grab/source/', expect_grab_src='/grab/source/') + '''Test folder grabbing with a trailing slash.''' + with pytest.warns(UserWarning, match="_metric_send_conn not set, " + "cannot send metrics"): + self._common(grab_src='/grab/source/', + expect_grab_src='/grab/source/') def test_no_trailing_slash(self): - with pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"): - self._common(grab_src='/grab/source', expect_grab_src='/grab/source/') + '''Test folder grabbing without a trailing slash.''' + with pytest.warns(UserWarning, match="_metric_send_conn not set, " + "cannot send metrics"): + self._common(grab_src='/grab/source', + expect_grab_src='/grab/source/') def _common(self, grab_src, expect_grab_src): source_root = Path('/workspace/source') @@ -30,9 +41,15 @@ def _common(self, grab_src, expect_grab_src): mock_config = SimpleNamespace(source_root=source_root, tool_box=ToolBox()) + # Since is_available calls run, in order to test a single run call, + # we patch is_available to be always true. with mock.patch('pathlib.Path.mkdir'): with mock.patch('fab.tools.tool.Tool.run') as mock_run: - grab_folder(mock_config, src=grab_src, dst_label=dst) + with mock.patch( + 'fab.tools.tool.Tool.is_available', + new_callable=mock.PropertyMock) as is_available: + is_available.return_value = True + grab_folder(mock_config, src=grab_src, dst_label=dst) expect_dst = mock_config.source_root / dst mock_run.assert_called_once_with( @@ -41,8 +58,10 @@ def _common(self, grab_src, expect_grab_src): class TestGrabFcm: + '''Test FCM functionality.''' def test_no_revision(self): + '''Test FCM without specifying a revision.''' source_root = Path('/workspace/source') source_url = '/www.example.com/bar' dst_label = 'bar' @@ -50,15 +69,23 @@ def test_no_revision(self): mock_config = SimpleNamespace(source_root=source_root, tool_box=ToolBox()) with mock.patch('pathlib.Path.mkdir'): - with mock.patch('fab.tools.tool.Tool.run') as mock_run, \ - pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"): - fcm_export(config=mock_config, src=source_url, dst_label=dst_label) + with mock.patch('fab.tools.tool.Tool.is_available', + new_callable=mock.PropertyMock) as is_available: + is_available.return_value = True + with mock.patch('fab.tools.tool.Tool.run') as mock_run, \ + pytest.warns(UserWarning, + match="_metric_send_conn not " + "set, cannot send metrics"): + fcm_export(config=mock_config, src=source_url, + dst_label=dst_label) mock_run.assert_called_once_with(['export', '--force', source_url, str(source_root / dst_label)], - env=None, cwd=None, capture_output=True) + env=None, cwd=None, + capture_output=True) def test_revision(self): + '''Test that the revision is passed on correctly in fcm export.''' source_root = Path('/workspace/source') source_url = '/www.example.com/bar' dst_label = 'bar' @@ -67,12 +94,19 @@ def test_revision(self): mock_config = SimpleNamespace(source_root=source_root, tool_box=ToolBox()) with mock.patch('pathlib.Path.mkdir'): - with mock.patch('fab.tools.tool.Tool.run') as mock_run, \ - pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"): - fcm_export(mock_config, src=source_url, dst_label=dst_label, revision=revision) + with mock.patch('fab.tools.tool.Tool.is_available', + new_callable=mock.PropertyMock) as is_available: + is_available.return_value = True + with mock.patch('fab.tools.tool.Tool.run') as mock_run, \ + pytest.warns( + UserWarning, match="_metric_send_conn not set, " + "cannot send metrics"): + fcm_export(mock_config, src=source_url, + dst_label=dst_label, revision=revision) mock_run.assert_called_once_with( - ['export', '--force', '--revision', '42', f'{source_url}', str(source_root / dst_label)], + ['export', '--force', '--revision', '42', f'{source_url}', + str(source_root / dst_label)], env=None, cwd=None, capture_output=True) # todo: test missing repo diff --git a/tests/unit_tests/tools/test_shell.py b/tests/unit_tests/tools/test_shell.py new file mode 100644 index 00000000..deab54d4 --- /dev/null +++ b/tests/unit_tests/tools/test_shell.py @@ -0,0 +1,61 @@ +############################################################################## +# (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 the shell implementation. +''' + +from unittest import mock + +from fab.tools import Category, Shell + + +def test_shell_constructor(): + '''Test the Shell constructor.''' + bash = Shell("bash") + assert bash.category == Category.SHELL + assert bash.name == "bash" + assert bash.exec_name == "bash" + + +def test_shell_check_available(): + '''Tests the is_available functionality.''' + bash = Shell("bash") + mock_result = mock.Mock(returncode=0) + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + assert bash.check_available() + tool_run.assert_called_once_with( + ["bash", "-c", "echo hello"], capture_output=True, env=None, + cwd=None, check=False) + + # Test behaviour if a runtime error happens: + with mock.patch("fab.tools.tool.Tool.run", + side_effect=RuntimeError("")) as tool_run: + assert not bash.check_available() + + +def test_shell_exec_single_arg(): + '''Test running a shell script without additional parameters.''' + ksh = Shell("ksh") + mock_result = mock.Mock(returncode=0) + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + ksh.exec("echo") + tool_run.assert_called_with(['ksh', '-c', 'echo'], + capture_output=True, env=None, cwd=None, + check=False) + + +def test_shell_exec_multiple_args(): + '''Test running a shell script with parameters.''' + ksh = Shell("ksh") + mock_result = mock.Mock(returncode=0) + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + ksh.exec(["some", "shell", "function"]) + tool_run.assert_called_with(['ksh', '-c', 'some', 'shell', 'function'], + capture_output=True, env=None, cwd=None, + check=False) diff --git a/tests/unit_tests/tools/test_tool_repository.py b/tests/unit_tests/tools/test_tool_repository.py index e9bfb0c1..0c7d77e5 100644 --- a/tests/unit_tests/tools/test_tool_repository.py +++ b/tests/unit_tests/tools/test_tool_repository.py @@ -151,17 +151,36 @@ def test_tool_repository_default_compiler_suite(): '''Tests the setting of default suite for compiler and linker.''' tr = ToolRepository() tr.set_default_compiler_suite("gnu") - for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER, - Category.LINKER]: - def_tool = tr.get_default(cat, mpi=False, openmp=False) - assert def_tool.suite == "gnu" - - tr.set_default_compiler_suite("intel-classic") - for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER, - Category.LINKER]: - def_tool = tr.get_default(cat, mpi=False, openmp=False) - assert def_tool.suite == "intel-classic" - with pytest.raises(RuntimeError) as err: - tr.set_default_compiler_suite("does-not-exist") - assert ("Cannot find 'FORTRAN_COMPILER' in the suite 'does-not-exist'" - in str(err.value)) + + # Mark all compiler and linker as available. + with mock.patch('fab.tools.tool.Tool.is_available', + new_callable=mock.PropertyMock) as is_available: + is_available.return_value = True + for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER, + Category.LINKER]: + def_tool = tr.get_default(cat, mpi=False, openmp=False) + assert def_tool.suite == "gnu" + + tr.set_default_compiler_suite("intel-classic") + for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER, + Category.LINKER]: + def_tool = tr.get_default(cat, mpi=False, openmp=False) + assert def_tool.suite == "intel-classic" + with pytest.raises(RuntimeError) as err: + tr.set_default_compiler_suite("does-not-exist") + assert ("Cannot find 'FORTRAN_COMPILER' in the suite 'does-not-exist'" + in str(err.value)) + + +def test_tool_repository_no_tool_available(): + '''Tests error handling if no tool is available.''' + + tr = ToolRepository() + tr.set_default_compiler_suite("gnu") + with mock.patch('fab.tools.tool.Tool.is_available', + new_callable=mock.PropertyMock) as is_available: + is_available.return_value = False + with pytest.raises(RuntimeError) as err: + tr.get_default(Category.SHELL) + assert ("Can't find available 'SHELL' tool. Tools are 'sh,bash,ksh," + "dash'" in str(err.value))