From b82289857b614093abd3e1cc256d41f9c57fefbc Mon Sep 17 00:00:00 2001 From: Patrick Dallaire Date: Thu, 23 Dec 2021 23:08:01 -0500 Subject: [PATCH] Add automatic expansion of --requirement list Related to issue kivy/python-for-android#2529 --- pythonforandroid/toolchain.py | 320 +++++++++++++++++++++++++++------- setup.py | 2 +- tests/test_toolchain.py | 139 +++++++++++++++ 3 files changed, 400 insertions(+), 61 deletions(-) diff --git a/pythonforandroid/toolchain.py b/pythonforandroid/toolchain.py index 2636eaca5b..72b41a3970 100644 --- a/pythonforandroid/toolchain.py +++ b/pythonforandroid/toolchain.py @@ -5,7 +5,6 @@ This module defines the entry point for command line and programmatic use. """ - from os import environ from pythonforandroid import __version__ from pythonforandroid.pythonpackage import get_dep_names_of_package @@ -227,6 +226,251 @@ def split_argument_list(arg_list): return re.split(r'[ ,]+', arg_list) +def __expand_requirements_arg_from_project_files(ctx, args): + """Parse additional requirements from setup.py or pyproject.toml file + and add to --requirements arg if --use_setup_py argument was specified""" + has_setup_py_or_toml = False + if getattr(args, "private", None) is not None: + project_dir = getattr(args, "private") + has_setup_py = os.path.exists( + os.path.join(project_dir, "setup.py")) + has_toml = os.path.exists( + os.path.join(project_dir, "pyproject.toml")) + has_setup_py_or_toml = has_setup_py or has_toml + + # Add dependencies from setup.py, but only if they are recipes + # (because otherwise, setup.py itself will install them later) + if has_setup_py_or_toml and getattr(args, "use_setup_py", False): + try: + info("Analyzing package dependencies. MAY TAKE A WHILE.") + # Get all the dependencies corresponding to a recipe: + dependencies = [ + dep.lower() for dep in + get_dep_names_of_package( + args.private, + keep_version_pins=True, + recursive=True, + verbose=True, + ) + ] + info("Dependencies obtained: " + str(dependencies)) + dependencies = [ + dependencie for dependencie in dependencies + if has_a_recipe(ctx, dependencie) + ] + + # Add dependencies to argument list: + if len(dependencies) > 0: + if len(args.requirements) > 0: + args.requirements += u"," + args.requirements += u",".join(dependencies) + + except ValueError: + # Not a python package, apparently. + warning( + "Processing failed, is this project a valid " + "package? Will continue WITHOUT setup.py deps." + ) + + +def has_a_recipe(ctx, requirement): + all_recipes = [ + recipe.lower() for recipe in + set(Recipe.list_recipes(ctx)) + ] + requirement_name = requirement.split('==')[0] + return requirement_name in all_recipes + + +def __run_pip_compile(requirements): + return shprint( + sh.bash, '-c', + "echo -e '{}' > requirements.in && " + "{} -m piptools compile --dry-run --annotation-style=line && " + "rm requirements.in".format( + '\n'.join(requirements), sys.executable)) + + +def __parse_pip_compile_output(output): + parsed_requirement_info_list = [] + for line in output.splitlines(): + match_data = re.match( + r'^([\w.-]+)(==([^\s]+)|\s+@\s+([^\s]+)).*' + r'#\s+via\s+([\w\s,.-]+)', line) + + if match_data: + parent_requirements = match_data.group(5).split(', ') + requirement_name = match_data.group(1) + requirement_version = match_data.group(3) + requirement_url = match_data.group(4) + + # Requirement is a "non-recipe" one we started with. + if '-r requirements.in' in parent_requirements: + parent_requirements.remove('-r requirements.in') + + if requirement_url: + # For wtv reason, pip-compile output truncates the slashes. + requirement_url = requirement_url.replace('file:/', 'file:///') + + parsed_requirement_info_list.append([ + requirement_name, + requirement_version, + requirement_url, + parent_requirements]) + + return parsed_requirement_info_list + + +def __run_pip_compile_and_parse_output(requirements): + return __parse_pip_compile_output(__run_pip_compile(requirements)) + + +def __is_requirement_indirectly_installed_by_recipe( + ctx, current_requirement_info, remaining_requirement_names): + + requirement_name, requirement_version, \ + requirement_url, parent_requirements = current_requirement_info + + # If any parent requirement has a recipe, this + # requirement ought also to be installed by it. + # Hence, it's better not to add this requirement the + # expanded list. + parent_requirements_with_recipe = [ + parent_requirement + for parent_requirement in parent_requirements + if has_a_recipe(ctx, parent_requirement) + ] + + # Any parent requirement removed for the expanded list + # implies that it and its own requirements (including + # this requirement) will be installed by a recipe. + # Hence, it's better not to add this requirement the + # expanded list. + parent_requirements_not_in_list = [ + parent_requirement + for parent_requirement in parent_requirements + if parent_requirement not in remaining_requirement_names + ] + + is_indrectly_installed_by_a_recipe = \ + len(parent_requirements) and \ + (parent_requirements_with_recipe or parent_requirements_not_in_list) + + if is_indrectly_installed_by_a_recipe and parent_requirements_with_recipe: + info('Concluding that {} is installed by {} recipe(s).'.format(requirement_name, parent_requirements_with_recipe)) + + elif is_indrectly_installed_by_a_recipe and parent_requirements_not_in_list: + info('Previously concluded that {} is/are installed by recipe(s). Consequently, so will {}.'.format( + parent_requirements_not_in_list, requirement_name)) + + return is_indrectly_installed_by_a_recipe + + +def __prune_requirements_installed_by_recipe(ctx, requirement_info_list): + original_requirement_count = -1 + + while len(requirement_info_list) != original_requirement_count: + original_requirement_count = len(requirement_info_list) + + for i, requirement_info in enumerate(reversed(requirement_info_list)): + index = original_requirement_count - i - 1 + + remaining_requirement_names = \ + [x[0] for x in requirement_info_list] + + if __is_requirement_indirectly_installed_by_recipe( + ctx, requirement_info, remaining_requirement_names): + info('\tRemoving {} from requirement list expansion.'.format( + requirement_info[0])) + + del requirement_info_list[index] + + +def __add_compiled_requirements_to_args( + ctx, args, compiled_requirment_info_list): + + for requirement_info in compiled_requirment_info_list: + requirement_name, requirement_version, \ + requirement_url, parent_requirements = requirement_info + + # If the requirement has a recipe, don't use specific + # version constraints determined by pip-compile. Some + # recipes may not support the specified version. Therefor, + # it's probably safer to just let them use their default + # version. User can still force the usage of specific + # version by explicitly declaring it with --requirements. + requirement_str = \ + requirement_name if has_a_recipe(ctx, requirement_name) else \ + '{}=={}'.format(requirement_name, requirement_version) + + requirement_names_arg = split_argument_list(re.sub( + r'==[^\s,]+', '', args.requirements)) + + # This expansion was carried out based on "non-recipe" + # requirements. Hence, the counter-part, requirements + # with a recipe, may already be part of list. + if not (requirement_url or requirement_name in requirement_names_arg): + args.requirements += ',' + requirement_str + + elif requirement_url and requirement_url not in requirement_names_arg: + args.requirements += ',' + requirement_url + + +def __expand_requirements_arg_from_pip_compile(ctx, args): + """Use pip-compile to generate requirement dependencies and add to + --requirements command line argument.""" + + non_recipe_requirements = [ + requirement for requirement in split_argument_list(args.requirements) + if not has_a_recipe(ctx, requirement) + ] + non_recipe_requirements_regex = \ + r',?\s+' + r'|,?\s+'.join(non_recipe_requirements) + args.requirements = \ + re.sub(non_recipe_requirements_regex, '', args.requirements) + + # Compile "non-recipe" requirements' dependencies and add to + # args.requirement. Otherwise, only recipe requirements' + # dependencies would get installed. + # More info https://github.com/kivy/python-for-android/issues/2529 + if non_recipe_requirements: + info("Compiling dependencies for: " + "{}".format(non_recipe_requirements)) + + parsed_requirement_info_list = \ + __run_pip_compile_and_parse_output(non_recipe_requirements) + + info("Requirements obtained from pip-compile: " + "{}".format(["{}{}".format(x[0], '==' + x[1] if x[1] else '[' + x[2] + ']') + for x in parsed_requirement_info_list])) + + __prune_requirements_installed_by_recipe( + ctx, parsed_requirement_info_list) + + info("Requirements remaining after recipe dependency \"prunage\": " + "{}".format(["{}{}".format(x[0], '==' + x[1] if x[1] else '[' + x[2] + ']') + for x in parsed_requirement_info_list])) + + __add_compiled_requirements_to_args( + ctx, args, parsed_requirement_info_list) + + +def expand_requirements_args(ctx, args): + """Expand --requirements arg value to include what may have not + been specified by the user, such as: + * requirements specified in local project setup.py or pyproject.toml + (if --use_setup_py was used) + * indirect requirements (i.e., the requirements of our requirements). + (e.g., if user specifies beautifulsoup4, the appropriate version of + soupsieve is added). + """ + __expand_requirements_arg_from_project_files(ctx, args) + __expand_requirements_arg_from_pip_compile(ctx, args) + + info('Expanded Requirements List: ' + '{}'.format(split_argument_list(args.requirements))) + + class NoAbbrevParser(argparse.ArgumentParser): """We want to disable argument abbreviation so as not to interfere with passing through arguments to build.py, but in python2 argparse @@ -645,65 +889,6 @@ def add_parser(subparsers, *args, **kwargs): self.ctx.with_debug_symbols = getattr( args, "with_debug_symbols", False ) - - have_setup_py_or_similar = False - if getattr(args, "private", None) is not None: - project_dir = getattr(args, "private") - if (os.path.exists(os.path.join(project_dir, "setup.py")) or - os.path.exists(os.path.join(project_dir, - "pyproject.toml"))): - have_setup_py_or_similar = True - - # Process requirements and put version in environ - if hasattr(args, 'requirements'): - requirements = [] - - # Add dependencies from setup.py, but only if they are recipes - # (because otherwise, setup.py itself will install them later) - if (have_setup_py_or_similar and - getattr(args, "use_setup_py", False)): - try: - info("Analyzing package dependencies. MAY TAKE A WHILE.") - # Get all the dependencies corresponding to a recipe: - dependencies = [ - dep.lower() for dep in - get_dep_names_of_package( - args.private, - keep_version_pins=True, - recursive=True, - verbose=True, - ) - ] - info("Dependencies obtained: " + str(dependencies)) - all_recipes = [ - recipe.lower() for recipe in - set(Recipe.list_recipes(self.ctx)) - ] - dependencies = set(dependencies).intersection( - set(all_recipes) - ) - # Add dependencies to argument list: - if len(dependencies) > 0: - if len(args.requirements) > 0: - args.requirements += u"," - args.requirements += u",".join(dependencies) - except ValueError: - # Not a python package, apparently. - warning( - "Processing failed, is this project a valid " - "package? Will continue WITHOUT setup.py deps." - ) - - # Parse --requirements argument list: - for requirement in split_argument_list(args.requirements): - if "==" in requirement: - requirement, version = requirement.split(u"==", 1) - os.environ["VERSION_{}".format(requirement)] = version - info('Recipe {}: version "{}" requested'.format( - requirement, version)) - requirements.append(requirement) - args.requirements = u",".join(requirements) - self.warn_on_deprecated_args(args) self.storage_dir = args.storage_dir @@ -723,6 +908,21 @@ def add_parser(subparsers, *args, **kwargs): self.ctx.activity_class_name = args.activity_class_name self.ctx.service_class_name = args.service_class_name + # Process requirements and put version in environ: + if getattr(args, 'requirements', []): + expand_requirements_args(self.ctx, args) + + # Handle specific version requirement constraints (e.g. foo==x.y) + requirements = [] + for requirement in split_argument_list(args.requirements): + if "==" in requirement: + requirement, version = requirement.split(u"==", 1) + os.environ["VERSION_{}".format(requirement)] = version + info('Recipe {}: version "{}" requested'.format( + requirement, version)) + requirements.append(requirement) + args.requirements = u",".join(requirements) + # Each subparser corresponds to a method command = args.subparser_name.replace('-', '_') getattr(self, command)(args) diff --git a/setup.py b/setup.py index 18bd3e4e45..ec03b1ea47 100644 --- a/setup.py +++ b/setup.py @@ -22,7 +22,7 @@ install_reqs = [ 'appdirs', 'colorama>=0.3.3', 'jinja2', 'six', 'enum34; python_version<"3.4"', 'sh>=1.10; sys_platform!="nt"', - 'pep517<0.7.0', 'toml', + 'pep517<0.7.0', 'toml', 'pip-tools' ] # (pep517 and toml are used by pythonpackage.py) diff --git a/tests/test_toolchain.py b/tests/test_toolchain.py index c6747885ca..a20f15209e 100644 --- a/tests/test_toolchain.py +++ b/tests/test_toolchain.py @@ -1,4 +1,5 @@ import io +import os import sys from os.path import join import pytest @@ -131,6 +132,144 @@ def test_create_no_sdk_dir(self): assert ex_info.value.message == ( 'Android SDK dir was not specified, exiting.') + def test_create_with_complex_requirements(self): + requirements = [ + 'python3==3.8.10', # direct requirement with recipe using version constraint + 'pandas', # direct requirement with recipe (no version constraint) + 'mfpymake==1.2.2', # direct requirement without recipe using version constraint + 'telenium', # direct requirement without recipe (no version constraint) + 'numpy==1.21.4', # indirect requirement with recipe using version constraint + 'mako==1.1.5', # indirect requirement without recipe using version constraint + # There's no reason to specify an indirect requirement unless we want to install a specific version. + ] + argv = [ + 'toolchain.py', + 'create', + '--sdk-dir=/tmp/android-sdk', + '--ndk-dir=/tmp/android-ndk', + '--bootstrap=service_only', + '--requirements={}'.format(','.join(requirements)), + '--dist-name=test_toolchain', + '--activity-class-name=abc.myapp.android.CustomPythonActivity', + '--service-class-name=xyz.myapp.android.CustomPythonService', + '--arch=arm64-v8a', + '--arch=armeabi-v7a' + ] + with patch_sys_argv(argv), mock.patch( + 'pythonforandroid.build.get_available_apis' + ) as m_get_available_apis, mock.patch( + 'pythonforandroid.build.get_toolchain_versions' + ) as m_get_toolchain_versions, mock.patch( + 'pythonforandroid.build.get_ndk_sysroot' + ) as m_get_ndk_sysroot, mock.patch( + 'pythonforandroid.toolchain.build_recipes' + ) as m_build_recipes, mock.patch( + 'pythonforandroid.toolchain.__run_pip_compile' + ) as m_run_pip_compile, mock.patch( + 'pythonforandroid.bootstraps.service_only.' + 'ServiceOnlyBootstrap.assemble_distribution' + ) as m_run_distribute: + m_get_available_apis.return_value = [27] + m_get_toolchain_versions.return_value = (['4.9'], True) + + # pip-compile can give different outputs depending many factors + # Here we use a sample output generated using python3.8 + m_run_pip_compile.return_value = \ + "#\n" \ + "# This file is autogenerated by pip-compile with python 3.9\n" \ + "# To update, run:\n" \ + "#\n" \ + "# pip-compile --annotation-style=line\n" \ + "#\n" \ + "certifi==2021.10.8 # via requests\n" \ + "charset-normalizer==2.0.9 # via requests\n" \ + "cheroot==8.5.2 # via cherrypy\n" \ + "cherrypy==18.6.1 # via telenium\n" \ + "idna==3.3 # via requests\n" \ + "importlib-resources==5.4.0 # via jaraco.text\n" \ + "jaraco.classes==3.2.1 # via jaraco.collections\n" \ + "jaraco.collections==3.4.0 # via cherrypy\n" \ + "jaraco.functools==3.5.0 # via cheroot, jaraco.text, tempora\n" \ + "jaraco.text==3.6.0 # via jaraco.collections\n" \ + "json-rpc==1.13.0 # via telenium\n" \ + "mako==1.1.5 # via -r requirements.in, telenium\n" \ + "markupsafe==2.0.1 # via mako\n" \ + "mfpymake==1.2.2 # via -r requirements.in\n" \ + "more-itertools==8.12.0 # via cheroot, cherrypy, jaraco.classes, jaraco.functools\n" \ + "networkx==2.6.3 # via mfpymake\n" \ + "numpy==1.21.5 # via mfpymake\n" \ + "portend==3.1.0 # via cherrypy\n" \ + "pytz==2021.3 # via tempora\n" \ + "requests==2.26.0 # via mfpymake\n" \ + "six==1.16.0 # via cheroot\n" \ + "telenium==0.5.0 # via -r requirements.in\n" \ + "tempora==4.1.2 # via portend\n" \ + "urllib3==1.26.7 # via requests\n" \ + "werkzeug==2.0.2 # via telenium\n" \ + "ws4py==0.5.1 # via telenium\n" \ + "zc.lockfile==2.0 # via cherrypy\n" \ + "zipp==3.6.0 # via importlib-resources\n" \ + "\n" \ + "# The following packages are considered to be unsafe in a requirements file:\n" \ + "# setuptools\n" \ + "Dry-run, so nothing updated.\n" + m_get_ndk_sysroot.return_value = ( + join(get_ndk_standalone("/tmp/android-ndk"), "sysroot"), + True, + ) + tchain = ToolchainCL() + assert tchain.ctx.activity_class_name == 'abc.myapp.android.CustomPythonActivity' + assert tchain.ctx.service_class_name == 'xyz.myapp.android.CustomPythonService' + assert m_get_available_apis.call_args_list in [ + [mock.call('/tmp/android-sdk')], # linux case + [mock.call('/private/tmp/android-sdk')] # macos case + ] + for callargs in m_get_toolchain_versions.call_args_list: + assert callargs in [ + mock.call("/tmp/android-ndk", mock.ANY), # linux case + mock.call("/private/tmp/android-ndk", mock.ANY), # macos case + ] + build_order = [ + 'android', 'cython', 'genericndkbuild', 'hostpython3', 'libbz2', + 'libffi', 'liblzma', 'numpy', 'openssl', 'pandas', 'pyjnius', + 'python3', 'pytz', 'setuptools', 'six', 'sqlite3' + ] + python_modules = [ + 'certifi', 'charset-normalizer', 'cheroot', 'cherrypy', + 'idna', 'importlib-resources', 'jaraco.classes', + 'jaraco.collections', 'jaraco.functools', 'jaraco.text', + 'json-rpc', 'mako', 'markupsafe', 'mfpymake', 'more-itertools', + 'networkx', 'portend', 'python-dateutil', 'requests', 'telenium', + 'tempora', 'urllib3', 'werkzeug', 'ws4py', 'zc.lockfile', 'zipp' + ] + context = mock.ANY + project_dir = None + + # For wtv reason, the build_order and python_modules list elements + # aren't always generated in the same order. + m_build_recipes_call_args = mock.call( + sorted(m_build_recipes.call_args_list[0][0][0]), + sorted(m_build_recipes.call_args_list[0][0][1]), + m_build_recipes.call_args_list[0][0][2], + m_build_recipes.call_args_list[0][0][3], + ignore_project_setup_py=m_build_recipes.call_args_list[0][1]['ignore_project_setup_py'] + ) + assert m_build_recipes_call_args == mock.call( + sorted(build_order), + sorted(python_modules), + context, + project_dir, + ignore_project_setup_py=False + ) + assert m_run_pip_compile.call_args_list == [mock.call( + ['mfpymake==1.2.2', 'telenium', 'mako==1.1.5'] + )] + assert m_run_distribute.call_args_list == [mock.call()] + assert 'VERSION_python3' in os.environ and os.environ['VERSION_python3'] == '3.8.10' + assert 'VERSION_mfpymake' in os.environ and os.environ['VERSION_mfpymake'] == '1.2.2' + assert 'VERSION_numpy' in os.environ and os.environ['VERSION_numpy'] == '1.21.4' + assert 'VERSION_mako' in os.environ and os.environ['VERSION_mako'] == '1.1.5' + @pytest.mark.skipif(sys.version_info < (3, 0), reason="requires python3") def test_recipes(self): """