From 61b161124bec26b73fd6932a07455b6e13a3d129 Mon Sep 17 00:00:00 2001 From: Christian Monch Date: Fri, 22 Nov 2024 17:52:17 +0100 Subject: [PATCH] feat: remove `use_shell` option --- README.md | 3 +- .../annexremotes/tests/test_hierarchies.py | 40 ++++++++++--------- .../annexremotes/tests/test_priority.py | 3 +- .../annexremotes/tests/test_remake_remote.py | 5 +-- datalad_remake/commands/tests/test_make.py | 3 +- datalad_remake/tests/test_complex_cases.py | 11 ++--- datalad_remake/utils/compute.py | 9 +---- examples/fmriprep-docker/fmriprep-docker | 2 - .../fmriprep-singularity/fmriprep-singularity | 2 - examples/one-to-many | 4 +- 10 files changed, 33 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index c7dfac1..b2c6ebc 100644 --- a/README.md +++ b/README.md @@ -65,8 +65,9 @@ Create a template and place it in the `.datalad/make/methods` directory: > cat > .datalad/make/methods/one-to-many < '{output}-1.txt'; echo content: {second} > '{output}-2.txt'", ] EOF diff --git a/datalad_remake/annexremotes/tests/test_hierarchies.py b/datalad_remake/annexremotes/tests/test_hierarchies.py index bf6c9ba..94aaee7 100644 --- a/datalad_remake/annexremotes/tests/test_hierarchies.py +++ b/datalad_remake/annexremotes/tests/test_hierarchies.py @@ -10,26 +10,28 @@ create_simple_computation_dataset, ) -test_method = """ -parameters = ['first', 'second', 'third'] -use_shell = 'true' -command = [ - "echo content: {first} > 'a.txt';", - "mkdir -p 'd2_subds0/d2_subds1/d2_subds2';", - "echo content: {second} > 'b.txt';", - "echo content: {third} > 'new.txt';", - "echo content: {first} > 'd2_subds0/a0.txt';", - "echo content: {second} > 'd2_subds0/b0.txt';", - "echo content: {third} > 'd2_subds0/new.txt';", - "echo content: {first} > 'd2_subds0/d2_subds1/a1.txt';", - "echo content: {second} > 'd2_subds0/d2_subds1/b1.txt';", - "echo content: {third} > 'd2_subds0/d2_subds1/new.txt';", - "echo content: {first} > 'd2_subds0/d2_subds1/d2_subds2/a2.txt';", - "echo content: {second} > 'd2_subds0/d2_subds1/d2_subds2/b2.txt';", - "echo content: {third} > 'd2_subds0/d2_subds1/d2_subds2/new.txt';", -] -""" +script = ( + "echo content: {first} > 'a.txt';" + "mkdir -p 'd2_subds0/d2_subds1/d2_subds2';" + "echo content: {second} > 'b.txt';" + "echo content: {third} > 'new.txt';" + "echo content: {first} > 'd2_subds0/a0.txt';" + "echo content: {second} > 'd2_subds0/b0.txt';" + "echo content: {third} > 'd2_subds0/new.txt';" + "echo content: {first} > 'd2_subds0/d2_subds1/a1.txt';" + "echo content: {second} > 'd2_subds0/d2_subds1/b1.txt';" + "echo content: {third} > 'd2_subds0/d2_subds1/new.txt';" + "echo content: {first} > 'd2_subds0/d2_subds1/d2_subds2/a2.txt';" + "echo content: {second} > 'd2_subds0/d2_subds1/d2_subds2/b2.txt';" + "echo content: {third} > 'd2_subds0/d2_subds1/d2_subds2/new.txt'" +) +test_method = '\n'.join( + [ + "parameters = ['first', 'second', 'third']", + 'command = ["bash", "-c", "' + script + '"]', + ] +) output_pattern_static = [ 'a.txt', diff --git a/datalad_remake/annexremotes/tests/test_priority.py b/datalad_remake/annexremotes/tests/test_priority.py index b58275b..93fae61 100644 --- a/datalad_remake/annexremotes/tests/test_priority.py +++ b/datalad_remake/annexremotes/tests/test_priority.py @@ -23,8 +23,7 @@ # priority code. template = """ parameters = ['content'] -use_shell = 'true' -command = ["echo from {label}: {{content}} > 'a.txt'"] +command = ["bash", "-c", "echo from {label}: {{content}} > 'a.txt'"] """ diff --git a/datalad_remake/annexremotes/tests/test_remake_remote.py b/datalad_remake/annexremotes/tests/test_remake_remote.py index 0a8693a..f6de407 100644 --- a/datalad_remake/annexremotes/tests/test_remake_remote.py +++ b/datalad_remake/annexremotes/tests/test_remake_remote.py @@ -18,10 +18,7 @@ template = """ parameters = ['content'] - -use_shell = 'true' - -command = ["echo content: {content} > 'a.txt'"] +command = ["bash", "-c", "echo content: {content} > 'a.txt'"] """ diff --git a/datalad_remake/commands/tests/test_make.py b/datalad_remake/commands/tests/test_make.py index 1927218..c28ef3e 100644 --- a/datalad_remake/commands/tests/test_make.py +++ b/datalad_remake/commands/tests/test_make.py @@ -12,8 +12,7 @@ test_method = """ parameters = ['name', 'file'] -use_shell = 'true' -command = ["echo Hello {name} > {file}"] +command = ["bash", "-c", "echo Hello {name} > {file}"] """ output_pattern = ['a.txt'] diff --git a/datalad_remake/tests/test_complex_cases.py b/datalad_remake/tests/test_complex_cases.py index 35e9370..e3530d9 100644 --- a/datalad_remake/tests/test_complex_cases.py +++ b/datalad_remake/tests/test_complex_cases.py @@ -7,22 +7,17 @@ template = """ parameters = ['line'] - -use_shell = 'true' - -command = ["echo {line} >> 'a.txt'"] +command = ["bash", "-c", "echo {line} >> 'a.txt'"] """ template_c1 = """ parameters = ['line'] -use_shell = 'true' -command = ["cat a.txt > c1.txt; echo {line} >> c1.txt"] +command = ["bash", "-c", "cat a.txt > c1.txt; echo {line} >> c1.txt"] """ template_c2 = """ parameters = ['line'] -use_shell = 'true' -command = ["cat c1.txt > c2.txt; echo {line} >> c2.txt"] +command = ["bash", "-c", "cat c1.txt > c2.txt; echo {line} >> c2.txt"] """ diff --git a/datalad_remake/utils/compute.py b/datalad_remake/utils/compute.py index 2f6fbc0..afa4979 100644 --- a/datalad_remake/utils/compute.py +++ b/datalad_remake/utils/compute.py @@ -74,10 +74,5 @@ def compute( substituted_command = substitute_arguments(template, substitutions, 'command') with chdir(root_directory): - if template.get('use_shell', 'false') == 'true': - cmd = ' '.join(substituted_command) - lgr.debug(f'compute: RUNNING: with shell=True: {cmd}') - subprocess.run(cmd, shell=True, check=True) # noqa: S602 - else: - lgr.debug(f'compute: RUNNING: {substituted_command}') - subprocess.run(substituted_command, check=True) + lgr.debug(f'compute: RUNNING: {substituted_command}') + subprocess.run(substituted_command, check=True) diff --git a/examples/fmriprep-docker/fmriprep-docker b/examples/fmriprep-docker/fmriprep-docker index 8ad8144..3baf945 100644 --- a/examples/fmriprep-docker/fmriprep-docker +++ b/examples/fmriprep-docker/fmriprep-docker @@ -18,8 +18,6 @@ parameters = ['input_dir', 'output_dir', 'participant_label', 'license_file'] -use_shell = 'false' - # Note: `{root_directory}` resolves to the directory of the dataset in which the # computation was started with `datalad make`. diff --git a/examples/fmriprep-singularity/fmriprep-singularity b/examples/fmriprep-singularity/fmriprep-singularity index 415dca2..044adb7 100644 --- a/examples/fmriprep-singularity/fmriprep-singularity +++ b/examples/fmriprep-singularity/fmriprep-singularity @@ -19,8 +19,6 @@ parameters = ['container', 'input_dir', 'output_dir', 'participant_label', 'license_file'] -use_shell = 'false' - # Note: `{root_directory}` resolves to the directory of the dataset in which the # computation was started with `datalad make`. diff --git a/examples/one-to-many b/examples/one-to-many index bc94284..a1a8858 100644 --- a/examples/one-to-many +++ b/examples/one-to-many @@ -8,12 +8,12 @@ parameters = ['first', 'second', 'output'] -use_shell = 'true' - # The command line that should be executed. The curly braces are placeholders # for the parameters that were defined above. They will be replaced with # the values provided in the parameter arguments of `datalad make`. # command = [ + "bash", + "-c", "echo content: {first} > '{output}-1.txt'; echo content: {second} > '{output}-2.txt'", ]