From 647a76e8f20a07944bcb989f9ac9d5b2ff3db8b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20GUICHARD?= Date: Wed, 19 Aug 2026 11:01:08 +0200 Subject: [PATCH 1/3] Fix failing read of toildir in ValueFrom.eval_prep when loadContents is true (#5568) --- src/toil/cwl/cwltoil.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 112dc06c57..c8fe0ea309 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -653,20 +653,23 @@ def eval_prep( for v in step_inputs.values(): val = cast(CWLObjectType, v) source_input = getattr(self.source, "input", {}) - if isinstance(val, dict) and isinstance(source_input, dict): - if ( - val.get("contents") is None - and source_input.get("loadContents") is True - ): - # This is safe to use even if we're bypassing the file - # store for the workflow. In that case, no toilfile:// or - # other special URIs will exist in the workflow to be read - # from, and ToilFsAccess still supports file:// URIs. - fs_access = functools.partial(ToilFsAccess, file_store=file_store) - with fs_access("").open(cast(str, val["location"]), "rb") as f: - val["contents"] = cwltool.builder.content_limit_respected_read( - f - ) + if ( + isinstance(val, dict) + and isinstance(source_input, dict) + and val.get("class") == "File" + and val.get("contents") is None + and val.get("location") is not None + and source_input.get("loadContents") is True + ): + # This is safe to use even if we're bypassing the file + # store for the workflow. In that case, no toilfile:// or + # other special URIs will exist in the workflow to be read + # from, and ToilFsAccess still supports file:// URIs. + fs_access = functools.partial(ToilFsAccess, file_store=file_store) + with fs_access("").open(cast(str, val["location"]), "rb") as f: + val["contents"] = cwltool.builder.content_limit_respected_read( + f + ) def resolve(self) -> Any: """ From 2fd0a8cd6314892075a165caf807772d0240e411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20GUICHARD?= Date: Wed, 19 Aug 2026 16:19:38 +0200 Subject: [PATCH 2/3] Restrict ValueFrom.eval_prep to load only one step input instead of all (#5568) --- src/toil/cwl/cwltoil.py | 54 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index c8fe0ea309..53d1130392 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -638,38 +638,35 @@ def __repr__(self) -> str: return f"ValueFrom({self.expr}, {self.source}, {self.req}, {self.container_engine})" def eval_prep( - self, step_inputs: CWLObjectType, file_store: AbstractFileStore + self, step_input: CWLObjectType, file_store: AbstractFileStore ) -> None: """ - Resolve the contents of any file in a set of inputs. + Prepare the step_input object before evaluation of valueFrom expression. - The inputs must be associated with the ValueFrom object's self.source. + Responsible for checking loadContents, when enabled load the contents + if the input is of type File. - Called when loadContents is specified. - - :param step_inputs: Workflow step inputs. + :param step_input: step input. :param file_store: A toil file store, needed to resolve toilfile:// paths. """ - for v in step_inputs.values(): - val = cast(CWLObjectType, v) - source_input = getattr(self.source, "input", {}) - if ( - isinstance(val, dict) - and isinstance(source_input, dict) - and val.get("class") == "File" - and val.get("contents") is None - and val.get("location") is not None - and source_input.get("loadContents") is True - ): - # This is safe to use even if we're bypassing the file - # store for the workflow. In that case, no toilfile:// or - # other special URIs will exist in the workflow to be read - # from, and ToilFsAccess still supports file:// URIs. - fs_access = functools.partial(ToilFsAccess, file_store=file_store) - with fs_access("").open(cast(str, val["location"]), "rb") as f: - val["contents"] = cwltool.builder.content_limit_respected_read( - f - ) + source_input = getattr(self.source, "input", {}) + if ( + isinstance(step_input, dict) + and isinstance(source_input, dict) + and step_input.get("class") == "File" + and step_input.get("contents") is None + and step_input.get("location") is not None + and source_input.get("loadContents") is True + ): + # This is safe to use even if we're bypassing the file + # store for the workflow. In that case, no toilfile:// or + # other special URIs will exist in the workflow to be read + # from, and ToilFsAccess still supports file:// URIs. + fs_access = functools.partial(ToilFsAccess, file_store=file_store) + with fs_access("").open(cast(str, step_input["location"]), "rb") as f: + step_input["contents"] = cwltool.builder.content_limit_respected_read( + f + ) def resolve(self) -> Any: """ @@ -771,8 +768,9 @@ def resolve_dict_w_promises( result: CWLObjectType = {} for k, v in dict_w_promises.items(): if isinstance(v, ValueFrom): - if file_store: - v.eval_prep(first_pass_results, file_store) + if file_store and first_pass_results[k]: + step_input = cast(CWLObjectType, first_pass_results[k]) + v.eval_prep(step_input, file_store) result[k] = v.do_eval(inputs=first_pass_results) else: result[k] = first_pass_results[k] From 68c72ac8bf59b5124b930b08f0b19b06d2a798fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20GUICHARD?= Date: Fri, 21 Aug 2026 16:42:58 +0200 Subject: [PATCH 3/3] Support step input loadContents for File array (#5568) --- src/toil/cwl/cwltoil.py | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 53d1130392..fdd05eb1a1 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -644,29 +644,31 @@ def eval_prep( Prepare the step_input object before evaluation of valueFrom expression. Responsible for checking loadContents, when enabled load the contents - if the input is of type File. + if the input is of type File/File[]. :param step_input: step input. :param file_store: A toil file store, needed to resolve toilfile:// paths. """ - source_input = getattr(self.source, "input", {}) - if ( - isinstance(step_input, dict) - and isinstance(source_input, dict) - and step_input.get("class") == "File" - and step_input.get("contents") is None - and step_input.get("location") is not None - and source_input.get("loadContents") is True - ): - # This is safe to use even if we're bypassing the file - # store for the workflow. In that case, no toilfile:// or - # other special URIs will exist in the workflow to be read - # from, and ToilFsAccess still supports file:// URIs. - fs_access = functools.partial(ToilFsAccess, file_store=file_store) - with fs_access("").open(cast(str, step_input["location"]), "rb") as f: - step_input["contents"] = cwltool.builder.content_limit_respected_read( - f - ) + values = step_input if isinstance(step_input, MutableSequence) else [step_input] + for val in values: + source_input = getattr(self.source, "input", {}) + if ( + isinstance(val, dict) + and isinstance(source_input, dict) + and val.get("class") == "File" + and val.get("contents") is None + and val.get("location") is not None + and source_input.get("loadContents") is True + ): + # This is safe to use even if we're bypassing the file + # store for the workflow. In that case, no toilfile:// or + # other special URIs will exist in the workflow to be read + # from, and ToilFsAccess still supports file:// URIs. + fs_access = functools.partial(ToilFsAccess, file_store=file_store) + with fs_access("").open(cast(str, val["location"]), "rb") as f: + val["contents"] = cwltool.builder.content_limit_respected_read( + f + ) def resolve(self) -> Any: """