Merge pull request #721 from common-workflow-language/warn_var_spool_cwl

Warn about using /var/spool/cwl

Author: mr-c

MANIFEST.in

@@ -1,3 +1,4 @@
+include MANIFEST.in
 include gittaggers.py Makefile cwltool.py
 include tests/*
 include tests/tmp1/tmp2/tmp3/.gitkeep

cwltool/builder.py

@@ -8,9 +8,10 @@
 import six
 from six import iteritems, string_types
 
-import avro
 import schema_salad.validate as validate
+import schema_salad.schema as schema
 from schema_salad.sourceline import SourceLine
+from schema_salad.schema import AvroSchemaFromJSONData
 
 from rdflib import Graph, URIRef
 from rdflib.namespace import OWL, RDFS
@@ -25,8 +26,6 @@
 
 _logger = logging.getLogger("cwltool")
 
-AvroSchemaFromJSONData = avro.schema.make_avsc_object
-
 CONTENT_LIMIT = 64 * 1024
 
 
@@ -85,7 +84,7 @@ def checkFormat(actualFile, inputFormats, ontology):
 
 class Builder(object):
     def __init__(self):  # type: () -> None
-        self.names = None  # type: avro.schema.Names
+        self.names = None  # type: schema.Names
         self.schemaDefs = None  # type: Dict[Text, Dict[Text, Any]]
         self.files = None  # type: List[Dict[Text, Text]]
         self.fs_access = None  # type: StdFsAccess

cwltool/factory.py

@@ -55,7 +55,8 @@ def __init__(self,
 
     def make(self, cwl):
         """Instantiate a CWL object from a CWl document."""
-        load = load_tool.load_tool(cwl, self.makeTool)
+        load = load_tool.load_tool(cwl, self.makeTool,
+                                   strict=self.execkwargs.get("strict", True))
         if isinstance(load, int):
             raise Exception("Error loading tool")
         return Callable(load, self)

cwltool/load_tool.py

@@ -10,15 +10,14 @@
 import json
 import copy
 from typing import (Any, Callable, Dict, Iterable, List, Mapping, Optional,
-        Text, Tuple, Union, cast)
+                    Text, Tuple, Union, cast)
 
 import requests.sessions
-from six import itervalues, string_types
+from six import iteritems, itervalues, string_types
 from six.moves import urllib
 
-import schema_salad.schema as schema
-from avro.schema import Names
 from ruamel.yaml.comments import CommentedMap, CommentedSeq
+import schema_salad.schema as schema
 from schema_salad.ref_resolver import ContextType, Fetcher, Loader, file_uri
 from schema_salad.sourceline import cmap, SourceLine
 from schema_salad.validate import ValidationException
@@ -55,23 +54,23 @@
 
 FetcherConstructorType = Callable[[Dict[Text, Union[Text, bool]],
     requests.sessions.Session], Fetcher]
+ResolverType = Callable[[Loader, Union[Text, Dict[Text, Any]]], Text]
 
 loaders = {}  # type: Dict[FetcherConstructorType, Loader]
 
 def default_loader(fetcher_constructor):
     # type: (Optional[FetcherConstructorType]) -> Loader
     if fetcher_constructor in loaders:
         return loaders[fetcher_constructor]
-    else:
-        loader = Loader(jobloaderctx, fetcher_constructor=fetcher_constructor)
-        loaders[fetcher_constructor] = loader
-        return loader
+    loader = Loader(jobloaderctx, fetcher_constructor=fetcher_constructor)
+    loaders[fetcher_constructor] = loader
+    return loader
 
 def resolve_tool_uri(argsworkflow,  # type: Text
-                     resolver=None,  # type: Callable[[Loader, Union[Text, Dict[Text, Any]]], Text]
+                     resolver=None,  # type: ResolverType
                      fetcher_constructor=None,  # type: FetcherConstructorType
                      document_loader=None  # type: Loader
-                     ):  # type: (...) -> Tuple[Text, Text]
+                    ):  # type: (...) -> Tuple[Text, Text]
 
     uri = None  # type: Text
     split = urllib.parse.urlsplit(argsworkflow)
@@ -107,7 +106,7 @@ def fetch_document(argsworkflow,  # type: Union[Text, Dict[Text, Any]]
     workflowobj = None  # type: CommentedMap
     if isinstance(argsworkflow, string_types):
         uri, fileuri = resolve_tool_uri(argsworkflow, resolver=resolver,
-                document_loader=document_loader)
+                                        document_loader=document_loader)
         workflowobj = document_loader.fetch(fileuri)
     elif isinstance(argsworkflow, dict):
         uri = "#" + Text(id(argsworkflow))
@@ -128,11 +127,12 @@ def _convert_stdstreams_to_files(workflowobj):
                 outputs = workflowobj.get('outputs', [])
                 if not isinstance(outputs, CommentedSeq):
                     raise ValidationException('"outputs" section is not '
-                            'valid.')
+                                              'valid.')
                 for out in workflowobj.get('outputs', []):
-                    if type(out) is not CommentedMap:
-                        raise ValidationException("Output '{}' is not a "
-                                    "valid OutputParameter.".format(out))
+                    if not isinstance(out, CommentedMap):
+                        raise ValidationException(
+                            "Output '{}' is not a valid "
+                            "OutputParameter.".format(out))
                     for streamtype in ['stdout', 'stderr']:
                         if out.get('type') == streamtype:
                             if 'outputBinding' in out:
@@ -142,8 +142,11 @@ def _convert_stdstreams_to_files(workflowobj):
                             if streamtype in workflowobj:
                                 filename = workflowobj[streamtype]
                             else:
-                                filename = Text(hashlib.sha1(json.dumps(workflowobj,
-                                            sort_keys=True).encode('utf-8')).hexdigest())
+                                filename = Text(
+                                    hashlib.sha1(json.dumps(workflowobj,
+                                                            sort_keys=True
+                                                           ).encode('utf-8')
+                                                ).hexdigest())
                                 workflowobj[streamtype] = filename
                             out['type'] = 'File'
                             out['outputBinding'] = cmap({'glob': filename})
@@ -174,9 +177,9 @@ def _add_blank_ids(workflowobj):
 
     if isinstance(workflowobj, dict):
         if ("run" in workflowobj and
-            isinstance(workflowobj["run"], dict) and
-            "id" not in workflowobj["run"] and
-            "$import" not in workflowobj["run"]):
+                isinstance(workflowobj["run"], dict) and
+                "id" not in workflowobj["run"] and
+                "$import" not in workflowobj["run"]):
             workflowobj["run"]["id"] = Text(uuid.uuid4())
         for entry in itervalues(workflowobj):
             _add_blank_ids(entry)
@@ -195,8 +198,8 @@ def validate_document(document_loader,  # type: Loader
                       overrides=None,  # type: List[Dict]
                       metadata=None,  # type: Optional[Dict]
                       do_validate=True
-                      ):
-    # type: (...) -> Tuple[Loader, Names, Union[Dict[Text, Any], List[Dict[Text, Any]]], Dict[Text, Any], Text]
+                     ):
+    # type: (...) -> Tuple[Loader, schema.Names, Union[Dict[Text, Any], List[Dict[Text, Any]]], Dict[Text, Any], Text]
     """Validate a CWL document."""
 
     if isinstance(workflowobj, list):
@@ -205,7 +208,8 @@ def validate_document(document_loader,  # type: Loader
         }, fn=uri)
 
     if not isinstance(workflowobj, dict):
-        raise ValueError("workflowjobj must be a dict, got '%s': %s" % (type(workflowobj), workflowobj))
+        raise ValueError("workflowjobj must be a dict, got '{}': {}".format(
+            type(workflowobj), workflowobj))
 
     jobobj = None
     if "cwl:tool" in workflowobj:
@@ -220,6 +224,33 @@ def validate_document(document_loader,  # type: Loader
 
         workflowobj = fetch_document(uri, fetcher_constructor=fetcher_constructor)[1]
 
+    def var_spool_cwl_detector(obj,           # type: Union[Mapping, Iterable, Text]
+                               item=None,     # type: Optional[Any]
+                               obj_key=None,  # type: Optional[Any]
+                              ):              # type: (...)->None
+        """ Detects any textual reference to /var/spool/cwl. """
+        if isinstance(obj, string_types):
+            if "var/spool/cwl" in obj:
+                message = SourceLine(
+                    item=item, key=obj_key, raise_type=Text,
+                    include_traceback=_logger.isEnabledFor(logging.DEBUG)).makeError(
+                        "Non-portable reference to /var/spool/cwl found: "
+                        "'{}'.\n Replace with /var/spool/cwl/ with "
+                        "$(runtime.outdir).".format(obj))
+                if not strict:
+                    _logger.warning(message)
+                else:
+                    raise ValidationException(message)
+            else:
+                return
+        elif isinstance(obj, Mapping):
+            for key, value in iteritems(obj):
+                var_spool_cwl_detector(value, obj, key)
+        elif isinstance(obj, Iterable):
+            for element in obj:
+                var_spool_cwl_detector(element, obj, None)
+    var_spool_cwl_detector(workflowobj)
+
     fileuri = urllib.parse.urldefrag(uri)[0]
     if "cwlVersion" not in workflowobj:
         if metadata and 'cwlVersion' in metadata:
@@ -233,7 +264,10 @@ def validate_document(document_loader,  # type: Loader
                 "will need to be upgraded first.")
 
     if not isinstance(workflowobj["cwlVersion"], (str, Text)):
-        raise Exception("'cwlVersion' must be a string, got %s" % type(workflowobj["cwlVersion"]))
+        with SourceLine(workflowobj, "cwlVersion", ValidationException):
+            raise ValidationException("'cwlVersion' must be a string, "
+                                      "got {}".format(
+                                          type(workflowobj["cwlVersion"])))
     # strip out version
     workflowobj["cwlVersion"] = re.sub(
         r"^(?:cwl:|https://w3id.org/cwl/cwl#)", "",
@@ -246,8 +280,10 @@ def validate_document(document_loader,  # type: Loader
                 version += " (with --enable-dev flag only)"
             versions.append(version)
         versions.sort()
-        raise ValidationException("The CWL reference runner no longer supports pre CWL v1.0 documents. "
-                                  "Supported versions are: \n{}".format("\n".join(versions)))
+        raise ValidationException(
+            "The CWL reference runner no longer supports pre CWL v1.0 "
+            "documents. Supported versions are: "
+            "\n{}".format("\n".join(versions)))
 
     (sch_document_loader, avsc_names) = \
         process.get_schema(workflowobj["cwlVersion"])[:2]
@@ -263,7 +299,8 @@ def validate_document(document_loader,  # type: Loader
     _add_blank_ids(workflowobj)
 
     workflowobj["id"] = fileuri
-    processobj, new_metadata = document_loader.resolve_all(workflowobj, fileuri, checklinks=do_validate)
+    processobj, new_metadata = document_loader.resolve_all(
+        workflowobj, fileuri, checklinks=do_validate)
     if not isinstance(processobj, (CommentedMap, CommentedSeq)):
         raise ValidationException("Workflow must be a dict or list.")
 
@@ -295,13 +332,12 @@ def validate_document(document_loader,  # type: Loader
 
 
 def make_tool(document_loader,  # type: Loader
-              avsc_names,  # type: Names
-              metadata,  # type: Dict[Text, Any]
-              uri,  # type: Text
-              makeTool,  # type: Callable[..., Process]
-              kwargs  # type: dict
-              ):
-    # type: (...) -> Process
+              avsc_names,       # type: schema.Names
+              metadata,         # type: Dict[Text, Any]
+              uri,              # type: Text
+              makeTool,         # type: Callable[..., Process]
+              kwargs            # type: Dict
+             ):  # type: (...) -> Process
     """Make a Python CWL object."""
     resolveduri = document_loader.resolve_ref(uri)[0]
 
@@ -340,28 +376,30 @@ def make_tool(document_loader,  # type: Loader
     return tool
 
 
-def load_tool(argsworkflow,  # type: Union[Text, Dict[Text, Any]]
-              makeTool,  # type: Callable[..., Process]
-              kwargs=None,  # type: Dict
-              enable_dev=False,  # type: bool
-              strict=True,  # type: bool
-              resolver=None,  # type: Callable[[Loader, Union[Text, Dict[Text, Any]]], Text]
+def load_tool(argsworkflow,              # type: Union[Text, Dict[Text, Any]]
+              makeTool,                  # type: Callable[..., Process]
+              kwargs=None,               # type: Dict
+              enable_dev=False,          # type: bool
+              strict=True,               # type: bool
+              resolver=None,             # type: ResolverType
               fetcher_constructor=None,  # type: FetcherConstructorType
               overrides=None
-              ):
-    # type: (...) -> Process
+             ):  # type: (...) -> Process
 
-    document_loader, workflowobj, uri = fetch_document(argsworkflow, resolver=resolver,
-                                                       fetcher_constructor=fetcher_constructor)
-    document_loader, avsc_names, processobj, metadata, uri = validate_document(
+    document_loader, workflowobj, uri = fetch_document(
+        argsworkflow, resolver=resolver, fetcher_constructor=fetcher_constructor)
+    document_loader, avsc_names, _, metadata, uri = validate_document(
         document_loader, workflowobj, uri, enable_dev=enable_dev,
         strict=strict, fetcher_constructor=fetcher_constructor,
         overrides=overrides, metadata=kwargs.get('metadata', None)
         if kwargs else None)
     return make_tool(document_loader, avsc_names, metadata, uri,
                      makeTool, kwargs if kwargs else {})
 
-def resolve_overrides(ov, ov_uri, baseurl):  # type: (CommentedMap, Text, Text) -> List[Dict[Text, Any]]
+def resolve_overrides(ov,      # Type: CommentedMap
+                      ov_uri,  # Type: Text
+                      baseurl  # type: Text
+                     ):  # type: (...) -> List[Dict[Text, Any]]
     ovloader = Loader(overrides_ctx)
     ret, _ = ovloader.resolve_all(ov, baseurl)
     if not isinstance(ret, CommentedMap):