fix(cli): fix special paths in workflow files and bump toil/cwltool (#3489)

Author: Panaetius

poetry.lock

@@ -61,8 +61,8 @@ click-option-group = "<0.6.0,>=0.5.2"
 click-plugins = "==1.1.1"
 coverage = { version = "<6.5,>=4.5.3", extras=["toml"], optional = true }
 cryptography = ">=38.0.0,<41.0.0"
-cwl-utils = ">=0.12,<0.18"
-cwltool = "==3.1.20220628170238"
+cwl-utils = ">=0.27,<0.28"
+cwltool = "==3.1.20230425144158"
 deal = ">=4.24.0,<5.0.0"
 deepdiff = ">=5.8,<7.0"
 deepmerge = "==1.0.1"
@@ -97,7 +97,7 @@ requests = ">=2.23.0,<2.31.1"
 rich = ">=9.3.0,<13.4.0"
 shellingham = "1.5.0.post1"
 tabulate = ">=0.7.7,<0.9.1"
-toil = "==5.7.1"
+toil = "==5.10.0"
 tqdm = "<4.62.4,>=4.48.1"
 werkzeug = ">=1.0.0,<2.2.4"
 yagup = ">=0.1.1"

pyproject.toml

@@ -20,11 +20,12 @@
 import io
 import json
 from enum import Enum
-from typing import Dict, List, Optional
+from typing import Dict, Iterator, List, Optional, cast
 
 import pyld
 import rdflib
 from rdflib import ConjunctiveGraph, Graph
+from rdflib.query import ResultRow
 from rdflib.tools.rdf2dot import LABEL_PROPERTIES, NODECOLOR, rdf2dot
 
 
@@ -189,7 +190,7 @@ def color(p):
         }
         """
 
-        for s, p, o in graph.query(sparql):
+        for s, p, o in cast(Iterator[ResultRow], graph.query(sparql)):
             sn = node(s)
             if p == rdflib.RDFS.label:
                 continue

renku/command/view_model/graph.py

@@ -26,7 +26,7 @@
 from urllib.parse import urlparse
 
 from cwl_utils.parser import load_document_by_uri
-from cwl_utils.parser.cwl_v1_0 import CommandLineTool, InitialWorkDirRequirement
+from cwl_utils.parser.cwl_v1_0 import CommandLineTool, Directory, File, InitialWorkDirRequirement
 from werkzeug.utils import secure_filename
 
 from renku.core.constant import RENKU_HOME
@@ -167,7 +167,7 @@ def _migrate_single_step(migration_context, cmd_line_tool, path, commit=None, pa
         matched_input = next(i for i in inputs if i.id.endswith(name))
         inputs.remove(matched_input)
 
-        path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(matched_input.default["path"])
+        path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(matched_input.default.path)
         stdin = path.resolve().relative_to(project_context.path)
         id_ = CommandInput.generate_id(base_id, "stdin")
 
@@ -237,7 +237,7 @@ def _migrate_single_step(migration_context, cmd_line_tool, path, commit=None, pa
                 pass
 
             if isinstance(matched_input.default, dict):
-                path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(matched_input.default["path"])
+                path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(matched_input.default.path)
             else:
                 path = Path(matched_input.default)
 
@@ -282,8 +282,8 @@ def _migrate_single_step(migration_context, cmd_line_tool, path, commit=None, pa
             if prefix and i.inputBinding.separate:
                 prefix += " "
 
-        if isinstance(i.default, dict) and "class" in i.default and i.default["class"] in ["File", "Directory"]:
-            path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(i.default["path"])
+        if isinstance(i.default, (File, Directory)):
+            path = project_context.metadata_path / OLD_WORKFLOW_PATH / Path(str(i.default.path))
             path = Path(os.path.realpath(path)).relative_to(project_context.path)
 
             run.inputs.append(

renku/core/migration/m_0005__2_cwl.py

@@ -148,7 +148,7 @@ def workflow_convert(
                     step_filename = Path(f"{uuid4()}.cwl")
                     step_path = (tmpdir / step_filename).resolve()
                     write_yaml(step_path, step.run.save())
-                    step.run = str(step_path)
+                    step.run = f"file://{step_path}"
             if filename is None:
                 filename = Path(f"parent_{uuid4()}.cwl")
         else:

renku/core/workflow/converters/cwl.py

@@ -21,10 +21,17 @@
 
 # Find all modified or added files, and do nothing if there aren't any.
 export RENKU_DISABLE_VERSION_CHECK=true
-IFS=$'\n' read -r -d '' -a MODIFIED_FILES \
-  <<< "$(git diff --name-only --cached --diff-filter=M)"
-IFS=$'\n' read -r -d '' -a ADDED_FILES \
-  <<< "$(git diff --name-only --cached --diff-filter=A)"
+
+declare -a MODIFIED_FILES=()
+while IFS= read -r -d '' file; do
+  MODIFIED_FILES+=( "$file" )
+done < <(git diff -z --name-only --cached --diff-filter=M)
+
+declare -a ADDED_FILES=()
+while IFS= read -r -d '' file; do
+  ADDED_FILES+=( "$file" )
+done < <(git diff -z --name-only --cached --diff-filter=A)
+
 
 if [ ${#MODIFIED_FILES[@]} -ne 0 ] || [ ${#ADDED_FILES[@]} -ne 0 ]; then
   # Verify that renku is installed; if not, warn and exit.

renku/data/pre-commit.sh

@@ -917,8 +917,10 @@ def get_existing_paths_in_revision(
                 if files:
                     # NOTE: check existing files
                     for batch in split_paths(*files):
-                        existing_paths = git.Git(working_dir=self.path).ls_tree(*batch, r=revision, name_only=True)
-                        result.extend(existing_paths.splitlines())
+                        existing_paths = git.Git(working_dir=self.path).ls_tree(
+                            *batch, r=revision, name_only=True, z=True
+                        )
+                        result.extend(existing_paths.strip("\x00").split("\x00"))
 
                 if dirs:
                     # NOTE: check existing dirs
@@ -930,7 +932,7 @@ def get_existing_paths_in_revision(
 
                 return result
             else:
-                existing_files = git.Git().ls_tree(r=revision, name_only=True).splitlines()
+                existing_files = git.Git().ls_tree(r=revision, name_only=True, z=True).strip("\x00").split("\x00")
                 existing_dirs = git.Git().ls_tree(r=revision, name_only=True, d=True).splitlines()
                 return existing_dirs + existing_files
         except git.GitCommandError as e:

renku/infrastructure/repository.py

@@ -711,7 +711,7 @@ def _flatten_dict(obj, key_string=""):
 
 
 @pytest.mark.parametrize("provider", available_workflow_providers())
-def test_workflow_execute_command_with_api_parameter_set(runner, run_shell, project, capsys, transaction_id, provider):
+def test_workflow_execute_command_with_api_parameter_set(runner, run_shell, project, transaction_id, provider):
     """Test executing a workflow with --set for a renku.ui.api.Parameter."""
     script = project.path / "script.py"
     output = project.path / "output"
@@ -740,7 +740,7 @@ def test_workflow_execute_command_with_api_parameter_set(runner, run_shell, proj
 
 
 @pytest.mark.parametrize("provider", available_workflow_providers())
-def test_workflow_execute_command_with_api_input_set(runner, run_shell, project, capsys, transaction_id, provider):
+def test_workflow_execute_command_with_api_input_set(runner, run_shell, project, transaction_id, provider):
     """Test executing a workflow with --set for a renku.ui.api.Input."""
     script = project.path / "script.py"
     output = project.path / "output"
@@ -775,7 +775,7 @@ def test_workflow_execute_command_with_api_input_set(runner, run_shell, project,
 
 
 @pytest.mark.parametrize("provider", available_workflow_providers())
-def test_workflow_execute_command_with_api_output_set(runner, run_shell, project, capsys, transaction_id, provider):
+def test_workflow_execute_command_with_api_output_set(runner, run_shell, project, transaction_id, provider):
     """Test executing a workflow with --set for a renku.ui.api.Output."""
     script = project.path / "script.py"
     output = project.path / "output"
@@ -806,7 +806,7 @@ def test_workflow_execute_command_with_api_output_set(runner, run_shell, project
     assert 0 == result.exit_code, format_result_exception(result)
 
 
-def test_workflow_execute_command_with_api_duplicate_output(runner, run_shell, project, capsys, transaction_id):
+def test_workflow_execute_command_with_api_duplicate_output(run_shell, project, transaction_id):
     """Test executing a workflow with duplicate output with differing path."""
     script = project.path / "script.py"
     output = project.path / "output"
@@ -824,7 +824,7 @@ def test_workflow_execute_command_with_api_duplicate_output(runner, run_shell, p
     assert b"Error: Invalid parameter value - Duplicate input/output name found: my-output\n" in result[0]
 
 
-def test_workflow_execute_command_with_api_valid_duplicate_output(runner, run_shell, project, capsys, transaction_id):
+def test_workflow_execute_command_with_api_valid_duplicate_output(run_shell, project, transaction_id):
     """Test executing a workflow with duplicate output with same path."""
     script = project.path / "script.py"
     output = project.path / "output"
@@ -844,7 +844,7 @@ def test_workflow_execute_command_with_api_valid_duplicate_output(runner, run_sh
     assert result[1] is None
 
 
-def test_workflow_execute_command_with_api_duplicate_input(runner, run_shell, project, capsys, transaction_id):
+def test_workflow_execute_command_with_api_duplicate_input(run_shell, project, transaction_id):
     """Test executing a workflow with duplicate input with differing path."""
     script = project.path / "script.py"
     input = project.path / "input"
@@ -862,7 +862,7 @@ def test_workflow_execute_command_with_api_duplicate_input(runner, run_shell, pr
     assert b"Error: Invalid parameter value - Duplicate input/output name found: my-input\n" in result[0]
 
 
-def test_workflow_execute_command_with_api_valid_duplicate_input(runner, run_shell, project, capsys, transaction_id):
+def test_workflow_execute_command_with_api_valid_duplicate_input(run_shell, project, transaction_id):
     """Test executing a workflow with duplicate input with same path."""
     script = project.path / "script.py"
     input = project.path / "input"