Use specified mtime in import_files

if the mtime is specified then try to copy files in file-to-file
transfers and reset the mtimes to the parameter value

3 files changed, 33 insertions, 26 deletions

diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py
index 015959ed4..93facd6aa 100644
--- a/src/buildstream/storage/_casbaseddirectory.py
+++ b/src/buildstream/storage/_casbaseddirectory.py
@@ -363,7 +363,7 @@ class CasBasedDirectory(Directory):
         *,
         filter_callback=None,
         report_written=True,
-        update_mtime=False,
+        update_mtime=None,
         can_link=False,
         properties=None
     ):
diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py
index 4c6c92fc3..7b745f777 100644
--- a/src/buildstream/storage/_filebaseddirectory.py
+++ b/src/buildstream/storage/_filebaseddirectory.py
@@ -30,7 +30,6 @@ See also: :ref:`sandboxing`.
 import os
 import shutil
 import stat
-import time
 
 from .directory import Directory, VirtualDirectoryError, _FileType
 from .. import utils
@@ -84,7 +83,7 @@ class FileBasedDirectory(Directory):
         *,
         filter_callback=None,
         report_written=True,
-        update_mtime=False,
+        update_mtime=None,
         can_link=False,
         properties=None
     ):
@@ -93,13 +92,15 @@ class FileBasedDirectory(Directory):
         from ._casbaseddirectory import CasBasedDirectory  # pylint: disable=cyclic-import
 
         if isinstance(external_pathspec, CasBasedDirectory):
-            if can_link and not update_mtime:
+            if can_link:
                 actionfunc = utils.safe_link
             else:
                 actionfunc = utils.safe_copy
 
             import_result = FileListResult()
-            self._import_files_from_cas(external_pathspec, actionfunc, filter_callback, result=import_result)
+            self._import_files_from_cas(
+                external_pathspec, actionfunc, filter_callback, update_mtime=update_mtime, result=import_result,
+            )
         else:
             if isinstance(external_pathspec, Directory):
                 source_directory = external_pathspec.external_directory
@@ -122,14 +123,10 @@ class FileBasedDirectory(Directory):
                     ignore_missing=False,
                     report_written=report_written,
                 )
+                if update_mtime:
+                    for f in import_result.files_written:
+                        os.utime(os.path.join(self.external_directory, f), times=(update_mtime, update_mtime))
 
-        # do not update times if these were set via nodes
-        properties = properties or []
-        if update_mtime and "MTime" not in properties:
-            cur_time = time.time()
-
-            for f in import_result.files_written:
-                os.utime(os.path.join(self.external_directory, f), times=(cur_time, cur_time))
         return import_result
 
     def import_single_file(self, external_pathspec, properties=None):
@@ -250,7 +247,9 @@ class FileBasedDirectory(Directory):
         else:
             return _FileType.SPECIAL_FILE
 
-    def _import_files_from_cas(self, source_directory, actionfunc, filter_callback, *, path_prefix="", result):
+    def _import_files_from_cas(
+        self, source_directory, actionfunc, filter_callback, *, path_prefix="", update_mtime=None, result
+    ):
         """ Import files from a CAS-based directory. """
 
         for name, entry in source_directory.index.items():
@@ -275,7 +274,12 @@ class FileBasedDirectory(Directory):
                     )
 
                 dest_subdir._import_files_from_cas(
-                    src_subdir, actionfunc, filter_callback, path_prefix=relative_pathname, result=result
+                    src_subdir,
+                    actionfunc,
+                    filter_callback,
+                    path_prefix=relative_pathname,
+                    result=result,
+                    update_mtime=update_mtime,
                 )
 
             if filter_callback and not filter_callback(relative_pathname):
@@ -300,18 +304,22 @@ class FileBasedDirectory(Directory):
                     src_path = source_directory.cas_cache.objpath(entry.digest)
 
                     # fallback to copying if we require mtime support on this file
-                    if entry.node_properties:
+                    if update_mtime or entry.node_properties:
                         utils.safe_copy(src_path, dest_path, result=result)
-                        mtime = None
-                        for prop in entry.node_properties:
-                            if prop.name == "MTime" and prop.value:
-                                mtime = prop.value
-                            else:
-                                raise ImplError("{} is not a supported node property.".format(prop.name))
+                        mtime = update_mtime
+                        # mtime property will override specified mtime
+                        # see https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/nodeproperties.md
+                        # for supported node property specifications
+                        if entry.node_properties:
+                            for prop in entry.node_properties:
+                                if prop.name == "MTime" and prop.value:
+                                    mtime = utils._parse_timestamp(prop.value)
+                                else:
+                                    raise ImplError("{} is not a supported node property.".format(prop.name))
                         if mtime:
                             utils._set_file_mtime(dest_path, mtime)
                     else:
-                        utils.safe_link(src_path, dest_path, result=result)
+                        actionfunc(src_path, dest_path, result=result)
 
                     if entry.is_executable:
                         os.chmod(
diff --git a/src/buildstream/storage/directory.py b/src/buildstream/storage/directory.py
index ecba82d60..f0aab7c10 100644
--- a/src/buildstream/storage/directory.py
+++ b/src/buildstream/storage/directory.py
@@ -82,7 +82,7 @@ class Directory:
         *,
         filter_callback: Optional[Callable[[str], bool]] = None,
         report_written: bool = True,
-        update_mtime: bool = False,
+        update_mtime: Optional[float] = None,
         can_link: bool = False,
         properties: Optional[List[str]] = None
     ) -> FileListResult:
@@ -99,12 +99,11 @@ class Directory:
             written. Defaults to true. If false, only a list of
             overwritten files is returned.
           update_mtime: Update the access and modification time
-            of each file copied to the current time.
+            of each file copied to the time specified in seconds.
           can_link: Whether it's OK to create a hard link to the
             original content, meaning the stored copy will change when the
             original files change. Setting this doesn't guarantee hard
-            links will be made. can_link will never be used if
-            update_mtime is set.
+            links will be made.
           properties: Optional list of strings representing file properties
             to capture when importing.