Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double quote escape for bash script #5280

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions aiida/cmdline/commands/cmd_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def set_code_builder(ctx, param, value):
@options_code.REMOTE_ABS_PATH()
@options_code.FOLDER()
@options_code.REL_PATH()
@options_code.USE_DOUBLE_QUOTES()
@options_code.PREPEND_TEXT()
@options_code.APPEND_TEXT()
@options.NON_INTERACTIVE()
Expand Down
9 changes: 9 additions & 0 deletions aiida/cmdline/params/options/commands/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ def validate_label_uniqueness(ctx, _, value):
help='[if --store-in-db]: Relative path of the executable inside the code-folder.'
)

USE_DOUBLE_QUOTES = OverridableOption(
'--use-double-quotes/--not-use-double-quotes',
is_eager=False,
default=False,
cls=InteractiveOption,
prompt='Whether use double quotes for code cmdline params?',
help='Whether the code executable parameters in script using the double quotes to escape.'
)

LABEL = options.LABEL.clone(
prompt='Label',
callback=validate_label_uniqueness,
Expand Down
15 changes: 14 additions & 1 deletion aiida/common/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ class CodeInfo(DefaultFieldsAttributeDict):
This attribute-dictionary contains the information needed to execute a code.
Possible attributes are:

* ``prepend_cmdline_params``: (optional) a list of strings, containing parameters to be
written in front of cmdline_params::

prepend_cmdline_params cmdline_params ...

* ``cmdline_params``: a list of strings, containing parameters to be written on
the command line right after the call to the code, as for example::

Expand Down Expand Up @@ -136,16 +141,24 @@ class CodeInfo(DefaultFieldsAttributeDict):

* ``withmpi``: if True, executes the code with mpirun (or another MPI installed
on the remote computer)
* ``use_double_quotes``: is a tuple contain three bool values which decide whether use double quotes for
prepend_cmdline_params, cmdline_params, append_cmdline_parames respectively. If True, the code cmdline parameters of
bash script escaped by quotes.
* ``code_uuid``: the uuid of the code associated to the CodeInfo
* ``custom_cmdline_string``: If this field provided, only the content of this string will print and
the string will print as it is without any escape
"""
_default_fields = (
'prepend_cmdline_params',
'cmdline_params', # as a list of strings
'stdin_name',
'stdout_name',
'stderr_name',
'join_files',
'withmpi',
'code_uuid'
'use_double_quotes', # tuple to control double quotes of (prepend_, cmdline_params, append_)
'code_uuid',
'custom_cmdline_string',
)


Expand Down
17 changes: 13 additions & 4 deletions aiida/common/escaping.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import re


def escape_for_bash(str_to_escape):
def escape_for_bash(str_to_escape, use_double_quotes=False):
"""
This function takes any string and escapes it in a way that
bash will interpret it as a single string.
Expand All @@ -34,14 +34,23 @@ def escape_for_bash(str_to_escape):
Finally, note that for python I have to enclose the string '"'"'
within triple quotes to make it work, getting finally: the complicated
string found below.

:param str_to_escape: the string to escape.
:param use_double_quotes: boolean, if ``True``, use double quotes instead of single quotes.
:return: the escaped string.
"""
if str_to_escape is None:
return ''

str_to_escape = str(str_to_escape)

escaped_quotes = str_to_escape.replace("'", """'"'"'""")
return f"'{escaped_quotes}'"
if use_double_quotes:
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
escaped_quotes = str_to_escape.replace('"', '''"'"'"''')
escaped = f'"{escaped_quotes}"'
else:
escaped_quotes = str_to_escape.replace("'", """'"'"'""")
escaped = f"'{escaped_quotes}'"

return escaped


# Mapping of "SQL" tokens into corresponding regex expressions
Expand Down
22 changes: 16 additions & 6 deletions aiida/engine/processes/calcjobs/calcjob.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ def define(cls, spec: CalcJobProcessSpec) -> None: # type: ignore[override]
help='If set to true, the submission script will load the system environment variables',)
spec.input('metadata.options.environment_variables', valid_type=dict, default=lambda: {},
help='Set a dictionary of custom environment variables for this calculation',)
spec.input('metadata.options.envvar_double_quotes', valid_type=bool, default=False,
help='If set to True, use double quotes instead of single quotes to escape the environment variables '
'specified in ``environment_variables``.',)
spec.input('metadata.options.priority', valid_type=str, required=False,
help='Set the priority of the job to be queued')
spec.input('metadata.options.max_memory_kb', valid_type=int, required=False,
Expand Down Expand Up @@ -702,18 +705,24 @@ def presubmit(self, folder: Folder) -> CalcInfo:
this_withmpi = code_info.withmpi

if this_withmpi:
this_argv = (
mpi_args + extra_mpirun_params + [this_code.get_execname()] +
(code_info.cmdline_params if code_info.cmdline_params is not None else [])
)
code_info.prepend_cmdline_params = mpi_args + extra_mpirun_params
else:
this_argv = [this_code.get_execname()
] + (code_info.cmdline_params if code_info.cmdline_params is not None else [])
code_info.prepend_cmdline_params = None

# Set code_info use_double_quotes based on computer and code setup if code_info not set by plugin
if code_info.use_double_quotes is None:
code_info.use_double_quotes = (
computer.get_use_double_quotes(), this_code.get_use_double_quotes(), False
)

this_argv = [this_code.get_execname()
] + (code_info.cmdline_params if code_info.cmdline_params is not None else [])

# overwrite the old cmdline_params and add codename and mpirun stuff
code_info.cmdline_params = this_argv

codes_info.append(code_info)

job_tmpl.codes_info = codes_info

# set the codes execution mode, default set to `SERIAL`
Expand All @@ -731,6 +740,7 @@ def presubmit(self, folder: Folder) -> CalcInfo:
job_tmpl.import_sys_environment = self.node.get_option('import_sys_environment')

job_tmpl.job_environment = self.node.get_option('environment_variables')
job_tmpl.envvar_double_quotes = self.node.get_option('envvar_double_quotes')

queue_name = self.node.get_option('queue_name')
account = self.node.get_option('account')
Expand Down
12 changes: 11 additions & 1 deletion aiida/manage/tests/pytest_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,15 @@ def test_1(aiida_local_code_factory):
:rtype: object
"""

def get_code(entry_point, executable, computer=aiida_localhost, label=None, prepend_text=None, append_text=None):
def get_code(
entry_point,
executable,
computer=aiida_localhost,
label=None,
use_double_quotes=False,
prepend_text=None,
append_text=None
):
"""Get local code.

Sets up code for given entry point on given computer.
Expand Down Expand Up @@ -201,6 +209,8 @@ def get_code(entry_point, executable, computer=aiida_localhost, label=None, prep
code.label = label
code.description = label

code.set_use_double_quotes(use_double_quotes)

if prepend_text is not None:
code.set_prepend_text(prepend_text)

Expand Down
6 changes: 6 additions & 0 deletions aiida/orm/computers.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,12 @@ def get_append_text(self) -> str:
def set_append_text(self, val: str) -> None:
self.set_property('append_text', str(val))

def get_use_double_quotes(self) -> bool:
return self.get_property('use_double_quotes', False)

def set_use_double_quotes(self, val: bool) -> None:
self.set_property('use_double_quotes', bool(val))

def get_mpirun_command(self) -> List[str]:
"""
Return the mpirun command. Must be a list of strings, that will be
Expand Down
14 changes: 14 additions & 0 deletions aiida/orm/nodes/data/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,20 @@ def get_input_plugin_name(self):
"""
return self.get_attribute('input_plugin', None)

def set_use_double_quotes(self, use_double_quotes):
"""
Set whether a code cmdline is escape with double quotes.
"""
from aiida.common.lang import type_check
type_check(use_double_quotes, bool)
self.set_attribute('use_double_quotes', use_double_quotes)

def get_use_double_quotes(self):
"""
Return whether a code is escape with double quotes (False, default) or not (True).
"""
return self.get_attribute('use_double_quotes', False)

def set_append_text(self, code):
"""
Pass a string of code that will be put in the scheduler script after the
Expand Down
2 changes: 2 additions & 0 deletions aiida/orm/utils/builders/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def new(self):
code.label = self._get_and_count('label', used)
code.description = self._get_and_count('description', used)
code.set_input_plugin_name(self._get_and_count('input_plugin', used))
code.set_use_double_quotes(self._get_and_count('use_double_quotes', used))
code.set_prepend_text(self._get_and_count('prepend_text', used))
code.set_append_text(self._get_and_count('append_text', used))

Expand Down Expand Up @@ -100,6 +101,7 @@ def get_code_spec(code):
spec['label'] = code.label
spec['description'] = code.description
spec['input_plugin'] = code.get_input_plugin_name()
spec['use_double_quotes'] = code.get_use_double_quotes()
spec['prepend_text'] = code.get_prepend_text()
spec['append_text'] = code.get_append_text()

Expand Down
10 changes: 7 additions & 3 deletions aiida/schedulers/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ class JobTemplate(DefaultFieldsAttributeDict): # pylint: disable=too-many-insta
* ``rerunnable``: if the job is rerunnable (boolean)
* ``job_environment``: a dictionary with environment variables to set
before the execution of the code.
* ``envvar_double_quotes``: if set to True, use double quotes instead of single quotes to escape the
environment variables specified in ``environment_variables``.
* ``working_directory``: the working directory for this job. During
submission, the transport will first do a 'chdir' to this directory,
and then possibly set a scheduler parameter, if this is supported
Expand Down Expand Up @@ -294,9 +296,10 @@ class JobTemplate(DefaultFieldsAttributeDict): # pylint: disable=too-many-insta
* ``append_text``: a (possibly multi-line) string to be inserted
in the scheduler script after the main execution line
* ``import_sys_environment``: import the system environment variables
* ``codes_info``: a list of aiida.common.datastructures.CalcInfo objects.
Each contains the information necessary to run a single code. At the
moment, it can contain:
* ``codes_info``: a list of (aiida.common.datastructures.CodeInfo, prepend_cmdline_params)
tuple objects. Each contains the information necessary to run a single code. At the
moment, where prepend_cmdline_params is a list of string of computer or scheduler
setting cmdline parameters while CodeInfo object can contain:

* ``cmdline_parameters``: a list of strings with the command line arguments
of the program to run. This is the main program to be executed.
Expand Down Expand Up @@ -328,6 +331,7 @@ class JobTemplate(DefaultFieldsAttributeDict): # pylint: disable=too-many-insta
'submit_as_hold',
'rerunnable',
'job_environment',
'envvar_double_quotes',
'working_directory',
'email',
'email_on_started',
Expand Down
18 changes: 13 additions & 5 deletions aiida/schedulers/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def _get_submit_script_environment_variables(self, template): # pylint: disable
lines = ['# ENVIRONMENT VARIABLES BEGIN ###']

for key, value in template.job_environment.items():
lines.append(f'export {key.strip()}={escape_for_bash(value)}')
lines.append(f'export {key.strip()}={escape_for_bash(value, template.envvar_double_quotes)}')

lines.append('# ENVIRONMENT VARIABLES END ###')

Expand Down Expand Up @@ -218,19 +218,27 @@ def _get_run_line(self, codes_info, codes_run_mode):
list_of_runlines = []

for code_info in codes_info:
if code_info.custom_cmdline_string:
list_of_runlines.append(code_info.custom_cmdline_string)
continue

command_to_exec_list = []
if code_info.prepend_cmdline_params:
for arg in code_info.prepend_cmdline_params:
command_to_exec_list.append(escape_for_bash(arg, use_double_quotes=code_info.use_double_quotes[0]))
for arg in code_info.cmdline_params:
command_to_exec_list.append(escape_for_bash(arg))
command_to_exec_list.append(escape_for_bash(arg, use_double_quotes=code_info.use_double_quotes[1]))

command_to_exec = ' '.join(command_to_exec_list)

stdin_str = f'< {escape_for_bash(code_info.stdin_name)}' if code_info.stdin_name else ''
stdout_str = f'> {escape_for_bash(code_info.stdout_name)}' if code_info.stdout_name else ''
stdin_str = f'< {escape_for_bash(code_info.stdin_name, code_info.use_double_quotes[2])}' if code_info.stdin_name else ''
stdout_str = f'> {escape_for_bash(code_info.stdout_name, code_info.use_double_quotes[2])}' if code_info.stdout_name else ''

join_files = code_info.join_files
if join_files:
stderr_str = '2>&1'
else:
stderr_str = f'2> {escape_for_bash(code_info.stderr_name)}' if code_info.stderr_name else ''
stderr_str = f'2> {escape_for_bash(code_info.stderr_name, code_info.use_double_quotes[2])}' if code_info.stderr_name else ''

output_string = f'{command_to_exec} {stdin_str} {stdout_str} {stderr_str}'

Expand Down
10 changes: 8 additions & 2 deletions tests/cmdline/commands/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,14 @@ def test_noninteractive_remote(run_cli_command, aiida_localhost, non_interactive
"""Test non-interactive remote code setup."""
label = 'noninteractive_remote'
options = [
'--non-interactive', f'--label={label}', '--description=description', '--input-plugin=core.arithmetic.add',
'--on-computer', f'--computer={aiida_localhost.label}', '--remote-abs-path=/remote/abs/path'
'--non-interactive',
f'--label={label}',
'--description=description',
'--input-plugin=core.arithmetic.add',
'--on-computer',
f'--computer={aiida_localhost.label}',
'--remote-abs-path=/remote/abs/path',
'--use-double-quotes',
]
run_cli_command(cmd_code.setup_code, options)
assert isinstance(load_code(label), Code)
Expand Down
28 changes: 26 additions & 2 deletions tests/common/test_escaping.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
from aiida.common.escaping import escape_for_bash


def test_escape_for_bash():
"""Tests various inputs for `aiida.common.escaping.escape_for_bash`."""
def test_escape_for_bash_single_quotes():
"""
Tests various inputs for `aiida.common.escaping.escape_for_bash`.
sigle quotes tests and also for backward compatible
"""

tests = (
[None, ''],
['string', "'string'"],
Expand All @@ -24,3 +28,23 @@ def test_escape_for_bash():

for string_input, string_escaped in tests:
assert escape_for_bash(string_input) == string_escaped


def test_escape_for_bash_double_quotes():
"""
Tests various inputs for `aiida.common.escaping.escape_for_bash`.
double quotes tests especially useful for ENV variables with $ symbol
"""

tests = (
[None, ''],
['string', '"string"'],
['string with space', '"string with space"'],
['string with a " double quote', '''"string with a "'"'" double quote"'''],
[1, '"1"'],
[2.0, '"2.0"'],
['$PWD', '"$PWD"'],
)

for string_input, string_escaped in tests:
assert escape_for_bash(string_input, use_double_quotes=True) == string_escaped
Loading