unify and fix tempdir creation logic

Author: mr-c

Makefile

@@ -26,7 +26,7 @@ MODULE=cwltool
 # `[[` conditional expressions.
 PYSOURCES=$(wildcard ${MODULE}/**.py tests/*.py) setup.py
 DEVPKGS=diff_cover black pylint coverage pep257 pydocstyle flake8 mypy\
-	pytest-xdist isort wheel -rtest-requirements.txt
+	pytest-xdist isort wheel autoflake -rtest-requirements.txt
 DEBDEVPKGS=pep8 python-autopep8 pylint python-coverage pydocstyle sloccount \
 	   python-flake8 python-mock shellcheck
 VERSION=3.0.$(shell TZ=UTC git log --first-parent --max-count=1 \
@@ -84,6 +84,9 @@ clean: FORCE
 sort_imports:
 	isort ${MODULE}/*.py tests/*.py setup.py
 
+remove_unused_imports: $(PYSOURCES)
+	autoflake --in-place --remove-all-unused-imports $^
+
 pep257: pydocstyle
 ## pydocstyle      : check Python code style
 pydocstyle: $(PYSOURCES)

cwltool.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# PYTHON_ARGCOMPLETE_OK
 """Convenience entry point for cwltool.
 
 This can be used instead of the recommended method of `./setup.py install`

cwltool/command_line_tool.py

@@ -8,7 +8,6 @@
 import os
 import re
 import shutil
-import tempfile
 import threading
 import urllib
 from functools import cmp_to_key, partial
@@ -936,15 +935,9 @@ def update_status_output_callback(
             )
         dockerReq, _ = self.get_requirement("DockerRequirement")
         if dockerReq is not None and runtimeContext.use_container:
-            out_dir, out_prefix = os.path.split(runtimeContext.tmp_outdir_prefix)
-            j.outdir = runtimeContext.outdir or tempfile.mkdtemp(
-                prefix=out_prefix, dir=out_dir
-            )
-            tmpdir_dir, tmpdir_prefix = os.path.split(runtimeContext.tmpdir_prefix)
-            j.tmpdir = runtimeContext.tmpdir or tempfile.mkdtemp(
-                prefix=tmpdir_prefix, dir=tmpdir_dir
-            )
-            j.stagedir = tempfile.mkdtemp(prefix=tmpdir_prefix, dir=tmpdir_dir)
+            j.outdir = runtimeContext.get_outdir()
+            j.tmpdir = runtimeContext.get_tmpdir()
+            j.stagedir = runtimeContext.create_tmpdir()
         else:
             j.outdir = builder.outdir
             j.tmpdir = builder.tmpdir

cwltool/context.py

@@ -1,5 +1,7 @@
 """Shared context objects that replace use of kwargs."""
 import copy
+import os
+import tempfile
 import threading
 from typing import IO, Any, Callable, Dict, Iterable, List, Optional, TextIO, Union
 
@@ -148,6 +150,35 @@ def __init__(self, kwargs: Optional[Dict[str, Any]] = None) -> None:
         self.default_stderr = None  # type: Optional[Union[IO[bytes], TextIO]]
         super(RuntimeContext, self).__init__(kwargs)
 
+    def get_outdir(self) -> str:
+        """Return self.outdir or create one with self.tmp_outdir_prefix."""
+        if self.outdir:
+            return self.outdir
+        return self.create_outdir()
+
+    def get_tmpdir(self) -> str:
+        """Return self.tmpdir or create one with self.tmpdir_prefix."""
+        if self.tmpdir:
+            return self.tmpdir
+        return self.create_tmpdir()
+
+    def get_stagedir(self) -> str:
+        """Return self.stagedir or create one with self.tmpdir_prefix."""
+        if self.stagedir:
+            return self.stagedir
+        tmp_dir, tmp_prefix = os.path.split(self.tmpdir_prefix)
+        return tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir)
+
+    def create_tmpdir(self) -> str:
+        """Create a temporary directory that respects self.tmpdir_prefix."""
+        tmp_dir, tmp_prefix = os.path.split(self.tmpdir_prefix)
+        return tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir)
+
+    def create_outdir(self) -> str:
+        """Create a temporary directory that respects self.tmp_outdir_prefix."""
+        out_dir, out_prefix = os.path.split(self.tmp_outdir_prefix)
+        return tempfile.mkdtemp(prefix=out_prefix, dir=out_dir)
+
     def copy(self):
         # type: () -> RuntimeContext
         return copy.copy(self)

cwltool/docker.py

@@ -7,7 +7,6 @@
 import shutil
 import subprocess  # nosec
 import sys
-import tempfile
 import threading
 from distutils import spawn
 from io import StringIO, open  # pylint: disable=redefined-builtin
@@ -23,10 +22,9 @@
 from .loghandler import _logger
 from .pathmapper import MapperEnt, PathMapper
 from .utils import (
-    DEFAULT_TMP_PREFIX,
     CWLObjectType,
+    create_tmp_dir,
     docker_windows_path_adjust,
-    ensure_non_writable,
     ensure_writable,
     onWindows,
 )
@@ -109,8 +107,8 @@ def __init__(
     def get_image(
         docker_requirement: Dict[str, str],
         pull_image: bool,
-        force_pull: bool = False,
-        tmp_outdir_prefix: str = DEFAULT_TMP_PREFIX,
+        force_pull: bool,
+        tmp_outdir_prefix: str,
     ) -> bool:
         """
         Retrieve the relevant Docker container image.
@@ -170,7 +168,7 @@ def get_image(
                 subprocess.check_call(cmd, stdout=sys.stderr)  # nosec
                 found = True
             elif "dockerFile" in docker_requirement:
-                dockerfile_dir = str(tempfile.mkdtemp(prefix=tmp_outdir_prefix))
+                dockerfile_dir = create_tmp_dir(tmp_outdir_prefix)
                 with open(os.path.join(dockerfile_dir, "Dockerfile"), "wb") as dfile:
                     dfile.write(docker_requirement["dockerFile"].encode("utf-8"))
                 cmd = [
@@ -236,8 +234,8 @@ def get_from_requirements(
         self,
         r: CWLObjectType,
         pull_image: bool,
-        force_pull: bool = False,
-        tmp_outdir_prefix: str = DEFAULT_TMP_PREFIX,
+        force_pull: bool,
+        tmp_outdir_prefix: str,
     ) -> Optional[str]:
         if not spawn.find_executable("docker"):
             raise WorkflowException("docker executable is not available")
@@ -294,8 +292,7 @@ def add_writable_file_volume(
                     os.makedirs(os.path.dirname(host_outdir_tgt))
                 shutil.copy(volume.resolved, host_outdir_tgt)
             else:
-                tmp_dir, tmp_prefix = os.path.split(tmpdir_prefix)
-                tmpdir = tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir)
+                tmpdir = create_tmp_dir(tmpdir_prefix)
                 file_copy = os.path.join(tmpdir, os.path.basename(volume.resolved))
                 shutil.copy(volume.resolved, file_copy)
                 self.append_volume(runtime, file_copy, volume.target, writable=True)
@@ -312,9 +309,8 @@ def add_writable_directory_volume(
         if volume.resolved.startswith("_:"):
             # Synthetic directory that needs creating first
             if not host_outdir_tgt:
-                tmp_dir, tmp_prefix = os.path.split(tmpdir_prefix)
                 new_dir = os.path.join(
-                    tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir),
+                    create_tmp_dir(tmpdir_prefix),
                     os.path.basename(volume.target),
                 )
                 self.append_volume(runtime, new_dir, volume.target, writable=True)
@@ -327,8 +323,7 @@ def add_writable_directory_volume(
                 )
             else:
                 if not host_outdir_tgt:
-                    tmp_dir, tmp_prefix = os.path.split(tmpdir_prefix)
-                    tmpdir = tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir)
+                    tmpdir = create_tmp_dir(tmpdir_prefix)
                     new_dir = os.path.join(tmpdir, os.path.basename(volume.resolved))
                     shutil.copytree(volume.resolved, new_dir)
                     self.append_volume(runtime, new_dir, volume.target, writable=True)
@@ -434,8 +429,7 @@ def create_runtime(
                     )
                     exit(2)
             else:
-                tmp_dir, tmp_prefix = os.path.split(runtimeContext.tmpdir_prefix)
-                cidfile_dir = tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir)
+                cidfile_dir = runtimeContext.create_tmpdir()
 
             cidfile_name = datetime.datetime.now().strftime("%Y%m%d%H%M%S-%f") + ".cid"
             if runtimeContext.cidfile_prefix is not None:

cwltool/executors.py

@@ -4,7 +4,6 @@
 import logging
 import math
 import os
-import tempfile
 import threading
 from abc import ABCMeta, abstractmethod
 from threading import Lock
@@ -32,7 +31,7 @@
 from .mutation import MutationManager
 from .process import Process, cleanIntermediate, relocateOutputs
 from .provenance_profile import ProvenanceProfile
-from .utils import DEFAULT_TMP_PREFIX, CWLObjectType, JobsType
+from .utils import CWLObjectType, JobsType
 from .workflow import Workflow
 from .workflow_job import WorkflowJob, WorkflowJobStep
 
@@ -99,9 +98,7 @@ def check_for_abstract_op(tool: CWLObjectType) -> None:
         if isinstance(original_outdir, str):
             finaloutdir = os.path.abspath(original_outdir)
         runtime_context = runtime_context.copy()
-        outdir = tempfile.mkdtemp(
-            prefix=getdefault(runtime_context.tmp_outdir_prefix, DEFAULT_TMP_PREFIX)
-        )
+        outdir = runtime_context.create_outdir()
         self.output_dirs.add(outdir)
         runtime_context.outdir = outdir
         runtime_context.mutation_manager = MutationManager()

cwltool/job.py

@@ -17,7 +17,6 @@
 from typing import (
     IO,
     Callable,
-    Dict,
     Iterable,
     List,
     Match,
@@ -38,20 +37,20 @@
 from typing_extensions import TYPE_CHECKING
 
 from .builder import Builder, HasReqsHints
-from .context import RuntimeContext, getdefault
+from .context import RuntimeContext
 from .errors import UnsupportedRequirement, WorkflowException
 from .loghandler import _logger
 from .pathmapper import MapperEnt, PathMapper
 from .process import stage_files
 from .secrets import SecretStore
 from .utils import (
-    DEFAULT_TMP_PREFIX,
     CWLObjectType,
     CWLOutputType,
     DirectoryType,
     OutputCallbackType,
     bytes2str_in_dicts,
     copytree_with_merge,
+    create_tmp_dir,
     ensure_non_writable,
     ensure_writable,
     onWindows,
@@ -380,11 +379,7 @@ def _execute(
                 stderr_path=stderr_path,
                 env=env,
                 cwd=self.outdir,
-                job_dir=tempfile.mkdtemp(
-                    prefix=getdefault(
-                        runtimeContext.tmp_outdir_prefix, DEFAULT_TMP_PREFIX
-                    )
-                ),
+                job_dir=runtimeContext.create_outdir(),
                 job_script_contents=job_script_contents,
                 timelimit=self.timelimit,
                 name=self.name,
@@ -600,8 +595,8 @@ def get_from_requirements(
         self,
         r: CWLObjectType,
         pull_image: bool,
-        force_pull: bool = False,
-        tmp_outdir_prefix: str = DEFAULT_TMP_PREFIX,
+        force_pull: bool,
+        tmp_outdir_prefix: str,
     ) -> Optional[str]:
         pass
 
@@ -656,9 +651,8 @@ def create_file_and_add_volume(
     ) -> str:
         """Create the file and add a mapping."""
         if not host_outdir_tgt:
-            tmp_dir, tmp_prefix = os.path.split(tmpdir_prefix)
             new_file = os.path.join(
-                tempfile.mkdtemp(prefix=tmp_prefix, dir=tmp_dir),
+                create_tmp_dir(tmpdir_prefix),
                 os.path.basename(volume.target),
             )
         writable = True if volume.type == "CreateWritableFile" else False
@@ -770,10 +764,8 @@ def run(
                         self.get_from_requirements(
                             docker_req,
                             runtimeContext.pull_image,
-                            getdefault(runtimeContext.force_docker_pull, False),
-                            getdefault(
-                                runtimeContext.tmp_outdir_prefix, DEFAULT_TMP_PREFIX
-                            ),
+                            runtimeContext.force_docker_pull,
+                            runtimeContext.tmp_outdir_prefix,
                         )
                     )
                 if img_id is None:

cwltool/load_tool.py

@@ -8,7 +8,6 @@
 import uuid
 from typing import (
     Any,
-    Callable,
     Dict,
     List,
     MutableMapping,
@@ -21,7 +20,6 @@
 
 from ruamel.yaml.comments import CommentedMap, CommentedSeq
 from schema_salad.exceptions import ValidationException
-from schema_salad.fetcher import Fetcher
 from schema_salad.ref_resolver import Loader, file_uri
 from schema_salad.schema import validate_doc
 from schema_salad.sourceline import SourceLine, cmap

cwltool/main.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
+# PYTHON_ARGCOMPLETE_OK
 """Entry point for cwltool."""
 
-import argcomplete
 import argparse
 import functools
 import io
@@ -30,6 +30,7 @@
     cast,
 )
 
+import argcomplete
 import coloredlogs
 import pkg_resources  # part of setuptools
 from ruamel import yaml
@@ -102,8 +103,6 @@
 )
 from .workflow import Workflow
 
-# PYTHON_ARGCOMPLETE_OK
-
 
 def _terminate_processes() -> None:
     """Kill all spawned processes.

cwltool/pack.py

@@ -6,7 +6,6 @@
     Any,
     Callable,
     Dict,
-    List,
     MutableMapping,
     MutableSequence,
     Optional,
@@ -20,7 +19,6 @@
 
 from .context import LoadingContext
 from .load_tool import fetch_document, resolve_and_validate_document
-from .loghandler import _logger
 from .process import shortname, uniquename
 from .update import ORDERED_VERSIONS, update
 from .utils import CWLObjectType, CWLOutputType

cwltool/pathmapper.py

@@ -4,37 +4,15 @@
 import stat
 import urllib
 import uuid
-from typing import (
-    Callable,
-    Dict,
-    Iterator,
-    List,
-    MutableMapping,
-    MutableSequence,
-    Optional,
-    Set,
-    Text,
-    Tuple,
-    Union,
-    cast,
-)
+from typing import Dict, Iterator, List, Optional, Tuple, cast
 
 from schema_salad.exceptions import ValidationException
 from schema_salad.ref_resolver import uri_file_path
 from schema_salad.sourceline import SourceLine
 
 from .loghandler import _logger
 from .stdfsaccess import abspath
-from .utils import (
-    CWLObjectType,
-    DirectoryType,
-    adjustDirObjs,
-    adjustFileObjs,
-    convert_pathsep_to_unix,
-    dedup,
-    downloadHttpFile,
-    visit_class,
-)
+from .utils import CWLObjectType, convert_pathsep_to_unix, dedup, downloadHttpFile
 
 MapperEnt = collections.namedtuple(
     "MapperEnt", ["resolved", "target", "type", "staged"]