Merge pull request #251 from PyAr/save-upgraded-venv

gilgamezh · web-flow · commit 6a8a4c4fbef6 · 2017-05-08T21:33:48.000-03:00
Select the best env from the stored ones in the case of multiple matching.
diff --git 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
 --------------------------
 
diff --git 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/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'),
+        ])
