From 8ef1e51a4a06a74e2eaa9cd188a7f9194d345c27 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Mon, 6 Nov 2017 14:38:25 +0000 Subject: [PATCH 1/2] Remove `requirement` from shell_commands() parameters --- lib/galaxy/tools/deps/__init__.py | 4 ++-- lib/galaxy/tools/deps/resolvers/__init__.py | 4 ++-- lib/galaxy/tools/deps/resolvers/conda.py | 4 ++-- .../tools/deps/resolvers/galaxy_packages.py | 23 +++++++++---------- lib/galaxy/tools/deps/resolvers/lmod.py | 2 +- lib/galaxy/tools/deps/resolvers/modules.py | 2 +- .../tools/deps/resolvers/resolver_mixins.py | 2 +- .../deps/resolvers/tool_shed_packages.py | 6 ++--- .../resolvers/unlinked_tool_shed_packages.py | 8 +++---- test/unit/tools/test_conda_resolution.py | 4 ++-- test/unit/tools/test_tool_deps.py | 8 +++---- 11 files changed, 33 insertions(+), 34 deletions(-) diff --git a/lib/galaxy/tools/deps/__init__.py b/lib/galaxy/tools/deps/__init__.py index 50b04bd28c2..acc2867b32d 100644 --- a/lib/galaxy/tools/deps/__init__.py +++ b/lib/galaxy/tools/deps/__init__.py @@ -110,7 +110,7 @@ class DependencyManager(object): def dependency_shell_commands(self, requirements, **kwds): requirement_to_dependency = self.requirements_to_dependencies(requirements, **kwds) - return [dependency.shell_commands(requirement) for requirement, dependency in requirement_to_dependency.items()] + return [dependency.shell_commands() for dependency in requirement_to_dependency.values()] def requirements_to_dependencies(self, requirements, **kwds): """ @@ -252,7 +252,7 @@ class CachedDependencyManager(DependencyManager): self.build_cache(requirements, **kwds) if os.path.exists(hashed_dependencies_dir): [dep.set_cache_path(hashed_dependencies_dir) for dep in cacheable_dependencies] - commands = [dep.shell_commands(req) for req, dep in resolved_dependencies.items()] + commands = [dep.shell_commands() for dep in resolved_dependencies.values()] return commands def hash_dependencies(self, resolved_dependencies): diff --git a/lib/galaxy/tools/deps/resolvers/__init__.py b/lib/galaxy/tools/deps/resolvers/__init__.py index 5c3811024e0..d7760f8b7d2 100644 --- a/lib/galaxy/tools/deps/resolvers/__init__.py +++ b/lib/galaxy/tools/deps/resolvers/__init__.py @@ -239,7 +239,7 @@ class Dependency(Dictifiable, object): cacheable = False @abstractmethod - def shell_commands(self, requirement): + def shell_commands(self): """ Return shell commands to enable this dependency. """ @@ -273,7 +273,7 @@ class NullDependency(Dependency): """ return "Dependency %s not found." % self.name - def shell_commands(self, requirement): + def shell_commands(self): return None diff --git a/lib/galaxy/tools/deps/resolvers/conda.py b/lib/galaxy/tools/deps/resolvers/conda.py index 3d61f057afa..7f0b72c5ab2 100644 --- a/lib/galaxy/tools/deps/resolvers/conda.py +++ b/lib/galaxy/tools/deps/resolvers/conda.py @@ -398,7 +398,7 @@ class MergedCondaDependency(Dependency): def version(self): return self._version - def shell_commands(self, requirement): + def shell_commands(self): if self._preserve_python_environment: # On explicit testing the only such requirement I am aware of is samtools - and it seems to work # fine with just appending the PATH as done below. Other tools may require additional @@ -464,7 +464,7 @@ class CondaDependency(Dependency): "You can try to shorten the path to the job_working_directory.") raise DependencyException("Conda dependency seemingly installed but failed to build job environment.") - def shell_commands(self, requirement): + def shell_commands(self): if not self.cache_path: # Build an isolated environment if not using a cached dependency manager self.build_environment() diff --git a/lib/galaxy/tools/deps/resolvers/galaxy_packages.py b/lib/galaxy/tools/deps/resolvers/galaxy_packages.py index 632dfc85cea..f3d5dea2f5b 100644 --- a/lib/galaxy/tools/deps/resolvers/galaxy_packages.py +++ b/lib/galaxy/tools/deps/resolvers/galaxy_packages.py @@ -27,23 +27,22 @@ class GalaxyPackageDependency(Dependency): dict_collection_visible_keys = Dependency.dict_collection_visible_keys + ['script', 'path', 'version', 'name'] dependency_type = 'galaxy_package' - def __init__(self, script, path, version, name, exact=True): + def __init__(self, script, path, name, type, version, exact=True): self.script = script self.path = path - self.version = version self.name = name + self.type = type + self.version = version self._exact = exact + assert self.script is not None or self.path is not None @property def exact(self): return self._exact - def shell_commands(self, requirement): + def shell_commands(self): base_path = self.path - if self.script is None and base_path is None: - log.warning("Failed to resolve dependency on '%s', ignoring", requirement.name) - commands = None - elif requirement.type == 'package' and self.script is None: + if self.type == 'package' and self.script is None: commands = 'PACKAGE_BASE=%s; export PACKAGE_BASE; PATH="%s/bin:$PATH"; export PATH' % (base_path, base_path) else: commands = 'PACKAGE_BASE=%s; export PACKAGE_BASE; . %s' % (base_path, self.script) @@ -83,7 +82,7 @@ class BaseGalaxyPackageDependencyResolver(DependencyResolver, UsesToolDependency def _find_dep_versioned(self, name, version, type='package', **kwds): base_path = self.base_path path = join(base_path, name, version) - return self._galaxy_package_dep(path, version, name, True) + return self._galaxy_package_dep(path, version, name, type, True) def _find_dep_default(self, name, type='package', exact=True, **kwds): base_path = self.base_path @@ -91,16 +90,16 @@ class BaseGalaxyPackageDependencyResolver(DependencyResolver, UsesToolDependency if islink(path): real_path = realpath(path) real_version = basename(real_path) - return self._galaxy_package_dep(real_path, real_version, name, exact) + return self._galaxy_package_dep(real_path, real_version, name, type, exact) else: return NullDependency(version=None, name=name) - def _galaxy_package_dep(self, path, version, name, exact): + def _galaxy_package_dep(self, path, version, name, type, exact): script = join(path, 'env.sh') if exists(script): - return self.dependency_type(script, path, version, name, exact) + return self.dependency_type(script, path, name, type, version, exact) elif exists(join(path, 'bin')): - return self.dependency_type(None, path, version, name, exact) + return self.dependency_type(None, path, name, type, version, exact) return NullDependency(version=version, name=name) diff --git a/lib/galaxy/tools/deps/resolvers/lmod.py b/lib/galaxy/tools/deps/resolvers/lmod.py index 6b2d35ba3de..91ee69f31bc 100644 --- a/lib/galaxy/tools/deps/resolvers/lmod.py +++ b/lib/galaxy/tools/deps/resolvers/lmod.py @@ -156,7 +156,7 @@ class LmodDependency(Dependency): def exact(self): return self._exact - def shell_commands(self, requirement): + def shell_commands(self): # Get the full module name in the form "tool_name/tool_version" module_to_load = self.module_name if self.module_version: diff --git a/lib/galaxy/tools/deps/resolvers/modules.py b/lib/galaxy/tools/deps/resolvers/modules.py index 50fd9c549d3..3cb97c04624 100644 --- a/lib/galaxy/tools/deps/resolvers/modules.py +++ b/lib/galaxy/tools/deps/resolvers/modules.py @@ -185,7 +185,7 @@ class ModuleDependency(Dependency): def exact(self): return self._exact - def shell_commands(self, requirement): + def shell_commands(self): module_to_load = self.module_name if self.module_version: module_to_load = '%s/%s' % (self.module_name, self.module_version) diff --git a/lib/galaxy/tools/deps/resolvers/resolver_mixins.py b/lib/galaxy/tools/deps/resolvers/resolver_mixins.py index 59adaba6503..efcb0bd1a9c 100644 --- a/lib/galaxy/tools/deps/resolvers/resolver_mixins.py +++ b/lib/galaxy/tools/deps/resolvers/resolver_mixins.py @@ -71,7 +71,7 @@ class HomebrewDependency(Dependency): def exact(self): return self._exact - def shell_commands(self, requirement): + def shell_commands(self): raw_commands = self.commands.replace("\n", ";") return raw_commands diff --git a/lib/galaxy/tools/deps/resolvers/tool_shed_packages.py b/lib/galaxy/tools/deps/resolvers/tool_shed_packages.py index c2c5153d126..dfd10ced586 100644 --- a/lib/galaxy/tools/deps/resolvers/tool_shed_packages.py +++ b/lib/galaxy/tools/deps/resolvers/tool_shed_packages.py @@ -20,7 +20,7 @@ class ToolShedPackageDependencyResolver(BaseGalaxyPackageDependencyResolver, Use installed_tool_dependency = self._get_installed_dependency(name, type, version=version, **kwds) if installed_tool_dependency: path = self._get_package_installed_dependency_path(installed_tool_dependency, name, version) - return self._galaxy_package_dep(path, version, name, True) + return self._galaxy_package_dep(path, version, name, type, True) else: return NullDependency(version=version, name=name) @@ -33,7 +33,7 @@ class ToolShedPackageDependencyResolver(BaseGalaxyPackageDependencyResolver, Use has_script_dep = is_galaxy_dep and dependency.script and dependency.path if has_script_dep: # Environment settings do not use versions. - return ToolShedDependency(dependency.script, dependency.path, None, name, True) + return ToolShedDependency(dependency.script, dependency.path, name, 'set_environment', None, True) return NullDependency(version=None, name=name) def _get_package_installed_dependency_path(self, installed_tool_dependency, name, version): @@ -59,7 +59,7 @@ class ToolShedPackageDependencyResolver(BaseGalaxyPackageDependencyResolver, Use tool_shed_repository.installed_changeset_revision)) if exists(path): script = join(path, 'env.sh') - return ToolShedDependency(script, path, None, name, True) + return ToolShedDependency(script, path, name, 'set_environment', None, True) return NullDependency(version=None, name=name) diff --git a/lib/galaxy/tools/deps/resolvers/unlinked_tool_shed_packages.py b/lib/galaxy/tools/deps/resolvers/unlinked_tool_shed_packages.py index 60ae6ac8329..b5911064804 100644 --- a/lib/galaxy/tools/deps/resolvers/unlinked_tool_shed_packages.py +++ b/lib/galaxy/tools/deps/resolvers/unlinked_tool_shed_packages.py @@ -67,7 +67,7 @@ class UnlinkedToolShedPackageDependencyResolver(BaseGalaxyPackageDependencyResol path = join(self.base_path, name, version) if exists(path): # First try the way without owner/name/revision - package = self._galaxy_package_dep(path, version, name, True) + package = self._galaxy_package_dep(path, version, name, type, True) if not isinstance(package, NullDependency): log.debug("Found dependency '%s' '%s' '%s' at '%s'", name, version, type, path) possibles.append(CandidateDependency(package, path)) @@ -79,7 +79,7 @@ class UnlinkedToolShedPackageDependencyResolver(BaseGalaxyPackageDependencyResol package_path = join(owner_path, package_name) for revision in listdir(package_path): revision_path = join(package_path, revision) - package = self._galaxy_package_dep(revision_path, version, name, True) + package = self._galaxy_package_dep(revision_path, version, name, type, True) if not isinstance(package, NullDependency): log.debug("Found dependency '%s' '%s' '%s' at '%s'", name, version, type, revision_path) possibles.append(CandidateDependency(package, package_path, owner)) @@ -145,11 +145,11 @@ class CandidateDependency(Dependency): self.path = path self.owner = owner - def shell_commands(self, requirement): + def shell_commands(self): """ Return shell commands to enable this dependency. """ - return self.dependency.shell_commands(requirement) + return self.dependency.shell_commands() __all__ = ('UnlinkedToolShedPackageDependencyResolver', ) diff --git a/test/unit/tools/test_conda_resolution.py b/test/unit/tools/test_conda_resolution.py index 791f32c381b..7db7d9093a4 100644 --- a/test/unit/tools/test_conda_resolution.py +++ b/test/unit/tools/test_conda_resolution.py @@ -32,7 +32,7 @@ def test_conda_resolution(): ) req = ToolRequirement(name="samtools", version=None, type="package") dependency = resolver.resolve(req, job_directory=job_dir) - assert dependency.shell_commands(None) is not None + assert dependency.shell_commands() is not None finally: shutil.rmtree(base_path) @@ -55,7 +55,7 @@ def test_against_conda_prefix_regression(): assert len(list(conda_util.installed_conda_targets(conda_context))) == 0 req = ToolRequirement(name="samtools", version="0.1.16", type="package") dependency = resolver.resolve(req, job_directory=job_dir) - assert dependency.shell_commands(None) is not None # install should not fail anymore + assert dependency.shell_commands() is not None # install should not fail anymore installed_targets = list(conda_util.installed_conda_targets(conda_context)) assert len(installed_targets) > 0 finally: diff --git a/test/unit/tools/test_tool_deps.py b/test/unit/tools/test_tool_deps.py index 00d0561bc13..23cc30962cf 100644 --- a/test/unit/tools/test_tool_deps.py +++ b/test/unit/tools/test_tool_deps.py @@ -287,7 +287,7 @@ echo 'FOO="bar"' ''') resolver = Bunch(modulecmd=mock_modulecmd, modulepath='/something') dependency = ModuleDependency(resolver, "foomodule", "1.0") - __assert_foo_exported(dependency.shell_commands(Bunch(type="package"))) + __assert_foo_exported(dependency.shell_commands()) def test_lmod_dependency_resolver(): @@ -420,7 +420,7 @@ echo 'FOO="bar"' ''') resolver = Bunch(lmodexec=mock_lmodexec, settargexec=None, modulepath='/path/to/modulefiles') dependency = LmodDependency(resolver, "foomodule", "1.0") - __assert_foo_exported(dependency.shell_commands(Bunch(type="package"))) + __assert_foo_exported(dependency.shell_commands()) def __write_script(path, contents): @@ -435,8 +435,8 @@ def test_galaxy_dependency_object_script(): # Create env.sh file that just exports variable FOO and verify it # shell_commands export it correctly. env_path = __setup_galaxy_package_dep(base_path, TEST_REPO_NAME, TEST_VERSION, "export FOO=\"bar\"") - dependency = GalaxyPackageDependency(env_path, os.path.dirname(env_path), TEST_REPO_NAME, TEST_VERSION) - __assert_foo_exported(dependency.shell_commands(Bunch(type="package"))) + dependency = GalaxyPackageDependency(env_path, os.path.dirname(env_path), TEST_REPO_NAME, 'package', TEST_VERSION) + __assert_foo_exported(dependency.shell_commands()) def test_shell_commands_built(): From 11a9abbb2d7276208beed66bd153ba0fe7881d2b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 5 Nov 2017 10:18:30 +0100 Subject: [PATCH 2/2] Return an OrderedSet of dependencies instead of a list This avoids activating mulled dependencies multiple times and fixes https://github.com/galaxyproject/galaxy/issues/4821. We use OrderedSet because the order of activation may matter for toolshed dependencies. --- lib/galaxy/tools/deps/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/deps/__init__.py b/lib/galaxy/tools/deps/__init__.py index acc2867b32d..1c173aa1333 100644 --- a/lib/galaxy/tools/deps/__init__.py +++ b/lib/galaxy/tools/deps/__init__.py @@ -13,6 +13,7 @@ from galaxy.util import ( hash_util, plugin_config ) +from galaxy.util.oset import OrderedSet from .requirements import ( ToolRequirement, @@ -109,8 +110,9 @@ class DependencyManager(object): return value def dependency_shell_commands(self, requirements, **kwds): - requirement_to_dependency = self.requirements_to_dependencies(requirements, **kwds) - return [dependency.shell_commands() for dependency in requirement_to_dependency.values()] + requirements_to_dependencies = self.requirements_to_dependencies(requirements, **kwds) + ordered_dependencies = OrderedSet(requirements_to_dependencies.values()) + return [dependency.shell_commands() for dependency in ordered_dependencies] def requirements_to_dependencies(self, requirements, **kwds): """