[WIP] Enhance distribution check when reusing distributions and...

pythonforandroid/bootstrap.py

@@ -126,7 +126,15 @@ def get_build_dir_name(self):
         return dir_name
 
     def get_build_dir(self):
-        return join(self.ctx.build_dir, 'bootstrap_builds', self.get_build_dir_name())
+        return join(
+            self.ctx.build_dir,
+            'bootstrap_builds',
+            self.get_build_dir_name(),
+            '{}__ndk_target_{}'.format(
+                '_'.join(arch.arch for arch in self.ctx.archs),
+                self.ctx.ndk_api,
+            ),
+        )
 
     def get_dist_dir(self, name):
         return join(self.ctx.dist_dir, name)

pythonforandroid/build.py

@@ -575,10 +575,10 @@ def build_recipes(build_order, python_modules, ctx, project_dir,
             info_main('Building {} for {}'.format(recipe.name, arch.arch))
             if recipe.should_build(arch):
                 recipe.build_arch(arch)
-                recipe.install_libraries(arch)
             else:
                 info('{} said it is already built, skipping'
                      .format(recipe.name))
+            recipe.install_libraries(arch)
 
         # 4) biglink everything
         info_main('# Biglinking object files')

pythonforandroid/distribution.py

@@ -2,8 +2,12 @@
 import glob
 import json
 
-from pythonforandroid.logger import (info, info_notify, warning, Err_Style, Err_Fore)
+from pythonforandroid import __version__
+from pythonforandroid.logger import (
+    info, debug, info_notify, warning, Err_Style, Err_Fore
+)
 from pythonforandroid.util import current_directory, BuildInterruptingException
+from pythonforandroid.recipe import Recipe
 from shutil import rmtree
 
 
@@ -19,14 +23,21 @@ class Distribution(object):
 
     name = None  # A name identifying the dist. May not be None.
     needs_build = False  # Whether the dist needs compiling
+    needs_clean_build = False  # Whether the build needs a full clean
     url = None
     dist_dir = None  # Where the dist dir ultimately is. Should not be None.
     ndk_api = None
+    android_api = None
+    p4a_version = None
 
     archs = []
     '''The arch targets that the dist is built for.'''
 
-    recipes = []
+    recipes = {}
+    '''A dictionary which holds recipes information. Each key is the name of
+    the recipe, and the value is recipe's version'''
+
+    recipes_to_rebuild = set()
 
     description = ''  # A long description
 
@@ -43,7 +54,6 @@ def __repr__(self):
 
     @classmethod
     def get_distribution(cls, ctx, name=None, recipes=[],
-                         ndk_api=None,
                          force_build=False,
                          extra_dist_dirs=[],
                          require_perfect_match=False,
@@ -83,23 +93,62 @@ def get_distribution(cls, ctx, name=None, recipes=[],
 
         name_match_dist = None
 
+        req_archs = [arch.arch for arch in ctx.archs]
+
+        recipes_with_version = cls.get_recipes_with_version(ctx, recipes)
+
         # 0) Check if a dist with that name already exists
         if name is not None and name:
-            possible_dists = [d for d in possible_dists if d.name == name]
+            possible_dists = [
+                d for d in possible_dists if d.name.startswith(name)
+            ]
             if possible_dists:
                 name_match_dist = possible_dists[0]
 
         # 1) Check if any existing dists meet the requirements
         _possible_dists = []
         for dist in possible_dists:
-            if (
-                ndk_api is not None and dist.ndk_api != ndk_api
-            ) or dist.ndk_api is None:
+            if any(
+                [
+                    dist.ndk_api != ctx.ndk_api,
+                    dist.android_api != ctx.android_api,
+                    set(dist.archs) != set(req_archs),
+                    dist.ndk_api is None,
+                ]
+            ):
                 continue
-            for recipe in recipes:
-                if recipe not in dist.recipes:
+            checked_dist_recipes = set()
+            for recipe_name, recipe_version in recipes_with_version.items():
+                if recipe_name not in dist.recipes.keys():
                     break
+                debug('Checking version for {} recipe: {} VS {}'.format(
+                    recipe_name, dist.recipes[recipe_name], recipe_version)
+                )
+                if dist.recipes[recipe_name] != recipe_version:
+                    warning(
+                        'Recipe {} has a newer version, '
+                        'adding it to recipes_to_rebuild'.format(recipe_name)
+                    )
+                    dist.recipes_to_rebuild.add(recipe_name)
+                    checked_dist_recipes.add(recipe_name)
             else:
+                # Check that the dist recipes has not changed (without
+                # considering the ones we already computed above)
+                for recipe_name, recipe_version in dist.recipes.items():
+                    if recipe_name in dist.recipes_to_rebuild:
+                        continue
+                    try:
+                        test_recipe = Recipe.get_recipe(recipe_name, ctx)
+                    except ValueError:
+                        # we don't have a recipe so skipping it
+                        continue
+                    if test_recipe.version != recipe_version:
+                        warning(
+                            'Recipe {} has a newer version,'
+                            ' adding to rebuild'.format(recipe_name)
+                        )
+                        dist.recipes_to_rebuild.add(recipe_name)
+
                 _possible_dists.append(dist)
         possible_dists = _possible_dists
 
@@ -110,37 +159,63 @@ def get_distribution(cls, ctx, name=None, recipes=[],
         else:
             info('No existing dists meet the given requirements!')
 
-        # If any dist has perfect recipes and ndk API, return it
+        # 2) Of the possible dists we will select the one which contains the
+        # requested recipes, unless we used the kwarg `force_build`
         for dist in possible_dists:
             if force_build:
                 continue
-            if ndk_api is not None and dist.ndk_api != ndk_api:
-                continue
-            if (set(dist.recipes) == set(recipes) or
-                (set(recipes).issubset(set(dist.recipes)) and
-                 not require_perfect_match)):
+            if set(dist.recipes.keys()) == set(recipes) or (
+                set(recipes).issubset(set(dist.recipes.keys()))
+                and not require_perfect_match
+            ):
                 info_notify('{} has compatible recipes, using this one'
                             .format(dist.name))
+                if dist.p4a_version != __version__:
+                    # We build this dist with a different version of p4a, so
+                    # we mark it as a dist that requires a clean build environ
+                    # (to avoid old cached builds issues)
+                    dist.needs_clean_build = True
+                elif dist.recipes_to_rebuild:
+                    dist.needs_build = True
+                    info_notify(
+                        'Some of the recipes has new versions,'
+                        ' we will update them:\n\t- {}'.format(
+                            '\n\t- '.join(dist.recipes_to_rebuild)
+                        )
+                    )
                 return dist
 
         assert len(possible_dists) < 2
 
-        # If there was a name match but we didn't already choose it,
+        # 3) If there was a name match but we didn't already choose it,
         # then the existing dist is incompatible with the requested
         # configuration and the build cannot continue
         if name_match_dist is not None and not allow_replace_dist:
             raise BuildInterruptingException(
-                'Asked for dist with name {name} with recipes ({req_recipes}) and '
-                'NDK API {req_ndk_api}, but a dist '
-                'with this name already exists and has either incompatible recipes '
-                '({dist_recipes}) or NDK API {dist_ndk_api}'.format(
+                '\n\tAsked for dist with name {name} and:'
+                '\n\t-> recipes: ({req_recipes})'
+                '\n\t-> NDK api: ({req_ndk_api})'
+                '\n\t-> android api ({req_android_api})'
+                '\n\t-> archs ({req_archs})'
+                '\n...but a dist with this name already exists and has either '
+                'incompatible:'
+                '\n\t-> recipes: ({dist_recipes})'
+                '\n\t-> NDK api: ({dist_ndk_api})'
+                '\n\t-> android api ({dist_android_api})'
+                '\n\t-> archs ({dist_archs})'.format(
                     name=name,
-                    req_ndk_api=ndk_api,
-                    dist_ndk_api=name_match_dist.ndk_api,
                     req_recipes=', '.join(recipes),
-                    dist_recipes=', '.join(name_match_dist.recipes)))
+                    req_ndk_api=ctx.ndk_api,
+                    req_android_api=ctx.android_api,
+                    req_archs=', '.join(req_archs),
+                    dist_recipes=', '.join(name_match_dist.recipes),
+                    dist_ndk_api=name_match_dist.ndk_api,
+                    dist_android_api=name_match_dist.android_api,
+                    dist_archs=', '.join(name_match_dist.archs),
+                )
+            )
 
-        # If we got this far, we need to build a new dist
+        # 4) If we got this far, we need to build a new dist
         dist = Distribution(ctx)
         dist.needs_build = True
 
@@ -151,10 +226,15 @@ def get_distribution(cls, ctx, name=None, recipes=[],
                 i += 1
             name = filen.format(i)
 
-        dist.name = name
+        # we add a suffix to our dist name, so we avoid to
+        # overwrite them when building for different arch
+        suffix_arch = f"_{{{'·'.join([arch.arch for arch in ctx.archs])}}}"
+        dist.name = name + suffix_arch
         dist.dist_dir = join(ctx.dist_dir, dist.name)
-        dist.recipes = recipes
+        dist.recipes = recipes_with_version
+        dist.archs = req_archs
         dist.ndk_api = ctx.ndk_api
+        dist.android_api = ctx.android_api
 
         return dist
 
@@ -186,53 +266,87 @@ def get_distributions(cls, ctx, extra_dist_dirs=[]):
                 dist.dist_dir = folder
                 dist.needs_build = False
                 dist.recipes = dist_info['recipes']
-                if 'archs' in dist_info:
-                    dist.archs = dist_info['archs']
-                if 'ndk_api' in dist_info:
-                    dist.ndk_api = dist_info['ndk_api']
-                else:
-                    dist.ndk_api = None
-                    warning(
-                        "Distribution {distname}: ({distdir}) has been "
-                        "built with an unknown api target, ignoring it, "
-                        "you might want to delete it".format(
-                            distname=dist.name,
-                            distdir=dist.dist_dir
+                for entry in {
+                    'archs', 'ndk_api', 'android_api', 'p4a_version'
+                }:
+                    setattr(dist, entry, dist_info.get(entry, None))
+                    if entry not in dist_info:
+                        warning(
+                            "Distribution {distname}: ({distdir}) has been "
+                            "built with an unknown {entry}, ignoring it, "
+                            "you might want to delete it".format(
+                                distname=dist.name,
+                                distdir=dist.dist_dir,
+                                entry=entry,
+                            )
                         )
-                    )
                 dists.append(dist)
         return dists
 
+    @classmethod
+    def get_recipes_with_version(cls, ctx, recipes_names):
+        recipes_with_version = {}
+        for name in recipes_names:
+            try:
+                version = Recipe.get_recipe(name, ctx).version
+            except ValueError:
+                # we don't have a recipe (it's a pure python module, and we
+                # don't know the version, so we set it to None)
+                version = None
+            recipes_with_version[name] = version
+        return recipes_with_version
+
     def save_info(self, dirn):
         '''
         Save information about the distribution in its dist_dir.
         '''
         with current_directory(dirn):
             info('Saving distribution info')
+            # Add version to recipes, so we can detect
+            # any change and rebuild them if necessary
+            recipes_with_version = self.get_recipes_with_version(
+                self.ctx, self.ctx.recipe_build_order + self.ctx.python_modules
+            )
+            py_version = self.ctx.python_recipe.major_minor_version_string
             with open('dist_info.json', 'w') as fileh:
-                json.dump({'dist_name': self.ctx.dist_name,
-                           'bootstrap': self.ctx.bootstrap.name,
-                           'archs': [arch.arch for arch in self.ctx.archs],
-                           'ndk_api': self.ctx.ndk_api,
-                           'use_setup_py': self.ctx.use_setup_py,
-                           'recipes': self.ctx.recipe_build_order + self.ctx.python_modules,
-                           'hostpython': self.ctx.hostpython,
-                           'python_version': self.ctx.python_recipe.major_minor_version_string},
-                          fileh)
+                json.dump(
+                    {
+                        'dist_name': self.ctx.dist_name,
+                        'bootstrap': self.ctx.bootstrap.name,
+                        'archs': [arch.arch for arch in self.ctx.archs],
+                        'ndk_api': self.ctx.ndk_api,
+                        'android_api': self.ctx.android_api,
+                        'use_setup_py': self.ctx.use_setup_py,
+                        'recipes': recipes_with_version,
+                        'hostpython': self.ctx.hostpython,
+                        'python_version': py_version,
+                        'p4a_version': __version__,
+                    },
+                    fileh,
+                    indent=4,
+                    sort_keys=True,
+                )
 
 
 def pretty_log_dists(dists, log_func=info):
     infos = []
     for dist in dists:
         ndk_api = 'unknown' if dist.ndk_api is None else dist.ndk_api
-        infos.append('{Fore.GREEN}{Style.BRIGHT}{name}{Style.RESET_ALL}: min API {ndk_api}, '
-                     'includes recipes ({Fore.GREEN}{recipes}'
-                     '{Style.RESET_ALL}), built for archs ({Fore.BLUE}'
-                     '{archs}{Style.RESET_ALL})'.format(
-                         ndk_api=ndk_api,
-                         name=dist.name, recipes=', '.join(dist.recipes),
-                         archs=', '.join(dist.archs) if dist.archs else 'UNKNOWN',
-                         Fore=Err_Fore, Style=Err_Style))
+        infos.append(
+            '{Fore.GREEN}{Style.BRIGHT}{name}{Style.RESET_ALL}: '
+            'min API {ndk_api}, '
+            'includes recipes ({Fore.GREEN}{recipes}{Style.RESET_ALL}), '
+            'recipes to rebuild({Fore.GREEN}{to_rebuild}{Style.RESET_ALL}), '
+            'built for archs ({Fore.BLUE}{archs}{Style.RESET_ALL})'.format(
+                ndk_api=ndk_api,
+                name=dist.name,
+                recipes=", ".join("{}".format(k) for k in dist.recipes.keys()),
+                to_rebuild=", ".join(dist.recipes_to_rebuild),
+                archs=', '.join(dist.archs) if dist.archs else 'UNKNOWN',
+                Fore=Err_Fore,
+                Style=Err_Style,
+            )
+        )
 
     for line in infos:
         log_func('\t' + line)