Symlink err (#1138)

* Check for file staging conflicts

* Directories can be merged, don't error.

* Dereference symlinks to avoid operating on the same file with different aliases

* Test that output files with same name get renamed

Author: Peter Amstutz

cwltool/argparser.py

@@ -252,7 +252,7 @@ def arg_parser():  # type: () -> argparse.ArgumentParser
 
     exgroup = parser.add_mutually_exclusive_group()
     exgroup.add_argument("--enable-color", action="store_true",
-                        help="Enable colored logging (default true)", default=True)
+                         help="Enable logging color (default enabled)", default=True)
     exgroup.add_argument("--disable-color", action="store_false", dest="enable_color",
                         help="Disable colored logging (default false)")
 

cwltool/pathmapper.py

@@ -340,8 +340,10 @@ def reversemap(self,
         return None
 
     def update(self, key, resolved, target, ctype, stage):
-        # type: (Text, Text, Text, Text, bool) -> None
-        self._pathmap[key] = MapperEnt(resolved, target, ctype, stage)
+        # type: (Text, Text, Text, Text, bool) -> MapperEnt
+        m = MapperEnt(resolved, target, ctype, stage)
+        self._pathmap[key] = m
+        return m
 
     def __contains__(self, key):  # type: (Text) -> bool
         """Test for the presence of the given relative path in this mapper."""

cwltool/process.py

@@ -28,7 +28,7 @@
 from typing_extensions import (TYPE_CHECKING,  # pylint: disable=unused-import
                                Text)
 from schema_salad import schema, validate
-from schema_salad.ref_resolver import Loader, file_uri
+from schema_salad.ref_resolver import Loader, file_uri, uri_file_path
 from schema_salad.sourceline import SourceLine, strip_dup_lineno
 
 from . import expression
@@ -39,7 +39,8 @@
 from .loghandler import _logger
 from .mutation import MutationManager  # pylint: disable=unused-import
 from .pathmapper import (PathMapper, adjustDirObjs, ensure_writable,
-                         get_listing, normalizeFilesDirs, visit_class)
+                         get_listing, normalizeFilesDirs, visit_class,
+                         MapperEnt)
 from .secrets import SecretStore  # pylint: disable=unused-import
 from .software_requirements import (  # pylint: disable=unused-import
     DependenciesConfiguration)
@@ -222,9 +223,30 @@ def stage_files(pathmapper,             # type: PathMapper
                 stage_func=None,        # type: Optional[Callable[..., Any]]
                 ignore_writable=False,  # type: bool
                 symlink=True,           # type: bool
-                secret_store=None       # type: Optional[SecretStore]
+                secret_store=None,      # type: Optional[SecretStore]
+                fix_conflicts=False     # type: bool
                ):  # type: (...) -> None
     """Link or copy files to their targets. Create them as needed."""
+
+    targets = {}  # type: Dict[Text, MapperEnt]
+    for key, entry in pathmapper.items():
+        if not 'File' in entry.type:
+            continue
+        if entry.target not in targets:
+            targets[entry.target] = entry
+        elif targets[entry.target].resolved != entry.resolved:
+            if fix_conflicts:
+                tgt = entry.target
+                i = 2
+                tgt = "%s_%s" % (tgt, i)
+                while tgt in targets:
+                    i += 1
+                    tgt = "%s_%s" % (tgt, i)
+                targets[tgt] = pathmapper.update(key, entry.resolved, tgt, entry.type, entry.staged)
+            else:
+                raise WorkflowException("File staging conflict, trying to stage both %s and %s to the same target %s" % (
+                    targets[entry.target].resolved, entry.resolved, entry.target))
+
     for key, entry in pathmapper.items():
         if not entry.staged:
             continue
@@ -329,9 +351,16 @@ def _relocate(src, dst):  # type: (Text, Text) -> None
             else:
                 shutil.copy2(src, dst)
 
+    def _realpath(ob):  # type: (Dict[Text, Any]) -> None
+        if ob["location"].startswith("file:"):
+            ob["location"] = file_uri(os.path.realpath(uri_file_path(ob["location"])))
+        if ob["location"].startswith("/"):
+            ob["location"] = os.path.realpath(ob["location"])
+
     outfiles = list(_collectDirEntries(outputObj))
+    visit_class(outfiles, ("File", "Directory"), _realpath)
     pm = path_mapper(outfiles, "", destination_path, separateDirs=False)
-    stage_files(pm, stage_func=_relocate, symlink=False)
+    stage_files(pm, stage_func=_relocate, symlink=False, fix_conflicts=True)
 
     def _check_adjust(a_file):  # type: (Dict[Text, Text]) -> Dict[Text, Text]
         a_file["location"] = file_uri(pm.mapper(a_file["location"])[1])

tests/test_relocate.py

@@ -1,8 +1,27 @@
+import sys
+import json
+
+if sys.version_info[0] < 3:
+    from StringIO import StringIO
+else:
+    from io import StringIO
+
 from cwltool.main import main
 
-from .util import get_data, needs_docker
+from .util import get_data, needs_docker, temp_dir
 
 @needs_docker
 def test_for_910():
     assert main([get_data('tests/wf/910.cwl')]) == 0
     assert main([get_data('tests/wf/910.cwl')]) == 0
+
+@needs_docker
+def test_for_conflict_file_names():
+    stream = StringIO()
+
+    with temp_dir() as tmp:
+        assert main(["--debug", "--outdir", tmp, get_data('tests/wf/conflict.cwl')], stdout=stream) == 0
+
+    out = json.loads(stream.getvalue())
+    assert out["b1"]["basename"] == out["b2"]["basename"]
+    assert out["b1"]["location"] != out["b2"]["location"]

tests/wf/conflict.cwl

@@ -0,0 +1,32 @@
+cwlVersion: v1.1
+$graph:
+  - class: CommandLineTool
+    id: makebzz
+    inputs: []
+    outputs:
+      bzz:
+        type: File
+        outputBinding:
+          glob: bzz
+    requirements:
+      ShellCommandRequirement: {}
+    arguments: [{shellQuote: false, valueFrom: "touch bzz"}]
+  - class: Workflow
+    id: main
+    inputs: []
+    outputs:
+      b1:
+        type: File
+        outputSource: step1/bzz
+      b2:
+        type: File
+        outputSource: step2/bzz
+    steps:
+      step1:
+        in: {}
+        out: [bzz]
+        run: '#makebzz'
+      step2:
+        in: {}
+        out: [bzz]
+        run: '#makebzz'