"checklinks" flag should not be based on do_validate (#1106)

* "checklinks" flag should not be based on do_validate

This flag is used internally by schema salad to determine when it is
appropriate to run checklinks, but checklinks must not be skipped
entirely as it includes updating fields in certain circumstances.

* Add http://arvados.org/cwl#ReuseRequirement extension to updates

* Make sure to use the cwlVersion declared at the root of the document.

* Add test for do_validate and outputSource.

* Use builder.outdir (container) not passed in outdir (host).

* Test for getting cwlVersion from root document in load_tool.

Author: Peter Amstutz

cwltool/command_line_tool.py

@@ -677,8 +677,8 @@ def collect_output(self,
                             globpatterns.extend(aslist(gb))
 
                     for gb in globpatterns:
-                        if gb.startswith(outdir):
-                            gb = gb[len(outdir) + 1:]
+                        if gb.startswith(builder.outdir):
+                            gb = gb[len(builder.outdir) + 1:]
                         elif gb == ".":
                             gb = outdir
                         elif gb.startswith("/"):

cwltool/load_tool.py

@@ -208,7 +208,7 @@ def resolve_and_validate_document(loadingContext,
 
     jobobj = None
     if "cwl:tool" in workflowobj:
-        jobobj, _ = loadingContext.loader.resolve_all(workflowobj, uri, checklinks=loadingContext.do_validate)
+        jobobj, _ = loadingContext.loader.resolve_all(workflowobj, uri)
         uri = urllib.parse.urljoin(uri, workflowobj["https://w3id.org/cwl/cwl#tool"])
         del cast(dict, jobobj)["https://w3id.org/cwl/cwl#tool"]
 
@@ -219,6 +219,11 @@ def resolve_and_validate_document(loadingContext,
     cwlVersion = loadingContext.metadata.get("cwlVersion")
     if not cwlVersion:
         cwlVersion = workflowobj.get("cwlVersion")
+    if not cwlVersion and fileuri != uri:
+        # The tool we're loading is a fragment of a bigger file.  Get
+        # the document root element and look for cwlVersion there.
+        metadata = fetch_document(fileuri, loadingContext)[1]
+        cwlVersion = metadata.get("cwlVersion")
     if not cwlVersion:
         raise ValidationException(
             "No cwlVersion found. "
@@ -281,9 +286,7 @@ def resolve_and_validate_document(loadingContext,
     if cwlVersion == "v1.0":
         _add_blank_ids(workflowobj)
 
-    workflowobj["id"] = fileuri
-    processobj, metadata = document_loader.resolve_all(
-        workflowobj, fileuri, checklinks=loadingContext.do_validate)
+    processobj, metadata = document_loader.resolve_all(workflowobj, fileuri)
     if loadingContext.metadata:
         metadata = loadingContext.metadata
     if not isinstance(processobj, (CommentedMap, CommentedSeq)):
@@ -301,6 +304,8 @@ def resolve_and_validate_document(loadingContext,
 
     # None means default behavior (do update)
     if loadingContext.do_update in (True, None):
+        if "cwlVersion" not in metadata:
+            metadata["cwlVersion"] = cwlVersion
         processobj = cast(CommentedMap, cmap(update.update(
             processobj, document_loader, fileuri, loadingContext.enable_dev, metadata)))
         if isinstance(processobj, MutableMapping):
@@ -309,6 +314,8 @@ def resolve_and_validate_document(loadingContext,
             document_loader.idx[metadata["id"]] = metadata
             for po in processobj:
                 document_loader.idx[po["id"]] = po
+        else:
+            raise Exception("'processobj' was not MutableMapping or MutableSequence %s" % type(processobj))
 
     if jobobj is not None:
         loadingContext.jobdefaults = jobobj

cwltool/update.py

@@ -27,11 +27,11 @@ def v1_0to1_1_0dev1(doc, loader, baseuri):  # pylint: disable=unused-argument
 
     rewrite = {
         "http://commonwl.org/cwltool#WorkReuse": "WorkReuse",
+        "http://arvados.org/cwl#ReuseRequirement": "WorkReuse",
         "http://commonwl.org/cwltool#TimeLimit": "ToolTimeLimit",
         "http://commonwl.org/cwltool#NetworkAccess": "NetworkAccess",
         "http://commonwl.org/cwltool#InplaceUpdateRequirement": "InplaceUpdateRequirement",
-        "http://commonwl.org/cwltool#LoadListingRequirement": "LoadListingRequirement",
-        "http://commonwl.org/cwltool#WorkReuse": "WorkReuse",
+        "http://commonwl.org/cwltool#LoadListingRequirement": "LoadListingRequirement"
     }
     def rewrite_requirements(t):
         if "requirements" in t:
@@ -73,6 +73,8 @@ def fix_inputBinding(t):
         proc.setdefault("hints", [])
         proc["hints"].insert(0, {"class": "NetworkAccess", "networkAccess": True})
         proc["hints"].insert(0, {"class": "LoadListingRequirement", "loadListing": "deep_listing"})
+        if "cwlVersion" in proc:
+            del proc["cwlVersion"]
 
     return (doc, "v1.1.0-dev1")
 

tests/test_load_tool.py

@@ -1,12 +1,15 @@
 from cwltool.load_tool import load_tool
 from cwltool.context import LoadingContext, RuntimeContext
 from cwltool.errors import WorkflowException
+from cwltool.update import INTERNAL_VERSION
 import pytest
 from .util import (get_data, get_main_output,
                    get_windows_safe_factory,
                    needs_docker, working_directory,
                    needs_singularity, temp_dir,
                    windows_needs_docker)
+from cwltool.resolver import Path, resolve_local
+from .test_fetch import norm
 
 @windows_needs_docker
 def test_check_version():
@@ -42,3 +45,37 @@ def test_use_metadata():
     tooldata = tool.tool.copy()
     del tooldata["cwlVersion"]
     tool2 = load_tool(tooldata, loadingContext)
+
+def test_checklink_outputSource():
+    """Test that outputSource is resolved correctly independent of value
+    of do_validate.
+
+    """
+
+    outsrc = norm(Path(get_data("tests/wf/1st-workflow.cwl")).as_uri())+"#argument/classfile"
+
+    loadingContext = LoadingContext({"do_validate": True})
+    tool = load_tool(get_data("tests/wf/1st-workflow.cwl"), loadingContext)
+    assert norm(tool.tool["outputs"][0]["outputSource"]) == outsrc
+
+    loadingContext = LoadingContext({"do_validate": False})
+    tool = load_tool(get_data("tests/wf/1st-workflow.cwl"), loadingContext)
+    assert norm(tool.tool["outputs"][0]["outputSource"]) == outsrc
+
+def test_load_graph_fragment():
+    """Test that outputSource is resolved correctly independent of value
+    of do_validate.
+
+    """
+
+    loadingContext = LoadingContext()
+    uri = Path(get_data("tests/wf/scatter-wf4.cwl")).as_uri()+"#main"
+    tool = load_tool(uri, loadingContext)
+
+    rs, metadata = tool.doc_loader.resolve_ref(uri)
+    # Reload from a dict (in 'rs'), not a URI.  The dict is a fragment
+    # of original document and doesn't have cwlVersion set, so test
+    # that it correctly looks up the root document to get the
+    # cwlVersion.
+    tool = load_tool(tool.tool, loadingContext)
+    assert tool.metadata["cwlVersion"] == INTERNAL_VERSION