From b92074e6ff87a19133b4d973577779c4ee6286d7 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 2 Feb 2016 11:29:37 +0000
Subject: [PATCH] Implement profile="XX.XX" on tools, better defaults for newer
 tools.

 - set -e in tool command scripts.
 - Disable implicit extra file collection. All dynamic extra file collection requires a <discover_datasets tag.
 - Disable format="input" - require explicit metadata targets (metadata_source, format_source).
 - Disable interpreter= - hacky and doesn't work as expected in too many circumstances. Use $__tool_directory__.
 - Disable $param_file - grepping the command for the string is impercise. If useful we can add flag to match the flag used to produce inputs as a JSON file.
 - Disable default version of 1.0.0, the IUC has made it clear they prefer all tools have an explicit version.

Fixes #1609.
---
 lib/galaxy/tools/__init__.py                  | 15 +++++++--
 lib/galaxy/tools/actions/__init__.py          |  8 +++--
 lib/galaxy/tools/evaluation.py                |  2 +-
 lib/galaxy/tools/parser/cwl.py                |  3 ++
 lib/galaxy/tools/parser/interface.py          | 11 +++++++
 .../tools/parser/output_collection_def.py     |  4 +--
 lib/galaxy/tools/parser/xml.py                | 31 ++++++++++++++++---
 lib/galaxy/tools/parser/yaml.py               |  5 +++
 test/unit/tools/test_evaluation.py            |  1 +
 9 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
index 3f292c508621..e16812ad3d28 100755
--- a/lib/galaxy/tools/__init__.py
+++ b/lib/galaxy/tools/__init__.py
@@ -20,6 +20,7 @@
 from paste import httpexceptions
 from six import string_types
 
+from galaxy.version import VERSION_MAJOR
 from galaxy import model
 from galaxy.managers import histories
 from galaxy.datatypes.metadata import JobExternalOutputMetadataWrapper
@@ -460,6 +461,8 @@ def parse( self, tool_source, guid=None ):
         """
         Read tool configuration from the element `root` and fill in `self`.
         """
+        self.legacy_defaults = tool_source.legacy_defaults
+        tool_profile = tool_source.parse_profile()
         # Get the UNIQUE id for the tool
         self.old_id = tool_source.parse_id()
         if guid is None:
@@ -469,6 +472,11 @@ def parse( self, tool_source, guid=None ):
         if not self.id:
             raise Exception( "Missing tool 'id'" )
 
+        if not self.legacy_defaults and VERSION_MAJOR < tool_profile:
+            template = "The tool %s targets version %s of Galaxy, you should upgrade Galaxy to ensure proper functioning of this tool."
+            message = template % (self.id, tool_profile)
+            log.warn(message)
+
         # Get the (user visible) name of the tool
         self.name = tool_source.parse_name()
         if not self.name:
@@ -476,8 +484,11 @@ def parse( self, tool_source, guid=None ):
 
         self.version = tool_source.parse_version()
         if not self.version:
-            # For backward compatibility, some tools may not have versions yet.
-            self.version = "1.0.0"
+            if self.legacy_defaults:
+                # For backward compatibility, some tools may not have versions yet.
+                self.version = "1.0.0"
+            else:
+                raise Exception( "Missing tool version.")
 
         # Support multi-byte tools
         self.is_multi_byte = tool_source.parse_is_multi_byte()
diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py
index 4f695d5062f8..92e99e15afdd 100644
--- a/lib/galaxy/tools/actions/__init__.py
+++ b/lib/galaxy/tools/actions/__init__.py
@@ -224,7 +224,10 @@ def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=Tru
 
         # Deal with input dataset names, 'dbkey' and types
         input_names = []
-        input_ext = 'data'
+        # format='input" previously would give you a random extension from
+        # the input extensions, now it should just give "input" as the output
+        # format.
+        input_ext = 'data' if tool.legacy_defaults else "input"
         input_dbkey = incoming.get( "dbkey", "?" )
         for name, data in reversed(inp_data.items()):
             if not data:
@@ -239,7 +242,8 @@ def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=Tru
             else:  # HDA
                 if data.hid:
                     input_names.append( 'data %s' % data.hid )
-            input_ext = data.ext
+            if tool.legacy_defaults:
+                input_ext = data.ext
 
             if data.dbkey not in [None, '?']:
                 input_dbkey = data.dbkey
diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py
index e6a2314601eb..689b45b3ae61 100644
--- a/lib/galaxy/tools/evaluation.py
+++ b/lib/galaxy/tools/evaluation.py
@@ -526,7 +526,7 @@ def __build_param_file( self ):
         param_dict = self.param_dict
         directory = self.local_working_directory
         command = self.tool.command
-        if command and "$param_file" in command:
+        if self.tool.legacy_defaults and command and "$param_file" in command:
             fd, param_filename = tempfile.mkstemp( dir=directory )
             os.close( fd )
             f = open( param_filename, "w" )
diff --git a/lib/galaxy/tools/parser/cwl.py b/lib/galaxy/tools/parser/cwl.py
index 66d20cdc11a3..606f56c532d9 100644
--- a/lib/galaxy/tools/parser/cwl.py
+++ b/lib/galaxy/tools/parser/cwl.py
@@ -133,6 +133,9 @@ def parse_requirements_and_containers(self):
             containers=containers,
         ))
 
+    def parse_profile(self):
+        return "16.04"
+
 
 class CwlPageSource(PageSource):
 
diff --git a/lib/galaxy/tools/parser/interface.py b/lib/galaxy/tools/parser/interface.py
index 25e38c6ac2b9..df24aa836126 100644
--- a/lib/galaxy/tools/parser/interface.py
+++ b/lib/galaxy/tools/parser/interface.py
@@ -11,6 +11,11 @@ class ToolSource(object):
     __metaclass__ = ABCMeta
     default_is_multi_byte = False
 
+    @property
+    def legacy_defaults(self):
+        # Restore oldest defaults for XML-based tools.
+        return self.parse_profile() == "legacy"
+
     @abstractmethod
     def parse_id(self):
         """ Parse an ID describing the abstract tool. This is not the
@@ -158,6 +163,12 @@ def parse_help(self):
         doesn't define help text.
         """
 
+    @abstractmethod
+    def parse_profile(self):
+        """ Return tool profile to use, currently this should be a Galaxy
+        version (> 16.04) or 'legacy'.
+        """
+
     def parse_tests_to_dict(self):
         return {'tests': []}
 
diff --git a/lib/galaxy/tools/parser/output_collection_def.py b/lib/galaxy/tools/parser/output_collection_def.py
index 4811fe8bce93..1790c89a41e6 100644
--- a/lib/galaxy/tools/parser/output_collection_def.py
+++ b/lib/galaxy/tools/parser/output_collection_def.py
@@ -23,9 +23,9 @@
 LEGACY_DEFAULT_DBKEY = None  # don't use __input__ for legacy default collection
 
 
-def dataset_collector_descriptions_from_elem( elem ):
+def dataset_collector_descriptions_from_elem( elem, legacy=True ):
     primary_dataset_elems = elem.findall( "discover_datasets" )
-    if not primary_dataset_elems:
+    if len(primary_dataset_elems) == 0 and legacy:
         return [ DEFAULT_DATASET_COLLECTOR_DESCRIPTION ]
     else:
         return map( lambda elem: DatasetCollectionDescription( **elem.attrib ), primary_dataset_elems )
diff --git a/lib/galaxy/tools/parser/xml.py b/lib/galaxy/tools/parser/xml.py
index 37be5fa1e30c..dd98a4c39518 100644
--- a/lib/galaxy/tools/parser/xml.py
+++ b/lib/galaxy/tools/parser/xml.py
@@ -112,7 +112,12 @@ def parse_environment_variables(self):
 
     def parse_interpreter(self):
         command_el = self._command_el
-        return (command_el is not None) and command_el.get("interpreter", None)
+        interpreter = (command_el is not None) and command_el.get("interpreter", None)
+        if not self.legacy_defaults:
+            log.warn("Deprecated interpeter attribute on command element is now ignored.")
+            interpreter = None
+
+        return interpreter
 
     def parse_version_command(self):
         version_cmd = self.root.find("version_command")
@@ -195,7 +200,7 @@ def _parse(data_elem, **kwds):
 
             dataset_collector_descriptions = None
             if collection_elem.find( "discover_datasets" ) is not None:
-                dataset_collector_descriptions = dataset_collector_descriptions_from_elem( collection_elem )
+                dataset_collector_descriptions = dataset_collector_descriptions_from_elem( collection_elem, legacy=False )
             structure = ToolOutputCollectionStructure(
                 collection_type=collection_type,
                 collection_type_source=collection_type_source,
@@ -262,7 +267,7 @@ def _parse_output(
         output.from_work_dir = data_elem.get("from_work_dir", None)
         output.hidden = string_as_bool( data_elem.get("hidden", "") )
         output.actions = ToolOutputActionGroup( output, data_elem.find( 'actions' ) )
-        output.dataset_collector_descriptions = dataset_collector_descriptions_from_elem( data_elem )
+        output.dataset_collector_descriptions = dataset_collector_descriptions_from_elem( data_elem, legacy=self.legacy_defaults )
         return output
 
     def parse_stdio(self):
@@ -277,6 +282,8 @@ def parse_stdio(self):
                 return aggressive_error_checks()
             else:
                 raise ValueError("Unknown detect_errors value encountered [%s]" % detect_errors)
+        elif len(self.root.findall('stdio')) == 0 and not self.legacy_defaults:
+            return error_on_exit_code()
         else:
             parser = StdioParser(self.root)
             return parser.stdio_exit_codes, parser.stdio_regexes
@@ -285,8 +292,10 @@ 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:
+        elif self.legacy_defaults:
             return False
+        else:
+            return True
 
     def parse_help(self):
         help_elem = self.root.find( 'help' )
@@ -307,6 +316,20 @@ def parse_tests_to_dict(self):
 
         return rval
 
+    @property
+    def legacy_defaults(self):
+        # Pre-16.04 or default XML defaults
+        # - Use standard error for error detection.
+        # - Don't run shells with -e
+        # - Auto-check for implicit multiple outputs.
+        # - Auto-check for $param_file.
+        # - Enable buggy interpreter attribute.
+        return self.parse_profile() == "legacy"
+
+    def parse_profile(self):
+        profile = self.root.get("profile", "legacy")
+        return profile
+
 
 def _test_elem_to_dict(test_elem, i):
     rval = dict(
diff --git a/lib/galaxy/tools/parser/yaml.py b/lib/galaxy/tools/parser/yaml.py
index a9fd1b94c698..e8990ef8dc20 100644
--- a/lib/galaxy/tools/parser/yaml.py
+++ b/lib/galaxy/tools/parser/yaml.py
@@ -168,6 +168,11 @@ def parse_tests_to_dict(self):
 
         return rval
 
+    def parse_profile(self):
+        """
+        """
+        return self.root_dict.get("profile", "16.04")
+
 
 def _parse_test(i, test_dict):
     inputs = test_dict["inputs"]
diff --git a/test/unit/tools/test_evaluation.py b/test/unit/tools/test_evaluation.py
index 63ff8d542f2b..66721a5ca044 100644
--- a/test/unit/tools/test_evaluation.py
+++ b/test/unit/tools/test_evaluation.py
@@ -256,6 +256,7 @@ def tool_directory( self ):
 class MockTool( object ):
 
     def __init__( self, app ):
+        self.legacy_defaults = True
         self.app = app
         self.hooks_called = []
         self.environment_variables = []