From 15ee8a2e623946afad1d841f4a3bb03e898e5878 Mon Sep 17 00:00:00 2001 From: Facundo Batista Date: Sun, 23 Apr 2017 22:00:24 -0300 Subject: [PATCH 1/2] Select the best env from the stored ones in the case of multiple matching. --- fades/cache.py | 110 +++++++++++++++++++++++++++++---------- fades/pipmanager.py | 1 - tests/test_cache.py | 122 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 202 insertions(+), 31 deletions(-) diff --git a/fades/cache.py b/fades/cache.py index 326e229..ac9fc2f 100644 --- a/fades/cache.py +++ b/fades/cache.py @@ -51,13 +51,14 @@ def _venv_match(self, installed, requirements): if not requirements: # special case for no requirements, where we can't actually # check anything: the venv is useful if nothing installed too - return not bool(installed) + return None if installed else [] + satisfying_deps = [] for repo, req_deps in requirements.items(): useful_inst = set() if repo not in installed: # the venv doesn't even have the repo - return False + return None if repo == REPO_VCS: inst_deps = {VCSDependency(url) for url in installed[repo].keys()} @@ -71,43 +72,96 @@ def _venv_match(self, installed, requirements): break else: # nothing installed satisfied that requirement - return False + return None # assure *all* that is installed is useful for the requirements - if useful_inst != inst_deps: - return False + if useful_inst == inst_deps: + satisfying_deps.extend(inst_deps) + else: + return None # it did it through! - return True + return satisfying_deps + + def _match_by_uuid(self, current_venvs, uuid): + """Select a venv matching exactly by uuid.""" + for venv_str in current_venvs: + venv = json.loads(venv_str) + env_path = venv.get('metadata', {}).get('env_path') + _, env_uuid = os.path.split(env_path) + if env_uuid == uuid: + return venv + + def _select_better_fit(self, matching_venvs): + """Receive a list of matching venvs, and decide which one is the best fit.""" + # keep the venvs in a separate array, to pick up the winner, and the (sorted, to compare + # each dependency with its equivalent) in other structure to later compare + venvs = [] + to_compare = [] + for matching, venv in matching_venvs: + to_compare.append(sorted(matching, key=lambda req: getattr(req, 'key', ''))) + venvs.append(venv) + + # compare each n-tuple of dependencies to see which one is bigger, and add score to the + # position of the winner + scores = [0] * len(venvs) + for dependencies in zip(*to_compare): + if not isinstance(dependencies[0], Distribution): + # only distribution URLs can be compared + continue + + winner = dependencies.index(max(dependencies)) + scores[winner] = scores[winner] + 1 + + # get the rightmost winner (in case of ties, to select the latest venv) + winner_pos = None + winner_score = -1 + for i, score in enumerate(scores): + if score >= winner_score: + winner_score = score + winner_pos = i + return venvs[winner_pos] + + def _match_by_requirements(self, current_venvs, requirements, interpreter, options): + """Select a venv matching interpreter and options, complying with requirements. + + Several venvs can be found in this case, will return the better fit. + """ + matching_venvs = [] + for venv_str in current_venvs: + venv = json.loads(venv_str) + + # simple filter, need to have exactly same options and interpreter + if venv.get('options') != options or venv.get('interpreter') != interpreter: + continue + + # requirements complying: result can be None (no comply) or a score to later sort + matching = self._venv_match(venv['installed'], requirements) + if matching is not None: + matching_venvs.append((matching, venv)) + + if not matching_venvs: + logger.debug("No matching venv found :(") + return + + return self._select_better_fit(matching_venvs) def _select(self, current_venvs, requirements=None, interpreter='', uuid='', options=None): """Select which venv satisfy the received requirements.""" - def get_match_by_uuid(uuid): - def match_by_uuid(env): - env_path = env.get('metadata', {}).get('env_path') - _, env_uuid = os.path.split(env_path) - return env_uuid == uuid - return match_by_uuid - - def match_by_req_and_interpreter(env): - return (env.get('options') == options and - env.get('interpreter') == interpreter and - self._venv_match(venv['installed'], requirements)) - if uuid: logger.debug("Searching a venv by uuid: %s", uuid) - match = get_match_by_uuid(uuid) + venv = self._match_by_uuid(current_venvs, uuid) else: - logger.debug("Searching a venv for reqs: %s and interpreter: %s", - requirements, interpreter) - match = match_by_req_and_interpreter + logger.debug("Searching a venv for: reqs=%s interpreter=%s options=%s", + requirements, interpreter, options) + venv = self._match_by_requirements(current_venvs, requirements, interpreter, options) - for venv_str in current_venvs: - venv = json.loads(venv_str) - if match(venv): - logger.debug("Found a matching venv! %s", venv) - return venv['metadata'] - logger.debug("No matching venv found :(") + if venv is None: + logger.debug("No matching venv found :(") + return + + logger.debug("Found a matching venv! %s", venv) + return venv['metadata'] def get_venv(self, requirements=None, interpreter='', uuid='', options=None): """Find a venv that serves these requirements, if any.""" diff --git a/fades/pipmanager.py b/fades/pipmanager.py index 08f53b9..a30fa39 100644 --- a/fades/pipmanager.py +++ b/fades/pipmanager.py @@ -24,7 +24,6 @@ import os import logging import shutil -import tempfile import contextlib from urllib import request diff --git a/tests/test_cache.py b/tests/test_cache.py index 29f3d50..92a500c 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -1,4 +1,4 @@ -# Copyright 2015-2016 Facundo Batista, Nicolás Demarchi +# Copyright 2015-2017 Facundo Batista, Nicolás Demarchi # # This program is free software: you can redistribute it and/or modify it # under the terms of the GNU General Public License version 3, as published @@ -25,7 +25,7 @@ from threading import Thread from unittest.mock import patch -from pkg_resources import parse_requirements +from pkg_resources import parse_requirements, Distribution from fades import cache, helpers, parsing @@ -35,6 +35,11 @@ def get_req(text): return list(parse_requirements(text)) +def get_distrib(*dep_ver_pairs): + """Build some Distributions with indicated info.""" + return [Distribution(project_name=dep, version=ver) for dep, ver in dep_ver_pairs] + + class TempfileTestCase(unittest.TestCase): """Basic functionality tests.""" @@ -359,6 +364,32 @@ def test_middle_match(self): 'interpreter': 'pythonX.Y', 'options': {'foo': 'bar'} }) + venv3 = json.dumps({ + 'metadata': 'venv3', + 'installed': {'pypi': {'dep': '7'}}, + 'interpreter': 'pythonX.Y', + 'options': {'foo': 'bar'} + }) + resp = self.venvscache._select([venv1, venv2, venv3], reqs, interpreter, uuid='', + options=options) + self.assertEqual(resp, 'venv2') + + def test_multiple_match_bigger_version(self): + reqs = {'pypi': get_req('dep')} + interpreter = 'pythonX.Y' + options = {'foo': 'bar'} + venv1 = json.dumps({ + 'metadata': 'venv1', + 'installed': {'pypi': {'dep': '3'}}, + 'interpreter': 'pythonX.Y', + 'options': {'foo': 'bar'} + }) + venv2 = json.dumps({ + 'metadata': 'venv2', + 'installed': {'pypi': {'dep': '7'}}, + 'interpreter': 'pythonX.Y', + 'options': {'foo': 'bar'} + }) venv3 = json.dumps({ 'metadata': 'venv3', 'installed': {'pypi': {'dep': '5'}}, @@ -367,6 +398,8 @@ def test_middle_match(self): }) resp = self.venvscache._select([venv1, venv2, venv3], reqs, interpreter, uuid='', options=options) + # matches venv2 because it has the bigger version for 'dep' (even if it's not the + # latest virtualenv created) self.assertEqual(resp, 'venv2') def test_multiple_deps_ok(self): @@ -582,3 +615,88 @@ def test_crazy_picky(self): self.assertEqual(self.check('>1.6,<1.9,!=1.9.6', '1.6.7'), 'ok') self.assertEqual(self.check('>1.6,<1.9,!=1.8.6', '1.8.7'), 'ok') self.assertEqual(self.check('>1.6,<1.9,!=1.9.6', '1.9.6'), None) + + +class BestFitTestCase(TempfileTestCase): + """Check the venv best fitting decissor.""" + + def setUp(self): + super().setUp() + self.venvscache = cache.VEnvsCache(self.tempfile) + + def check(self, possible_venvs): + """Assert that the selected venv is the best fit one.""" + self.assertEqual(self.venvscache._select_better_fit(possible_venvs), 'venv_best_fit') + + def test_one_simple(self): + self.check([ + (get_distrib(('dep', '3')), 'venv_best_fit'), + ]) + + def test_one_double(self): + self.check([ + (get_distrib(('dep1', '3'), ('dep2', '3')), 'venv_best_fit'), + ]) + + def test_two_simple(self): + self.check([ + (get_distrib(('dep', '5')), 'venv_best_fit'), + (get_distrib(('dep', '3')), 'venv_1'), + ]) + + def test_two_double_evident(self): + self.check([ + (get_distrib(('dep1', '5'), ('dep2', '7')), 'venv_best_fit'), + (get_distrib(('dep1', '3'), ('dep2', '6')), 'venv_1'), + ]) + + def test_two_double_mixed_1(self): + # tie! the one chosen is the last one + self.check([ + (get_distrib(('dep1', '3'), ('dep2', '9')), 'venv_1'), + (get_distrib(('dep1', '5'), ('dep2', '7')), 'venv_best_fit'), + ]) + + def test_two_double_mixed_2(self): + # tie! the one chosen is the last one + self.check([ + (get_distrib(('dep1', '5'), ('dep2', '7')), 'venv_1'), + (get_distrib(('dep1', '3'), ('dep2', '9')), 'venv_best_fit'), + ]) + + def test_two_triple(self): + self.check([ + (get_distrib(('dep1', '3'), ('dep2', '9'), ('dep3', '4')), 'venv_best_fit'), + (get_distrib(('dep1', '5'), ('dep2', '7'), ('dep3', '2')), 'venv_1'), + ]) + + def test_unordered(self): + # assert it compares each dependency with its equivalent + self.check([ + (get_distrib(('dep2', '3'), ('dep1', '2'), ('dep3', '8')), 'venv_best_fit'), + (get_distrib(('dep1', '7'), ('dep3', '5'), ('dep2', '2')), 'venv_1'), + ]) + + def test_big(self): + self.check([ + (get_distrib(('dep1', '3'), ('dep2', '2')), 'venv_1'), + (get_distrib(('dep1', '4'), ('dep2', '2')), 'venv_2'), + (get_distrib(('dep1', '5'), ('dep2', '7')), 'venv_best_fit'), + (get_distrib(('dep1', '5'), ('dep2', '6')), 'venv_3'), + ]) + + def test_vcs_alone(self): + self.check([ + ([parsing.VCSDependency('someurl')], 'venv_best_fit'), + ]) + + def test_vcs_mixed_simple(self): + self.check([ + ([parsing.VCSDependency('someurl')] + get_distrib(('dep', '3')), 'venv_best_fit'), + ]) + + def test_vcs_mixed_multiple(self): + self.check([ + ([parsing.VCSDependency('someurl')] + get_distrib(('dep', '3')), 'venv_best_fit'), + ([parsing.VCSDependency('someurl')] + get_distrib(('dep', '1')), 'venv_1'), + ]) From 4ba6c21a84525c2235ef3c212c8c2d7d371d680a Mon Sep 17 00:00:00 2001 From: Facundo Batista Date: Mon, 24 Apr 2017 09:40:14 -0300 Subject: [PATCH 2/2] Better explanation of version handling in the README. --- README.rst | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 3feb82a..20a58dd 100644 --- a/README.rst +++ b/README.rst @@ -236,7 +236,8 @@ don't specify a version, it will install the latest one from PyPI. For example, you do ``fades -d foobar`` and it installs foobar in version 7. At some point, there is a new version of foobar in PyPI, version 8, but if do ``fades -d foobar`` it will just reuse previously -created virtualenv, with version 7, not using the new one! +created virtualenv, with version 7, not downloading the new version and +creating a new virtualenv with it! You can tell fades to do otherwise, just do:: @@ -245,9 +246,13 @@ You can tell fades to do otherwise, just do:: ...and *fades* will search updates for the package on PyPI, and as it will found version 8, will create a new virtualenv using the latest version. -You can even use this parameter when specifying the package version. Say -you call ``fades -d foobar==7``, *fades* will install version 7 no matter -which one is the latest. But if you do:: +From this moment on, if you request ``fades -d foobar`` it will bring the +virtualenv with the new version. If you want to get a virtualenv with +not-the-latest version for any dependency, just specify the proper versions. + +You can even use the ``--check-updates`` parameter when specifying the package +version. Say you call ``fades -d foobar==7``, *fades* will install version 7 no +matter which one is the latest. But if you do:: fades -d foobar==7 --check-updates @@ -270,6 +275,7 @@ Examples: ``fades --python-options=-B foo.py`` + Setting options using config files ---------------------------------- @@ -327,6 +333,7 @@ ie, add a cron task that perform this command:: fades --clean-unused-venvs=42 + Some command line examples --------------------------