From d44296c9db95c2fb27c455a6b6fe1d023a4c4524 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 6 Mar 2025 11:22:36 +1100 Subject: [PATCH 1/2] Merge main into develop (#394) * Improve support for MPI wrappers (#352) * Update clang binding module. (#376) * Cleaner handling of linking external libraries (#374) * Support additional compilers (#385) * Cray * Icx (LLVM Intel) * nVidia * Psyclone 3 support (#388) * Fixed typo in comment. --------- Co-authored-by: Matthew Hambley --- source/fab/tools/psyclone.py | 47 +++---------- tests/unit_tests/tools/test_psyclone.py | 89 ++++++------------------- 2 files changed, 27 insertions(+), 109 deletions(-) diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index 066b05be..6e4adbbf 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -38,20 +38,10 @@ def check_available(self) -> bool: # First get the version (and confirm that PSyclone is installed): try: - # Older versions of PSyclone (2.3.1 and earlier) expect a filename - # even when --version is used, and won't produce version info - # without this. So provide a dummy file (which does not need to - # exist), and check the error for details to see if PSyclone does - # not exist, or if the error is because of the non-existing file - version_output = self.run(["--version", "does_not_exist"], - capture_output=True) - except RuntimeError as err: - # If the command is not found, the error contains the following: - if "could not be executed" in str(err): - return False - # Otherwise, psyclone likely complained about the not existing - # file. Continue and try to find version information in the output: - version_output = str(err) + version_output = self.run(["--version"], capture_output=True) + except RuntimeError: + # Something is wrong, report as not available + return False # Search for the version info: exp = r"PSyclone version: (\d[\d.]+\d)" @@ -64,29 +54,8 @@ def check_available(self) -> bool: # Now convert the version info to integer. The regular expression # match guarantees that we have integer numbers now: - version = tuple(int(x) for x in matches.groups()[0].split('.')) - - if version == (2, 5, 0): - # The behaviour of PSyclone changes from 2.5.0 to the next - # release. But since head-of-trunk still reports 2.5.0, we - # need to run additional tests to see if we have the official - # 2.5.0 release, or current trunk (which already has the new - # command line options). PSyclone needs an existing file - # in order to work, so use __file__ to present this file. - # PSyclone will obviously abort since this is not a Fortran - # file, but we only need to check the error message to - # see if the domain name is incorrect (--> current trunk) - # or not (2.5.0 release) - try: - self.run(["-api", "nemo", __file__], capture_output=True) - except RuntimeError as err: - if "Unsupported PSyKAL DSL / API 'nemo' specified" in str(err): - # It is current development. Just give it a version number - # greater than 2.5.0 for now, till the official release - # is done. - version = (2, 5, 0, 1) - - self._version = version + self._version = tuple(int(x) for x in matches.groups()[0].split('.')) + return True def process(self, @@ -156,7 +125,7 @@ def process(self, # transformation tool only, so calling PSyclone without api is # actually valid. if api: - if self._version > (2, 5, 0): + if self._version >= (3, 0, 0): api_param = "--psykal-dsl" # Mapping from old names to new names: mapping = {"dynamo0.3": "lfric", @@ -176,7 +145,7 @@ def process(self, # Make mypy happy - we tested above that transformed_file is # specified when no api is specified. assert transformed_file - if self._version > (2, 5, 0): + if self._version >= (3, 0, 0): # New version: no API, parameter, but -o for output name: parameters.extend(["-o", transformed_file]) else: diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py index 52cd2404..1d2d3f2a 100644 --- a/tests/unit_tests/tools/test_psyclone.py +++ b/tests/unit_tests/tools/test_psyclone.py @@ -40,70 +40,24 @@ def test_psyclone_constructor(): assert psyclone.flags == [] -def test_psyclone_check_available_2_4_0(): - '''Tests the is_available functionality with version 2.4.0. - We get only one call. +@pytest.mark.parametrize("version", ["2.4.0", "2.5.0", "3.0.0", "3.1.0"]) +def test_psyclone_check_available_and_version(version): + '''Tests the is_available functionality and version number detection + with PSyclone. Note that the version number is only used internally, + so we test with the private attribute. ''' psyclone = Psyclone() - - mock_result = get_mock_result("2.4.0") + version_tuple = tuple(int(i) for i in version.split(".")) + mock_result = get_mock_result(version) with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: assert psyclone.check_available() - tool_run.assert_called_once_with( - ["psyclone", "--version", mock.ANY], capture_output=True, - env=None, cwd=None, check=False) - - -def test_psyclone_check_available_2_5_0(): - '''Tests the is_available functionality with PSyclone 2.5.0. - We get two calls. First version, then check if nemo API exists - ''' - psyclone = Psyclone() + assert psyclone._version == version_tuple - mock_result = get_mock_result("2.5.0") - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result) as tool_run: - assert psyclone.check_available() - tool_run.assert_any_call( - ["psyclone", "--version", mock.ANY], capture_output=True, - env=None, cwd=None, check=False) - tool_run.assert_any_call( - ["psyclone", "-api", "nemo", mock.ANY], capture_output=True, + tool_run.assert_called_once_with( + ["psyclone", "--version"], 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: - with pytest.warns(UserWarning, - match="Unexpected version information " - "for PSyclone: ''."): - assert not psyclone.check_available() - - -def test_psyclone_check_available_after_2_5_0(): - '''Tests the is_available functionality with releases after 2.5.0. - We get two calls. First version, then check if nemo API exists - ''' - psyclone = Psyclone() - - # We detect the dummy version '2.5.0.1' if psyclone reports 2.5.0 - # but the command line option "-api nemo" is not accepted. - # So we need to return two results from our mock objects: first - # success for version 2.5.0, then a failure with an appropriate - # error message: - mock_result1 = get_mock_result("2.5.0") - mock_result2 = get_mock_result("Unsupported PSyKAL DSL / " - "API 'nemo' specified") - mock_result2.returncode = 1 - - # "Unsupported PSyKAL DSL / API 'nemo' specified" - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result1) as tool_run: - tool_run.side_effect = [mock_result1, mock_result2] - assert psyclone.check_available() - assert psyclone._version == (2, 5, 0, 1) - def test_psyclone_check_available_errors(): '''Test various errors that can happen in check_available. @@ -270,14 +224,12 @@ def test_psyclone_process_nemo_api_old_psyclone(version): ("gocean", "gocean") ]) def test_psyclone_process_api_new_psyclone(api): - '''Test running the new PSyclone version. Since this version is not - yet released, we use the Fab internal version number 2.5.0.1 for - now. It uses new API names, and we need to check that the old style - names are converted to the new names. + '''Test running PSyclone 3.0.0. It uses new API names, and we need to + check that the old style names are converted to the new names. ''' api_in, api_out = api psyclone = Psyclone() - mock_result = get_mock_result("2.5.0.1") + mock_result = get_mock_result("3.0.0") transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() with mock.patch('fab.tools.tool.subprocess.run', @@ -298,12 +250,11 @@ def test_psyclone_process_api_new_psyclone(api): def test_psyclone_process_no_api_new_psyclone(): - '''Test running the new PSyclone version without an API. Since this - version is not yet released, we use the Fab internal version number - 2.5.0.1 for now. + '''Test running the PSyclone 3.0.0 without an API, i.e. as transformation + only. ''' psyclone = Psyclone() - mock_result = get_mock_result("2.5.0.1") + mock_result = get_mock_result("3.0.0") transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() @@ -324,13 +275,11 @@ def test_psyclone_process_no_api_new_psyclone(): def test_psyclone_process_nemo_api_new_psyclone(): - '''Test running PSyclone. Since this version is not yet released, we use - the Fab internal version number 2.5.0.1 for now. This tests that - backwards compatibility of using the nemo api works, i.e. '-api nemo' is - just removed. + '''Test running PSyclone 3.0.0 and test that backwards compatibility of + using the nemo api works, i.e. '-api nemo' is just removed. ''' psyclone = Psyclone() - mock_result = get_mock_result("2.5.0.1") + mock_result = get_mock_result("3.0.0") transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() From c021b83cfb20ea3ffd8027dc3898efeb91bbcab4 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 6 Mar 2025 11:49:08 +1100 Subject: [PATCH 2/2] Only add 'sh' as shell not bash etc. --- source/fab/tools/tool_repository.py | 10 ++++++---- tests/unit_tests/tools/test_tool_repository.py | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/fab/tools/tool_repository.py b/source/fab/tools/tool_repository.py index a9749757..f514c27e 100644 --- a/source/fab/tools/tool_repository.py +++ b/source/fab/tools/tool_repository.py @@ -72,10 +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"]: + # Add a standard shell. Additional shells (bash, ksh, dash) + # can be created by just adding their names to the list. While Fab + # itself does not need this, it is a very convenient tool for user + # configurations (e.g. to query `nf-config` etc), since we don't + # allow a shell to be used in Python's subprocess. + for shell_name in ["sh"]: self.add_tool(Shell(shell_name)) # Now create the potential mpif90 and Cray ftn wrapper diff --git a/tests/unit_tests/tools/test_tool_repository.py b/tests/unit_tests/tools/test_tool_repository.py index 012487d4..e557c175 100644 --- a/tests/unit_tests/tools/test_tool_repository.py +++ b/tests/unit_tests/tools/test_tool_repository.py @@ -178,5 +178,5 @@ def test_tool_repository_no_tool_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)) + assert ("Can't find available 'SHELL' tool. Tools are 'sh'" + in str(err.value))