Make test_get_bootstraps_from_recipes() deterministic and better

pythonforandroid/bootstrap.py

@@ -1,10 +1,11 @@
+import functools
+import glob
+import importlib
+import os
 from os.path import (join, dirname, isdir, normpath, splitext, basename)
 from os import listdir, walk, sep
 import sh
 import shlex
-import glob
-import importlib
-import os
 import shutil
 
 from pythonforandroid.logger import (warning, shprint, info, logger,
@@ -138,36 +139,52 @@ def run_distribute(self):
         self.distribution.save_info(self.dist_dir)
 
     @classmethod
-    def list_bootstraps(cls):
+    def all_bootstraps(cls):
         '''Find all the available bootstraps and return them.'''
         forbidden_dirs = ('__pycache__', 'common')
         bootstraps_dir = join(dirname(__file__), 'bootstraps')
+        result = set()
         for name in listdir(bootstraps_dir):
             if name in forbidden_dirs:
                 continue
             filen = join(bootstraps_dir, name)
             if isdir(filen):
-                yield name
+                result.add(name)
+        return result
 
     @classmethod
     def get_bootstrap_from_recipes(cls, recipes, ctx):
         '''Returns a bootstrap whose recipe requirements do not conflict with
         the given recipes.'''
         info('Trying to find a bootstrap that matches the given recipes.')
         bootstraps = [cls.get_bootstrap(name, ctx)
-                      for name in cls.list_bootstraps()]
-        acceptable_bootstraps = []
+                      for name in cls.all_bootstraps()]
+        acceptable_bootstraps = set()
+        known_web_packages = {"flask"}  # to pick webview over service_only
+        default_recipe_priorities = [
+            "webview", "sdl2", "service_only"  # last is highest
+        ]
+        recipes_with_deps_lists = expand_dependencies(recipes, ctx)
+        # ^^ NOTE: these are just the default priorities if no special rules
+        # apply (which you can find in the code below), so basically if no
+        # known graphical lib or web lib is used - in which case service_only
+        # is the most reasonable guess.
+
+        # Find out which bootstraps are acceptable:
         for bs in bootstraps:
             if not bs.can_be_chosen_automatically:
                 continue
-            possible_dependency_lists = expand_dependencies(bs.recipe_depends)
+            possible_dependency_lists = expand_dependencies(bs.recipe_depends, ctx)
             for possible_dependencies in possible_dependency_lists:
                 ok = True
+                # Check if the bootstap's dependencies have an internal conflict:
                 for recipe in possible_dependencies:
                     recipe = Recipe.get_recipe(recipe, ctx)
                     if any([conflict in recipes for conflict in recipe.conflicts]):
                         ok = False
                         break
+                # Check if bootstrap's dependencies conflict with chosen
+                # packages:
                 for recipe in recipes:
                     try:
                         recipe = Recipe.get_recipe(recipe, ctx)
@@ -180,14 +197,61 @@ def get_bootstrap_from_recipes(cls, recipes, ctx):
                         ok = False
                         break
                 if ok and bs not in acceptable_bootstraps:
-                    acceptable_bootstraps.append(bs)
+                    acceptable_bootstraps.add(bs)
+
         info('Found {} acceptable bootstraps: {}'.format(
             len(acceptable_bootstraps),
             [bs.name for bs in acceptable_bootstraps]))
-        if acceptable_bootstraps:
-            info('Using the first of these: {}'
-                 .format(acceptable_bootstraps[0].name))
-            return acceptable_bootstraps[0]
+
+        def have_dependency_in_recipes(dep):
+            for dep_list in recipes_with_deps_lists:
+                if dep in dep_list:
+                    return True
+            return False
+
+        # Special rule: return SDL2 bootstrap if there's an sdl2 dep:
+        if (have_dependency_in_recipes("sdl2") and
+                "sdl2" in [b.name for b in acceptable_bootstraps]
+                ):
+            info('Using sdl2 bootstrap since it is in dependencies')
+            return cls.get_bootstrap("sdl2", ctx)
+
+        # Special rule: return "webview" if we depend on common web recipe:
+        for possible_web_dep in known_web_packages:
+            if have_dependency_in_recipes(possible_web_dep):
+                # We have a web package dep!
+                if "webview" in [b.name for b in acceptable_bootstraps]:
+                    info('Using webview bootstrap since common web packages '
+                         'were found {}'.format(
+                             known_web_packages.intersection(recipes)
+                         ))
+                    return cls.get_bootstrap("webview", ctx)
+
+        # Otherwise, go by a default priority ordering to pick best one:
+        def bootstrap_priority(a, b):
+            def rank_bootstrap(bootstrap):
+                """ Returns a ranking index for each bootstrap,
+                    with higher priority ranked with higher number. """
+                if bootstrap.name in default_recipe_priorities:
+                    return default_recipe_priorities.index(bootstrap.name) + 1
+                return 0
+            # Rank bootstraps in order:
+            rank_a = rank_bootstrap(a)
+            rank_b = rank_bootstrap(b)
+            if rank_a != rank_b:
+                return (rank_b - rank_a)
+            else:
+                return (a.name - b.name)  # alphabetic sort for determinism
+
+        prioritized_acceptable_bootstraps = sorted(
+            list(acceptable_bootstraps),
+            key=functools.cmp_to_key(bootstrap_priority)
+        )
+
+        if prioritized_acceptable_bootstraps:
+            info('Using the highest ranked/first of these: {}'
+                 .format(prioritized_acceptable_bootstraps[0].name))
+            return prioritized_acceptable_bootstraps[0]
         return None
 
     @classmethod
@@ -299,9 +363,26 @@ def fry_eggs(self, sitepackages):
                 shprint(sh.rm, '-rf', d)
 
 
-def expand_dependencies(recipes):
+def expand_dependencies(recipes, ctx):
+    """ This function expands to lists of all different available
+        alternative recipe combinations, with the dependencies added in
+        ONLY for all the not-with-alternative recipes.
+        (So this is like the deps graph very simplified and incomplete, but
+         hopefully good enough for most basic bootstrap compatibility checks)
+    """
+
+    # Add in all the deps of recipes where there is no alternative:
+    recipes_with_deps = list(recipes)
+    for entry in recipes:
+        if not isinstance(entry, (tuple, list)) or len(entry) == 1:
+            if isinstance(entry, (tuple, list)):
+                entry = entry[0]
+            recipe = Recipe.get_recipe(entry, ctx)
+            recipes_with_deps += recipe.depends
+
+    # Split up lists by available alternatives:
     recipe_lists = [[]]
-    for recipe in recipes:
+    for recipe in recipes_with_deps:
         if isinstance(recipe, (tuple, list)):
             new_recipe_lists = []
             for alternative in recipe:
@@ -311,6 +392,6 @@ def expand_dependencies(recipes):
                     new_recipe_lists.append(new_list)
             recipe_lists = new_recipe_lists
         else:
-            for old_list in recipe_lists:
-                old_list.append(recipe)
+            for existing_list in recipe_lists:
+                existing_list.append(recipe)
     return recipe_lists

tests/test_bootstrap.py

@@ -1,6 +1,6 @@
+
 import os
 import sh
-
 import unittest
 
 try:
@@ -9,12 +9,14 @@
     # `Python 2` or lower than `Python 3.3` does not
     # have the `unittest.mock` module built-in
     import mock
-from pythonforandroid.bootstrap import Bootstrap
+from pythonforandroid.bootstrap import Bootstrap, expand_dependencies
 from pythonforandroid.distribution import Distribution
 from pythonforandroid.recipe import Recipe
 from pythonforandroid.archs import ArchARMv7_a
 from pythonforandroid.build import Context
 
+from test_graph import get_fake_recipe
+
 
 class BaseClassSetupBootstrap(object):
     """
@@ -100,33 +102,92 @@ def test_build_dist_dirs(self):
             bs.get_common_dir().endswith("pythonforandroid/bootstraps/common")
         )
 
-    def test_list_bootstraps(self):
+    def test_all_bootstraps(self):
         """A test which will initialize a bootstrap and will check if the
         method :meth:`~pythonforandroid.bootstrap.Bootstrap.list_bootstraps`
         returns the expected values, which should be: `empty", `service_only`,
         `webview` and `sdl2`
         """
         expected_bootstraps = {"empty", "service_only", "webview", "sdl2"}
-        set_of_bootstraps = set(Bootstrap().list_bootstraps())
+        set_of_bootstraps = Bootstrap().all_bootstraps()
         self.assertEqual(
             expected_bootstraps, expected_bootstraps & set_of_bootstraps
         )
         self.assertEqual(len(expected_bootstraps), len(set_of_bootstraps))
 
+    def test_expand_dependencies(self):
+        # Test depenency expansion of a recipe with no alternatives:
+        expanded_result_1 = expand_dependencies(["pysdl2"], self.ctx)
+        assert({"sdl2", "pysdl2", "python3"} in
+               [set(s) for s in expanded_result_1])
+
+        # Test all alternatives are listed (they won't have dependencies
+        # expanded since expand_dependencies() is too simplistic):
+        expanded_result_2 = expand_dependencies([("pysdl2", "kivy")], self.ctx)
+        assert([["pysdl2"], ["kivy"]] == expanded_result_2)
+
     def test_get_bootstraps_from_recipes(self):
-        """A test which will initialize a bootstrap and will check if the
-        method :meth:`~pythonforandroid.bootstrap.Bootstrap.
-        get_bootstraps_from_recipes` returns the expected values
-        """
-        recipes_sdl2 = {"sdl2", "python3", "kivy"}
-        bs = Bootstrap().get_bootstrap_from_recipes(recipes_sdl2, self.ctx)
+        import pythonforandroid.recipe
+        original_get_recipe = pythonforandroid.recipe.Recipe.get_recipe
+
+        @mock.patch("pythonforandroid.recipe.Recipe.get_recipe")
+        def do_test_get_bootstraps_from_recipes(mock_get_recipe):
+            """A test which will initialize a bootstrap and will check if the
+            method :meth:`~pythonforandroid.bootstrap.Bootstrap.
+            get_bootstraps_from_recipes` returns the expected values
+            """
+
+            # Use original get_recipe() until we need something else later:
+            mock_get_recipe.side_effect = original_get_recipe
+
+            # Test that SDL2 works with kivy:
+            recipes_sdl2 = {"sdl2", "python3", "kivy"}
+            bs = Bootstrap().get_bootstrap_from_recipes(recipes_sdl2, self.ctx)
+            self.assertEqual(bs.name, "sdl2")
+
+            # Test that pysdl2 or kivy alone will also yield SDL2 (dependency):
+            recipes_pysdl2_only = {"pysdl2"}
+            bs = Bootstrap().get_bootstrap_from_recipes(recipes_pysdl2_only, self.ctx)
+            self.assertEqual(bs.name, "sdl2")
+            recipes_kivy_only = {"kivy"}
+            bs = Bootstrap().get_bootstrap_from_recipes(recipes_kivy_only, self.ctx)
+            self.assertEqual(bs.name, "sdl2")
+
+            # Test that something conflicting with sdl2 won't give sdl2:
+            def _add_sdl2_conflicting_recipe(name, ctx):
+                if name == "conflictswithsdl2":
+                    if name not in pythonforandroid.recipe.Recipe.recipes:
+                        pythonforandroid.recipe.Recipe.recipes[name] = (
+                            get_fake_recipe("sdl2", conflicts=["sdl2"])
+                        )
+                return original_get_recipe(name, ctx)
+            mock_get_recipe.side_effect = _add_sdl2_conflicting_recipe
+            recipes_with_sdl2_conflict = {"python3", "conflictswithsdl2"}
+            bs = Bootstrap().get_bootstrap_from_recipes(
+                recipes_with_sdl2_conflict, self.ctx
+            )
+            self.assertNotEqual(bs.name, "sdl2")
 
-        self.assertEqual(bs.name, "sdl2")
+            # Test using flask will default to webview:
+            recipes_with_flask = {"python3", "flask"}
+            bs = Bootstrap().get_bootstrap_from_recipes(
+                recipes_with_flask, self.ctx
+            )
+            self.assertEqual(bs.name, "webview")
+
+            # Test using random packages will default to service_only:
+            recipes_with_no_sdl2_or_web = {"python3", "numpy"}
+            bs = Bootstrap().get_bootstrap_from_recipes(
+                recipes_with_no_sdl2_or_web, self.ctx
+            )
+            self.assertEqual(bs.name, "service_only")
+
+            # test wrong recipes
+            wrong_recipes = {"python2", "python3", "pyjnius"}
+            bs = Bootstrap().get_bootstrap_from_recipes(wrong_recipes, self.ctx)
+            self.assertIsNone(bs)
 
-        # test wrong recipes
-        wrong_recipes = {"python2", "python3", "pyjnius"}
-        bs = Bootstrap().get_bootstrap_from_recipes(wrong_recipes, self.ctx)
-        self.assertIsNone(bs)
+        do_test_get_bootstraps_from_recipes()
 
     @mock.patch("pythonforandroid.bootstrap.ensure_dir")
     def test_prepare_dist_dir(self, mock_ensure_dir):