storage: Support mtime property in Directory.import_files()

3 files changed, 86 insertions, 20 deletions

diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py
index 51b75ac19..624d071dd 100644
--- a/src/buildstream/storage/_casbaseddirectory.py
+++ b/src/buildstream/storage/_casbaseddirectory.py
@@ -29,13 +29,14 @@ See also: :ref:`sandboxing`.
 
 import os
 import stat
+import copy
 import tarfile as tarfilelib
 from io import StringIO
 
 from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2
 from .directory import Directory, VirtualDirectoryError, _FileType
 from ._filebaseddirectory import FileBasedDirectory
-from ..utils import FileListResult, BST_ARBITRARY_TIMESTAMP
+from ..utils import FileListResult, BST_ARBITRARY_TIMESTAMP, _get_file_mtimestamp
 
 
 class IndexEntry:
@@ -50,7 +51,8 @@ class IndexEntry:
         target=None,
         is_executable=False,
         buildstream_object=None,
-        modified=False
+        modified=False,
+        node_properties=None
     ):
         self.name = name
         self.type = entrytype
@@ -59,6 +61,7 @@ class IndexEntry:
         self.is_executable = is_executable
         self.buildstream_object = buildstream_object
         self.modified = modified
+        self.node_properties = copy.deepcopy(node_properties)
 
     def get_directory(self, parent):
         if not self.buildstream_object:
@@ -126,7 +129,11 @@ class CasBasedDirectory(Directory):
             self.index[entry.name] = IndexEntry(entry.name, _FileType.DIRECTORY, digest=entry.digest)
         for entry in pb2_directory.files:
             self.index[entry.name] = IndexEntry(
-                entry.name, _FileType.REGULAR_FILE, digest=entry.digest, is_executable=entry.is_executable
+                entry.name,
+                _FileType.REGULAR_FILE,
+                digest=entry.digest,
+                is_executable=entry.is_executable,
+                node_properties=list(entry.node_properties),
             )
         for entry in pb2_directory.symlinks:
             self.index[entry.name] = IndexEntry(entry.name, _FileType.SYMLINK, target=entry.target)
@@ -150,11 +157,20 @@ class CasBasedDirectory(Directory):
 
         return newdir
 
-    def _add_file(self, basename, filename, modified=False, can_link=False):
+    def _add_file(self, basename, filename, modified=False, can_link=False, properties=None):
         entry = IndexEntry(filename, _FileType.REGULAR_FILE, modified=modified or filename in self.index)
         path = os.path.join(basename, filename)
         entry.digest = self.cas_cache.add_object(path=path, link_directly=can_link)
         entry.is_executable = os.access(path, os.X_OK)
+        properties = properties or []
+        # see https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/nodeproperties.md
+        # for supported node property specifications
+        entry.node_properties = []
+        if "MTime" in properties:
+            node_property = remote_execution_pb2.NodeProperty()
+            node_property.name = "MTime"
+            node_property.value = _get_file_mtimestamp(path)
+            entry.node_properties.append(node_property)
         self.index[filename] = entry
 
         self.__invalidate_digest()
@@ -333,6 +349,7 @@ class CasBasedDirectory(Directory):
                             digest=entry.digest,
                             is_executable=entry.is_executable,
                             modified=True,
+                            node_properties=entry.node_properties,
                         )
                         self.__invalidate_digest()
                     else:
@@ -341,7 +358,14 @@ class CasBasedDirectory(Directory):
                     result.files_written.append(relative_pathname)
 
     def import_files(
-        self, external_pathspec, *, filter_callback=None, report_written=True, update_mtime=None, can_link=False
+        self,
+        external_pathspec,
+        *,
+        filter_callback=None,
+        report_written=True,
+        update_mtime=None,
+        can_link=False,
+        properties=None
     ):
         """ See superclass Directory for arguments """
 
@@ -356,7 +380,7 @@ class CasBasedDirectory(Directory):
             # content into this CasBasedDirectory using CAS-to-CAS import
             # to write the report, handle possible conflicts (if the target
             # directory is not empty) and apply the optional filter.
-            digest = self.cas_cache.import_directory(external_pathspec)
+            digest = self.cas_cache.import_directory(external_pathspec, properties=properties)
             external_pathspec = CasBasedDirectory(self.cas_cache, digest=digest)
 
         assert isinstance(external_pathspec, CasBasedDirectory)
@@ -368,13 +392,14 @@ class CasBasedDirectory(Directory):
 
         return result
 
-    def import_single_file(self, external_pathspec):
+    def import_single_file(self, external_pathspec, properties=None):
         result = FileListResult()
         if self._check_replacement(os.path.basename(external_pathspec), os.path.dirname(external_pathspec), result):
             self._add_file(
                 os.path.dirname(external_pathspec),
                 os.path.basename(external_pathspec),
                 modified=os.path.basename(external_pathspec) in result.overwritten,
+                properties=properties,
             )
             result.files_written.append(external_pathspec)
         return result
@@ -639,6 +664,9 @@ class CasBasedDirectory(Directory):
                     filenode.name = name
                     filenode.digest.CopyFrom(entry.digest)
                     filenode.is_executable = entry.is_executable
+                    if entry.node_properties:
+                        node_properties = sorted(entry.node_properties, key=lambda prop: prop.name)
+                        filenode.node_properties.extend(node_properties)
                 elif entry.type == _FileType.SYMLINK:
                     symlinknode = pb2_directory.symlinks.add()
                     symlinknode.name = name
diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py
index c2ff06a66..7b745f777 100644
--- a/src/buildstream/storage/_filebaseddirectory.py
+++ b/src/buildstream/storage/_filebaseddirectory.py
@@ -78,20 +78,29 @@ class FileBasedDirectory(Directory):
         return current_dir
 
     def import_files(
-        self, external_pathspec, *, filter_callback=None, report_written=True, update_mtime=None, can_link=False
+        self,
+        external_pathspec,
+        *,
+        filter_callback=None,
+        report_written=True,
+        update_mtime=None,
+        can_link=False,
+        properties=None
     ):
         """ See superclass Directory for arguments """
 
         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
@@ -114,13 +123,13 @@ 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))
 
-        if update_mtime:
-            for f in import_result.files_written:
-                os.utime(os.path.join(self.external_directory, f), times=(update_mtime, update_mtime))
         return import_result
 
-    def import_single_file(self, external_pathspec):
+    def import_single_file(self, external_pathspec, properties=None):
         dstpath = os.path.join(self.external_directory, os.path.basename(external_pathspec))
         result = FileListResult()
         if os.path.exists(dstpath):
@@ -238,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():
@@ -263,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):
@@ -286,7 +302,25 @@ class FileBasedDirectory(Directory):
 
                 if entry.type == _FileType.REGULAR_FILE:
                     src_path = source_directory.cas_cache.objpath(entry.digest)
-                    actionfunc(src_path, dest_path, result=result)
+
+                    # fallback to copying if we require mtime support on this file
+                    if update_mtime or entry.node_properties:
+                        utils.safe_copy(src_path, dest_path, result=result)
+                        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:
+                        actionfunc(src_path, dest_path, result=result)
+
                     if entry.is_executable:
                         os.chmod(
                             dest_path,
@@ -298,6 +332,7 @@ class FileBasedDirectory(Directory):
                             | stat.S_IROTH
                             | stat.S_IXOTH,
                         )
+
                 else:
                     assert entry.type == _FileType.SYMLINK
                     os.symlink(entry.target, dest_path)
diff --git a/src/buildstream/storage/directory.py b/src/buildstream/storage/directory.py
index 771c11ae7..f0aab7c10 100644
--- a/src/buildstream/storage/directory.py
+++ b/src/buildstream/storage/directory.py
@@ -32,7 +32,7 @@ See also: :ref:`sandboxing`.
 """
 
 
-from typing import Callable, Optional, Union
+from typing import Callable, Optional, Union, List
 
 from .._exceptions import BstError
 from ..exceptions import ErrorDomain
@@ -83,7 +83,8 @@ class Directory:
         filter_callback: Optional[Callable[[str], bool]] = None,
         report_written: bool = True,
         update_mtime: Optional[float] = None,
-        can_link: bool = False
+        can_link: bool = False,
+        properties: Optional[List[str]] = None
     ) -> FileListResult:
         """Imports some or all files from external_path into this directory.
 
@@ -103,6 +104,8 @@ class Directory:
             original content, meaning the stored copy will change when the
             original files change. Setting this doesn't guarantee hard
             links will be made.
+          properties: Optional list of strings representing file properties
+            to capture when importing.
 
         Yields:
           A report of files imported and overwritten.
@@ -111,7 +114,7 @@ class Directory:
 
         raise NotImplementedError()
 
-    def import_single_file(self, external_pathspec):
+    def import_single_file(self, external_pathspec, properties=None):
         """Imports a single file from an external path"""
         raise NotImplementedError()