From c2ac19275064b4c5b07685133f28bb509fd6be7d Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 6 Jan 2017 14:28:43 -0500 Subject: [PATCH] Implement resolve_all on Conda dependency resolver. This is a somewhat substantial reversal in the typical way that the Conda resolver works. Everything related to caching, copying, linking, and building environments on the fly should now only apply if both of the following conditions are met (1) there is more than one requirement tag in a tool and (2) not all of them are resolvable exactly by the Conda resolver. For recipes that don't meet both of these two criteria - the normal case I would suspect going forward - Galaxy will just look for a hashed environment for these requirements built for all the requirements at once whenever the requirements are installed. The new 90% case, such environments should be much less buggy for two primary reasons. - #3299 is solved - in other words Conda is deferred to and if packages have potential conflicts - Conda can choose the right combination of build specifiers to resolve things correctly. - Environments are no longer built on a per-job basis - this means file system problems related to linking and copying aren't really an issue and complexity related to caching can be safely ignored. My guess is we should re-write all the Conda docs to make the other use case seem like a corner case - because hopefully it is now. This commit includes a test tool that wouldn't work without the rewrite I believe. --- lib/galaxy/tools/deps/conda_util.py | 21 +++- lib/galaxy/tools/deps/resolvers/conda.py | 110 ++++++++++++++++++ .../tools/mulled_example_conflict.xml | 17 +++ test/functional/tools/samples_tool_conf.xml | 1 + 4 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 test/functional/tools/mulled_example_conflict.xml diff --git a/lib/galaxy/tools/deps/conda_util.py b/lib/galaxy/tools/deps/conda_util.py index 496828f99af..6632bd0fdd6 100644 --- a/lib/galaxy/tools/deps/conda_util.py +++ b/lib/galaxy/tools/deps/conda_util.py @@ -364,6 +364,17 @@ def install_conda(conda_context=None): os.remove(script_path) +def install_conda_targets(conda_targets, env_name, conda_context=None): + conda_context = _ensure_conda_context(conda_context) + conda_context.ensure_channels_configured() + create_args = [ + "--name", env_name, # enviornment for package + ] + for conda_target in conda_targets: + create_args.append(conda_target.package_specifier) + return conda_context.exec_create(create_args) + + def install_conda_target(conda_target, conda_context=None): """ Install specified target into a its own environment. """ @@ -376,10 +387,14 @@ def install_conda_target(conda_target, conda_context=None): return conda_context.exec_create(create_args) -def cleanup_failed_install(conda_target, conda_context=None): +def cleanup_failed_install_of_environment(env, conda_context=None): conda_context = _ensure_conda_context(conda_context) - if conda_context.has_env(conda_target.install_environment): - conda_context.exec_remove([conda_target.install_environment]) + if conda_context.has_env(env): + conda_context.exec_remove([env]) + + +def cleanup_failed_install(conda_target, conda_context=None): + cleanup_failed_install_of_environment(conda_target.install_environment, conda_context=conda_context) def best_search_result(conda_target, conda_context=None, channels_override=None): diff --git a/lib/galaxy/tools/deps/resolvers/conda.py b/lib/galaxy/tools/deps/resolvers/conda.py index f384d1028e4..56e368db41b 100644 --- a/lib/galaxy/tools/deps/resolvers/conda.py +++ b/lib/galaxy/tools/deps/resolvers/conda.py @@ -11,10 +11,13 @@ import galaxy.tools.deps.installable from ..conda_util import ( build_isolated_environment, cleanup_failed_install, + cleanup_failed_install_of_environment, CondaContext, CondaTarget, + hash_conda_packages, install_conda, install_conda_target, + install_conda_targets, installed_conda_targets, is_conda_target_installed, USE_PATH_EXEC_DEFAULT, @@ -102,6 +105,71 @@ class CondaDependencyResolver(DependencyResolver, ListableDependencyResolver, In def clean(self, **kwds): return self.conda_context.exec_clean() + def install_all(self, conda_targets): + env = self.merged_environment_name(conda_targets) + return_code = install_conda_targets(conda_targets, env, conda_context=self.conda_context) + if return_code != 0: + is_installed = False + else: + # Recheck if installed + is_installed = self.conda_context.has_env(env) + + if not is_installed: + log.debug("Removing failed conda install of {}".format(str(conda_targets))) + cleanup_failed_install_of_environment(env, conda_context=self.conda_context) + + return is_installed + + def resolve_all(self, requirements, **kwds): + if len(requirements) == 0: + return False + + if not os.path.isdir(self.conda_context.conda_prefix): + return False + + for requirement in requirements: + if requirement.type != "package": + return False + + conda_targets = [] + for requirement in requirements: + version = requirement.version + if self.versionless: + version = None + + conda_targets.append(CondaTarget(requirement.name, version=version)) + + preserve_python_environment = kwds.get("preserve_python_environment", False) + + env = self.merged_environment_name(conda_targets) + dependencies = [] + + is_installed = self.conda_context.has_env(env) + if not is_installed and self.auto_install: + is_installed = self.install_all(conda_targets) + + if is_installed: + for requirement in requirements: + dependency = MergedCondaDependency( + self.conda_context, + self.conda_context.env_path(env), + exact=self.versionless or requirement.version is None, + name=requirement.name, + version=requirement.version, + preserve_python_environment=preserve_python_environment, + ) + dependencies.append(dependency) + + return dependencies + + def merged_environment_name(self, conda_targets): + if len(conda_targets) > 1: + # For continuity with mulled containers this is kind of nice. + return "mulled-v1-%s" % hash_conda_packages(conda_targets) + else: + assert len(conda_targets) == 1 + return conda_targets[0].install_environment + def resolve(self, name, version, type, **kwds): # Check for conda just not being there, this way we can enable # conda by default and just do nothing in not configured. @@ -193,6 +261,48 @@ class CondaDependencyResolver(DependencyResolver, ListableDependencyResolver, In return self.conda_context.conda_prefix +class MergedCondaDependency(Dependency): + dict_collection_visible_keys = Dependency.dict_collection_visible_keys + ['environment_path', 'name', 'version'] + dependency_type = 'conda' + + def __init__(self, conda_context, environment_path, exact, name=None, version=None, preserve_python_environment=False): + self.activate = conda_context.activate + self.conda_context = conda_context + self.environment_path = environment_path + self._exact = exact + self._name = name + self._version = version + self.cache_path = None + self._preserve_python_environment = preserve_python_environment + + @property + def exact(self): + return self._exact + + @property + def name(self): + return self._name + + @property + def version(self): + return self._version + + def shell_commands(self, requirement): + 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 + # variables in the future. + return """export PATH=$PATH:'%s/bin' """ % ( + self.environment_path, + ) + else: + return """[ "$CONDA_DEFAULT_ENV" = "%s" ] || . %s '%s' > conda_activate.log 2>&1 """ % ( + self.environment_path, + self.activate, + self.environment_path + ) + + class CondaDependency(Dependency): dict_collection_visible_keys = Dependency.dict_collection_visible_keys + ['environment_path', 'name', 'version'] dependency_type = 'conda' diff --git a/test/functional/tools/mulled_example_conflict.xml b/test/functional/tools/mulled_example_conflict.xml new file mode 100644 index 00000000000..4df76c2fab1 --- /dev/null +++ b/test/functional/tools/mulled_example_conflict.xml @@ -0,0 +1,17 @@ + + + &1 | grep -q structural + ]]> + + lumpy-sv + numpy + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 58ecb3b6f5a..ab7ed822054 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -123,6 +123,7 @@ +