From cd2cf494fe1ede4ae5b43417f0da21b2ca6473a0 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 17 Jan 2017 20:01:37 -0500 Subject: [PATCH] Allow finer grain specification of requirements. This adapts CWL-style ``SoftwareRequirement`` ``specs`` to solve #1927. Here I'm trying to implement the CWL specification in a way that helps enable the feasibility of Conda packaging in Galaxy. It is a delicate balancing act aimed to upset as many interested parties as I can. To understand the problem - consider the ``blast+`` requirement found in the Galaxy wrappers. It looks something like this: ``` ``` Great, that works for Galaxy and Tool Shed packages. It doesn't work for bioconda at all. I think this problem is fairly uncommon - most packages have simple names shared across Debian, Brew, Conda, etc... - but it does happen in some cases that there are inconsistencies. Some people have taken to duplicating the requirement - this is bad and should not be done since they are mutually exclusive and Galaxy will attempt to resolve both. This introduces the following syntax for tools with profile >= 16.10: ``` ``` This allows finer grain resolution of the requirement without sacrificing the abstract name at the top. It allows the name and the version to be adapted by resolvers as needed (hopefully rarely so). This syntax is the future facing one, but obviously this tool would not work on older Galaxy versions. To remedy this - an alternative syntax can be used for tools targetting Galaxy verions pre-16.10: ``` blast+ ``` This syntax sucks - but it does give newer Galaxies the ability to resolve the specifications without breaking the more simple functionality for older Galaxies. For more information on the CWL side of this - checkout the discussion on https://github.com/common-workflow-language/cwltool/pull/214. The CWL specification information is defined at http://www.commonwl.org/v1.0/CommandLineTool.html#SoftwarePackage. Conflicts: lib/galaxy/tools/deps/__init__.py test/functional/tools/samples_tool_conf.xml --- lib/galaxy/tools/deps/__init__.py | 13 +++- lib/galaxy/tools/deps/requirements.py | 76 +++++++++++++++++-- lib/galaxy/tools/deps/resolvers/__init__.py | 28 +++++++ lib/galaxy/tools/deps/resolvers/conda.py | 5 +- lib/galaxy/tools/xsd/galaxy.xsd | 39 ++++++---- .../tools/requirement_specification_1.xml | 18 +++++ .../tools/requirement_specification_2.xml | 16 ++++ 7 files changed, 171 insertions(+), 24 deletions(-) create mode 100644 test/functional/tools/requirement_specification_1.xml create mode 100644 test/functional/tools/requirement_specification_2.xml diff --git a/lib/galaxy/tools/deps/__init__.py b/lib/galaxy/tools/deps/__init__.py index 6f254d3965a6..cc68b3c6249f 100644 --- a/lib/galaxy/tools/deps/__init__.py +++ b/lib/galaxy/tools/deps/__init__.py @@ -141,6 +141,7 @@ def _requirements_to_dependencies_dict(self, requirements, **kwds): # Check requirements all at once all_unmet = len(requirement_to_dependency) == 0 if all_unmet and hasattr(resolver, "resolve_all"): + # TODO: Handle specs. dependencies = resolver.resolve_all(resolvable_requirements, **kwds) if dependencies: assert len(dependencies) == len(resolvable_requirements) @@ -155,7 +156,17 @@ def _requirements_to_dependencies_dict(self, requirements, **kwds): if requirement in requirement_to_dependency: continue - dependency = resolver.resolve( requirement.name, requirement.version, requirement.type, **kwds ) + name = requirement.name + version = requirement.version + specs = requirement.specs + + if hasattr(resolver, "find_specification"): + spec = resolver.find_specification(specs) + if spec is not None: + name = spec.short_name + version = spec.version or version + + dependency = resolver.resolve( name, version, requirement.type, **kwds ) if require_exact and not dependency.exact: continue diff --git a/lib/galaxy/tools/deps/requirements.py b/lib/galaxy/tools/deps/requirements.py index bb2cfaa612f8..2b041fc0348d 100644 --- a/lib/galaxy/tools/deps/requirements.py +++ b/lib/galaxy/tools/deps/requirements.py @@ -14,29 +14,66 @@ class ToolRequirement( object ): run (for example, a program, package, or library). Requirements can optionally assert a specific version. """ - def __init__( self, name=None, type=None, version=None ): + def __init__( self, name=None, type=None, version=None, specs=[] ): self.name = name self.type = type self.version = version + self.specs = specs def to_dict( self ): - return dict(name=self.name, type=self.type, version=self.version) + specs = [s.to_dict() for s in self.specs] + return dict(name=self.name, type=self.type, version=self.version, specs=specs) @staticmethod def from_dict( dict ): version = dict.get( "version", None ) name = dict.get("name", None) type = dict.get("type", None) - return ToolRequirement( name=name, type=type, version=version ) + specs = [RequirementSpecification.from_dict(s) for s in dict.get("specs", [])] + return ToolRequirement( name=name, type=type, version=version, specs=specs ) def __eq__(self, other): - return self.name == other.name and self.type == other.type and self.version == other.version + return self.name == other.name and self.type == other.type and self.version == other.version and self.specs == other.specs def __ne__(self, other): return not self.__eq__(other) def __hash__(self): - return hash((self.name, self.type, self.version)) + return hash((self.name, self.type, self.version, frozenset(self.specs))) + + +class RequirementSpecification(object): + """Refine a requirement using a URI.""" + + def __init__(self, uri, version=None): + self.uri = uri + self.version = version + + @property + def specifies_version(self): + return self.version is not None + + @property + def short_name(self): + return self.uri.split("/")[-1] + + def to_dict(self): + return dict(uri=self.uri, version=self.version) + + @staticmethod + def from_dict(dict): + uri = dict.get["uri"] + version = dict.get("version", None) + return RequirementSpecification(uri=uri, version=version) + + def __eq__(self, other): + return self.uri == other.uri and self.version == other.version + + def __ne__(self, other): + return not self.__eq__(other) + + def __hash__(self): + return hash((self.uri, self.version)) class ToolRequirements(object): @@ -173,10 +210,29 @@ def parse_requirements_from_xml( xml_root ): requirements = ToolRequirements() for requirement_elem in requirement_elems: - name = xml_text( requirement_elem ) + if "name" in requirement_elem.attrib: + name = requirement_elem.get( "name" ) + spec_elems = requirement_elem.findall("specification") + specs = map(specification_from_element, spec_elems) + else: + name = xml_text( requirement_elem ) + spec_uris_raw = requirement_elem.attrib.get("specification_uris", "") + specs = [] + for spec_uri in spec_uris_raw.split(","): + if not spec_uri: + continue + version = None + if "@" in spec_uri: + uri, version = spec_uri.split("@", 1) + else: + uri = spec_uri + uri = uri.strip() + if version: + version = version.strip() + specs.append(RequirementSpecification(uri, version)) type = requirement_elem.get( "type", DEFAULT_REQUIREMENT_TYPE ) version = requirement_elem.get( "version", DEFAULT_REQUIREMENT_VERSION ) - requirement = ToolRequirement( name=name, type=type, version=version ) + requirement = ToolRequirement( name=name, type=type, version=version, specs=specs ) requirements.append( requirement ) container_elems = [] @@ -188,6 +244,12 @@ def parse_requirements_from_xml( xml_root ): return requirements, containers +def specification_from_element(specification_elem): + uri = specification_elem.get("uri", None) + version = specification_elem.get("version", None) + return RequirementSpecification(uri, version) + + def container_from_element(container_elem): identifier = xml_text(container_elem) type = container_elem.get("type", DEFAULT_CONTAINER_TYPE) diff --git a/lib/galaxy/tools/deps/resolvers/__init__.py b/lib/galaxy/tools/deps/resolvers/__init__.py index c24c107887a3..bebf7539e8a0 100644 --- a/lib/galaxy/tools/deps/resolvers/__init__.py +++ b/lib/galaxy/tools/deps/resolvers/__init__.py @@ -67,6 +67,34 @@ def _to_requirement(self, name, version=None): return ToolRequirement(name=name, type="package", version=version) +class SpecificationAwareDependencyResolver: + """Mix this into a :class:`DependencyResolver` to implement URI specification matching. + + Allows adapting generic requirements to more specific URIs - to tailor name + or version to specified resolution system. + """ + __metaclass__ = ABCMeta + + @abstractmethod + def find_specification(self, specs): + """Find closest matching specification for discovered resolver.""" + + +class SpecificationPatternDependencyResolver: + """Implement the :class:`SpecificationAwareDependencyResolver` with a regex pattern.""" + + @abstractproperty + def _specification_pattern(self): + """Pattern of URI to match against.""" + + def find_specification(self, specs): + pattern = self._specification_pattern + for spec in specs: + if pattern.match(spec.uri): + return spec + return None + + class InstallableDependencyResolver: """ Mix this into a ``DependencyResolver`` and implement to indicate the dependency resolver can attempt to install new dependencies. diff --git a/lib/galaxy/tools/deps/resolvers/conda.py b/lib/galaxy/tools/deps/resolvers/conda.py index f6fe1d3925f5..992fbd879e91 100644 --- a/lib/galaxy/tools/deps/resolvers/conda.py +++ b/lib/galaxy/tools/deps/resolvers/conda.py @@ -5,6 +5,7 @@ import logging import os +import re import galaxy.tools.deps.installable @@ -29,6 +30,7 @@ InstallableDependencyResolver, ListableDependencyResolver, NullDependency, + SpecificationPatternDependencyResolver, ) @@ -39,9 +41,10 @@ log = logging.getLogger(__name__) -class CondaDependencyResolver(DependencyResolver, ListableDependencyResolver, InstallableDependencyResolver): +class CondaDependencyResolver(DependencyResolver, ListableDependencyResolver, InstallableDependencyResolver, SpecificationPatternDependencyResolver): dict_collection_visible_keys = DependencyResolver.dict_collection_visible_keys + ['conda_prefix', 'versionless', 'ensure_channels', 'auto_install'] resolver_type = "conda" + _specification_pattern = re.compile(r"https\:\/\/anaconda.org\/\w+\/\w+") def __init__(self, dependency_manager, **kwds): self.versionless = _string_as_bool(kwds.get('versionless', 'false')) diff --git a/lib/galaxy/tools/xsd/galaxy.xsd b/lib/galaxy/tools/xsd/galaxy.xsd index 5b589306ac3e..6790c324a0cb 100644 --- a/lib/galaxy/tools/xsd/galaxy.xsd +++ b/lib/galaxy/tools/xsd/galaxy.xsd @@ -227,7 +227,7 @@ complete descriptions of the runtime of a tool. - + - - - - - This value defines the which type of the 3rd party module required by this tool. - - - - - For package type requirements this value defines a specific version of the tool dependency. - - - - + + + + + + This value defines the which type of the 3rd party module required by this tool. + + + + + For package type requirements this value defines a specific version of the tool dependency. + + + + + Name of requirement (if body of ``requirement`` element contains specification URIs). + + + + + URIs and versions of requirement specification. + + diff --git a/test/functional/tools/requirement_specification_1.xml b/test/functional/tools/requirement_specification_1.xml new file mode 100644 index 000000000000..00744e114f4d --- /dev/null +++ b/test/functional/tools/requirement_specification_1.xml @@ -0,0 +1,18 @@ + + $out_file1 ; + echo "Moo" >> $out_file1 ; + ]]> + + + + + + + + + + + + + diff --git a/test/functional/tools/requirement_specification_2.xml b/test/functional/tools/requirement_specification_2.xml new file mode 100644 index 000000000000..e3190f068ccf --- /dev/null +++ b/test/functional/tools/requirement_specification_2.xml @@ -0,0 +1,16 @@ + + $out_file1 ; + echo "Moo" >> $out_file1 ; + ]]> + + + blast+ + + + + + + + +