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 ENV_VAR part #5349

Merged
merged 2 commits into from
Feb 13, 2022
Merged
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
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:
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
4 changes: 4 additions & 0 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.environment_variables_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 @@ -731,6 +734,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.environment_variables_double_quotes = self.node.get_option('environment_variables_double_quotes')

queue_name = self.node.get_option('queue_name')
account = self.node.get_option('account')
Expand Down
4 changes: 4 additions & 0 deletions aiida/schedulers/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ 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.
* ``environment_variables_double_quotes``: if set to True, use double quotes
instead of single quotes to escape the environment variables specified
in ``environment_variables``.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in ``environment_variables``.
in ``job_environment``.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@unkcpz just noticed this last mistake in the docstring. Sorry for having missed it. If you correct this, I will merge the PR. Thanks a lot

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sphuber Thanks. Updated.

* ``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 @@ -328,6 +331,7 @@ class JobTemplate(DefaultFieldsAttributeDict): # pylint: disable=too-many-insta
'submit_as_hold',
'rerunnable',
'job_environment',
'environment_variables_double_quotes',
'working_directory',
'email',
'email_on_started',
Expand Down
2 changes: 1 addition & 1 deletion 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.environment_variables_double_quotes)}')

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

Expand Down
29 changes: 17 additions & 12 deletions tests/common/test_escaping.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,24 @@
# For further information please visit http://www.aiida.net #
###########################################################################
"""Tests for the :mod:`aiida.common.escaping`."""
import pytest

from aiida.common.escaping import escape_for_bash


def test_escape_for_bash():
@pytest.mark.parametrize(('to_escape, expected_single_quotes, expected_double_quotes'), (
(None, '', ''),
('string', "'string'", '"string"'),
('string with space', "'string with space'", '"string with space"'),
(
"""string with ' single and " double quote""", """'string with '"'"' single and " double quote'""",
'''"string with ' single and "'"'" double quote"'''
),
(1, "'1'", '"1"'),
(2.0, "'2.0'", '"2.0"'),
('$PWD', "'$PWD'", '"$PWD"'),
))
def test_escape_for_bash(to_escape, expected_single_quotes, expected_double_quotes):
"""Tests various inputs for `aiida.common.escaping.escape_for_bash`."""
tests = (
[None, ''],
['string', "'string'"],
['string with space', "'string with space'"],
["string with a ' single quote", """'string with a '"'"' single quote'"""],
[1, "'1'"],
[2.0, "'2.0'"],
)

for string_input, string_escaped in tests:
assert escape_for_bash(string_input) == string_escaped
assert escape_for_bash(to_escape, use_double_quotes=False) == expected_single_quotes
assert escape_for_bash(to_escape, use_double_quotes=True) == expected_double_quotes
5 changes: 3 additions & 2 deletions tests/schedulers/test_sge.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ def test_submit_script(self):
job_tmpl.priority = None
job_tmpl.max_wallclock_seconds = '3600' # "23:59:59"
job_tmpl.job_environment = {'HOME': '/home/users/dorigm7s/', 'WIENROOT': '$HOME:/WIEN2k'}
job_tmpl.environment_variables_double_quotes = True

submit_script_text = sge.get_submit_script(job_tmpl)

Expand All @@ -335,8 +336,8 @@ def test_submit_script(self):
self.assertTrue('#$ -q FavQ.q' in submit_script_text)
self.assertTrue('#$ -l h_rt=01:00:00' in submit_script_text)
self.assertTrue('# ENVIRONMENT VARIABLES BEGIN ###' in submit_script_text)
self.assertTrue("export HOME='/home/users/dorigm7s/'" in submit_script_text)
self.assertTrue("export WIENROOT='$HOME:/WIEN2k'" in submit_script_text)
self.assertTrue('export HOME="/home/users/dorigm7s/"' in submit_script_text)
self.assertTrue('export WIENROOT="$HOME:/WIEN2k"' in submit_script_text)
self.assertFalse('#$ -r yes' in submit_script_text)

def test_submit_script_rerunnable(self): # pylint: disable=no-self-use
Expand Down