move hints vs. requirements specific error checking earlier

MANIFEST.in

@@ -11,6 +11,7 @@ include tests/loop-ext/*
 include tests/tmp1/tmp2/tmp3/.gitkeep
 include tests/tmp4/alpha/*
 include tests/wf/*
+include tests/wf/inp-filelist.txt
 include tests/wf/operation/*
 include tests/override/*
 include tests/reloc/*.cwl

build-cwltool-docker.sh

@@ -8,4 +8,4 @@ ${engine} run -t -v /var/run/docker.sock:/var/run/docker.sock \
 	-v /tmp:/tmp \
 	-v "$PWD":/tmp/cwltool \
 	quay.io/commonwl/cwltool_module /bin/sh -c \
-	"apk add gcc bash git && pip install -r/tmp/cwltool/test-requirements.txt ; pytest -k 'not (test_bioconda or test_double_overwrite or test_env_filtering or test_biocontainers or test_disable_file_overwrite_without_ext or test_disable_file_creation_in_outdir_with_ext or test_write_write_conflict or test_directory_literal_with_real_inputs_inside or test_revsort_workflow or test_stdin_with_id_preset or test_no_compute_chcksum or test_packed_workflow_execution[tests/wf/count-lines1-wf.cwl-tests/wf/wc-job.json-False] or test_sequential_workflow or test_single_process_subwf_subwf_inline_step)' --ignore-glob '*test_udocker.py' -n 0 -v -rs --pyargs cwltool"
+	"apk add gcc bash git && pip install -r/tmp/cwltool/test-requirements.txt ; pytest -k 'not (test_bioconda or test_double_overwrite or test_env_filtering or test_biocontainers or test_disable_file_overwrite_without_ext or test_disable_file_creation_in_outdir_with_ext or test_write_write_conflict or test_directory_literal_with_real_inputs_inside or test_revsort_workflow or test_stdin_with_id_preset or test_no_compute_chcksum or test_packed_workflow_execution[tests/wf/count-lines1-wf.cwl-tests/wf/wc-job.json-False] or test_sequential_workflow or test_single_process_subwf_subwf_inline_step or test_cache_dockerreq_hint_instead_of_req)' --ignore-glob '*test_udocker.py' -n 0 -v -rs --pyargs cwltool"

cwltool/argparser.py

@@ -415,9 +415,7 @@ def arg_parser() -> argparse.ArgumentParser:
         dest="rm_container",
     )
 
-    cidgroup = container_group.add_argument_group(
-        "Recording the Docker container identifier into a file"
-    )
+    cidgroup = parser.add_argument_group("Recording the software container identifier into a file")
     cidgroup.add_argument(
         # Disabled as containerid is now saved by default
         "--record-container-id",
@@ -430,15 +428,15 @@ def arg_parser() -> argparse.ArgumentParser:
     cidgroup.add_argument(
         "--cidfile-dir",
         type=str,
-        help="Store the Docker container ID into a file in the specified directory.",
+        help="Store the software container ID into a file in the specified directory.",
         default=None,
         dest="cidfile_dir",
     )
 
     cidgroup.add_argument(
         "--cidfile-prefix",
         type=str,
-        help="Specify a prefix to the container ID filename. "
+        help="Specify a prefix to the software container ID filename. "
         "Final file name will be followed by a timestamp. "
         "The default is no prefix.",
         default=None,

cwltool/command_line_tool.py

@@ -431,28 +431,6 @@
                 return SingularityCommandLineJob
             elif runtimeContext.user_space_docker_cmd:
                 return UDockerCommandLineJob
-            if mpiReq is not None:
-                if mpiRequired:
-                    if dockerRequired:
-                        raise UnsupportedRequirement(
-                            "No support for Docker and MPIRequirement both being required"
-                        )
-                    else:
-                        _logger.warning(
-                            "MPI has been required while Docker is hinted, discarding Docker hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
-                        return CommandLineJob
-                else:
-                    if dockerRequired:
-                        _logger.warning(
-                            "Docker has been required while MPI is hinted, discarding MPI hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
-                    else:
-                        raise UnsupportedRequirement(
-                            "Both Docker and MPI have been hinted - don't know what to do"
-                        )
             if runtimeContext.podman:
                 return PodmanCommandLineJob
             return DockerCommandLineJob
@@ -492,7 +470,14 @@
         for ls in cast(list[CWLObjectType], fn.get("listing", [])):
             self.updatePathmap(os.path.join(outdir, cast(str, fn["basename"])), pathmap, ls)
 
-    def _initialworkdir(self, j: JobBase, builder: Builder) -> None:
+    def _initialworkdir(self, j: Optional[JobBase], builder: Builder) -> None:
+        """
+        Test and initialize the working directory.
+
+        :param j: A :py:class:`~cwltool.job.CommandLineJob` or a
+                  specialized container-based job.
+                  If 'None', then only tests will be performed, no setup.
+        """
         initialWorkdir, _ = self.get_requirement("InitialWorkDirRequirement")
         if initialWorkdir is None:
             return
@@ -760,6 +745,9 @@
                         "is in 'requirements'."
                     )
 
+        if j is None:
+            return  # Only testing
+
         with SourceLine(initialWorkdir, "listing", WorkflowException, debug):
             j.generatefiles["listing"] = ls
             for entry in ls:
@@ -824,6 +812,7 @@
                 _check_adjust,
             )
             visit_class([cachebuilder.files, cachebuilder.bindings], ("File"), _checksum)
+            self._initialworkdir(None, cachebuilder)  # test the initial working directory
 
             cmdline = flatten(list(map(cachebuilder.generate_arg, cachebuilder.bindings)))
             docker_req, _ = self.get_requirement("DockerRequirement")

cwltool/process.py

@@ -810,12 +810,47 @@ def inc(d: list[int]) -> None:
         tmpdir = ""
         stagedir = ""
 
-        docker_req, _ = self.get_requirement("DockerRequirement")
+        docker_req, docker_required = self.get_requirement("DockerRequirement")
         default_docker = None
+        mpi_req, mpi_required = self.get_requirement(MPIRequirementName)
 
         if docker_req is None and runtime_context.default_container:
             default_docker = runtime_context.default_container
 
+        if (
+            docker_req is not None
+            and runtime_context.use_container
+            and not runtime_context.singularity
+            and not runtime_context.user_space_docker_cmd
+            and mpi_req is not None
+        ):
+            if mpi_required:
+                if docker_required:
+                    raise UnsupportedRequirement(
+                        "No support for DockerRequirement and MPIRequirement "
+                        "both being required, unless Singularity or uDocker is being used."
+                    )
+                else:
+                    _logger.warning(
+                        "MPI has been required while DockerRequirement is hinted "
+                        "and neither Singularity nor uDocker is being used, discarding Docker hint(s)."
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
+                    docker_req = None
+                    docker_required = False
+            else:
+                if docker_required:
+                    _logger.warning(
+                        "Docker has been required (and neither Singularity nor "
+                        "uDocker is being used) while MPI is hinted, discarding MPI hint(s)/"
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
+                else:
+                    raise UnsupportedRequirement(
+                        "Both Docker and MPI have been hinted and neither "
+                        "Singularity nor uDocker are being used - don't know what to do."
+                    )
+
         if (docker_req or default_docker) and runtime_context.use_container:
             if docker_req is not None:
                 # Check if docker output directory is absolute

tests/test_caching.py

@@ -0,0 +1,172 @@
+import re
+from pathlib import Path
+
+import pytest
+
+from .util import get_data, get_main_output, needs_docker
+
+test_factors = [(""), ("--parallel"), ("--debug"), ("--parallel --debug")]
+
+
+@needs_docker
+@pytest.mark.parametrize("factor", test_factors)
+def test_wf_without_container(tmp_path: Path, factor: str) -> None:
+    """Confirm that we can run a workflow without a container."""
+    test_file = "hello-workflow.cwl"
+    cache_dir = str(tmp_path / "cwltool_cache")
+    commands = factor.split()
+    commands.extend(
+        [
+            "--cachedir",
+            cache_dir,
+            "--outdir",
+            str(tmp_path / "outdir"),
+            get_data("tests/wf/" + test_file),
+            "--usermessage",
+            "hello",
+        ]
+    )
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "completed success" in stderr
+    assert error_code == 0
+
+
+@needs_docker
+@pytest.mark.parametrize("factor", test_factors)
+def test_issue_740_fixed(tmp_path: Path, factor: str) -> None:
+    """Confirm that re-running a particular workflow with caching succeeds."""
+    test_file = "cache_test_workflow.cwl"
+    cache_dir = str(tmp_path / "cwltool_cache")
+    commands = factor.split()
+    commands.extend(["--cachedir", cache_dir, get_data("tests/wf/" + test_file)])
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "completed success" in stderr
+    assert error_code == 0
+
+    commands = factor.split()
+    commands.extend(["--cachedir", cache_dir, get_data("tests/wf/" + test_file)])
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "Output of job will be cached in" not in stderr
+    assert error_code == 0, stderr
+
+
+@needs_docker
+@pytest.mark.parametrize("factor", test_factors)
+def test_cache_relative_paths(tmp_path: Path, factor: str) -> None:
+    """Confirm that re-running a particular workflow with caching succeeds."""
+    test_file = "secondary-files.cwl"
+    test_job_file = "secondary-files-job.yml"
+    cache_dir = str(tmp_path / "cwltool_cache")
+    commands = factor.split()
+    commands.extend(
+        [
+            "--out",
+            str(tmp_path / "out"),
+            "--cachedir",
+            cache_dir,
+            get_data(f"tests/{test_file}"),
+            get_data(f"tests/{test_job_file}"),
+        ]
+    )
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "completed success" in stderr
+    assert error_code == 0
+
+    commands = factor.split()
+    commands.extend(
+        [
+            "--out",
+            str(tmp_path / "out2"),
+            "--cachedir",
+            cache_dir,
+            get_data(f"tests/{test_file}"),
+            get_data(f"tests/{test_job_file}"),
+        ]
+    )
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "Output of job will be cached in" not in stderr
+    assert error_code == 0, stderr
+
+    assert (tmp_path / "cwltool_cache" / "27903451fc1ee10c148a0bdeb845b2cf").exists()
+
+
+@pytest.mark.parametrize("factor", test_factors)
+def test_cache_default_literal_file(tmp_path: Path, factor: str) -> None:
+    """Confirm that running a CLT with a default literal file with caching succeeds."""
+    test_file = "tests/wf/extract_region_specs.cwl"
+    cache_dir = str(tmp_path / "cwltool_cache")
+    commands = factor.split()
+    commands.extend(
+        [
+            "--out",
+            str(tmp_path / "out"),
+            "--cachedir",
+            cache_dir,
+            get_data(test_file),
+        ]
+    )
+    error_code, _, stderr = get_main_output(commands)
+
+    stderr = re.sub(r"\s\s+", " ", stderr)
+    assert "completed success" in stderr
+    assert error_code == 0
+
+
+@needs_docker
+@pytest.mark.parametrize("factor", test_factors)
+def test_cache_dockerreq_hint_instead_of_req(tmp_path: Path, factor: str) -> None:
+    """The cache must not be checked when there is an invalid use of an absolute path in iwdr.listing."""
+    cache_dir = str(tmp_path / "cwltool_cache")
+    test_job_file = "tests/wf/loadContents-input.yml"
+    # First, run the iwd-container-entryname1 conformance tests with caching turned on
+    test1_file = "tests/wf/iwd-container-entryname1.cwl"
+    commands1 = factor.split()
+    commands1.extend(
+        [
+            "--out",
+            str(tmp_path / "out1"),
+            "--cachedir",
+            cache_dir,
+            get_data(test1_file),
+            get_data(test_job_file),
+        ]
+    )
+    error_code1, _, stderr1 = get_main_output(commands1)
+
+    stderr1 = re.sub(r"\s\s+", " ", stderr1)
+    assert "completed success" in stderr1
+    assert error_code1 == 0
+    # Second, run the iwd-container-entryname3 test, which should fail
+    # even though it would be a cache hit, except that its DockerRequirement is
+    # in `hints` instead of `requirements` and one of the initial working directory
+    # items has an absolute path starting with `/`.
+    test2_file = "tests/wf/iwd-container-entryname3.cwl"
+    commands2 = factor.split()
+    commands2.extend(
+        [
+            "--out",
+            str(tmp_path / "out2"),
+            "--cachedir",
+            cache_dir,
+            get_data(test2_file),
+            get_data(test_job_file),
+        ]
+    )
+    error_code2, _, stderr2 = get_main_output(commands2)
+
+    stderr2 = re.sub(r"\s\s+", " ", stderr2)
+    assert (
+        "at index 0 of listing is invalid, name can only start with '/' "
+        "when DockerRequirement is in 'requirements" in stderr2
+    )
+    assert error_code2 == 1

tests/test_environment.py

@@ -204,7 +204,7 @@ def test_basic(crt_params: CheckHolder, tmp_path: Path, monkeypatch: pytest.Monk
         "USEDVAR": "VARVAL",
         "UNUSEDVAR": "VARVAL",
     }
-    args = crt_params.flags + [f"--tmpdir-prefix={tmp_prefix}"]
+    args = crt_params.flags + [f"--tmpdir-prefix={tmp_prefix}", "--debug"]
     env = get_tool_env(
         tmp_path,
         args,