Merge branch 'master' into 10401-limit-dir-expansion

diff --cc sdk/cwl/arvados_cwl/arvcontainer.py
Simple merge

diff --cc sdk/cwl/arvados_cwl/pathmapper.py
index be114ebbc3f4a17cdb19821f552b19b1aebf846b,a8619a8598a538d5ba7353390bc63e316a76a648..cddb4088b7bfcbbdb211d6785ba650e8ea36901e
--- 1/sdk/cwl/arvados_cwl/pathmapper.py
--- 2/sdk/cwl/arvados_cwl/pathmapper.py
+++ b/sdk/cwl/arvados_cwl/pathmapper.py
@@@ -30,14 -31,12 +31,14 @@@ class ArvPathMapper(PathMapper)
  
      def visit(self, srcobj, uploadfiles):
          src = srcobj["location"]
 -        if srcobj["class"] == "File":
 -            if "#" in src:
 -                src = src[:src.index("#")]
 -            if isinstance(src, basestring) and ArvPathMapper.pdh_path.match(src):
 -                self._pathmap[src] = MapperEnt(src, self.collection_pattern % urllib.unquote(src[5:]), "File")
 -            if src not in self._pathmap:
 +        if "#" in src:
 +            src = src[:src.index("#")]
 +
 +        if isinstance(src, basestring) and ArvPathMapper.pdh_dirpath.match(src):
-             self._pathmap[src] = MapperEnt(src, self.collection_pattern % src[5:], srcobj["class"], True)
++            self._pathmap[src] = MapperEnt(src, self.collection_pattern % urllib.unquote(src[5:]), srcobj["class"], True)
 +
 +        if src not in self._pathmap:
 +            if src.startswith("file:"):
                  # Local FS ref, may need to be uploaded or may be on keep
                  # mount.
                  ab = abspath(src, self.input_basedir)
@@@ -48,21 -45,22 +49,21 @@@
                      if isinstance(st, arvados.commands.run.UploadFile):
                          uploadfiles.add((src, ab, st))
                      elif isinstance(st, arvados.commands.run.ArvFile):
-                         self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % st.fn[5:], "File", True)
 -                        self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % urllib.unquote(st.fn[5:]), "File")
 -                    elif src.startswith("_:"):
 -                        if "contents" in srcobj:
 -                            pass
 -                        else:
 -                            raise WorkflowException("File literal '%s' is missing contents" % src)
 -                    elif src.startswith("arvwf:"):
 -                        self._pathmap[src] = MapperEnt(src, src, "File")
++                        self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % urllib.unquote(st.fn[5:]), "File", True)
                      else:
                          raise WorkflowException("Input file path '%s' is invalid" % st)
 -            if "secondaryFiles" in srcobj:
 -                for l in srcobj["secondaryFiles"]:
 -                    self.visit(l, uploadfiles)
 -        elif srcobj["class"] == "Directory":
 -            if isinstance(src, basestring) and ArvPathMapper.pdh_dirpath.match(src):
 -                self._pathmap[src] = MapperEnt(src, self.collection_pattern % urllib.unquote(src[5:]), "Directory")
 +            elif src.startswith("_:"):
 +                if srcobj["class"] == "File" and "contents" not in srcobj:
 +                    raise WorkflowException("File literal '%s' is missing `contents`" % src)
 +                if srcobj["class"] == "Directory" and "listing" not in srcobj:
 +                    raise WorkflowException("Directory literal '%s' is missing `listing`" % src)
 +            else:
 +                self._pathmap[src] = MapperEnt(src, src, srcobj["class"], True)
 +
 +        with SourceLine(srcobj, "secondaryFiles", WorkflowException):
 +            for l in srcobj.get("secondaryFiles", []):
 +                self.visit(l, uploadfiles)
 +        with SourceLine(srcobj, "listing", WorkflowException):
              for l in srcobj.get("listing", []):
                  self.visit(l, uploadfiles)
  
@@@ -93,7 -91,7 +94,7 @@@
              loc = k["location"]
              if loc in already_uploaded:
                  v = already_uploaded[loc]
-                 self._pathmap[loc] = MapperEnt(v.resolved, self.collection_pattern % v.resolved[5:], "File", True)
 -                self._pathmap[loc] = MapperEnt(v.resolved, self.collection_pattern % urllib.unquote(v.resolved[5:]), "File")
++                self._pathmap[loc] = MapperEnt(v.resolved, self.collection_pattern % urllib.unquote(v.resolved[5:]), "File", True)
  
          for srcobj in referenced_files:
              self.visit(srcobj, uploadfiles)
@@@ -108,8 -106,7 +109,8 @@@
                                               project=self.arvrunner.project_uuid)
  
          for src, ab, st in uploadfiles:
-             self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % st.keepref,
 -            self._pathmap[src] = MapperEnt(urllib.quote(st.fn, "/:+@"), self.collection_pattern % st.fn[5:], "File")
++            self._pathmap[src] = MapperEnt(urllib.quote(st.fn, "/:+@"), self.collection_pattern % st.fn[5:],
 +                                           "Directory" if os.path.isdir(ab) else "File", True)
              self.arvrunner.add_uploaded(src, self._pathmap[src])
  
          for srcobj in referenced_files:

diff --cc sdk/cwl/tests/test_pathmapper.py
index a81a8531b998757698260d59cea8b9a982bd97f4,3b6af04b293e8f48e320fa6508716d3c6d27faf6..b39a9842845f7af01c64d62c713819ecaf1aca1c
--- 1/sdk/cwl/tests/test_pathmapper.py
--- 2/sdk/cwl/tests/test_pathmapper.py
+++ b/sdk/cwl/tests/test_pathmapper.py
@@@ -41,19 -40,19 +41,25 @@@ class TestPathmap(unittest.TestCase)
                           p._pathmap)
  
      @mock.patch("arvados.commands.run.uploadfiles")
--    def test_upload(self, upl):
++    @mock.patch("arvados.commands.run.statfile")
++    def test_upload(self, statfile, upl):
          """Test pathmapper uploading files."""
  
          arvrunner = arvados_cwl.ArvCwlRunner(self.api)
  
++        def statfile_mock(prefix, fn, fnPattern="$(file %s/%s)", dirPattern="$(dir %s/%s/)"):
++            st = arvados.commands.run.UploadFile("", "tests/hw.py")
++            return st
++
          upl.side_effect = upload_mock
++        statfile.side_effect = statfile_mock
  
          p = ArvPathMapper(arvrunner, [{
              "class": "File",
--            "location": "tests/hw.py"
++            "location": "file:tests/hw.py"
          }], "", "/test/%s", "/test/%s/%s")
  
-         self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
 -        self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File')},
++        self.assertEqual({'file:tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
                           p._pathmap)
  
      @mock.patch("arvados.commands.run.uploadfiles")
@@@ -61,16 -60,16 +67,16 @@@
          """Test pathmapper handling previously uploaded files."""
  
          arvrunner = arvados_cwl.ArvCwlRunner(self.api)
-         arvrunner.add_uploaded('tests/hw.py', MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='', type='File', staged=True))
 -        arvrunner.add_uploaded('tests/hw.py', MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='', type='File'))
++        arvrunner.add_uploaded('file:tests/hw.py', MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='', type='File', staged=True))
  
          upl.side_effect = upload_mock
  
          p = ArvPathMapper(arvrunner, [{
              "class": "File",
--            "location": "tests/hw.py"
++            "location": "file:tests/hw.py"
          }], "", "/test/%s", "/test/%s/%s")
  
-         self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
 -        self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File')},
++        self.assertEqual({'file:tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
                           p._pathmap)
  
      @mock.patch("arvados.commands.run.uploadfiles")
@@@ -90,8 -89,8 +96,8 @@@
  
          p = ArvPathMapper(arvrunner, [{
              "class": "File",
--            "location": "tests/hw.py"
++            "location": "file:tests/hw.py"
          }], "", "/test/%s", "/test/%s/%s")
  
-         self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
 -        self.assertEqual({'tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File')},
++        self.assertEqual({'file:tests/hw.py': MapperEnt(resolved='keep:99999999999999999999999999999991+99/hw.py', target='/test/99999999999999999999999999999991+99/hw.py', type='File', staged=True)},
                           p._pathmap)

diff --cc sdk/cwl/tests/wf/scatter2.cwl
index f73ec2b13d3c4919dd99755bae65bbda8ff560a3,f73ec2b13d3c4919dd99755bae65bbda8ff560a3..83b3537d528ea55446f26e7862ece767333fd8db
--- 1/sdk/cwl/tests/wf/scatter2.cwl
--- 2/sdk/cwl/tests/wf/scatter2.cwl
+++ b/sdk/cwl/tests/wf/scatter2.cwl
@@@ -44,6 -44,6 +44,7 @@@ steps
            out: [out]
            run:
              class: CommandLineTool
++            id: subtool
              inputs:
                sleeptime:
                  type: int

diff --cc sdk/cwl/tests/wf/scatter2_subwf.cwl
index df4d992c359a42eeb619826aa14ce4b87aeab19d,df4d992c359a42eeb619826aa14ce4b87aeab19d..dd4c7054a94c5c9dc73785eafbbc36e2bdff55b5
--- 1/sdk/cwl/tests/wf/scatter2_subwf.cwl
--- 2/sdk/cwl/tests/wf/scatter2_subwf.cwl
+++ b/sdk/cwl/tests/wf/scatter2_subwf.cwl
@@@ -50,9 -50,9 +50,10 @@@
            "run": {
              "baseCommand": "sleep",
              "class": "CommandLineTool",
++            "id": "#main/sleep1/subtool",
              "inputs": [
                {
--                "id": "#main/sleep1/sleeptime",
++                "id": "#main/sleep1/subtool/sleeptime",
                  "inputBinding": {
                    "position": 1
                  },
@@@ -61,7 -61,7 +62,7 @@@
              ],
              "outputs": [
                {
--                "id": "#main/sleep1/out",
++                "id": "#main/sleep1/subtool/out",
                  "outputBinding": {
                    "outputEval": "out"
                  },