Merge branch 'juerg/fast-import' into 'master'

Improve import performance

See merge request BuildStream/buildstream!1190

12 files changed, 183 insertions, 103 deletions

diff --git a/buildstream/_artifactcache.py b/buildstream/_artifactcache.py
index 9986d689f..330365025 100644
--- a/buildstream/_artifactcache.py
+++ b/buildstream/_artifactcache.py
@@ -692,9 +692,8 @@ class ArtifactCache():
     #     logsdir (CasBasedDirectory): A CasBasedDirectory containing the artifact's logs
     #
     def get_artifact_logs(self, ref):
-        descend = ["logs"]
         cache_id = self.cas.resolve_ref(ref, update_mtime=True)
-        vdir = CasBasedDirectory(self.cas, digest=cache_id).descend(descend)
+        vdir = CasBasedDirectory(self.cas, digest=cache_id).descend('logs')
         return vdir
 
     ################################################
diff --git a/buildstream/element.py b/buildstream/element.py
index 9d1333721..47ca04c28 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -178,6 +178,12 @@ class Element(Plugin):
     *Since: 1.4*
     """
 
+    BST_RUN_COMMANDS = True
+    """Whether the element may run commands using Sandbox.run.
+
+    *Since: 1.4*
+    """
+
     def __init__(self, context, project, meta, plugin_conf):
 
         self.__cache_key_dict = None            # Dict for cache key calculation
@@ -663,14 +669,14 @@ class Element(Plugin):
 
         with self.timed_activity("Staging {}/{}".format(self.name, self._get_brief_display_key())):
             artifact_vdir, _ = self.__get_artifact_directory()
-            files_vdir = artifact_vdir.descend(['files'])
+            files_vdir = artifact_vdir.descend('files')
 
             # Hard link it into the staging area
             #
             vbasedir = sandbox.get_virtual_directory()
             vstagedir = vbasedir \
                 if path is None \
-                else vbasedir.descend(path.lstrip(os.sep).split(os.sep))
+                else vbasedir.descend(*path.lstrip(os.sep).split(os.sep))
 
             split_filter = self.__split_filter_func(include, exclude, orphans)
 
@@ -1439,7 +1445,7 @@ class Element(Plugin):
 
         # Stage all sources that need to be copied
         sandbox_vroot = sandbox.get_virtual_directory()
-        host_vdirectory = sandbox_vroot.descend(directory.lstrip(os.sep).split(os.sep), create=True)
+        host_vdirectory = sandbox_vroot.descend(*directory.lstrip(os.sep).split(os.sep), create=True)
         self._stage_sources_at(host_vdirectory, mount_workspaces=mount_workspaces, usebuildtree=sandbox._usebuildtree)
 
     # _stage_sources_at():
@@ -1478,7 +1484,7 @@ class Element(Plugin):
             # Check if we have a cached buildtree to use
             elif usebuildtree:
                 artifact_vdir, _ = self.__get_artifact_directory()
-                import_dir = artifact_vdir.descend(['buildtree'])
+                import_dir = artifact_vdir.descend('buildtree')
                 if import_dir.is_empty():
                     detail = "Element type either does not expect a buildtree or it was explictily cached without one."
                     self.warn("WARNING: {} Artifact contains an empty buildtree".format(self.name), detail=detail)
@@ -1624,6 +1630,13 @@ class Element(Plugin):
             with _signals.terminator(cleanup_rootdir), \
                 self.__sandbox(rootdir, output_file, output_file, self.__sandbox_config) as sandbox:  # noqa
 
+                if not self.BST_RUN_COMMANDS:
+                    # Element doesn't need to run any commands in the sandbox.
+                    #
+                    # Disable Sandbox.run() to allow CasBasedDirectory for all
+                    # sandboxes.
+                    sandbox._disable_run()
+
                 # By default, the dynamic public data is the same as the static public data.
                 # The plugin's assemble() method may modify this, though.
                 self.__dynamic_public = _yaml.node_copy(self.__public)
@@ -1663,7 +1676,7 @@ class Element(Plugin):
                     if workspace and self.__staged_sources_directory:
                         sandbox_vroot = sandbox.get_virtual_directory()
                         path_components = self.__staged_sources_directory.lstrip(os.sep).split(os.sep)
-                        sandbox_vpath = sandbox_vroot.descend(path_components)
+                        sandbox_vpath = sandbox_vroot.descend(*path_components)
                         try:
                             sandbox_vpath.import_files(workspace.get_absolute_path())
                         except UtilError as e2:
@@ -1684,7 +1697,7 @@ class Element(Plugin):
             if collect is not None:
                 try:
                     sandbox_vroot = sandbox.get_virtual_directory()
-                    collectvdir = sandbox_vroot.descend(collect.lstrip(os.sep).split(os.sep))
+                    collectvdir = sandbox_vroot.descend(*collect.lstrip(os.sep).split(os.sep))
                 except VirtualDirectoryError:
                     # No collect directory existed
                     collectvdir = None
@@ -1721,7 +1734,7 @@ class Element(Plugin):
                 sandbox_vroot = sandbox.get_virtual_directory()
                 try:
                     sandbox_build_dir = sandbox_vroot.descend(
-                        self.get_variable('build-root').lstrip(os.sep).split(os.sep))
+                        *self.get_variable('build-root').lstrip(os.sep).split(os.sep))
                     buildtreevdir.import_files(sandbox_build_dir)
                 except VirtualDirectoryError:
                     # Directory could not be found. Pre-virtual
@@ -2645,7 +2658,7 @@ class Element(Plugin):
         filter_func = self.__split_filter_func(include=include, exclude=exclude, orphans=orphans)
 
         artifact_vdir, _ = self.__get_artifact_directory()
-        files_vdir = artifact_vdir.descend(['files'])
+        files_vdir = artifact_vdir.descend('files')
 
         element_files = files_vdir.list_relative_paths()
 
diff --git a/buildstream/plugins/elements/compose.py b/buildstream/plugins/elements/compose.py
index 12520ce4c..f45ffd76a 100644
--- a/buildstream/plugins/elements/compose.py
+++ b/buildstream/plugins/elements/compose.py
@@ -156,7 +156,7 @@ class ComposeElement(Element):
         # instead of into a subdir. The element assemble() method should
         # support this in some way.
         #
-        installdir = vbasedir.descend(['buildstream', 'install'], create=True)
+        installdir = vbasedir.descend('buildstream', 'install', create=True)
 
         # We already saved the manifest for created files in the integration phase,
         # now collect the rest of the manifest.
diff --git a/buildstream/plugins/elements/filter.py b/buildstream/plugins/elements/filter.py
index 5a2db2c44..232f4ccca 100644
--- a/buildstream/plugins/elements/filter.py
+++ b/buildstream/plugins/elements/filter.py
@@ -160,6 +160,12 @@ class FilterElement(Element):
     # added, to reduce the potential for confusion
     BST_FORBID_SOURCES = True
 
+    # This plugin has been modified to avoid the use of Sandbox.get_directory
+    BST_VIRTUAL_DIRECTORY = True
+
+    # Filter elements do not run any commands
+    BST_RUN_COMMANDS = False
+
     def configure(self, node):
         self.node_validate(node, [
             'include', 'exclude', 'include-orphans'
diff --git a/buildstream/plugins/elements/import.py b/buildstream/plugins/elements/import.py
index 3ae979d7a..73884214f 100644
--- a/buildstream/plugins/elements/import.py
+++ b/buildstream/plugins/elements/import.py
@@ -41,6 +41,9 @@ class ImportElement(Element):
     # This plugin has been modified to avoid the use of Sandbox.get_directory
     BST_VIRTUAL_DIRECTORY = True
 
+    # Import elements do not run any commands
+    BST_RUN_COMMANDS = False
+
     def configure(self, node):
         self.node_validate(node, [
             'source', 'target'
@@ -75,14 +78,14 @@ class ImportElement(Element):
         self._stage_sources_in_sandbox(sandbox, 'input', mount_workspaces=False)
 
         rootdir = sandbox.get_virtual_directory()
-        inputdir = rootdir.descend(['input'])
-        outputdir = rootdir.descend(['output'], create=True)
+        inputdir = rootdir.descend('input')
+        outputdir = rootdir.descend('output', create=True)
 
         # The directory to grab
-        inputdir = inputdir.descend(self.source.strip(os.sep).split(os.sep))
+        inputdir = inputdir.descend(*self.source.strip(os.sep).split(os.sep))
 
         # The output target directory
-        outputdir = outputdir.descend(self.target.strip(os.sep).split(os.sep), create=True)
+        outputdir = outputdir.descend(*self.target.strip(os.sep).split(os.sep), create=True)
 
         if inputdir.is_empty():
             raise ElementError("{}: No files were found inside directory '{}'"
diff --git a/buildstream/plugins/elements/stack.py b/buildstream/plugins/elements/stack.py
index 138afedf7..b26281fe4 100644
--- a/buildstream/plugins/elements/stack.py
+++ b/buildstream/plugins/elements/stack.py
@@ -63,7 +63,7 @@ class StackElement(Element):
         # the actual artifact data in a subdirectory, then we
         # will be able to store some additional state in the
         # artifact cache, and we can also remove this hack.
-        vrootdir.descend(['output', 'bst'], create=True)
+        vrootdir.descend('output', 'bst', create=True)
 
         # And we're done
         return '/output'
diff --git a/buildstream/sandbox/_sandboxremote.py b/buildstream/sandbox/_sandboxremote.py
index 9ca4738be..348ebca1b 100644
--- a/buildstream/sandbox/_sandboxremote.py
+++ b/buildstream/sandbox/_sandboxremote.py
@@ -317,7 +317,7 @@ class SandboxRemote(Sandbox):
         for mark in self._get_marked_directories():
             directory = mark['directory']
             # Create each marked directory
-            upload_vdir.descend(directory.split(os.path.sep), create=True)
+            upload_vdir.descend(*directory.split(os.path.sep), create=True)
 
         # Generate action_digest first
         input_root_digest = upload_vdir._get_digest()
diff --git a/buildstream/sandbox/sandbox.py b/buildstream/sandbox/sandbox.py
index ca4652599..f11ddea1d 100644
--- a/buildstream/sandbox/sandbox.py
+++ b/buildstream/sandbox/sandbox.py
@@ -117,6 +117,7 @@ class Sandbox():
         self.__env = None
         self.__mount_sources = {}
         self.__allow_real_directory = kwargs['allow_real_directory']
+        self.__allow_run = True
 
         # Plugin ID for logging
         plugin = kwargs.get('plugin', None)
@@ -190,8 +191,9 @@ class Sandbox():
 
         """
         if self._vdir is None or self._never_cache_vdirs:
-            if 'BST_CAS_DIRECTORIES' in os.environ:
-                self._vdir = CasBasedDirectory(self.__context.artifactcache.cas)
+            if self._use_cas_based_directory():
+                cascache = self.__context.get_cascache()
+                self._vdir = CasBasedDirectory(cascache)
             else:
                 self._vdir = FileBasedDirectory(self._root)
         return self._vdir
@@ -278,6 +280,9 @@ class Sandbox():
            not exist yet, even if a workspace is being used.
         """
 
+        if not self.__allow_run:
+            raise SandboxError("Sandbox.run() has been disabled")
+
         # Fallback to the sandbox default settings for
         # the cwd and env.
         #
@@ -386,6 +391,22 @@ class Sandbox():
     def _create_batch(self, main_group, flags, *, collect=None):
         return _SandboxBatch(self, main_group, flags, collect=collect)
 
+    # _use_cas_based_directory()
+    #
+    # Whether to use CasBasedDirectory as sandbox root. If this returns `False`,
+    # FileBasedDirectory will be used.
+    #
+    # Returns:
+    #    (bool): Whether to use CasBasedDirectory
+    #
+    def _use_cas_based_directory(self):
+        # Use CasBasedDirectory as sandbox root if neither Sandbox.get_directory()
+        # nor Sandbox.run() are required. This allows faster staging.
+        if not self.__allow_real_directory and not self.__allow_run:
+            return True
+
+        return 'BST_CAS_DIRECTORIES' in os.environ
+
     ################################################
     #               Private methods                #
     ################################################
@@ -562,6 +583,15 @@ class Sandbox():
         else:
             callback()
 
+    # _disable_run()
+    #
+    # Raise exception if `Sandbox.run()` is called. This enables use of
+    # CasBasedDirectory for faster staging when command execution is not
+    # required.
+    #
+    def _disable_run(self):
+        self.__allow_run = False
+
 
 # _SandboxBatch()
 #
diff --git a/buildstream/scriptelement.py b/buildstream/scriptelement.py
index 697cd2822..3327f818e 100644
--- a/buildstream/scriptelement.py
+++ b/buildstream/scriptelement.py
@@ -249,7 +249,7 @@ class ScriptElement(Element):
                                              .format(element.name, item['destination']),
                                              silent_nested=True):
                         virtual_dstdir = sandbox.get_virtual_directory()
-                        virtual_dstdir.descend(item['destination'].lstrip(os.sep).split(os.sep), create=True)
+                        virtual_dstdir.descend(*item['destination'].lstrip(os.sep).split(os.sep), create=True)
                         element.stage_dependency_artifacts(sandbox, Scope.RUN, path=item['destination'])
 
             with sandbox.batch(SandboxFlags.NONE):
@@ -269,7 +269,7 @@ class ScriptElement(Element):
                                 dep.integrate(sandbox)
 
         install_root_path_components = self.__install_root.lstrip(os.sep).split(os.sep)
-        sandbox.get_virtual_directory().descend(install_root_path_components, create=True)
+        sandbox.get_virtual_directory().descend(*install_root_path_components, create=True)
 
     def assemble(self, sandbox):
 
diff --git a/buildstream/storage/_casbaseddirectory.py b/buildstream/storage/_casbaseddirectory.py
index 43497eaf7..91e11888d 100644
--- a/buildstream/storage/_casbaseddirectory.py
+++ b/buildstream/storage/_casbaseddirectory.py
@@ -55,6 +55,12 @@ class IndexEntry():
 
         return self.buildstream_object
 
+    def get_digest(self):
+        if self.digest:
+            return self.digest
+        else:
+            return self.buildstream_object._get_digest()
+
 
 class ResolutionException(VirtualDirectoryError):
     """ Superclass of all exceptions that can be raised by
@@ -155,8 +161,9 @@ class CasBasedDirectory(Directory):
     def _add_file(self, basename, filename, modified=False):
         entry = IndexEntry(filename, _FileType.REGULAR_FILE,
                            modified=modified or filename in self.index)
-        entry.digest = self.cas_cache.add_object(path=os.path.join(basename, filename))
-        entry.is_executable = os.access(os.path.join(basename, filename), os.X_OK)
+        path = os.path.join(basename, filename)
+        entry.digest = self.cas_cache.add_object(path=path)
+        entry.is_executable = os.access(path, os.X_OK)
         self.index[filename] = entry
 
         self.__invalidate_digest()
@@ -175,13 +182,12 @@ class CasBasedDirectory(Directory):
 
         self.__invalidate_digest()
 
-    def descend(self, subdirectory_spec, create=False):
+    def descend(self, *paths, create=False):
         """Descend one or more levels of directory hierarchy and return a new
         Directory object for that directory.
 
         Arguments:
-        * subdirectory_spec (list of strings): A list of strings which are all directory
-          names.
+        * *paths (str): A list of strings which are all directory names.
         * create (boolean): If this is true, the directories will be created if
           they don't already exist.
 
@@ -193,47 +199,38 @@ class CasBasedDirectory(Directory):
 
         """
 
-        # It's very common to send a directory name instead of a list and this causes
-        # bizarre errors, so check for it here
-        if not isinstance(subdirectory_spec, list):
-            subdirectory_spec = [subdirectory_spec]
-
-        # Because of the way split works, it's common to get a list which begins with
-        # an empty string. Detect these and remove them.
-        while subdirectory_spec and subdirectory_spec[0] == "":
-            subdirectory_spec.pop(0)
+        current_dir = self
 
-        # Descending into [] returns the same directory.
-        if not subdirectory_spec:
-            return self
+        for path in paths:
+            # Skip empty path segments
+            if not path:
+                continue
 
-        if subdirectory_spec[0] in self.index:
-            entry = self.index[subdirectory_spec[0]]
-            if entry.type == _FileType.DIRECTORY:
-                subdir = entry.get_directory(self)
-                return subdir.descend(subdirectory_spec[1:], create)
-            else:
-                error = "Cannot descend into {}, which is a '{}' in the directory {}"
-                raise VirtualDirectoryError(error.format(subdirectory_spec[0],
-                                                         self.index[subdirectory_spec[0]].type,
-                                                         self))
-        else:
-            if create:
-                newdir = self._add_directory(subdirectory_spec[0])
-                return newdir.descend(subdirectory_spec[1:], create)
+            entry = current_dir.index.get(path)
+            if entry:
+                if entry.type == _FileType.DIRECTORY:
+                    current_dir = entry.get_directory(current_dir)
+                else:
+                    error = "Cannot descend into {}, which is a '{}' in the directory {}"
+                    raise VirtualDirectoryError(error.format(path,
+                                                             current_dir.index[path].type,
+                                                             current_dir))
             else:
-                error = "'{}' not found in {}"
-                raise VirtualDirectoryError(error.format(subdirectory_spec[0], str(self)))
-        return None
+                if create:
+                    current_dir = current_dir._add_directory(path)
+                else:
+                    error = "'{}' not found in {}"
+                    raise VirtualDirectoryError(error.format(path, str(current_dir)))
 
-    def _check_replacement(self, name, path_prefix, fileListResult):
+        return current_dir
+
+    def _check_replacement(self, name, relative_pathname, fileListResult):
         """ Checks whether 'name' exists, and if so, whether we can overwrite it.
         If we can, add the name to 'overwritten_files' and delete the existing entry.
         Returns 'True' if the import should go ahead.
         fileListResult.overwritten and fileListResult.ignore are updated depending
         on the result. """
         existing_entry = self.index.get(name)
-        relative_pathname = os.path.join(path_prefix, name)
         if existing_entry is None:
             return True
         elif existing_entry.type == _FileType.DIRECTORY:
@@ -285,11 +282,11 @@ class CasBasedDirectory(Directory):
                 continue
 
             if direntry.is_file(follow_symlinks=False):
-                if self._check_replacement(direntry.name, path_prefix, result):
+                if self._check_replacement(direntry.name, relative_pathname, result):
                     self._add_file(source_directory, direntry.name, modified=relative_pathname in result.overwritten)
                     result.files_written.append(relative_pathname)
             elif direntry.is_symlink():
-                if self._check_replacement(direntry.name, path_prefix, result):
+                if self._check_replacement(direntry.name, relative_pathname, result):
                     self._copy_link_from_filesystem(source_directory, direntry.name)
                     result.files_written.append(relative_pathname)
 
@@ -303,18 +300,43 @@ class CasBasedDirectory(Directory):
             is_dir = entry.type == _FileType.DIRECTORY
 
             if is_dir:
-                src_subdir = source_directory.descend(name)
+                create_subdir = name not in self.index
+
+                if create_subdir and not filter_callback:
+                    # If subdirectory does not exist yet and there is no filter,
+                    # we can import the whole source directory by digest instead
+                    # of importing each directory entry individually.
+                    subdir_digest = entry.get_digest()
+                    dest_entry = IndexEntry(name, _FileType.DIRECTORY, digest=subdir_digest)
+                    self.index[name] = dest_entry
+                    self.__invalidate_digest()
+
+                    # However, we still need to iterate over the directory entries
+                    # to fill in `result.files_written`.
+
+                    # Use source subdirectory object if it already exists,
+                    # otherwise create object for destination subdirectory.
+                    # This is based on the assumption that the destination
+                    # subdirectory is more likely to be modified later on
+                    # (e.g., by further import_files() calls).
+                    if entry.buildstream_object:
+                        subdir = entry.buildstream_object
+                    else:
+                        subdir = dest_entry.get_directory(self)
 
-                try:
-                    create_subdir = name not in self.index
-                    dest_subdir = self.descend(name, create=create_subdir)
-                except VirtualDirectoryError:
-                    filetype = self.index[name].type
-                    raise VirtualDirectoryError('Destination is a {}, not a directory: /{}'
-                                                .format(filetype, relative_pathname))
+                    subdir.__add_files_to_result(path_prefix=relative_pathname, result=result)
+                else:
+                    src_subdir = source_directory.descend(name)
 
-                dest_subdir._partial_import_cas_into_cas(src_subdir, filter_callback,
-                                                         path_prefix=relative_pathname, result=result)
+                    try:
+                        dest_subdir = self.descend(name, create=create_subdir)
+                    except VirtualDirectoryError:
+                        filetype = self.index[name].type
+                        raise VirtualDirectoryError('Destination is a {}, not a directory: /{}'
+                                                    .format(filetype, relative_pathname))
+
+                    dest_subdir._partial_import_cas_into_cas(src_subdir, filter_callback,
+                                                             path_prefix=relative_pathname, result=result)
 
             if filter_callback and not filter_callback(relative_pathname):
                 if is_dir and create_subdir and dest_subdir.is_empty():
@@ -325,7 +347,7 @@ class CasBasedDirectory(Directory):
                 continue
 
             if not is_dir:
-                if self._check_replacement(name, path_prefix, result):
+                if self._check_replacement(name, relative_pathname, result):
                     if entry.type == _FileType.REGULAR_FILE:
                         self.index[name] = IndexEntry(name, _FileType.REGULAR_FILE,
                                                       digest=entry.digest,
@@ -556,13 +578,13 @@ class CasBasedDirectory(Directory):
         return self.__digest
 
     def _objpath(self, path):
-        subdir = self.descend(path[:-1])
+        subdir = self.descend(*path[:-1])
         entry = subdir.index[path[-1]]
         return self.cas_cache.objpath(entry.digest)
 
     def _exists(self, path):
         try:
-            subdir = self.descend(path[:-1])
+            subdir = self.descend(*path[:-1])
             return path[-1] in subdir.index
         except VirtualDirectoryError:
             return False
@@ -572,3 +594,14 @@ class CasBasedDirectory(Directory):
             self.__digest = None
             if self.parent:
                 self.parent.__invalidate_digest()
+
+    def __add_files_to_result(self, *, path_prefix="", result):
+        for name, entry in self.index.items():
+            # The destination filename, relative to the root where the import started
+            relative_pathname = os.path.join(path_prefix, name)
+
+            if entry.type == _FileType.DIRECTORY:
+                subdir = self.descend(name)
+                subdir.__add_files_to_result(path_prefix=relative_pathname, result=result)
+            else:
+                result.files_written.append(relative_pathname)
diff --git a/buildstream/storage/_filebaseddirectory.py b/buildstream/storage/_filebaseddirectory.py
index 4b0fd917b..742200379 100644
--- a/buildstream/storage/_filebaseddirectory.py
+++ b/buildstream/storage/_filebaseddirectory.py
@@ -46,35 +46,32 @@ class FileBasedDirectory(Directory):
     def __init__(self, external_directory=None):
         self.external_directory = external_directory
 
-    def descend(self, subdirectory_spec, create=False):
+    def descend(self, *paths, create=False):
         """ See superclass Directory for arguments """
-        # It's very common to send a directory name instead of a list and this causes
-        # bizarre errors, so check for it here
-        if not isinstance(subdirectory_spec, list):
-            subdirectory_spec = [subdirectory_spec]
-
-        # Because of the way split works, it's common to get a list which begins with
-        # an empty string. Detect these and remove them.
-        while subdirectory_spec and subdirectory_spec[0] == "":
-            subdirectory_spec.pop(0)
-
-        if not subdirectory_spec:
-            return self
-
-        new_path = os.path.join(self.external_directory, subdirectory_spec[0])
-        try:
-            st = os.lstat(new_path)
-            if not stat.S_ISDIR(st.st_mode):
-                raise VirtualDirectoryError("Cannot descend into '{}': '{}' is not a directory"
-                                            .format(subdirectory_spec[0], new_path))
-        except FileNotFoundError:
-            if create:
-                os.mkdir(new_path)
-            else:
-                raise VirtualDirectoryError("Cannot descend into '{}': '{}' does not exist"
-                                            .format(subdirectory_spec[0], new_path))
 
-        return FileBasedDirectory(new_path).descend(subdirectory_spec[1:], create)
+        current_dir = self
+
+        for path in paths:
+            # Skip empty path segments
+            if not path:
+                continue
+
+            new_path = os.path.join(current_dir.external_directory, path)
+            try:
+                st = os.lstat(new_path)
+                if not stat.S_ISDIR(st.st_mode):
+                    raise VirtualDirectoryError("Cannot descend into '{}': '{}' is not a directory"
+                                                .format(path, new_path))
+            except FileNotFoundError:
+                if create:
+                    os.mkdir(new_path)
+                else:
+                    raise VirtualDirectoryError("Cannot descend into '{}': '{}' does not exist"
+                                                .format(path, new_path))
+
+            current_dir = FileBasedDirectory(new_path)
+
+        return current_dir
 
     def import_files(self, external_pathspec, *,
                      filter_callback=None,
@@ -160,7 +157,7 @@ class FileBasedDirectory(Directory):
                     tf.addfile(tarinfo, f)
             elif tarinfo.isdir():
                 tf.addfile(tarinfo)
-                self.descend(filename.split(os.path.sep)).export_to_tar(tf, arcname, mtime)
+                self.descend(*filename.split(os.path.sep)).export_to_tar(tf, arcname, mtime)
             else:
                 tf.addfile(tarinfo)
 
diff --git a/buildstream/storage/directory.py b/buildstream/storage/directory.py
index 70054f78c..bad818fef 100644
--- a/buildstream/storage/directory.py
+++ b/buildstream/storage/directory.py
@@ -52,13 +52,12 @@ class Directory():
     def __init__(self, external_directory=None):
         raise NotImplementedError()
 
-    def descend(self, subdirectory_spec, create=False):
+    def descend(self, *paths, create=False):
         """Descend one or more levels of directory hierarchy and return a new
         Directory object for that directory.
 
         Args:
-          subdirectory_spec (list of str): A list of strings which are all directory
-            names.
+          *paths (str): A list of strings which are all directory names.
           create (boolean): If this is true, the directories will be created if
             they don't already exist.