#3 Converted PSyclone to be a tool.

source/fab/newtools/__init__.py

@@ -13,6 +13,7 @@
                                    Gfortran, Icc, Ifort)
 from fab.newtools.flags import Flags
 from fab.newtools.linker import Linker
+from fab.newtools.psyclone import Psyclone
 from fab.newtools.preprocessor import Cpp, CppFortran, Fpp, Preprocessor
 from fab.newtools.tool import Tool, VendorTool
 # Order here is important to avoid a circular import
@@ -37,6 +38,7 @@
            "Ifort",
            "Linker",
            "Preprocessor",
+           "Psyclone",
            "Subversion",
            "Tool",
            "ToolBox",

source/fab/newtools/psyclone.py

@@ -0,0 +1,60 @@
+##############################################################################
+# (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 the tool class for PSyclone.
+
+"""
+
+from pathlib import Path
+from typing import List, Optional, Union
+
+from fab.newtools.categories import Categories
+from fab.newtools.tool import Tool
+
+
+class Psyclone(Tool):
+    '''This is the base class for `PSyclone`.
+    '''
+
+    def __init__(self):
+        super().__init__("psyclone", "psyclone", Categories.PSYCLONE)
+
+    def check_available(self):
+        '''Checks if psyclone is available. We do this by requesting the
+        psyclone version.
+        '''
+        try:
+            self.run("--version")
+        except (RuntimeError, FileNotFoundError):
+            return False
+        return True
+
+    def process(self, api: str,
+                x90_file: Union[Path, str],
+                psy_file: Union[Path, str],
+                alg_file: Union[Path, str],
+                transformation_script: Optional[Union[Path, str]] = None,
+                additional_parameters: Optional[List[str]] = None,
+                kernel_roots: Optional[List[str]] = None,
+                ):
+        '''Create the archive with the specified name, containing the
+        listed members.
+        :param output_fpath: the output path.
+        :param members: the list of objects to be added to the archive.
+        '''
+        parameters = ["-api", api, "-l", "all",
+                      "-opsy", str(psy_file),
+                      "-oalg", str(alg_file)]
+        if transformation_script:
+            parameters.extend(["-s", str(transformation_script)])
+        if additional_parameters:
+            parameters.extend(additional_parameters)
+        if kernel_roots:
+            roots_with_dash_d = sum([['-d', str(k)] for k in kernel_roots], [])
+            parameters.extend(roots_with_dash_d)
+        parameters.append(str(x90_file))
+        print("XX", parameters)
+        return self.run(additional_parameters=parameters)

source/fab/newtools/tool_repository.py

@@ -15,7 +15,7 @@
 from typing import Any, Type
 
 from fab.newtools import (Ar, Categories, Cpp, CppFortran, Gcc, Gfortran,
-                          Icc, Ifort, Linker)
+                          Icc, Ifort, Linker, Psyclone)
 from fab.newtools.versioning import Fcm, Git, Subversion
 
 
@@ -56,7 +56,7 @@ def __init__(self):
         # TODO: sort the defaults so that they actually work (since not all
         # tools FAB knows about are available). For now, disable Fpp:
         for cls in [Gcc, Icc, Gfortran, Ifort, Cpp, CppFortran,
-                    Fcm, Git, Subversion, Ar]:
+                    Fcm, Git, Subversion, Ar, Psyclone]:
             self.add_tool(cls)
 
     def add_tool(self, cls: Type[Any]):

source/fab/steps/psyclone.py

@@ -15,10 +15,9 @@
 import warnings
 from itertools import chain
 from pathlib import Path
-from typing import Dict, List, Optional, Set, Union, Tuple
+from typing import Dict, List, Optional, Set, Tuple
 
 from fab.build_config import BuildConfig
-from fab.tools import run_command
 
 from fab.artefacts import ArtefactsGetter, CollectionConcat, SuffixFilter
 from fab.parse.fortran import FortranAnalyser, AnalysedFortran
@@ -32,15 +31,6 @@
 logger = logging.getLogger(__name__)
 
 
-def tool_available() -> bool:
-    """Check if the psyclone tool is available at the command line."""
-    try:
-        run_command(['psyclone', '-h'])
-    except (RuntimeError, FileNotFoundError):
-        return False
-    return True
-
-
 # todo: should this be part of the psyclone step?
 def preprocess_x90(config, common_flags: Optional[List[str]] = None):
     common_flags = common_flags or []
@@ -318,37 +308,46 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]):
     prebuild_hash = _gen_prebuild_hash(x90_file, mp_payload)
 
     # These are the filenames we expect to be output for this x90 input file.
-    # There will always be one modified_alg, and 0-1 generated.
+    # There will always be one modified_alg, and 0-1 generated psy file.
     modified_alg: Path = x90_file.with_suffix('.f90')
     modified_alg = input_to_output_fpath(config=mp_payload.config, input_path=modified_alg)
-    generated: Path = x90_file.parent / (str(x90_file.stem) + '_psy.f90')
-    generated = input_to_output_fpath(config=mp_payload.config, input_path=generated)
+    psy_file: Path = x90_file.parent / (str(x90_file.stem) + '_psy.f90')
+    psy_file = input_to_output_fpath(config=mp_payload.config, input_path=psy_file)
 
-    generated.parent.mkdir(parents=True, exist_ok=True)
+    psy_file.parent.mkdir(parents=True, exist_ok=True)
 
     # do we already have prebuilt results for this x90 file?
     prebuilt_alg, prebuilt_gen = _get_prebuild_paths(
-        mp_payload.config.prebuild_folder, modified_alg, generated, prebuild_hash)
+        mp_payload.config.prebuild_folder, modified_alg, psy_file, prebuild_hash)
     if prebuilt_alg.exists():
         # todo: error handling in here
         msg = f'found prebuilds for {x90_file}:\n    {prebuilt_alg}'
         shutil.copy2(prebuilt_alg, modified_alg)
         if prebuilt_gen.exists():
             msg += f'\n    {prebuilt_gen}'
-            shutil.copy2(prebuilt_gen, generated)
+            shutil.copy2(prebuilt_gen, psy_file)
         log_or_dot(logger=logger, msg=msg)
 
     else:
+        config = mp_payload.config
+        psyclone = config.tool_box[Categories.PSYCLONE]
         try:
+            transformation_script = mp_payload.transformation_script
+            psyclone.process(api="dynamo0.3",
+                             x90_file=x90_file,
+                             psy_file=psy_file,
+                             alg_file=modified_alg,
+                             transformation_script=transformation_script,
+                             kernel_roots=mp_payload.kernel_roots,
+                             additional_parameters=mp_payload.cli_args)
+
             # logger.info(f'running psyclone on {x90_file}')
-            run_psyclone(generated, modified_alg, x90_file,
-                         mp_payload.kernel_roots, mp_payload.transformation_script, mp_payload.cli_args)
 
             shutil.copy2(modified_alg, prebuilt_alg)
             msg = f'created prebuilds for {x90_file}:\n    {prebuilt_alg}'
-            if Path(generated).exists():
+            if Path(psy_file).exists():
                 msg += f'\n    {prebuilt_gen}'
-                shutil.copy2(generated, prebuilt_gen)
+                shutil.copy2(psy_file, prebuilt_gen)
             log_or_dot(logger=logger, msg=msg)
 
         except Exception as err:
@@ -357,12 +356,12 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]):
 
     # do we have handwritten overrides for either of the files we just created?
     modified_alg = _check_override(modified_alg, mp_payload)
-    generated = _check_override(generated, mp_payload)
+    psy_file = _check_override(psy_file, mp_payload)
 
     # return the output files from psyclone
     result: List[Path] = [modified_alg]
-    if Path(generated).exists():
-        result.append(generated)
+    if Path(psy_file).exists():
+        result.append(psy_file)
 
     # we also want to return the prebuild artefact files we created,
     # which are just copies, in the prebuild folder, with hashes in the filenames.
@@ -403,35 +402,12 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs):
     return prebuild_hash
 
 
-def _get_prebuild_paths(prebuild_folder, modified_alg, generated, prebuild_hash):
+def _get_prebuild_paths(prebuild_folder, modified_alg, psy_file, prebuild_hash):
     prebuilt_alg = Path(prebuild_folder / f'{modified_alg.stem}.{prebuild_hash}{modified_alg.suffix}')
-    prebuilt_gen = Path(prebuild_folder / f'{generated.stem}.{prebuild_hash}{generated.suffix}')
+    prebuilt_gen = Path(prebuild_folder / f'{psy_file.stem}.{prebuild_hash}{psy_file.suffix}')
     return prebuilt_alg, prebuilt_gen
 
 
-def run_psyclone(generated, modified_alg, x90_file, kernel_roots,
-                 transformation_script, cli_args) -> None:
-
-    # -d specifies "a root directory structure containing kernel source"
-    kernel_args: Union[List[str], list] = sum([['-d', k] for k in kernel_roots], [])
-
-    # transformation python script
-    transform_options = ['-s', transformation_script] if transformation_script else []
-
-    command = [
-        'psyclone', '-api', 'dynamo0.3',
-        '-l', 'all',
-        *kernel_args,
-        '-opsy', generated,  # filename of generated PSy code
-        '-oalg', modified_alg,  # filename of transformed algorithm code
-        *transform_options,
-        *cli_args,
-        x90_file,
-    ]
-
-    run_command(command)
-
-
 def _check_override(check_path: Path, mp_payload: MpCommonArgs):
     """
     Delete the file if there's an override for it.

tests/system_tests/psyclone/test_psyclone_system_test.py

@@ -17,8 +17,8 @@
 from fab.steps.find_source_files import find_source_files
 from fab.steps.grab.folder import grab_folder
 from fab.steps.preprocess import preprocess_fortran
-from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone, tool_available
-from fab.newtools import ToolBox
+from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone
+from fab.newtools import ToolBox, Psyclone
 from fab.util import file_checksum
 
 SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90'
@@ -61,7 +61,7 @@ def test_make_parsable_x90(tmp_path):
     unlink(parsable_x90_path)
 
 
-class TestX90Analyser(object):
+class TestX90Analyser():
 
     expected_analysis_result = AnalysedX90(
         fpath=EXPECT_PARSABLE_X90,
@@ -93,7 +93,7 @@ def test_prebuild(self, tmp_path):
         assert analysed_x90 == self.expected_analysis_result
 
 
-class Test_analysis_for_prebuilds(object):
+class Test_analysis_for_prebuilds():
 
     def test_analyse(self, tmp_path):
 
@@ -124,8 +124,8 @@ def test_analyse(self, tmp_path):
         }
 
 
-@pytest.mark.skipif(not tool_available(), reason="psyclone cli tool not available")
-class TestPsyclone(object):
+@pytest.mark.skipif(not Psyclone().is_available, reason="psyclone cli tool not available")
+class TestPsyclone():
     """
     Basic run of the psyclone step.
 
@@ -185,11 +185,11 @@ def test_prebuild(self, tmp_path, config):
             self.steps(config)
 
         # make sure no work gets done the second time round
-        with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_x90_walk:
-            with mock.patch('fab.parse.fortran.FortranAnalyser.walk_nodes') as mock_fortran_walk:
-                with mock.patch('fab.steps.psyclone.run_psyclone') as mock_run:
-                    with config, pytest.warns(UserWarning, match="no transformation script specified"):
-                        self.steps(config)
+        with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_x90_walk, \
+                mock.patch('fab.parse.fortran.FortranAnalyser.walk_nodes') as mock_fortran_walk, \
+                mock.patch('fab.newtools.psyclone.Psyclone.process') as mock_run, \
+                config, pytest.warns(UserWarning, match="no transformation script specified"):
+            self.steps(config)
 
         mock_x90_walk.assert_not_called()
         mock_fortran_walk.assert_not_called()

tests/unit_tests/tools/test_psyclone.py

@@ -0,0 +1,52 @@
+##############################################################################
+# (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 PSyclone implementation.
+'''
+
+from unittest import mock
+
+from fab.newtools import (Categories, Psyclone)
+
+
+def test_psyclone_constructor():
+    '''Test the psyclone constructor.'''
+    psyclone = Psyclone()
+    assert psyclone.category == Categories.PSYCLONE
+    assert psyclone.name == "psyclone"
+    assert psyclone.exec_name == "psyclone"
+    assert psyclone.flags == []
+
+
+def test_psyclone_check_available():
+    '''Tests the is_available functionality.'''
+    psyclone = Psyclone()
+    with mock.patch("fab.newtools.tool.Tool.run") as tool_run:
+        assert psyclone.check_available()
+    tool_run.assert_called_once_with("--version")
+
+    # Test behaviour if a runtime error happens:
+    with mock.patch("fab.newtools.tool.Tool.run",
+                    side_effect=RuntimeError("")) as tool_run:
+        assert not psyclone.check_available()
+
+
+def test_psyclone_process():
+    '''Test running PSyclone.'''
+    psyclone = Psyclone()
+    with mock.patch("fab.newtools.tool.Tool.run") as tool_run:
+        psyclone.process(api="dynamo0.3",
+                         x90_file="x90_file",
+                         psy_file="psy_file",
+                         alg_file="alg_file",
+                         transformation_script="transformation_script",
+                         kernel_roots=["root1", "root2"],
+                         additional_parameters=["-c", "psyclone.cfg"])
+    tool_run.assert_called_with(
+        additional_parameters=['-api', 'dynamo0.3', '-l', 'all', '-opsy',
+                               'psy_file', '-oalg', 'alg_file', '-s',
+                               'transformation_script', '-c', 'psyclone.cfg',
+                               '-d', 'root1', '-d', 'root2', 'x90_file'])