WIP Add support for optional secondary files in v1.1.0 (#804)

* Add support for optional secondary files in v1.1.0

* Refactor to assume schema salad applies secondaryFilesDSL

* Bump schema-salad version pin

* Update embedded v1.1 schema

Author: bogdang989

cwltool/builder.py

@@ -1,17 +1,22 @@
 from __future__ import absolute_import
 
 import copy
+import os
 import logging
 from typing import (Any, Callable, Dict, List, MutableMapping, MutableSequence,
                     Optional, Set, Tuple, Union)
 
+from typing_extensions import Text, Type, TYPE_CHECKING  # pylint: disable=unused-import
+# move to a regular typing import when Python 3.3-3.6 is no longer supported
+
 from rdflib import Graph, URIRef  # pylint: disable=unused-import
 from rdflib.namespace import OWL, RDFS
 from ruamel.yaml.comments import CommentedMap
 from schema_salad import validate
 from schema_salad.schema import Names, convert_to_dict
 from schema_salad.avro.schema import make_avsc_object, Schema
 from schema_salad.sourceline import SourceLine
+from schema_salad.ref_resolver import uri_file_path
 from six import iteritems, string_types
 from typing_extensions import (TYPE_CHECKING,  # pylint: disable=unused-import
                                Text, Type)
@@ -131,6 +136,7 @@ def __init__(self,
                  outdir,               # type: Text
                  tmpdir,               # type: Text
                  stagedir,             # type: Text
+                 cwl_version,          # type: Text
                 ):  # type: (...) -> None
 
         self.job = job
@@ -140,6 +146,7 @@ def __init__(self,
         self.names = names
         self.requirements = requirements
         self.hints = hints
+        self.cwl_version = cwl_version
         self.resources = resources
         self.mutation_manager = mutation_manager
         self.formatgraph = formatgraph
@@ -284,26 +291,39 @@ def _capture_files(f):
                     if "secondaryFiles" not in datum:
                         datum["secondaryFiles"] = []
                     for sf in aslist(schema["secondaryFiles"]):
-                        if isinstance(sf, MutableMapping) or "$(" in sf or "${" in sf:
-                            sfpath = self.do_eval(sf, context=datum)
+                        sf_required = True
+                        if isinstance(sf, MutableMapping) and "pattern" in sf and self.cwl_version in ['v1.1.0-dev1']:
+                            if 'required' in sf:
+                                sf_required = self.do_eval(sf['required'], context=datum)
+                        elif isinstance(sf, string_types):
+                            sf = {"pattern": sf}
                         else:
-                            sfpath = substitute(datum["basename"], sf)
+                            raise validate.ValidationException("Not a secondary file definition: %s" % sf)
+
+                        if "$(" in sf["pattern"] or "${" in sf["pattern"]:
+                            sfpath = self.do_eval(sf["pattern"], context=datum)
+                        else:
+                            sfpath = substitute(datum["basename"], sf["pattern"])
+
                         for sfname in aslist(sfpath):
+                            if not sfname:
+                                continue
                             found = False
                             for d in datum["secondaryFiles"]:
                                 if not d.get("basename"):
                                     d["basename"] = d["location"][d["location"].rindex("/")+1:]
                                 if d["basename"] == sfname:
                                     found = True
                             if not found:
+                                sf_location = datum["location"][0:datum["location"].rindex("/")+1]+sfname
                                 if isinstance(sfname, MutableMapping):
                                     datum["secondaryFiles"].append(sfname)
-                                elif discover_secondaryFiles:
+                                elif discover_secondaryFiles and os.path.exists(uri_file_path(sf_location)):
                                     datum["secondaryFiles"].append({
-                                        "location": datum["location"][0:datum["location"].rindex("/")+1]+sfname,
+                                        "location": sf_location,
                                         "basename": sfname,
                                         "class": "File"})
-                                else:
+                                elif sf_required:
                                     raise WorkflowException("Missing required secondary file '%s' from file object: %s" % (
                                         sfname, json_dumps(datum, indent=4)))
 

cwltool/command_line_tool.py

@@ -15,6 +15,9 @@
 from typing import (Any, Callable, Dict, Generator, List, Mapping, MutableMapping,
                     MutableSequence, Optional, Set, Union, cast)
 
+from typing_extensions import Text, Type, TYPE_CHECKING  # pylint: disable=unused-import
+# move to a regular typing import when Python 3.3-3.6 is no longer supported
+
 import shellescape
 from schema_salad import validate
 from schema_salad.avro.schema import Schema
@@ -759,18 +762,37 @@ def collect_output(self,
                             primary.setdefault("secondaryFiles", [])
                             pathprefix = primary["path"][0:primary["path"].rindex("/")+1]
                             for sf in aslist(schema["secondaryFiles"]):
+                                if isinstance(sf, MutableMapping) and 'pattern' in sf:
+                                    if 'required' in sf:
+                                        sf_required = sf['required']
+                                    else:
+                                        sf_required = False
+                                    sf = sf['pattern']
+                                else:
+                                    sf_required = False
+
                                 if isinstance(sf, MutableMapping) or "$(" in sf or "${" in sf:
                                     sfpath = builder.do_eval(sf, context=primary)
                                     subst = False
                                 else:
+                                    if sf.endswith('?') and \
+                                            self.metadata['cwlVersion'] in ['v1.1.0-dev1']:
+                                        sf_required = False
+                                        sf = sf[:-1]
                                     sfpath = sf
                                     subst = True
                                 for sfitem in aslist(sfpath):
+                                    if not sfitem:
+                                        continue
                                     if isinstance(sfitem, string_types):
                                         if subst:
                                             sfitem = {"path": substitute(primary["path"], sfitem)}
                                         else:
                                             sfitem = {"path": pathprefix+sfitem}
+                                    if not os.path.exists(sfitem['path']) and sf_required:
+                                        raise WorkflowException(
+                                            "Missing required secondary file '%s'" % (
+                                                sfitem["path"]))
                                     if "path" in sfitem and "location" not in sfitem:
                                         revmap(sfitem)
                                     if fs_access.isfile(sfitem["location"]):

cwltool/process.py

@@ -673,6 +673,7 @@ def inc(d):  # type: (List[int]) -> None
                                             or tempfile.mkdtemp())
                 stagedir = fs_access.realpath(runtime_context.stagedir
                                               or tempfile.mkdtemp())
+        cwl_version = self.metadata["cwlVersion"]
 
         builder = Builder(job,
                           files,
@@ -694,7 +695,8 @@ def inc(d):  # type: (List[int]) -> None
                           load_listing,
                           outdir,
                           tmpdir,
-                          stagedir)
+                          stagedir,
+                          cwl_version)
 
         bindings.extend(builder.bind_input(
             self.inputs_record_schema, job,

cwltool/schemas/v1.1.0-dev1/CommandLineTool.yml

@@ -50,6 +50,7 @@ $graph:
 
       ## Changelog
 
+        * Clarify documentation around `valueFrom` and `null` inputs.
         * Default values for some fields are now expressed in the schema.
         * When defining record types with `CommandInputRecordSchema`, fields of
           type `File` may now include `format`, `loadContents`,
@@ -69,9 +70,12 @@ $graph:
         * Clarify semantics of `shellQuote`
         * Expressions are now allowed to evaluate to `null` or `Dirent` in
           [InitialWorkDirRequirement.listing](#InitialWorkDirRequirement).
+        * Clarify behavior of secondaryFiles on output.
         * [Addition](#Requirements_and_hints) of `cwl:requirements` field to
-          input object documents
+          input object documents.
         * Clarify behavior of `glob` for absolute paths and symlinks.
+        * Clarify behavior of `glob` to include directories.
+        * `secondaryFiles` can now be explicitly marked as `required` or not.
 
       See also the [CWL Workflow Description, v1.1.0-dev1 changelog](Workflow.html#Changelog).
 
@@ -202,6 +206,9 @@ $graph:
         `InputParameter.default` field) must be applied before evaluating the
         expression.
 
+        If the value of the associated input parameter is `null`, `valueFrom` is
+        not evaluated and nothing is added to the command line.
+
         When a binding is part of the `CommandLineTool.arguments` field,
         the `valueFrom` field is required.
 
@@ -240,12 +247,12 @@ $graph:
         - type: array
           items: string
       doc: |
-        Find files relative to the output directory, using POSIX glob(3)
-        pathname matching.  If an array is provided, find files that match any
-        pattern in the array.  If an expression is provided, the expression must
-        return a string or an array of strings, which will then be evaluated as
-        one or more glob patterns.  Must only match and return files which
-        actually exist.
+        Find files or directories relative to the output directory, using POSIX
+        glob(3) pathname matching.  If an array is provided, find files or
+        directories that match any pattern in the array.  If an expression is
+        provided, the expression must return a string or an array of strings,
+        which will then be evaluated as one or more glob patterns.  Must only
+        match and return files/directories which actually exist.
 
         If the value of glob is a relative path pattern (does not
         begin with a slash '/') then it is resolved relative to the
@@ -259,14 +266,14 @@ $graph:
 
         A glob may match a path within the output directory which is
         actually a symlink to another file.  In this case, the
-        expected behavior is for the resulting File object to take the
+        expected behavior is for the resulting File/Directory object to take the
         `basename` (and corresponding `nameroot` and `nameext`) of the
-        symlink.  The `location` of the File is implementation
-        dependent, but logically the File should have the same content
-        as the symlink target.  Platforms may stage output files to
+        symlink.  The `location` of the File/Directory is implementation
+        dependent, but logically the File/Directory should have the same content
+        as the symlink target.  Platforms may stage output files/directories to
         cloud storage that lack the concept of a symlink.  In
-        this case file content may be duplicated, or (to avoid
-        duplication) the File `location` may refer to the symlink
+        this case file content and directories may be duplicated, or (to avoid
+        duplication) the File/Directory `location` may refer to the symlink
         target.
 
         It is an error if a symlink in the output directory (or any

cwltool/schemas/v1.1.0-dev1/Process.yml

@@ -237,7 +237,9 @@ $graph:
         - "null"
         - type: array
           items: [File, Directory]
-      jsonldPredicate: "cwl:secondaryFiles"
+      jsonldPredicate:
+        _id: "cwl:secondaryFiles"
+        secondaryFilesDSL: true
       doc: |
         A list of additional files or directories that are associated with the
         primary file and must be transferred alongside the primary file.
@@ -458,18 +460,25 @@ $graph:
     secondaryFiles:
       type:
         - "null"
-        - string
-        - Expression
+        - SecondaryFileSchema
         - type: array
-          items: [string, Expression]
-      jsonldPredicate: "cwl:secondaryFiles"
+          items: SecondaryFileSchema
+      jsonldPredicate:
+        _id: "cwl:secondaryFiles"
+        secondaryFilesDSL: true
       doc: |
         Only valid when `type: File` or is an array of `items: File`.
 
-        Provides a pattern or expression specifying files or directories that
-        must be included alongside the primary file.  All listed secondary
-        files must be present.  An implementation may fail workflow execution
-        if an expected secondary file does not exist.
+        Provides a pattern or expression specifying files or
+        directories that should be included alongside the primary
+        file.  Secondary files may be required or optional.  When not
+        explicitly specified, secondary files specified for `inputs`
+        are required and `outputs` are optional.  An implementation
+        must include matching Files and Directories in the
+        `secondaryFiles` property of the primary file.  These Files
+        and Directories must be transferred and staged alongside the
+        primary file.  An implementation may fail workflow execution
+        if a required secondary file does not exist.
 
         If the value is an expression, the value of `self` in the expression
         must be the primary input or output File object to which this binding
@@ -480,6 +489,8 @@ $graph:
         `path` or `location` and `basename` fields set, or an array consisting
         of strings or File or Directory objects.  It is legal to reference an
         unchanged File or Directory object taken from input as a secondaryFile.
+        The expression may return "null" in which case there is no secondaryFile
+        from that expression.
 
         To work on non-filename-preserving storage systems, portable tool
         descriptions should avoid constructing new values from `location`, but
@@ -490,11 +501,13 @@ $graph:
         it specifies that the following pattern should be applied to the path
         of the primary file to yield a filename relative to the primary File:
 
-          1. If string begins with one or more caret `^` characters, for each
+          1. If string ends with `?` character, remove the last `?` and mark
+            the resulting secondary file as optional.
+          2. If string begins with one or more caret `^` characters, for each
             caret, remove the last file extension from the path (the last
             period `.` and all following characters).  If there are no file
             extensions, the path is unchanged.
-          2. Append the remainder of the string to the end of the file path.
+          3. Append the remainder of the string to the end of the file path.
 
     streamable:
       type: boolean?
@@ -850,3 +863,50 @@ $graph:
         type: array
         items: CommandInputSchema
       doc: The list of type definitions.
+
+- name: SecondaryFileSchema
+  type: record
+  fields:
+    - name: pattern
+      type:
+        - string
+        - Expression
+      doc: |
+        Provides a pattern or expression specifying files or directories that
+        should be included alongside the primary file.
+
+        If the value is an expression, the value of `self` in the expression
+        must be the primary input or output File object to which this binding
+        applies.  The `basename`, `nameroot` and `nameext` fields must be
+        present in `self`.  For `CommandLineTool` outputs the `path` field must
+        also be present.  The expression must return a filename string relative
+        to the path to the primary File, a File or Directory object with either
+        `path` or `location` and `basename` fields set, or an array consisting
+        of strings or File or Directory objects.  It is legal to reference an
+        unchanged File or Directory object taken from input as a secondaryFile.
+        The expression may return "null" in which case there is no secondaryFile
+        from that expression.
+
+        To work on non-filename-preserving storage systems, portable tool
+        descriptions should avoid constructing new values from `location`, but
+        should construct relative references using `basename` or `nameroot`
+        instead.
+
+        If a value in `secondaryFiles` is a string that is not an expression,
+        it specifies that the following pattern should be applied to the path
+        of the primary file to yield a filename relative to the primary File:
+
+          1. If string begins with one or more caret `^` characters, for each
+            caret, remove the last file extension from the path (the last
+            period `.` and all following characters).  If there are no file
+            extensions, the path is unchanged.
+          2. If string ends with `?` character, remove the last `?` and mark
+            the resulting secondary file as optional.
+          3. Append the remainder of the string to the end of the file path.
+    - name: required
+      type: ["null", boolean, string, Expression]
+      doc: |
+        An implementation must not fail workflow execution if `required` is
+        set to `false` and the expected secondary file does not exist.
+        Default value for `required` field is `true` for secondary files on
+        input and `false` for secondary files on output.

cwltool/schemas/v1.1.0-dev1/Workflow.yml

@@ -68,6 +68,7 @@ $graph:
         * `WorkflowStepInput` now has a `label` field
         * [Addition](#Requirements_and_hints) of `cwl:requirments` field to
           input object documents
+        * `secondaryFiles` can now be explicitly marked as `required` or not.
 
       See also the [CWL Command Line Tool Description, v1.1.0-dev1 changelog](CommandLineTool.html#Changelog).
 
@@ -327,7 +328,7 @@ $graph:
         1. `null` if there is no `source` field
         2. the value of the parameter(s) specified in the `source` field when this
         workflow input parameter **is not** specified in this workflow step's `scatter` field.
-        3. an element of the parameter specified in the `source` field when this workflow input 
+        3. an element of the parameter specified in the `source` field when this workflow input
         parameter **is** specified in this workflow step's `scatter` field.
 
         The value of `inputs` in the parameter reference or expression must be

requirements.txt

@@ -2,7 +2,7 @@ requests>=2.4.3
 ruamel.yaml>=0.12.4,<=0.15.77
 rdflib>=4.2.2,<4.3
 shellescape>=3.4.1,<3.5
-schema-salad>=3.0,<3.1
+schema-salad>=3.1,<4
 typing>=3.5.3; python_version<"3.6"
 pathlib2==2.3.2; python_version<"3"
 prov==1.5.1

setup.py

@@ -53,7 +53,7 @@
           'ruamel.yaml >= 0.12.4, <= 0.15.77',
           'rdflib >= 4.2.2, < 4.3.0',
           'shellescape >= 3.4.1, < 3.5',
-          'schema-salad >= 3.0, < 3.1',
+          'schema-salad >= 3.1, < 4',
           'mypy-extensions',
           'six >= 1.9.0',  # >= 1.9.0 required by prov
           'psutil',

tests/secondary-files-job.yml

@@ -0,0 +1,3 @@
+fasta_path:
+  class: File
+  location: 2.fasta