From 3f670a8fa284458127140d0cd3abfac5c2c0ca78 Mon Sep 17 00:00:00 2001 From: Dan Rose Date: Fri, 27 Sep 2019 23:35:06 -0500 Subject: [PATCH] switch to ProcessPool It turns out most of what I was doing with multiprocessing.Process was reinventing Pool.apply. This also naturally throttles the number of simultaneous processes, which was a possible concern. Move setup.py-specific stuff to run_setup_py. I expect to reuse this for python build, plus isolating it makes the subprocess workload minimal. --- colcon_core/package_identification/python.py | 77 +++++--------------- colcon_core/run_setup_py.py | 46 ++++++++++++ 2 files changed, 64 insertions(+), 59 deletions(-) create mode 100644 colcon_core/run_setup_py.py diff --git a/colcon_core/package_identification/python.py b/colcon_core/package_identification/python.py index e9adcc16..31b65d48 100644 --- a/colcon_core/package_identification/python.py +++ b/colcon_core/package_identification/python.py @@ -1,9 +1,10 @@ # Copyright 2016-2019 Dirk Thomas -# Copyright 2019 Rover Robotics +# Copyright 2019 Rover Robotics via Dan Rose # Licensed under the Apache License, Version 2.0 import multiprocessing import os +from traceback import format_exc from typing import Optional import warnings @@ -12,9 +13,12 @@ from colcon_core.package_identification \ import PackageIdentificationExtensionPoint from colcon_core.plugin_system import satisfies_version +from colcon_core.run_setup_py import run_setup_py from distlib.util import parse_requirement from distlib.version import NormalizedVersion +_process_pool = multiprocessing.Pool() + class PythonPackageIdentification(PackageIdentificationExtensionPoint): """Identify Python packages with `setup.py` and opt. `setup.cfg` files.""" @@ -104,65 +108,20 @@ def get_setup_result(setup_py, *, env: Optional[dict]): if env is not None: env_copy.update(env) - conn_recv, conn_send = multiprocessing.Pipe(duplex=False) - with conn_send: - p = multiprocessing.Process( - target=_get_setup_result_target, - args=(os.path.abspath(str(setup_py)), env_copy, conn_send), - ) - p.start() - p.join() - with conn_recv: - result_or_exception_string = conn_recv.recv() - - if isinstance(result_or_exception_string, dict): - return result_or_exception_string - raise RuntimeError( - 'Failure when trying to run setup script {}:\n{}' - .format(setup_py, result_or_exception_string)) - - -def _get_setup_result_target(setup_py: str, env: dict, conn_send): - """ - Run setup.py in a modified environment. - - Helper function for get_setup_metadata. The resulting dict or error - will be sent via conn_send instead of returned or thrown. - - :param setup_py: Absolute path to a setup.py script - :param env: Environment variables to set before running setup.py - :param conn_send: Connection to send the result as either a dict or an - error string - """ - import distutils.core - import traceback try: - # need to be in setup.py's parent dir to detect any setup.cfg - os.chdir(os.path.dirname(setup_py)) - - os.environ.clear() - os.environ.update(env) - - result = distutils.core.run_setup( - str(setup_py), ('--dry-run',), stop_after='config') - - # could just return all attrs in result.__dict__, but we take this - # opportunity to filter a few things that don't need to be there - conn_send.send({ - attr: value for attr, value in result.__dict__.items() - if ( - # These *seem* useful but always have the value 0. - # Look for their values in the 'metadata' object instead. - attr not in result.display_option_names - # Getter methods - and not callable(value) - # Private properties - and not attr.startswith('_') - # Objects that are generally not picklable - and attr not in ('cmdclass', 'distclass', 'ext_modules') - )}) - except BaseException: - conn_send.send(traceback.format_exc()) + return _process_pool.apply( + run_setup_py, + kwds={ + 'cwd': os.path.abspath(str(setup_py.parent)), + 'env': env_copy, + 'script_args': ('--dry-run',), + 'stop_after': 'config' + } + ) + except Exception as e: + raise RuntimeError( + 'Failure when trying to run setup script {}: {}' + .format(setup_py, format_exc())) from e def create_dependency_descriptor(requirement_string): diff --git a/colcon_core/run_setup_py.py b/colcon_core/run_setup_py.py new file mode 100644 index 00000000..fa4c88ae --- /dev/null +++ b/colcon_core/run_setup_py.py @@ -0,0 +1,46 @@ +# Copyright 2019 Rover Robotics via Dan Rose +# Licensed under the Apache License, Version 2.0 + +import distutils.core +import os + + +def run_setup_py(cwd: str, env: dict, script_args=(), stop_after='run'): + """ + Modify the current process and run setup.py. + + This should be run in a subprocess so as not to dirty the state of the + current process. + + :param cwd: Absolute path to a directory containing a setup.py script + :param env: Environment variables to set before running setup.py + :param script_args: command-line arguments to pass to setup.py + :param stop_after: which + :returns: The properties of a Distribution object, minus some useless + and/or unpicklable properties + """ + # need to be in setup.py's parent dir to detect any setup.cfg + os.chdir(cwd) + + os.environ.clear() + os.environ.update(env) + + result = distutils.core.run_setup( + 'setup.py', script_args=script_args, stop_after=stop_after) + + # could just return all attrs in result.__dict__, but we take this + # opportunity to filter a few things that don't need to be there + return { + attr: value for attr, value in result.__dict__.items() + if ( + # These *seem* useful but always have the value 0. + # Look for their values in the 'metadata' object instead. + attr not in result.display_option_names + # Getter methods + and not callable(value) + # Private properties + and not attr.startswith('_') + # Objects that are generally not picklable + and attr not in ('cmdclass', 'distclass', 'ext_modules') + ) + }