Merge pull request #829 from common-workflow-language/windows_fixes

Windows fixes

Also normalizes the Factory() interface by removing the use of kwargs

Author: mr-c

cwltool/docker.py

@@ -36,21 +36,26 @@ def _get_docker_machine_mounts():  # type: () -> List[Text]
             if 'DOCKER_MACHINE_NAME' not in os.environ:
                 __docker_machine_mounts = []
             else:
-                __docker_machine_mounts = [u'/' + line.split(None, 1)[0]
-                        for line in subprocess.check_output(
-                            ['docker-machine', 'ssh',
-                                os.environ['DOCKER_MACHINE_NAME'],
-                                'mount', '-t', 'vboxsf'],
-                            universal_newlines=True).splitlines()]
+                __docker_machine_mounts = [
+                    u'/' + line.split(None, 1)[0] for line in
+                    subprocess.check_output(
+                        ['docker-machine', 'ssh',
+                         os.environ['DOCKER_MACHINE_NAME'], 'mount', '-t',
+                         'vboxsf'],
+                        universal_newlines=True).splitlines()]
     return __docker_machine_mounts
 
 def _check_docker_machine_path(path):  # type: (Optional[Text]) -> None
     if not path:
         return
+    if onWindows():
+        path = path.lower()
     mounts = _get_docker_machine_mounts()
     if mounts:
         found = False
         for mount in mounts:
+            if onWindows():
+                mount = mount.lower()
             if path.startswith(mount):
                 found = True
                 break
@@ -59,9 +64,11 @@ def _check_docker_machine_path(path):  # type: (Optional[Text]) -> None
                 "Input path {path} is not in the list of host paths mounted "
                 "into the Docker virtual machine named {name}. Already mounted "
                 "paths: {mounts}.\n"
-                "See https://docs.docker.com/toolbox/toolbox_install_windows/#optional-add-shared-directories"
-                " for instructions on how to add this path to your VM.".format(path=path,
-                    name=os.environ["DOCKER_MACHINE_NAME"], mounts=mounts))
+                "See https://docs.docker.com/toolbox/toolbox_install_windows/"
+                "#optional-add-shared-directories for instructions on how to "
+                "add this path to your VM.".format(
+                    path=path, name=os.environ["DOCKER_MACHINE_NAME"],
+                    mounts=mounts))
 
 
 class DockerCommandLineJob(ContainerCommandLineJob):

cwltool/factory.py

@@ -1,8 +1,8 @@
 from __future__ import absolute_import
 
 import os
-from typing import Callable as tCallable # pylint: disable=unused-import
-from typing import (Any, # pylint: disable=unused-import
+from typing import Callable as tCallable   # pylint: disable=unused-import
+from typing import (Any,  # pylint: disable=unused-import
                     Dict, Optional, Text, Tuple, Union)
 
 from . import load_tool
@@ -29,44 +29,34 @@ def __init__(self, t, factory):  # type: (Process, Factory) -> None
 
     def __call__(self, **kwargs):
         # type: (**Any) -> Union[Text, Dict[Text, Text]]
-        runtimeContext = self.factory.runtimeContext.copy()
-        runtimeContext.basedir = os.getcwd()
-        out, status = self.factory.executor(self.t, kwargs, runtimeContext)
+        runtime_context = self.factory.runtime_context.copy()
+        runtime_context.basedir = os.getcwd()
+        out, status = self.factory.executor(self.t, kwargs, runtime_context)
         if status != "success":
             raise WorkflowStatus(out, status)
         else:
             return out
 
 class Factory(object):
     def __init__(self,
-                 executor=None,            # type: tCallable[...,Tuple[Dict[Text,Any], Text]]
-                 loadingContext=None,      # type: LoadingContext
-                 runtimeContext=None,      # type: RuntimeContext
-                 **kwargs
+                 executor=None,        # type: tCallable[...,Tuple[Dict[Text,Any], Text]]
+                 loading_context=None,  # type: LoadingContext
+                 runtime_context=None   # type: RuntimeContext
                 ):  # type: (...) -> None
         if executor is None:
             executor = SingleJobExecutor()
         self.executor = executor
-
-        new_exec_kwargs = get_default_args()
-        new_exec_kwargs.update(kwargs)
-        new_exec_kwargs.pop("job_order")
-        new_exec_kwargs.pop("workflow")
-        new_exec_kwargs.pop("outdir")
-
-        if loadingContext is None:
-            self.loadingContext = LoadingContext(new_exec_kwargs)
-        else:
-            self.loadingContext = loadingContext
-
-        if runtimeContext is None:
-            self.runtimeContext = RuntimeContext(new_exec_kwargs)
+        self.loading_context = loading_context
+        if loading_context is None:
+            self.loading_context = LoadingContext()
+        if runtime_context is None:
+            self.runtime_context = RuntimeContext()
         else:
-            self.runtimeContext = runtimeContext
+            self.runtime_context = runtime_context
 
     def make(self, cwl):
         """Instantiate a CWL object from a CWl document."""
-        load = load_tool.load_tool(cwl, self.loadingContext)
+        load = load_tool.load_tool(cwl, self.loading_context)
         if isinstance(load, int):
             raise Exception("Error loading tool")
         return Callable(load, self)

cwltool/main.py

@@ -592,12 +592,13 @@ def main(argsl=None,                   # type: List[str]
             conf_file = getattr(args, "beta_dependency_resolvers_configuration", None)  # Text
             use_conda_dependencies = getattr(args, "beta_conda_dependencies", None)  # Text
 
-            job_script_provider = None  # type: Optional[DependenciesConfiguration]
             if conf_file or use_conda_dependencies:
                 runtimeContext.job_script_provider = DependenciesConfiguration(args)
 
-            runtimeContext.find_default_container = \
-                functools.partial(find_default_container, args)
+            runtimeContext.find_default_container = functools.partial(
+                find_default_container,
+                default_container=runtimeContext.default_container,
+                use_biocontainers=args.beta_use_biocontainers)
             runtimeContext.make_fs_access = getdefault(runtimeContext.make_fs_access, StdFsAccess)
             (out, status) = executor(tool,
                                      initialized_job_order_object,
@@ -674,14 +675,14 @@ def loc_to_path(obj):
         _logger.addHandler(defaultStreamHandler)
 
 
-def find_default_container(args, builder):
-    # type: (argparse.Namespace, HasReqsHints) -> Optional[Text]
-    default_container = None
-    if args.default_container:
-        default_container = args.default_container
-    elif args.beta_use_biocontainers:
-        default_container = get_container_from_software_requirements(args, builder)
-
+def find_default_container(builder,                  # type: HasReqsHints
+                           default_container=None,   # type: Text
+                           use_biocontainers=None,  # type: bool
+                          ):  # type: (...) -> Optional[Text]
+    """Default finder for default containers."""
+    if not default_container and use_biocontainers:
+        default_container = get_container_from_software_requirements(
+            use_biocontainers, builder)
     return default_container
 
 

cwltool/software_requirements.py

@@ -100,9 +100,9 @@ def get_dependencies(builder):
     return ToolRequirements.from_list(dependencies)
 
 
-def get_container_from_software_requirements(args, builder):
-    # type: (argparse.Namespace, HasReqsHints) -> Optional[Text]
-    if args.beta_use_biocontainers:
+def get_container_from_software_requirements(use_biocontainers, builder):
+    # type: (bool, HasReqsHints) -> Optional[Text]
+    if use_biocontainers:
         ensure_galaxy_lib_available()
         from galaxy.tools.deps.containers import ContainerRegistry, AppInfo, ToolInfo, DOCKER_CONTAINER_TYPE
         app_info = AppInfo(

tests/test_examples.py

@@ -19,6 +19,7 @@
 from cwltool.errors import WorkflowException
 from cwltool.main import main
 from cwltool.utils import onWindows, subprocess
+from cwltool.context import RuntimeContext
 
 from .util import (get_data, get_windows_safe_factory, needs_docker,
                    needs_singularity, windows_needs_docker)
@@ -152,16 +153,21 @@ def test_factory_bad_outputs(self):
 
     def test_default_args(self):
         f = cwltool.factory.Factory()
-        assert f.runtimeContext.use_container is True
-        assert f.runtimeContext.on_error == "stop"
+        assert f.runtime_context.use_container is True
+        assert f.runtime_context.on_error == "stop"
 
     def test_redefined_args(self):
-        f = cwltool.factory.Factory(use_container=False, on_error="continue")
-        assert f.runtimeContext.use_container is False
-        assert f.runtimeContext.on_error == "continue"
+        runtime_context = RuntimeContext()
+        runtime_context.use_container = False
+        runtime_context.on_error = "continue"
+        f = cwltool.factory.Factory(runtime_context=runtime_context)
+        assert f.runtime_context.use_container is False
+        assert f.runtime_context.on_error == "continue"
 
     def test_partial_scatter(self):
-        f = cwltool.factory.Factory(on_error="continue")
+        runtime_context = RuntimeContext()
+        runtime_context.on_error = "continue"
+        f = cwltool.factory.Factory(runtime_context=runtime_context)
         fail = f.make(get_data("tests/wf/scatterfail.cwl"))
         try:
             fail()
@@ -173,7 +179,9 @@ def test_partial_scatter(self):
             self.fail("Should have raised WorkflowStatus")
 
     def test_partial_output(self):
-        f = cwltool.factory.Factory(on_error="continue")
+        runtime_context = RuntimeContext()
+        runtime_context.on_error = "continue"
+        f = cwltool.factory.Factory(runtime_context=runtime_context)
         fail = f.make(get_data("tests/wf/wffail.cwl"))
         try:
             fail()
@@ -248,36 +256,32 @@ def loadref(base, p):
 
         sc.sort(key=lambda k: k["basename"])
 
-        self.assertEquals([{
-            "basename": "bar.cwl",
-            "nameroot": "bar",
-            "class": "File",
-            "nameext": ".cwl",
-            "location": "file:///example/bar.cwl"
-        },
-            {
-                "basename": "data.txt",
-                "nameroot": "data",
-                "class": "File",
-                "nameext": ".txt",
-                "location": "file:///example/data.txt"
-            },
-            {
-                "basename": "data2",
-                "class": "Directory",
-                "location": "file:///example/data2",
-                "listing": [{
-                    "basename": "data3.txt",
-                    "nameroot": "data3",
-                    "class": "File",
-                    "nameext": ".txt",
-                    "location": "file:///example/data3.txt",
-                    "secondaryFiles": [{
-                        "class": "File",
-                        "basename": "data5.txt",
-                        "location": "file:///example/data5.txt",
-                        "nameext": ".txt",
-                        "nameroot": "data5"
+        self.assertEquals([
+            {"basename": "bar.cwl",
+             "nameroot": "bar",
+             "class": "File",
+             "nameext": ".cwl",
+             "location": "file:///example/bar.cwl"},
+            {"basename": "data.txt",
+             "nameroot": "data",
+             "class": "File",
+             "nameext": ".txt",
+             "location": "file:///example/data.txt"},
+            {"basename": "data2",
+             "class": "Directory",
+             "location": "file:///example/data2",
+             "listing": [
+                 {"basename": "data3.txt",
+                  "nameroot": "data3",
+                  "class": "File",
+                  "nameext": ".txt",
+                  "location": "file:///example/data3.txt",
+                  "secondaryFiles": [
+                      {"class": "File",
+                       "basename": "data5.txt",
+                       "location": "file:///example/data5.txt",
+                       "nameext": ".txt",
+                       "nameroot": "data5"
                     }]
                 }]
             }, {
@@ -313,39 +317,27 @@ def test_trick_scandeps(self):
 
 class TestDedup(unittest.TestCase):
     def test_dedup(self):
-        ex = [{
-            "class": "File",
-            "location": "file:///example/a"
-        },
-            {
-                "class": "File",
-                "location": "file:///example/a"
-            },
-            {
-                "class": "File",
-                "location": "file:///example/d"
-            },
-            {
-                "class": "Directory",
-                "location": "file:///example/c",
-                "listing": [{
-                    "class": "File",
-                    "location": "file:///example/d"
-                }]
-            }]
-
-        self.assertEquals([{
-            "class": "File",
-            "location": "file:///example/a"
-        },
-            {
-                "class": "Directory",
-                "location": "file:///example/c",
-                "listing": [{
-                    "class": "File",
-                    "location": "file:///example/d"
-                }]
-            }], cwltool.pathmapper.dedup(ex))
+        ex = [{"class": "File",
+               "location": "file:///example/a"},
+              {"class": "File",
+               "location": "file:///example/a"},
+              {"class": "File",
+               "location": "file:///example/d"},
+              {"class": "Directory",
+               "location": "file:///example/c",
+               "listing": [
+                   {"class": "File",
+                    "location": "file:///example/d"}]}]
+
+        self.assertEquals([
+            {"class": "File",
+             "location": "file:///example/a"},
+            {"class": "Directory",
+             "location": "file:///example/c",
+             "listing": [
+                 {"class": "File",
+                  "location": "file:///example/d"}]}],
+            cwltool.pathmapper.dedup(ex))
 
 
 class TestTypeCompare(unittest.TestCase):
@@ -623,11 +615,9 @@ def test_print_dot(self):
 
 class TestCmdLine(unittest.TestCase):
     def get_main_output(self, new_args):
-        process = subprocess.Popen([
-                                       sys.executable,
-                                       "-m",
-                                       "cwltool"
-                                   ] + new_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        process = subprocess.Popen(
+            [sys.executable, "-m", "cwltool"] + new_args,
+            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
         stdout, stderr = process.communicate()
         return process.returncode, stdout.decode(), stderr.decode()
@@ -684,15 +674,15 @@ def test_issue_740_fixed(self):
 class TestChecksum(TestCmdLine):
 
     def test_compute_checksum(self):
-        f = cwltool.factory.Factory(compute_checksum=True,
-                use_container=onWindows())
+        runtime_context = RuntimeContext()
+        runtime_context.compute_checksum = True
+        runtime_context.use_container = onWindows()
+        f = cwltool.factory.Factory(runtime_context=runtime_context)
         echo = f.make(get_data("tests/wf/cat-tool.cwl"))
-        output = echo(file1={
-                "class": "File",
-                "location": get_data("tests/wf/whale.txt")
-            },
-            reverse=False
-        )
+        output = echo(
+            file1={"class": "File",
+                   "location": get_data("tests/wf/whale.txt")},
+            reverse=False)
         self.assertEquals(output['output']["checksum"], "sha1$327fc7aedf4f6b69a42a7c8b808dc5a7aff61376")
 
     def test_no_compute_checksum(self):
@@ -721,5 +711,6 @@ def test_singularity_workflow(self):
         self.assertIn("completed success", stderr)
         self.assertEquals(error_code, 0)
 
+
 if __name__ == '__main__':
     unittest.main()