From db787f1d69ab58b49cb20e0c0f1db92180408cb1 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Mon, 15 Nov 2021 14:42:28 +0100 Subject: [PATCH 1/7] skip $import: #local-ref --- cwlupgrader/main.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cwlupgrader/main.py b/cwlupgrader/main.py index 97833f2..95d3fdf 100755 --- a/cwlupgrader/main.py +++ b/cwlupgrader/main.py @@ -195,12 +195,14 @@ def process_imports( if isinstance(document, CommentedMap): for key, value in document.items(): if key == "$import": - if value not in imports: + if value not in imports and not value.startswith("#"): + with SourceLine(document, key, Exception): + import_doc = load_cwl_document( + Path(document.lc.filename).parent / value + ) write_cwl_document( updater( - load_cwl_document( - Path(document.lc.filename).parent / value - ), + import_doc, outdir, ), Path(value).name, From 61b40c7ce26a64ef5f4140c1804e36de5747b3e0 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Mon, 15 Nov 2021 16:06:53 +0100 Subject: [PATCH 2/7] Better support for $import or "run: " references to parent directories. --- cwlupgrader/main.py | 85 +++++++++++++++++++++++------------------- tests/test_complete.py | 6 +-- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/cwlupgrader/main.py b/cwlupgrader/main.py index 95d3fdf..43ade7c 100755 --- a/cwlupgrader/main.py +++ b/cwlupgrader/main.py @@ -31,7 +31,9 @@ def parse_args(args: List[str]) -> argparse.Namespace: """Argument parser.""" parser = argparse.ArgumentParser( description="Tool to upgrade CWL documents from one version to another. " - "Supports upgrading 'draft-3', 'v1.0', and 'v1.1' to 'v1.2'", + "Supports upgrading 'draft-3', 'v1.0', and 'v1.1' to 'v1.2'. For workflows " + 'that include "$import" or "run:" reference that refer to parent ' + "directories, start cwl-upgrader from the topmost directory of your project.", formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) parser.add_argument( @@ -40,11 +42,11 @@ def parse_args(args: List[str]) -> argparse.Namespace: parser.add_argument( "--v1.1-only", dest="v1_1_only", - help="Don't upgrade past cwlVersion: v1.1", + help="Don't upgrade past cwlVersion: v1.1.", action="store_true", ) parser.add_argument( - "--dir", help="Directory in which to save converted files", default=Path.cwd() + "--dir", help="Directory in which to save converted files.", default=Path.cwd() ) parser.add_argument( "inputs", @@ -64,10 +66,21 @@ def main(args: Optional[List[str]] = None) -> int: def run(args: argparse.Namespace) -> int: """Main function.""" imports: Set[str] = set() - if args.dir and not os.path.exists(args.dir): - os.makedirs(args.dir) - for path in args.inputs: + if args.dir: + out_dir = Path(args.dir) + out_dir.mkdir(parents=True, exist_ok=True) + else: + out_dir = Path.cwd() + for p in args.inputs: + path = Path(os.path.normpath(p)) _logger.info("Processing %s", path) + if not p.startswith("../") and path.resolve() != path: + # common_path = os.path.commonpath([out_dir.resolve(), path.resolve()]) + # current_out_dir = out_dir / os.path.relpath(path, start=common_path) + current_out_dir = out_dir / path.parent + current_out_dir.mkdir(parents=True, exist_ok=True) + else: + current_out_dir = out_dir document = load_cwl_document(path) if "cwlVersion" not in document: _logger.warn("No cwlVersion found in %s, skipping it.", path) @@ -89,17 +102,17 @@ def run(args: argparse.Namespace) -> int: target_version = "latest" upgraded_document = upgrade_document( document, - args.dir, + current_out_dir, target_version=target_version, imports=imports, ) - write_cwl_document(upgraded_document, Path(path).name, args.dir) + write_cwl_document(upgraded_document, current_out_dir / (path.name)) return 0 def upgrade_document( document: Any, - output_dir: str, + output_dir: Path, target_version: Optional[str] = "latest", imports: Optional[Set[str]] = None, ) -> Any: @@ -158,7 +171,7 @@ def upgrade_document( return main_updater(document, output_dir) -def load_cwl_document(path: str) -> Any: +def load_cwl_document(path: Union[str, Path]) -> Any: """ Load the given path using the Ruamel YAML round-trip loader. @@ -171,7 +184,7 @@ def load_cwl_document(path: str) -> Any: return document -def write_cwl_document(document: Any, name: str, dirname: str) -> None: +def write_cwl_document(document: Any, path: Path) -> None: """ Serialize the document using the Ruamel YAML round trip dumper. @@ -179,7 +192,6 @@ def write_cwl_document(document: Any, name: str, dirname: str) -> None: set the executable bit if it is a CWL document. """ ruamel.yaml.scalarstring.walk_tree(document) - path = Path(dirname) / name with open(path, "w") as handle: if "cwlVersion" in document: handle.write("#!/usr/bin/env cwl-runner\n") @@ -189,7 +201,7 @@ def write_cwl_document(document: Any, name: str, dirname: str) -> None: def process_imports( - document: Any, imports: Set[str], updater: Callable[[Any, str], Any], outdir: str + document: Any, imports: Set[str], updater: Callable[[Any, Path], Any], outdir: Path ) -> None: """Find any '$import's and process them.""" if isinstance(document, CommentedMap): @@ -200,14 +212,9 @@ def process_imports( import_doc = load_cwl_document( Path(document.lc.filename).parent / value ) - write_cwl_document( - updater( - import_doc, - outdir, - ), - Path(value).name, - outdir, - ) + new_path = (outdir / value).resolve() + new_path.parent.mkdir(parents=True, exist_ok=True) + write_cwl_document(updater(import_doc, outdir), new_path) imports.add(value) else: process_imports(value, imports, updater, outdir) @@ -216,7 +223,7 @@ def process_imports( process_imports(entry, imports, updater, outdir) -def v1_0_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: +def v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: """CWL v1.0.x to v1.1 transformation loop.""" _v1_0_to_v1_1(document, outdir) for key, value in document.items(): @@ -231,20 +238,20 @@ def v1_0_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: return sort_v1_0(document) -def v1_0_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def v1_0_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: """CWL v1.0.x to v1.2 transformation.""" document = v1_0_to_v1_1(document, outdir) document["cwlVersion"] = "v1.2" return document -def v1_1_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def v1_1_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: """CWL v1.1 to v1.2 transformation.""" document["cwlVersion"] = "v1.2" return document -def draft3_to_v1_0(document: CommentedMap, outdir: str) -> CommentedMap: +def draft3_to_v1_0(document: CommentedMap, outdir: Path) -> CommentedMap: """Transformation loop.""" _draft3_to_v1_0(document, outdir) if isinstance(document, MutableMapping): @@ -260,17 +267,17 @@ def draft3_to_v1_0(document: CommentedMap, outdir: str) -> CommentedMap: return sort_v1_0(document) -def draft3_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: +def draft3_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: """transformation loop.""" return v1_0_to_v1_1(draft3_to_v1_0(document, outdir), outdir) -def draft3_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def draft3_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: """transformation loop.""" return v1_1_to_v1_2(v1_0_to_v1_1(draft3_to_v1_0(document, outdir), outdir), outdir) -def _draft3_to_v1_0(document: CommentedMap, outdir: str) -> CommentedMap: +def _draft3_to_v1_0(document: CommentedMap, outdir: Path) -> CommentedMap: """Inner loop for transforming draft-3 to v1.0.""" if "class" in document: if document["class"] == "Workflow": @@ -295,11 +302,11 @@ def _draft3_to_v1_0(document: CommentedMap, outdir: str) -> CommentedMap: return document -def _draft3_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: +def _draft3_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: return v1_0_to_v1_1(_draft3_to_v1_0(document, outdir), outdir) -def _draft3_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def _draft3_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: return _draft3_to_v1_1(document, outdir) # nothing needs doing for 1.2 @@ -318,7 +325,7 @@ def _draft3_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: } -def _v1_0_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: +def _v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: """Inner loop for transforming draft-3 to v1.0.""" if "class" in document: if document["class"] == "Workflow": @@ -350,11 +357,13 @@ def _v1_0_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: isinstance(entry["run"], str) and "#" not in entry["run"] ): - path = Path(document.lc.filename).parent / entry["run"] - process = v1_0_to_v1_1( - load_cwl_document(str(path)), outdir - ) - write_cwl_document(process, path.name, outdir) + path = ( + Path(document.lc.filename).parent / entry["run"] + ).resolve() + process = v1_0_to_v1_1(load_cwl_document(path), outdir) + new_path = (outdir / entry["run"]).resolve() + new_path.parent.mkdir(parents=True, exist_ok=True) + write_cwl_document(process, new_path) elif isinstance(entry["run"], str) and "#" in entry["run"]: pass # reference to $graph entry else: @@ -397,11 +406,11 @@ def _v1_0_to_v1_1(document: CommentedMap, outdir: str) -> CommentedMap: return document -def _v1_0_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def _v1_0_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: return _v1_0_to_v1_1(document, outdir) # nothing needs doing for v1.2 -def _v1_1_to_v1_2(document: CommentedMap, outdir: str) -> CommentedMap: +def _v1_1_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: return document diff --git a/tests/test_complete.py b/tests/test_complete.py index 0e98386..6c8e715 100644 --- a/tests/test_complete.py +++ b/tests/test_complete.py @@ -37,21 +37,21 @@ def test_draft3_tool_long_form_arrays(tmp_path: Path) -> None: def test_invalid_target(tmp_path: Path) -> None: """Test for invalid target version""" doc = load_cwl_document(get_data("testdata/v1.0/listing_deep1.cwl")) - result = upgrade_document(doc, str(tmp_path), "invalid-version") + result = upgrade_document(doc, tmp_path, "invalid-version") assert result is None def test_v1_0_to_v1_1(tmp_path: Path) -> None: """Basic CWL v1.0 to CWL v1.1 test.""" doc = load_cwl_document(get_data("testdata/v1.0/listing_deep1.cwl")) - upgraded = upgrade_document(doc, str(tmp_path), "v1.1") + upgraded = upgrade_document(doc, tmp_path, "v1.1") assert doc == upgraded def test_v1_1_to_v1_2(tmp_path: Path) -> None: """Basic CWL v1.1 to CWL v1.2 test.""" doc = load_cwl_document(get_data("testdata/v1.1/listing_deep1.cwl")) - upgraded = upgrade_document(doc, str(tmp_path), "v1.2") + upgraded = upgrade_document(doc, tmp_path, "v1.2") assert doc == upgraded From 02c0fbc75a980cb7ecb81b47eeb2239234eb8f7a Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Mon, 15 Nov 2021 17:22:18 +0100 Subject: [PATCH 3/7] issue warning if we write outside of the user-specified output directory --- cwlupgrader/main.py | 128 ++++++++++++++++++++++++++--------------- tests/test_complete.py | 6 +- 2 files changed, 86 insertions(+), 48 deletions(-) diff --git a/cwlupgrader/main.py b/cwlupgrader/main.py index 43ade7c..0d74ee6 100755 --- a/cwlupgrader/main.py +++ b/cwlupgrader/main.py @@ -66,21 +66,16 @@ def main(args: Optional[List[str]] = None) -> int: def run(args: argparse.Namespace) -> int: """Main function.""" imports: Set[str] = set() - if args.dir: - out_dir = Path(args.dir) - out_dir.mkdir(parents=True, exist_ok=True) - else: - out_dir = Path.cwd() + root_out_dir = Path(args.dir) + root_out_dir.mkdir(parents=True, exist_ok=True) for p in args.inputs: path = Path(os.path.normpath(p)) _logger.info("Processing %s", path) if not p.startswith("../") and path.resolve() != path: - # common_path = os.path.commonpath([out_dir.resolve(), path.resolve()]) - # current_out_dir = out_dir / os.path.relpath(path, start=common_path) - current_out_dir = out_dir / path.parent + current_out_dir = root_out_dir / path.parent current_out_dir.mkdir(parents=True, exist_ok=True) else: - current_out_dir = out_dir + current_out_dir = root_out_dir document = load_cwl_document(path) if "cwlVersion" not in document: _logger.warn("No cwlVersion found in %s, skipping it.", path) @@ -103,6 +98,7 @@ def run(args: argparse.Namespace) -> int: upgraded_document = upgrade_document( document, current_out_dir, + root_out_dir, target_version=target_version, imports=imports, ) @@ -113,6 +109,7 @@ def run(args: argparse.Namespace) -> int: def upgrade_document( document: Any, output_dir: Path, + root_dir: Path, target_version: Optional[str] = "latest", imports: Optional[Set[str]] = None, ) -> Any: @@ -167,8 +164,8 @@ def upgrade_document( pass # does not happen? How to do the case that base version is v1.0? else: _logger.error(f"Unsupported cwlVersion: {version}") - process_imports(document, imports, inner_updater, output_dir) - return main_updater(document, output_dir) + process_imports(document, imports, inner_updater, output_dir, root_dir) + return main_updater(document, output_dir, root_dir) def load_cwl_document(path: Union[str, Path]) -> Any: @@ -200,8 +197,23 @@ def write_cwl_document(document: Any, path: Path) -> None: path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) +def check_path(root_dir: Path, new_path: Path) -> None: + """Issue a warning if the new path is outside of the root path.""" + common_path = Path(os.path.commonpath([root_dir.resolve(), new_path.resolve()])) + if common_path != root_dir: + _logger.warn( + "Writing file, '%s', outside of the output directory, '%s'.", + os.path.normpath(new_path), + root_dir, + ) + + def process_imports( - document: Any, imports: Set[str], updater: Callable[[Any, Path], Any], outdir: Path + document: Any, + imports: Set[str], + updater: Callable[[Any, Path, Path], Any], + out_dir: Path, + root_dir: Path, ) -> None: """Find any '$import's and process them.""" if isinstance(document, CommentedMap): @@ -212,72 +224,85 @@ def process_imports( import_doc = load_cwl_document( Path(document.lc.filename).parent / value ) - new_path = (outdir / value).resolve() + new_path = (out_dir / value).resolve() + check_path(root_dir, new_path) new_path.parent.mkdir(parents=True, exist_ok=True) - write_cwl_document(updater(import_doc, outdir), new_path) + write_cwl_document(updater(import_doc, out_dir, root_dir), new_path) imports.add(value) else: - process_imports(value, imports, updater, outdir) + process_imports(value, imports, updater, out_dir, root_dir) elif isinstance(document, MutableSequence): for entry in document: - process_imports(entry, imports, updater, outdir) + process_imports(entry, imports, updater, out_dir, root_dir) -def v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: +def v1_0_to_v1_1(document: CommentedMap, out_dir: Path, root_dir: Path) -> CommentedMap: """CWL v1.0.x to v1.1 transformation loop.""" - _v1_0_to_v1_1(document, outdir) + _v1_0_to_v1_1(document, out_dir, root_dir) for key, value in document.items(): with SourceLine(document, key, Exception): if isinstance(value, CommentedMap): - document[key] = _v1_0_to_v1_1(value, outdir) + document[key] = _v1_0_to_v1_1(value, out_dir, root_dir) elif isinstance(value, list): for index, entry in enumerate(value): if isinstance(entry, CommentedMap): - value[index] = _v1_0_to_v1_1(entry, outdir) + value[index] = _v1_0_to_v1_1(entry, out_dir, root_dir) document["cwlVersion"] = "v1.1" return sort_v1_0(document) -def v1_0_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: +def v1_0_to_v1_2(document: CommentedMap, out_dir: Path, root_dir: Path) -> CommentedMap: """CWL v1.0.x to v1.2 transformation.""" - document = v1_0_to_v1_1(document, outdir) + document = v1_0_to_v1_1(document, out_dir, root_dir) document["cwlVersion"] = "v1.2" return document -def v1_1_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: +def v1_1_to_v1_2(document: CommentedMap, out_dir: Path, root_dir: Path) -> CommentedMap: """CWL v1.1 to v1.2 transformation.""" document["cwlVersion"] = "v1.2" return document -def draft3_to_v1_0(document: CommentedMap, outdir: Path) -> CommentedMap: +def draft3_to_v1_0( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: """Transformation loop.""" - _draft3_to_v1_0(document, outdir) + _draft3_to_v1_0(document, out_dir, root_dir) if isinstance(document, MutableMapping): for key, value in document.items(): with SourceLine(document, key, Exception): if isinstance(value, CommentedMap): - document[key] = _draft3_to_v1_0(value, outdir) + document[key] = _draft3_to_v1_0(value, out_dir, root_dir) elif isinstance(value, list): for index, entry in enumerate(value): if isinstance(entry, CommentedMap): - value[index] = _draft3_to_v1_0(entry, outdir) + value[index] = _draft3_to_v1_0(entry, out_dir, root_dir) document["cwlVersion"] = "v1.0" return sort_v1_0(document) -def draft3_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: +def draft3_to_v1_1( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: """transformation loop.""" - return v1_0_to_v1_1(draft3_to_v1_0(document, outdir), outdir) + return v1_0_to_v1_1(draft3_to_v1_0(document, out_dir, root_dir), out_dir, root_dir) -def draft3_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: +def draft3_to_v1_2( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: """transformation loop.""" - return v1_1_to_v1_2(v1_0_to_v1_1(draft3_to_v1_0(document, outdir), outdir), outdir) + return v1_1_to_v1_2( + v1_0_to_v1_1(draft3_to_v1_0(document, out_dir, root_dir), out_dir, root_dir), + out_dir, + root_dir, + ) -def _draft3_to_v1_0(document: CommentedMap, outdir: Path) -> CommentedMap: +def _draft3_to_v1_0( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: """Inner loop for transforming draft-3 to v1.0.""" if "class" in document: if document["class"] == "Workflow": @@ -302,12 +327,16 @@ def _draft3_to_v1_0(document: CommentedMap, outdir: Path) -> CommentedMap: return document -def _draft3_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: - return v1_0_to_v1_1(_draft3_to_v1_0(document, outdir), outdir) +def _draft3_to_v1_1( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: + return v1_0_to_v1_1(_draft3_to_v1_0(document, out_dir, root_dir), out_dir, root_dir) -def _draft3_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: - return _draft3_to_v1_1(document, outdir) # nothing needs doing for 1.2 +def _draft3_to_v1_2( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: + return _draft3_to_v1_1(document, out_dir, root_dir) # nothing needs doing for 1.2 WORKFLOW_INPUT_INPUTBINDING = ( @@ -325,7 +354,9 @@ def _draft3_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: } -def _v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: +def _v1_0_to_v1_1( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: """Inner loop for transforming draft-3 to v1.0.""" if "class" in document: if document["class"] == "Workflow": @@ -339,7 +370,7 @@ def _v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: upgrade_v1_0_hints_and_reqs(entry) if "run" in entry and isinstance(entry["run"], CommentedMap): process = entry["run"] - _v1_0_to_v1_1(process, outdir) + _v1_0_to_v1_1(process, out_dir, root_dir) if "cwlVersion" in process: del process["cwlVersion"] elif isinstance(steps, MutableMapping): @@ -350,18 +381,21 @@ def _v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: if "run" in entry: if isinstance(entry["run"], CommentedMap): process = entry["run"] - _v1_0_to_v1_1(process, outdir) + _v1_0_to_v1_1(process, out_dir, root_dir) if "cwlVersion" in process: del process["cwlVersion"] elif ( isinstance(entry["run"], str) and "#" not in entry["run"] ): - path = ( + run_path = ( Path(document.lc.filename).parent / entry["run"] ).resolve() - process = v1_0_to_v1_1(load_cwl_document(path), outdir) - new_path = (outdir / entry["run"]).resolve() + process = v1_0_to_v1_1( + load_cwl_document(run_path), out_dir, root_dir + ) + new_path = out_dir / entry["run"] + check_path(root_dir, new_path) new_path.parent.mkdir(parents=True, exist_ok=True) write_cwl_document(process, new_path) elif isinstance(entry["run"], str) and "#" in entry["run"]: @@ -406,11 +440,15 @@ def _v1_0_to_v1_1(document: CommentedMap, outdir: Path) -> CommentedMap: return document -def _v1_0_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: - return _v1_0_to_v1_1(document, outdir) # nothing needs doing for v1.2 +def _v1_0_to_v1_2( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: + return _v1_0_to_v1_1(document, out_dir, root_dir) # nothing needs doing for v1.2 -def _v1_1_to_v1_2(document: CommentedMap, outdir: Path) -> CommentedMap: +def _v1_1_to_v1_2( + document: CommentedMap, out_dir: Path, root_dir: Path +) -> CommentedMap: return document diff --git a/tests/test_complete.py b/tests/test_complete.py index 6c8e715..8093b48 100644 --- a/tests/test_complete.py +++ b/tests/test_complete.py @@ -37,21 +37,21 @@ def test_draft3_tool_long_form_arrays(tmp_path: Path) -> None: def test_invalid_target(tmp_path: Path) -> None: """Test for invalid target version""" doc = load_cwl_document(get_data("testdata/v1.0/listing_deep1.cwl")) - result = upgrade_document(doc, tmp_path, "invalid-version") + result = upgrade_document(doc, tmp_path, tmp_path, "invalid-version") assert result is None def test_v1_0_to_v1_1(tmp_path: Path) -> None: """Basic CWL v1.0 to CWL v1.1 test.""" doc = load_cwl_document(get_data("testdata/v1.0/listing_deep1.cwl")) - upgraded = upgrade_document(doc, tmp_path, "v1.1") + upgraded = upgrade_document(doc, tmp_path, tmp_path, "v1.1") assert doc == upgraded def test_v1_1_to_v1_2(tmp_path: Path) -> None: """Basic CWL v1.1 to CWL v1.2 test.""" doc = load_cwl_document(get_data("testdata/v1.1/listing_deep1.cwl")) - upgraded = upgrade_document(doc, tmp_path, "v1.2") + upgraded = upgrade_document(doc, tmp_path, tmp_path, "v1.2") assert doc == upgraded From 59139ba3cc9e4d6297ccc2823a4879cdaae20bd6 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Fri, 26 May 2023 10:45:04 +0200 Subject: [PATCH 4/7] add tests --- testdata/v1.0/params_inc.yml | 120 ++++++++++++++++++++++++++++++++ testdata/v1.0/subdir/params.cwl | 19 +++++ testdata/v1.1/params_inc.yml | 120 ++++++++++++++++++++++++++++++++ testdata/v1.1/subdir/params.cwl | 18 +++++ tests/test_import.py | 31 +++++++++ 5 files changed, 308 insertions(+) create mode 100644 testdata/v1.0/params_inc.yml create mode 100644 testdata/v1.0/subdir/params.cwl create mode 100644 testdata/v1.1/params_inc.yml create mode 100755 testdata/v1.1/subdir/params.cwl create mode 100644 tests/test_import.py diff --git a/testdata/v1.0/params_inc.yml b/testdata/v1.0/params_inc.yml new file mode 100644 index 0000000..930d2e8 --- /dev/null +++ b/testdata/v1.0/params_inc.yml @@ -0,0 +1,120 @@ + - id: t1 + type: Any + outputBinding: + outputEval: $(inputs) + - id: t2 + type: Any + outputBinding: + outputEval: $(inputs.bar) + - id: t3 + type: Any + outputBinding: + outputEval: $(inputs['bar']) + - id: t4 + type: Any + outputBinding: + outputEval: $(inputs["bar"]) + + - id: t5 + type: Any + outputBinding: + outputEval: $(inputs.bar.baz) + - id: t6 + type: Any + outputBinding: + outputEval: $(inputs['bar'].baz) + - id: t7 + type: Any + outputBinding: + outputEval: $(inputs['bar']["baz"]) + - id: t8 + type: Any + outputBinding: + outputEval: $(inputs.bar['baz']) + + - id: t9 + type: Any + outputBinding: + outputEval: $(inputs.bar['b az']) + - id: t10 + type: Any + outputBinding: + outputEval: $(inputs.bar['b\'az']) + - id: t11 + type: Any + outputBinding: + outputEval: $(inputs.bar["b'az"]) + - id: t12 + type: "null" + outputBinding: + outputEval: $(inputs.bar['b"az']) + + - id: t13 + type: Any + outputBinding: + outputEval: -$(inputs.bar.baz) + - id: t14 + type: Any + outputBinding: + outputEval: -$(inputs['bar'].baz) + - id: t15 + type: Any + outputBinding: + outputEval: -$(inputs['bar']["baz"]) + - id: t16 + type: Any + outputBinding: + outputEval: -$(inputs.bar['baz']) + + - id: t17 + type: Any + outputBinding: + outputEval: $(inputs.bar.baz) $(inputs.bar.baz) + - id: t18 + type: Any + outputBinding: + outputEval: $(inputs['bar'].baz) $(inputs['bar'].baz) + - id: t19 + type: Any + outputBinding: + outputEval: $(inputs['bar']["baz"]) $(inputs['bar']["baz"]) + - id: t20 + type: Any + outputBinding: + outputEval: $(inputs.bar['baz']) $(inputs.bar['baz']) + + - id: t21 + type: Any + outputBinding: + outputEval: $(inputs.bar['b az']) $(inputs.bar['b az']) + - id: t22 + type: Any + outputBinding: + outputEval: $(inputs.bar['b\'az']) $(inputs.bar['b\'az']) + - id: t23 + type: Any + outputBinding: + outputEval: $(inputs.bar["b'az"]) $(inputs.bar["b'az"]) + - id: t24 + type: Any + outputBinding: + outputEval: $(inputs.bar['b"az']) $(inputs.bar['b"az']) + + - id: t25 + type: Any + outputBinding: + outputEval: $(inputs.bar.buz[1]) + - id: t26 + type: Any + outputBinding: + outputEval: $(inputs.bar.buz[1]) $(inputs.bar.buz[1]) + + - id: t27 + type: "null" + outputBinding: + outputEval: $(null) + + - id: t28 + type: int + outputBinding: + outputEval: $(inputs.bar.buz.length) diff --git a/testdata/v1.0/subdir/params.cwl b/testdata/v1.0/subdir/params.cwl new file mode 100644 index 0000000..bb32821 --- /dev/null +++ b/testdata/v1.0/subdir/params.cwl @@ -0,0 +1,19 @@ +class: CommandLineTool +cwlVersion: v1.0 +hints: + ResourceRequirement: + ramMin: 8 +inputs: + bar: + type: Any + default: { + "baz": "zab1", + "b az": 2, + "b'az": true, + 'b"az': null, + "buz": ['a', 'b', 'c'] + } + +outputs: {"$import": ../params_inc.yml} + +baseCommand: "true" diff --git a/testdata/v1.1/params_inc.yml b/testdata/v1.1/params_inc.yml new file mode 100644 index 0000000..274c385 --- /dev/null +++ b/testdata/v1.1/params_inc.yml @@ -0,0 +1,120 @@ +- id: t1 + type: Any + outputBinding: + outputEval: $(inputs) +- id: t2 + type: Any + outputBinding: + outputEval: $(inputs.bar) +- id: t3 + type: Any + outputBinding: + outputEval: $(inputs['bar']) +- id: t4 + type: Any + outputBinding: + outputEval: $(inputs["bar"]) + +- id: t5 + type: Any + outputBinding: + outputEval: $(inputs.bar.baz) +- id: t6 + type: Any + outputBinding: + outputEval: $(inputs['bar'].baz) +- id: t7 + type: Any + outputBinding: + outputEval: $(inputs['bar']["baz"]) +- id: t8 + type: Any + outputBinding: + outputEval: $(inputs.bar['baz']) + +- id: t9 + type: Any + outputBinding: + outputEval: $(inputs.bar['b az']) +- id: t10 + type: Any + outputBinding: + outputEval: $(inputs.bar['b\'az']) +- id: t11 + type: Any + outputBinding: + outputEval: $(inputs.bar["b'az"]) +- id: t12 + type: "null" + outputBinding: + outputEval: $(inputs.bar['b"az']) + +- id: t13 + type: Any + outputBinding: + outputEval: -$(inputs.bar.baz) +- id: t14 + type: Any + outputBinding: + outputEval: -$(inputs['bar'].baz) +- id: t15 + type: Any + outputBinding: + outputEval: -$(inputs['bar']["baz"]) +- id: t16 + type: Any + outputBinding: + outputEval: -$(inputs.bar['baz']) + +- id: t17 + type: Any + outputBinding: + outputEval: $(inputs.bar.baz) $(inputs.bar.baz) +- id: t18 + type: Any + outputBinding: + outputEval: $(inputs['bar'].baz) $(inputs['bar'].baz) +- id: t19 + type: Any + outputBinding: + outputEval: $(inputs['bar']["baz"]) $(inputs['bar']["baz"]) +- id: t20 + type: Any + outputBinding: + outputEval: $(inputs.bar['baz']) $(inputs.bar['baz']) + +- id: t21 + type: Any + outputBinding: + outputEval: $(inputs.bar['b az']) $(inputs.bar['b az']) +- id: t22 + type: Any + outputBinding: + outputEval: $(inputs.bar['b\'az']) $(inputs.bar['b\'az']) +- id: t23 + type: Any + outputBinding: + outputEval: $(inputs.bar["b'az"]) $(inputs.bar["b'az"]) +- id: t24 + type: Any + outputBinding: + outputEval: $(inputs.bar['b"az']) $(inputs.bar['b"az']) + +- id: t25 + type: Any + outputBinding: + outputEval: $(inputs.bar.buz[1]) +- id: t26 + type: Any + outputBinding: + outputEval: $(inputs.bar.buz[1]) $(inputs.bar.buz[1]) + +- id: t27 + type: "null" + outputBinding: + outputEval: $(null) + +- id: t28 + type: int + outputBinding: + outputEval: $(inputs.bar.buz.length) diff --git a/testdata/v1.1/subdir/params.cwl b/testdata/v1.1/subdir/params.cwl new file mode 100755 index 0000000..697cf97 --- /dev/null +++ b/testdata/v1.1/subdir/params.cwl @@ -0,0 +1,18 @@ +#!/usr/bin/env cwl-runner +cwlVersion: v1.1 +class: CommandLineTool +requirements: + NetworkAccess: + networkAccess: true + LoadListingRequirement: + loadListing: deep_listing +hints: + ResourceRequirement: + ramMin: 8 +inputs: + bar: + type: Any + default: {"baz": "zab1", "b az": 2, "b'az": true, 'b"az': null, "buz": ['a', 'b', + 'c']} +baseCommand: "true" +outputs: {"$import": ../params_inc.yml} diff --git a/tests/test_import.py b/tests/test_import.py new file mode 100644 index 0000000..85c9d48 --- /dev/null +++ b/tests/test_import.py @@ -0,0 +1,31 @@ +import filecmp +from pathlib import Path + +from cwlupgrader.main import load_cwl_document, main, upgrade_document +import pytest +import logging +from .util import get_data +import re + +def test_import_parent_directory(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """Confirm that $import to a superior directory still preserves the directory structure.""" + caplog.set_level(logging.WARN) + out_dir = tmp_path/"out" + out_dir.mkdir() + doc = load_cwl_document(get_data("testdata/v1.0/subdir/params.cwl")) + upgraded = upgrade_document(doc, out_dir, out_dir, "v1.1") + expected = load_cwl_document(get_data("testdata/v1.1/subdir/params.cwl")) + assert upgraded == expected + assert len(caplog.records) == 1 + assert re.search(re.escape(f"Writing file, '{tmp_path}/params_inc.yml', outside of the output directory, '{out_dir}'."), caplog.records[0].getMessage()) + +def test_import_parent_directory_safe(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """Confirm no warning when $import to a superior directory (but still in the current working directory) still preserves the directory structure.""" + caplog.set_level(logging.WARN) + out_dir = tmp_path/"out" + out_dir.mkdir() + doc = load_cwl_document(get_data("testdata/v1.0/subdir/params.cwl")) + upgraded = upgrade_document(doc, out_dir, tmp_path, "v1.1") + expected = load_cwl_document(get_data("testdata/v1.1/subdir/params.cwl")) + assert upgraded == expected + assert len(caplog.records) == 0 From 1197c22b5467336dc68a593642811ff4db16d45d Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Fri, 26 May 2023 12:49:12 +0200 Subject: [PATCH 5/7] fixes --- MANIFEST.in | 2 +- cwlupgrader/main.py | 17 ++++++++--------- tests/test_import.py | 29 +++++++++++++++++++++-------- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 613beaa..3d0430d 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,3 @@ include MANIFEST.in Makefile include cwlupgrader/py.typed -recursive-include testdata *.cwl +recursive-include testdata *.cwl *.yml diff --git a/cwlupgrader/main.py b/cwlupgrader/main.py index 35f780e..fa4ea62 100755 --- a/cwlupgrader/main.py +++ b/cwlupgrader/main.py @@ -12,10 +12,9 @@ from pathlib import Path from typing import Any, Callable, Dict, List, MutableMapping, Optional, Set, Union -from schema_salad.sourceline import SourceLine, add_lc_filename, cmap - import ruamel.yaml from ruamel.yaml.comments import CommentedMap # for consistent sort order +from schema_salad.sourceline import SourceLine, add_lc_filename, cmap _logger = logging.getLogger("cwl-upgrader") # pylint: disable=invalid-name defaultStreamHandler = logging.StreamHandler() # pylint: disable=invalid-name @@ -203,7 +202,7 @@ def load_cwl_document(path: Union[str, Path]) -> Any: def write_cwl_document(document: Any, path: Path) -> None: - """ + r""" Serialize the document using the Ruamel YAML round trip dumper. Will also prepend "#!/usr/bin/env cwl-runner\n" and @@ -294,7 +293,7 @@ def v1_1_to_v1_2(document: CommentedMap, out_dir: Path, root_dir: Path) -> Comme def draft3_to_v1_0( document: CommentedMap, out_dir: Path, root_dir: Path ) -> CommentedMap: - """Transformation loop.""" + """Transform loop for draft-3 to v1.0.""" _draft3_to_v1_0(document, out_dir, root_dir) if isinstance(document, MutableMapping): for key, value in document.items(): @@ -312,14 +311,14 @@ def draft3_to_v1_0( def draft3_to_v1_1( document: CommentedMap, out_dir: Path, root_dir: Path ) -> CommentedMap: - """transformation loop.""" + """Transform loop for draft-3 to v1.1.""" return v1_0_to_v1_1(draft3_to_v1_0(document, out_dir, root_dir), out_dir, root_dir) def draft3_to_v1_2( document: CommentedMap, out_dir: Path, root_dir: Path ) -> CommentedMap: - """transformation loop.""" + """Transform loop for draft-3 to v1.2.""" return v1_1_to_v1_2( v1_0_to_v1_1(draft3_to_v1_0(document, out_dir, root_dir), out_dir, root_dir), out_dir, @@ -405,7 +404,7 @@ def _v1_0_to_v1_1( process = v1_0_to_v1_1( load_cwl_document(str(path)), out_dir, root_dir ) - write_cwl_document(process, path.name, out_dir) + write_cwl_document(process, out_dir / path.name) elif isinstance(steps, MutableMapping): for step_name in steps: with SourceLine(steps, step_name, Exception): @@ -491,7 +490,7 @@ def _v1_1_to_v1_2( with SourceLine(steps, index, Exception): if "run" in entry and isinstance(entry["run"], CommentedMap): process = entry["run"] - _v1_1_to_v1_2(process, out_dir) + _v1_1_to_v1_2(process, out_dir, root_dir) if "cwlVersion" in process: del process["cwlVersion"] @@ -504,7 +503,7 @@ def _v1_1_to_v1_2( process = v1_1_to_v1_2( load_cwl_document(str(path)), out_dir, root_dir ) - write_cwl_document(process, path.name, out_dir, root_dir) + write_cwl_document(process, out_dir / path.name) elif isinstance(steps, MutableMapping): for step_name in steps: with SourceLine(steps, step_name, Exception): diff --git a/tests/test_import.py b/tests/test_import.py index 85c9d48..405b2cd 100644 --- a/tests/test_import.py +++ b/tests/test_import.py @@ -1,28 +1,41 @@ import filecmp +import logging +import re from pathlib import Path -from cwlupgrader.main import load_cwl_document, main, upgrade_document import pytest -import logging + +from cwlupgrader.main import load_cwl_document, main, upgrade_document + from .util import get_data -import re -def test_import_parent_directory(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + +def test_import_parent_directory( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: """Confirm that $import to a superior directory still preserves the directory structure.""" caplog.set_level(logging.WARN) - out_dir = tmp_path/"out" + out_dir = tmp_path / "out" out_dir.mkdir() doc = load_cwl_document(get_data("testdata/v1.0/subdir/params.cwl")) upgraded = upgrade_document(doc, out_dir, out_dir, "v1.1") expected = load_cwl_document(get_data("testdata/v1.1/subdir/params.cwl")) assert upgraded == expected assert len(caplog.records) == 1 - assert re.search(re.escape(f"Writing file, '{tmp_path}/params_inc.yml', outside of the output directory, '{out_dir}'."), caplog.records[0].getMessage()) + assert re.search( + re.escape( + f"Writing file, '{tmp_path}/params_inc.yml', outside of the output directory, '{out_dir}'." + ), + caplog.records[0].getMessage(), + ) + -def test_import_parent_directory_safe(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: +def test_import_parent_directory_safe( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: """Confirm no warning when $import to a superior directory (but still in the current working directory) still preserves the directory structure.""" caplog.set_level(logging.WARN) - out_dir = tmp_path/"out" + out_dir = tmp_path / "out" out_dir.mkdir() doc = load_cwl_document(get_data("testdata/v1.0/subdir/params.cwl")) upgraded = upgrade_document(doc, out_dir, tmp_path, "v1.1") From f5774554335ee27813a7cdbf9f1bc608eb3680a7 Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Fri, 26 May 2023 13:02:36 +0200 Subject: [PATCH 6/7] fixes --- MANIFEST.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MANIFEST.in b/MANIFEST.in index 3d0430d..5c6393a 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,4 @@ include MANIFEST.in Makefile include cwlupgrader/py.typed -recursive-include testdata *.cwl *.yml +recursive-include testdata *.cwl +recursive-include testdata *.yml From 6d2a6003ea8094db6dfe5b2c1b9c0320c47cde9b Mon Sep 17 00:00:00 2001 From: "Michael R. Crusoe" Date: Fri, 26 May 2023 13:07:37 +0200 Subject: [PATCH 7/7] fixes --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 54bf456..d97c30e 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,9 @@ packages=["cwlupgrader", "cwlupgrader.tests"], include_package_data=True, package_dir={"cwlupgrader.tests": "tests"}, - package_data={"cwlupgrader.tests": ["../testdata/**/*.cwl"]}, + package_data={ + "cwlupgrader.tests": ["../testdata/**/*.cwl", "../testdata/**/*.yml"] + }, install_requires=[ "setuptools", "ruamel.yaml >= 0.16.0, < 0.17.27;python_version>='3.10'",