Merge pull request #912 from psafont/fix-move

Fix folder relocations on existing folders

Author: mr-c

cwltool/docker.py

@@ -218,9 +218,10 @@ def add_volumes(self, pathmapper, runtime, secret_store=None):
             if not vol.staged:
                 continue
             host_outdir_tgt = None  # type: Optional[Text]
-            if vol.target.startswith(container_outdir+"/"):
+            if vol.target.startswith(container_outdir + "/"):
                 host_outdir_tgt = os.path.join(
                     host_outdir, vol.target[len(container_outdir)+1:])
+
             if vol.type in ("File", "Directory"):
                 if not vol.resolved.startswith("_:"):
                     _check_docker_machine_path(docker_windows_path_adjust(
@@ -244,7 +245,8 @@ def add_volumes(self, pathmapper, runtime, secret_store=None):
             elif vol.type == "WritableDirectory":
                 if vol.resolved.startswith("_:"):
                     if host_outdir_tgt:
-                        os.makedirs(host_outdir_tgt, 0o0755)
+                        if not os.path.exists(host_outdir_tgt):
+                            os.makedirs(host_outdir_tgt, 0o0755)
                     else:
                         raise WorkflowException(
                             "Unable to compute host_outdir_tgt for "
@@ -268,6 +270,9 @@ def add_volumes(self, pathmapper, runtime, secret_store=None):
                 else:
                     contents = vol.resolved
                 if host_outdir_tgt:
+                    dirname = os.path.dirname(host_outdir_tgt)
+                    if not os.path.exists(dirname):
+                        os.makedirs(dirname, 0o0755)
                     with open(host_outdir_tgt, "wb") as file_literal:
                         file_literal.write(contents.encode("utf-8"))
                 else:

cwltool/process.py

@@ -299,29 +299,30 @@ def _relocate(src, dst):
         if src == dst:
             return
 
-        if action == "move":
-            # do not move anything if we are trying to move an entity from
-            # outside of the source directories
-            if any(src.startswith(path + "/") for path in source_directories):
-                _logger.debug("Moving %s to %s", src, dst)
-                if fs_access.isdir(src) and fs_access.isdir(dst):
-                    # merge directories
-                    for dir_entry in scandir(src):
-                        _relocate(dir_entry, fs_access.join(
-                            dst, dir_entry.name))
-                else:
-                    shutil.move(src, dst)
-                    return
-
-        _logger.debug("Copying %s to %s", src, dst)
-        if fs_access.isdir(src):
-            if os.path.isdir(dst):
-                shutil.rmtree(dst)
-            elif os.path.isfile(dst):
-                os.unlink(dst)
-            shutil.copytree(src, dst)
-        else:
-            shutil.copy2(src, dst)
+        # If the source is not contained in source_directories we're not allowed to delete it
+        src_can_deleted = any(os.path.commonprefix([p, src]) == p for p in source_directories)
+
+        _action = "move" if action == "move" and src_can_deleted else "copy"
+
+        if _action == "move":
+            _logger.debug("Moving %s to %s", src, dst)
+            if fs_access.isdir(src) and fs_access.isdir(dst):
+                # merge directories
+                for dir_entry in scandir(src):
+                    _relocate(dir_entry.path, fs_access.join(dst, dir_entry.name))
+            else:
+                shutil.move(src, dst)
+
+        elif _action == "copy":
+            _logger.debug("Copying %s to %s", src, dst)
+            if fs_access.isdir(src):
+                if os.path.isdir(dst):
+                    shutil.rmtree(dst)
+                elif os.path.isfile(dst):
+                    os.unlink(dst)
+                shutil.copytree(src, dst)
+            else:
+                shutil.copy2(src, dst)
 
     outfiles = list(_collectDirEntries(outputObj))
     pm = path_mapper(outfiles, "", destination_path, separateDirs=False)
@@ -357,7 +358,8 @@ def relink(relinked,  # type: Dict[Text, Text]
                         os.unlink(path)
                         os.symlink(os.path.relpath(link_name, path), path)
                 else:
-                    if any(real_path.startswith(path + "/") for path in source_directories):
+                    if any(os.path.commonprefix([path, real_path]) == path \
+                           for path in source_directories):
                         os.unlink(path)
                         os.rename(real_path, path)
                         relinked[real_path] = path

tests/test_relocate.py

@@ -0,0 +1,8 @@
+from cwltool.main import main
+
+from .util import get_data, needs_docker
+
+@needs_docker
+def test_for_910():
+    assert main([get_data('tests/wf/910.cwl')]) == 0
+    assert main([get_data('tests/wf/910.cwl')]) == 0

tests/wf/910.cwl

@@ -0,0 +1,31 @@
+cwlVersion: v1.0
+class: CommandLineTool
+
+requirements:
+- class: InlineJavascriptRequirement
+- class: InitialWorkDirRequirement
+  listing: |
+    ${
+      return [{"class": "Directory",
+               "basename": "default",
+               "listing": [{"class": "File",
+                            "basename": "file.txt",
+                            "contents": "Hello world"}
+                           ],
+               "writable": true}]
+    }
+
+hints:
+- class: DockerRequirement
+  dockerPull: ubuntu
+
+inputs: []
+
+outputs:
+
+  output_folder:
+    type: Directory
+    outputBinding:
+      glob: "*"
+
+baseCommand: [ls]