Merge branch '19109-upload-secondary' refs #19109

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/.licenseignore b/.licenseignore
index 387aeda9445daa25d45dac26dea99b4745456809..203c378bdcfa1dadc483234dcea453c91462066f 100644 (file)
--- a/.licenseignore
+++ b/.licenseignore
@@ -50,6 +50,7 @@ docker/jobs/1078ECD7.key
 */script/rails
 sdk/cwl/tests/input/blorp.txt
 sdk/cwl/tests/tool/blub.txt
+sdk/cwl/tests/19109-upload-secondary/*
 sdk/cwl/tests/federation/data/*
 sdk/cwl/tests/fake-keep-mount/fake_collection_dir/.arvados#collection
 sdk/go/manifest/testdata/*_manifest

diff --git a/sdk/cwl/arvados_cwl/runner.py b/sdk/cwl/arvados_cwl/runner.py
index 50b3bb94d8934c1234220c25a3aa6bfb1a3075a0..f232178c5d5400ab90aad300d2f1d2d70909b98d 100644 (file)
--- a/sdk/cwl/arvados_cwl/runner.py
+++ b/sdk/cwl/arvados_cwl/runner.py
@@ -129,6 +129,9 @@ def set_secondary(fsaccess, builder, inputschema, secondaryspec, primary, discov
             set_secondary(fsaccess, builder, i, secondaryspec, primary, discovered)
         return
 
+    if inputschema == "File":
+        inputschema = {"type": "File"}
+
     if isinstance(inputschema, basestring):
         sd = search_schemadef(inputschema, reversed(builder.hints+builder.requirements))
         if sd:
@@ -164,10 +167,13 @@ def set_secondary(fsaccess, builder, inputschema, secondaryspec, primary, discov
             set_secondary(fsaccess, builder, {"type": inputschema["items"]}, secondaryspec, p, discovered)
 
     elif (inputschema["type"] == "File" and
-          secondaryspec and
           isinstance(primary, Mapping) and
-          primary.get("class") == "File" and
-          "secondaryFiles" not in primary):
+          primary.get("class") == "File"):
+
+        if "secondaryFiles" in primary or not secondaryspec:
+            # Nothing to do.
+            return
+
         #
         # Found a file, check for secondaryFiles
         #
@@ -175,9 +181,9 @@ def set_secondary(fsaccess, builder, inputschema, secondaryspec, primary, discov
         primary["secondaryFiles"] = secondaryspec
         for i, sf in enumerate(aslist(secondaryspec)):
             if builder.cwlVersion == "v1.0":
-                pattern = builder.do_eval(sf, context=primary)
+                pattern = sf
             else:
-                pattern = builder.do_eval(sf["pattern"], context=primary)
+                pattern = sf["pattern"]
             if pattern is None:
                 continue
             if isinstance(pattern, list):
@@ -214,7 +220,20 @@ def set_secondary(fsaccess, builder, inputschema, secondaryspec, primary, discov
                     "Expression must return list, object, string or null")
 
             if pattern is not None:
-                sfpath = substitute(primary["location"], pattern)
+                if "${" in pattern or "$(" in pattern:
+                    sfname = builder.do_eval(pattern, context=primary)
+                else:
+                    sfname = substitute(primary["basename"], pattern)
+
+                if sfname is None:
+                    continue
+
+                p_location = primary["location"]
+                if "/" in p_location:
+                    sfpath = (
+                        p_location[0 : p_location.rindex("/") + 1]
+                        + sfname
+                    )
 
             required = builder.do_eval(required, context=primary)
 

diff --git a/sdk/cwl/tests/19109-upload-secondary.cwl b/sdk/cwl/tests/19109-upload-secondary.cwl
new file mode 100644 (file)
index 0000000..933496b
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary.cwl
@@ -0,0 +1,42 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.2
+class: Workflow
+
+requirements:
+  InlineJavascriptRequirement: {}
+
+inputs:
+  file1:
+    type: File?
+    secondaryFiles:
+      - pattern: .tbi
+        required: true
+  file2:
+    type: File
+    secondaryFiles:
+      - pattern: |
+          ${
+          return self.basename + '.tbi';
+          }
+        required: true
+outputs:
+  out:
+    type: File
+    outputSource: cat/out
+  out2:
+    type: File
+    outputSource: cat2/out
+steps:
+  cat:
+    in:
+      inp: file1
+    run: cat2.cwl
+    out: [out]
+  cat2:
+    in:
+      inp: file2
+    run: cat2.cwl
+    out: [out]

diff --git a/sdk/cwl/tests/19109-upload-secondary.yml b/sdk/cwl/tests/19109-upload-secondary.yml
new file mode 100644 (file)
index 0000000..af4147a
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary.yml
@@ -0,0 +1,10 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+file1:
+  class: File
+  location: 19109-upload-secondary/file1.txt
+file2:
+  class: File
+  location: 19109-upload-secondary/file2.txt

diff --git a/sdk/cwl/tests/19109-upload-secondary/file1.txt b/sdk/cwl/tests/19109-upload-secondary/file1.txt
new file mode 100644 (file)
index 0000000..41549db
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary/file1.txt
@@ -0,0 +1 @@
+strawberry

diff --git a/sdk/cwl/tests/19109-upload-secondary/file1.txt.tbi b/sdk/cwl/tests/19109-upload-secondary/file1.txt.tbi
new file mode 100644 (file)
index 0000000..c9438bd
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary/file1.txt.tbi
@@ -0,0 +1 @@
+blueberry
\ No newline at end of file

diff --git a/sdk/cwl/tests/19109-upload-secondary/file2.txt b/sdk/cwl/tests/19109-upload-secondary/file2.txt
new file mode 100644 (file)
index 0000000..637a09b
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary/file2.txt
@@ -0,0 +1 @@
+banana

diff --git a/sdk/cwl/tests/19109-upload-secondary/file2.txt.tbi b/sdk/cwl/tests/19109-upload-secondary/file2.txt.tbi
new file mode 100644 (file)
index 0000000..4d5e625
--- /dev/null
+++ b/sdk/cwl/tests/19109-upload-secondary/file2.txt.tbi
@@ -0,0 +1 @@
+mango
\ No newline at end of file

diff --git a/sdk/cwl/tests/arvados-tests.yml b/sdk/cwl/tests/arvados-tests.yml
index 9e691bdba558593b6f06423611037e016b88a930..2f309cfe81e6aae5a26ebacdea842d957d07ab0b 100644 (file)
--- a/sdk/cwl/tests/arvados-tests.yml
+++ b/sdk/cwl/tests/arvados-tests.yml
@@ -449,3 +449,23 @@
   output: {}
   tool: 18994-basename/wf_ren.cwl
   doc: "Test issue 18994 - correctly stage file with modified basename"
+
+- job: 19109-upload-secondary.yml
+  output: {
+    "out": {
+        "basename": "file1.catted",
+        "class": "File",
+        "location": "file1.catted",
+        "size": 20,
+        "checksum": "sha1$c4cead17cebdd829f38c48e18a28f1da72339ef7"
+    },
+    "out2": {
+        "basename": "file2.catted",
+        "checksum": "sha1$6f71c5d1512519ede45bedfdd624e05fd8037b0d",
+        "class": "File",
+        "location": "file2.catted",
+        "size": 12
+    }
+  }
+  tool: 19109-upload-secondary.cwl
+  doc: "Test issue 19109 - correctly discover & upload secondary files"

diff --git a/sdk/cwl/tests/cat2.cwl b/sdk/cwl/tests/cat2.cwl
new file mode 100644 (file)
index 0000000..82d93ef
--- /dev/null
+++ b/sdk/cwl/tests/cat2.cwl
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+cwlVersion: v1.2
+class: CommandLineTool
+inputs:
+  - id: inp
+    type: File
+    secondaryFiles:
+      - pattern: .tbi
+        required: true
+stdout: $(inputs.inp.nameroot).catted
+outputs:
+  out:
+    type: stdout
+arguments: [cat, '$(inputs.inp.path)', '$(inputs.inp.secondaryFiles[0].path)']