588 single file rsync

CONTRIBUTORS.md

@@ -10,10 +10,11 @@ To indicate your agreement, add your details to the the following table.
 If you are not submitting contributions on behalf of an organisation please use
 "n/a" for your affiliation.
 
-| GitHub Username | Real Name       | Affiliation |
-|-----------------|-----------------|-------------|
-| MatthewHambley  | Matthew Hambley | Met Office  |
-| yaswant         | Yaswant Pradhan | Met Office  |
+| GitHub Username | Real Name       | Affiliation                      |
+|-----------------|-----------------|----------------------------------|
+| MatthewHambley  | Matthew Hambley | Met Office                       |
+| yaswant         | Yaswant Pradhan | Met Office                       |
+| hiker           | Joerg Henrichs  | Bureau of Meteorology, Australia |
 
 ---
 

source/fab/api.py

@@ -21,6 +21,7 @@
 from fab.steps.compile_fortran import compile_fortran
 from fab.steps.find_source_files import Exclude, find_source_files, Include
 from fab.steps.grab.fcm import fcm_export
+from fab.steps.grab.files import grab_files
 from fab.steps.grab.folder import grab_folder
 from fab.steps.grab.git import git_checkout
 from fab.steps.grab.prebuild import grab_pre_build
@@ -67,6 +68,7 @@
     "FlagList",
     "get_fab_workspace",
     "git_checkout",
+    "grab_files",
     "grab_folder",
     "grab_pre_build",
     "find_source_files",

source/fab/fab_base/fab_base.py

@@ -28,7 +28,7 @@
 from fab.steps.compile_c import compile_c
 from fab.steps.compile_fortran import compile_fortran
 from fab.steps.find_source_files import find_source_files, Exclude, Include
-from fab.steps.grab.folder import grab_folder
+from fab.steps.grab.files import grab_files
 from fab.steps.link import link_exe, link_shared_object
 from fab.steps.preprocess import preprocess_c, preprocess_fortran
 from fab.tools.category import Category
@@ -656,7 +656,7 @@ def grab_files_step(self) -> None:
         if type(self).__name__ == "FabBase":
             # Do a simple build based on files in "." if FabBase is
             # started by itself (and not inherited):
-            grab_folder(self.config, src=".")
+            grab_files(self.config, src=".")
 
     def find_source_files_step(
             self,

source/fab/steps/grab/files.py

@@ -0,0 +1,38 @@
+# ##############################################################################
+#  (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 grab_files function.
+"""
+
+from pathlib import Path
+from typing import Union
+
+from fab.steps import step
+from fab.tools.category import Category
+
+
+@step
+def grab_files(config, src: Union[Path, str], dst_label: str = ''):
+    """
+    Copy a source file or folder to the project workspace.
+
+    :param config:
+        The :class:`fab.build_config.BuildConfig` object where we can read
+        settings such as the project workspace folder or the multiprocessing
+        flag.
+    :param src:
+        The source location to grab. Either a directory or a file.
+    :param dst_label:
+        The name of a sub folder, in the project workspace, in which to put
+        the source. If not specified, the code is copied into the root of the
+        source folder.
+
+    """
+    dst = config.source_root / dst_label
+    dst.mkdir(parents=True, exist_ok=True)
+    rsync = config.tool_box.get_tool(Category.RSYNC)
+    rsync.execute(src=Path(src), dst=dst)

source/fab/steps/grab/folder.py

@@ -3,29 +3,33 @@
 #  For further details please refer to the file COPYRIGHT
 #  which you should have received as part of this distribution
 # ##############################################################################
+
+"""
+This file contains the grab_folder function.
+"""
+import logging
 from pathlib import Path
 from typing import Union
 
-from fab.steps import step
-from fab.tools.category import Category
+from fab.steps.grab.files import grab_files
+
+logger = logging.getLogger(__name__)
 
 
-@step
 def grab_folder(config, src: Union[Path, str], dst_label: str = ''):
     """
-    Copy a source folder to the project workspace.
+    Copy a source folder to the project workspace. This function is
+    deprecated, use `grab_files` instead.
 
     :param config:
         The :class:`fab.build_config.BuildConfig` object where we can read settings
         such as the project workspace folder or the multiprocessing flag.
     :param src:
-        The source location to grab. The nature of this parameter is depends on the subclass.
+        The source directory or file to grab.
     :param dst_label:
         The name of a sub folder, in the project workspace, in which to put the source.
         If not specified, the code is copied into the root of the source folder.
 
     """
-    _dst = config.source_root / dst_label
-    _dst.mkdir(parents=True, exist_ok=True)
-    rsync = config.tool_box.get_tool(Category.RSYNC)
-    rsync.execute(src=src, dst=_dst)
+    logger.warning("Using deprecated `grab_folder`. Use `grab_files` instead.")
+    grab_files(config, src, dst_label)

source/fab/tools/rsync.py

@@ -7,7 +7,6 @@
 """This file contains the Rsync class for synchronising file trees.
 """
 
-import os
 from pathlib import Path
 from typing import Union
 
@@ -25,18 +24,22 @@ def __init__(self):
         super().__init__("rsync", "rsync", Category.RSYNC)
 
     def execute(self, src: Path,
-                dst: Path):
+                dst: Path) -> str:
         '''Execute an rsync command from src to dst. It supports
         ~ expansion for src, and makes sure that `src` end with a `/`
-        so that rsync does not create a sub-directory.
+        if src is a directory so that rsync does not create a sub-directory.
 
         :param src: the input path.
         :param dst: destination path.
         '''
-        src_str = os.path.expanduser(str(src))
-        if not src_str.endswith('/'):
-            src_str += '/'
-
+        src_abs = src.expanduser().resolve()
+        if src_abs.is_dir():
+            # Ensure that a directory name ends with a '/'
+            src_str = f"{src_abs}/"
+        else:
+            src_str = str(src_abs)
+
+        # Note that run will change Path to str internally
         parameters: list[Union[str, Path]] = [
             '--times', '--links', '--stats', '-ru', src_str, dst]
         return self.run(additional_parameters=parameters)

tests/unit_tests/fab_base/test_fab_base.py

@@ -390,7 +390,7 @@ def test_build_binary(monkeypatch) -> None:
     # We need to patch a lot of Fab functions (to avoid dependencies
     # on the runtime environment):
     mocks = {}
-    for function_name in ["grab_folder", "find_source_files",
+    for function_name in ["grab_files", "find_source_files",
                           "preprocess_c", "preprocess_fortran",
                           "compile_fortran", "compile_c", "analyse"]:
         patcher = mock.patch(f"fab.fab_base.fab_base.{function_name}")
@@ -400,8 +400,8 @@ def test_build_binary(monkeypatch) -> None:
         fab_base.build()
     assert "No target objects defined, linking aborted" in str(err.value)
 
-    mocks["grab_folder"][0].stop()
-    mocks["grab_folder"][1].assert_called_once_with(
+    mocks["grab_files"][0].stop()
+    mocks["grab_files"][1].assert_called_once_with(
         fab_base.config, src=".")
 
     mocks["find_source_files"][0].stop()
@@ -443,16 +443,16 @@ def test_build_static_lib(monkeypatch) -> None:
     # We need to patch a lot of Fab functions (to avoid dependencies
     # on the runtime environment):
     mocks = {}
-    for function_name in ["grab_folder", "find_source_files", "preprocess_c",
+    for function_name in ["grab_files", "find_source_files", "preprocess_c",
                           "preprocess_fortran", "compile_fortran",
                           "compile_c", "analyse", "archive_objects"]:
         patcher = mock.patch(f"fab.fab_base.fab_base.{function_name}")
         mocks[function_name] = (patcher, patcher.start())
 
     fab_base.build()
 
-    mocks["grab_folder"][0].stop()
-    mocks["grab_folder"][1].assert_called_once_with(
+    mocks["grab_files"][0].stop()
+    mocks["grab_files"][1].assert_called_once_with(
         fab_base.config, src=".")
 
     mocks["find_source_files"][0].stop()
@@ -495,16 +495,16 @@ def test_build_shared_lib(monkeypatch) -> None:
     # We need to patch a lot of Fab functions (to avoid dependencies
     # on the runtime environment):
     mocks = {}
-    for function_name in ["grab_folder", "find_source_files", "preprocess_c",
+    for function_name in ["grab_files", "find_source_files", "preprocess_c",
                           "preprocess_fortran", "compile_fortran",
                           "compile_c", "analyse", "link_shared_object"]:
         patcher = mock.patch(f"fab.fab_base.fab_base.{function_name}")
         mocks[function_name] = (patcher, patcher.start())
 
     fab_base.build()
 
-    mocks["grab_folder"][0].stop()
-    mocks["grab_folder"][1].assert_called_once_with(
+    mocks["grab_files"][0].stop()
+    mocks["grab_files"][1].assert_called_once_with(
         fab_base.config, src=".")
 
     mocks["find_source_files"][0].stop()

tests/unit_tests/parse/fortran/test_contained_subroutine.py

@@ -16,7 +16,7 @@
 from fab.steps.analyse import analyse
 from fab.steps.compile_fortran import compile_fortran
 from fab.steps.find_source_files import find_source_files
-from fab.steps.grab.folder import grab_folder
+from fab.steps.grab.files import grab_files
 from fab.steps.link import link_exe
 from fab.tools.category import Category
 from fab.tools.tool_box import ToolBox
@@ -47,7 +47,7 @@ def test_contained_subroutine(tmp_path):
     with BuildConfig(fab_workspace=tmp_path, tool_box=tb,
                      project_label='contained_subroutine',
                      multiprocessing=False) as config:
-        grab_folder(config, PROJECT_SOURCE)
+        grab_files(config, PROJECT_SOURCE)
         find_source_files(config)
         analyse(config, root_symbols='main')
         build_tree = config.artefact_store[ArtefactSet.BUILD_TREES]["main"]

tests/unit_tests/steps/test_grab.py

@@ -6,6 +6,7 @@
 """
 Validate methods to obtain source.
 """
+import logging
 from pathlib import Path
 
 from pyfakefs.fake_filesystem import FakeFilesystem
@@ -14,12 +15,13 @@
 
 from fab.build_config import BuildConfig
 from fab.steps.grab.fcm import fcm_export
+from fab.steps.grab.files import grab_files
 from fab.steps.grab.folder import grab_folder
 from fab.tools.tool_box import ToolBox
 from fab.tools.tool_repository import ToolRepository
 
 
-class TestGrabFolder:
+class TestGrabFiles:
     """
     Tests file directory grabbing.
     """
@@ -30,15 +32,49 @@ class TestGrabFolder:
             ['/grab/source', '/grab/source/']
         ]
     )
-    def test_source_path(self,
+    def test_grab_files(self,
+                        source: str,
+                        expected: str,
+                        stub_tool_repository: ToolRepository,
+                        fs: FakeFilesystem,
+                        fake_process: FakeProcess) -> None:
+        """
+        Tests file directory grabbery.
+        """
+        fs.create_dir("/grab/source")
+        version_command = ['rsync', '--version']
+        fake_process.register(version_command, stdout='1.2.3')
+        grab_command = ['rsync', '--times', '--links', '--stats',
+                        '-ru', expected, '/fab/project/source/bar']
+        fake_process.register(grab_command)
+
+        config = BuildConfig('project', ToolBox(),
+                             mpi=False, openmp=False, multiprocessing=False,
+                             fab_workspace=Path('/fab'))
+
+        with warns(UserWarning,
+                   match="_metric_send_conn not set, cannot send metrics"):
+            grab_files(config, src=source, dst_label='bar')
+        assert fake_process.call_count(grab_command) == 1
+
+    @mark.parametrize(
+        ['source', 'expected'],
+        [
+            ['/grab/source/', '/grab/source/'],
+            ['/grab/source', '/grab/source/']
+        ]
+    )
+    def test_grab_folder(self,
                          source: str,
                          expected: str,
                          stub_tool_repository: ToolRepository,
                          fs: FakeFilesystem,
-                         fake_process: FakeProcess) -> None:
+                         fake_process: FakeProcess,
+                         caplog) -> None:
         """
         Tests file directory grabbery.
         """
+        fs.create_dir("/grab/source")
         version_command = ['rsync', '--version']
         fake_process.register(version_command, stdout='1.2.3')
         grab_command = ['rsync', '--times', '--links', '--stats',
@@ -51,7 +87,10 @@ def test_source_path(self,
 
         with warns(UserWarning,
                    match="_metric_send_conn not set, cannot send metrics"):
-            grab_folder(config, src=source, dst_label='bar')
+            with caplog.at_level(logging.WARNING):
+                grab_folder(config, src=source, dst_label='bar')
+            assert ("Using deprecated `grab_folder`. Use `grab_files` instead."
+                    in caplog.text)
         assert fake_process.call_count(grab_command) == 1
 
 

tests/unit_tests/test_api.py

@@ -38,6 +38,8 @@ def test_import_from_api() -> None:
         "file_checksum",
         "get_fab_workspace",
         "git_checkout",
+        "grab_files",
+        "grab_folder",
         "grab_folder",
         "grab_pre_build",
         "find_source_files",

tests/unit_tests/tools/test_rsync.py

@@ -45,23 +45,36 @@ def test_check_available(fake_process: FakeProcess) -> None:
     ]
 
 
-def test_rsync_create(fake_process: FakeProcess) -> None:
+def test_rsync_create(fake_process: FakeProcess,
+                      change_into_tmpdir: Path) -> None:
     """
     Tests performing a sync. Ensure source always ends with a '/'.
     """
-    with_command = ['rsync', '--times', '--links', '--stats', '-ru', '/src/', '/dst']
-    fake_process.register(with_command)
-    without_command = ['rsync', '--times', '--links', '--stats', '-ru', '/src/', '/dst']
-    fake_process.register(without_command)
+    tmp_dir = change_into_tmpdir
+    # Create a directory"
+    directory = tmp_dir / "directory"
+    directory.mkdir()
+    file = tmp_dir / "file"
+    file.write_text("A file\n")
 
     rsync = Rsync()
 
-    # Test 1: src with /
-    rsync.execute(src=Path("/src/"), dst=Path("/dst"))
+    # Test 1: Directory must have a '/' at the end:
+    dir_command = ['rsync', '--times', '--links', '--stats', '-ru',
+                   f'{directory}/', '/dst']
+    fake_process.register(dir_command)
+    rsync.execute(src=directory, dst=Path("/dst"))
 
-    # Test 2: src without /
-    rsync.execute(src=Path("/src"), dst=Path("/dst"))
+    # Test 2: a file should not have a '/' at the end. First ensure
+    # that file does indeed not have a '/' at the end (Path should discard
+    # trailing / ... but just in case:)
+    assert str(file)[-1] != "/"
+    file_command = ['rsync', '--times', '--links', '--stats', '-ru',
+                    f'{file}', '/dst']
+    fake_process.register(file_command)
+
+    rsync.execute(src=file, dst=Path("/dst"))
 
     assert call_list(fake_process) == [
-        with_command, without_command
+        dir_command, file_command
     ]