From d020522650a9bfc86c22923a01fd5d7c07c65326 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 2 Feb 2016 11:02:29 +0000 Subject: [PATCH] Introduce strict shell command option. Newer tool formats (CWL and YAML just enable this by default), but XML tools can add a strict="true" to the command block to force "set -e" handling on the shell part of tool execution. Run the framework tests verifying the new and older default behavior using the following commands: ``` ./run_tests.sh -framework -id strict_shell_default_off ./run_tests.sh -framework -id strict_shell ``` --- lib/galaxy/jobs/__init__.py | 4 ++++ lib/galaxy/jobs/command_factory.py | 10 +++++++- lib/galaxy/tools/__init__.py | 3 +++ lib/galaxy/tools/parser/cwl.py | 3 +++ lib/galaxy/tools/parser/interface.py | 6 +++++ lib/galaxy/tools/parser/xml.py | 7 ++++++ lib/galaxy/tools/parser/yaml.py | 4 ++++ test/functional/tools/samples_tool_conf.xml | 2 ++ test/functional/tools/strict_shell.xml | 23 +++++++++++++++++++ .../tools/strict_shell_default_off.xml | 20 ++++++++++++++++ test/unit/jobs/test_command_factory.py | 1 + 11 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 test/functional/tools/strict_shell.xml create mode 100644 test/functional/tools/strict_shell_default_off.xml diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 7a2c0cc14df..8376d2b08aa 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -793,6 +793,10 @@ class JobWrapper( object ): def shell(self): return self.job_destination.shell or getattr(self.app.config, 'default_job_shell', DEFAULT_JOB_SHELL) + @property + def strict_shell(self): + return self.tool.strict_shell + @property def commands_in_new_shell(self): return self.app.config.commands_in_new_shell diff --git a/lib/galaxy/jobs/command_factory.py b/lib/galaxy/jobs/command_factory.py index 10fe078293c..91b53defa7c 100644 --- a/lib/galaxy/jobs/command_factory.py +++ b/lib/galaxy/jobs/command_factory.py @@ -87,7 +87,15 @@ def __externalize_commands(job_wrapper, shell, commands_builder, remote_command_ integrity_injection = "" if check_script_integrity(config): integrity_injection = INTEGRITY_INJECTION - script_contents = u"#!%s\n%s%s" % (shell, integrity_injection, tool_commands) + set_e = "" + if job_wrapper.strict_shell: + set_e = "set -e\n" + script_contents = u"#!%s\n%s%s%s" % ( + shell, + integrity_injection, + set_e, + tool_commands + ) write_script(local_container_script, script_contents, config) commands = local_container_script if 'working_directory' in remote_command_params: diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index b84a62364c2..3f292c50862 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -562,6 +562,9 @@ class Tool( object, Dictifiable ): # Parse result handling for tool exit codes and stdout/stderr messages: self.parse_stdio( tool_source ) + + self.strict_shell = tool_source.parse_strict_shell() + # Any extra generated config files for the tool self.__parse_config_files(tool_source) # Action diff --git a/lib/galaxy/tools/parser/cwl.py b/lib/galaxy/tools/parser/cwl.py index f19007c71f1..66d20cdc11a 100644 --- a/lib/galaxy/tools/parser/cwl.py +++ b/lib/galaxy/tools/parser/cwl.py @@ -61,6 +61,9 @@ class CwlToolSource(ToolSource): def parse_help(self): return "" + def parse_strict_shell(self): + return True + def parse_stdio(self): # TODO: remove duplication with YAML from galaxy.jobs.error_level import StdioErrorLevel diff --git a/lib/galaxy/tools/parser/interface.py b/lib/galaxy/tools/parser/interface.py index ab8138329d4..25e38c6ac2b 100644 --- a/lib/galaxy/tools/parser/interface.py +++ b/lib/galaxy/tools/parser/interface.py @@ -139,6 +139,12 @@ class ToolSource(object): dictionaries for use by Tool. """ + @abstractmethod + def parse_strict_shell(self): + """ Return True if tool commands should be executed with + set -e. + """ + @abstractmethod def parse_stdio(self): """ Builds lists of ToolStdioExitCode and ToolStdioRegex objects diff --git a/lib/galaxy/tools/parser/xml.py b/lib/galaxy/tools/parser/xml.py index e0e1f973a62..37be5fa1e30 100644 --- a/lib/galaxy/tools/parser/xml.py +++ b/lib/galaxy/tools/parser/xml.py @@ -281,6 +281,13 @@ class XmlToolSource(ToolSource): parser = StdioParser(self.root) return parser.stdio_exit_codes, parser.stdio_regexes + def parse_strict_shell(self): + command_el = self._command_el + if command_el is not None: + return string_as_bool(command_el.get("strict", "False")) + else: + return False + def parse_help(self): help_elem = self.root.find( 'help' ) return help_elem.text if help_elem is not None else None diff --git a/lib/galaxy/tools/parser/yaml.py b/lib/galaxy/tools/parser/yaml.py index 0455d619d99..a9fd1b94c69 100644 --- a/lib/galaxy/tools/parser/yaml.py +++ b/lib/galaxy/tools/parser/yaml.py @@ -64,6 +64,10 @@ class YamlToolSource(ToolSource): page_source = YamlPageSource(self.root_dict.get("inputs", {})) return PagesSource([page_source]) + def parse_strict_shell(self): + # TODO: Add ability to disable this. + return True + def parse_stdio(self): return error_on_exit_code() diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 48084ab8ff6..8aa278536de 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -23,6 +23,8 @@ + +