Skip to content

Commit 1994471

Browse files
committed
Better package identification
Previously, we either monkey-patched setuptools and harvested the arguments passed to setuptools.setup or we parsed setup.cfg Now we run the setup.py script with distutils.core.run_setup to return properties from the resulting Distribution object.
1 parent 5d9c766 commit 1994471

File tree

4 files changed

+118
-63
lines changed

4 files changed

+118
-63
lines changed

colcon_core/package_identification/python.py

Lines changed: 111 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,23 @@
11
# Copyright 2016-2019 Dirk Thomas
2+
# Copyright 2019 Rover Robotics
23
# Licensed under the Apache License, Version 2.0
34

5+
import multiprocessing
6+
import os
7+
from typing import Optional
8+
import warnings
9+
410
from colcon_core.dependency_descriptor import DependencyDescriptor
511
from colcon_core.package_identification import logger
612
from colcon_core.package_identification \
713
import PackageIdentificationExtensionPoint
814
from colcon_core.plugin_system import satisfies_version
915
from distlib.util import parse_requirement
1016
from distlib.version import NormalizedVersion
11-
try:
12-
from setuptools.config import read_configuration
13-
except ImportError as e:
14-
from pkg_resources import get_distribution
15-
from pkg_resources import parse_version
16-
setuptools_version = get_distribution('setuptools').version
17-
minimum_version = '30.3.0'
18-
if parse_version(setuptools_version) < parse_version(minimum_version):
19-
e.msg += ', ' \
20-
"'setuptools' needs to be at least version {minimum_version}, if" \
21-
' a newer version is not available from the package manager use ' \
22-
"'pip3 install -U setuptools' to update to the latest version" \
23-
.format_map(locals())
24-
raise
2517

2618

2719
class PythonPackageIdentification(PackageIdentificationExtensionPoint):
28-
"""Identify Python packages with `setup.cfg` files."""
20+
"""Identify Python packages with `setup.py` and opt. `setup.cfg` files."""
2921

3022
def __init__(self): # noqa: D107
3123
super().__init__()
@@ -41,69 +33,136 @@ def identify(self, desc): # noqa: D102
4133
if not setup_py.is_file():
4234
return
4335

44-
setup_cfg = desc.path / 'setup.cfg'
45-
if not setup_cfg.is_file():
46-
return
36+
# after this point, we are convinced this is a Python package,
37+
# so we should fail with an Exception instead of silently
38+
39+
config = get_setup_result(setup_py, env=None)
4740

48-
config = get_configuration(setup_cfg)
49-
name = config.get('metadata', {}).get('name')
41+
name = config['metadata'].name
5042
if not name:
51-
return
43+
raise RuntimeError(
44+
"The Python package in '{setup_py.parent}' has an invalid "
45+
'package name'.format_map(locals()))
5246

5347
desc.type = 'python'
5448
if desc.name is not None and desc.name != name:
55-
msg = 'Package name already set to different value'
56-
logger.error(msg)
57-
raise RuntimeError(msg)
49+
raise RuntimeError(
50+
"The Python package in '{setup_py.parent}' has the name "
51+
"'{name}' which is different from the already set package "
52+
"name '{desc.name}'".format_map(locals()))
5853
desc.name = name
5954

60-
version = config.get('metadata', {}).get('version')
61-
desc.metadata['version'] = version
55+
desc.metadata['version'] = config['metadata'].version
6256

63-
options = config.get('options', {})
64-
dependencies = extract_dependencies(options)
65-
for k, v in dependencies.items():
66-
desc.dependencies[k] |= v
57+
for dependency_type, option_name in [
58+
('build', 'setup_requires'),
59+
('run', 'install_requires'),
60+
('test', 'tests_require')
61+
]:
62+
desc.dependencies[dependency_type] = {
63+
create_dependency_descriptor(d)
64+
for d in config[option_name] or ()}
6765

6866
def getter(env):
69-
nonlocal options
70-
return options
67+
nonlocal setup_py
68+
return get_setup_result(setup_py, env=env)
7169

7270
desc.metadata['get_python_setup_options'] = getter
7371

7472

7573
def get_configuration(setup_cfg):
7674
"""
77-
Read the setup.cfg file.
75+
Return the configuration values defined in the setup.cfg file.
76+
77+
The function exists for backward compatibility with older versions of
78+
colcon-ros.
7879
7980
:param setup_cfg: The path of the setup.cfg file
8081
:returns: The configuration data
8182
:rtype: dict
8283
"""
83-
return read_configuration(str(setup_cfg))
84+
warnings.warn(
85+
'colcon_core.package_identification.python.get_configuration() will '
86+
'be removed in the future', DeprecationWarning, stacklevel=2)
87+
config = get_setup_result(setup_cfg.parent / 'setup.py', env=None)
88+
return {
89+
'metadata': {'name': config['metadata'].name},
90+
'options': config
91+
}
8492

8593

86-
def extract_dependencies(options):
94+
def get_setup_result(setup_py, *, env: Optional[dict]):
8795
"""
88-
Get the dependencies of the package.
96+
Spin up a subprocess to run setup.py, with the given environment.
8997
90-
:param options: The dictionary from the options section of the setup.cfg
91-
file
92-
:returns: The dependencies
93-
:rtype: dict(string, set(DependencyDescriptor))
98+
:param setup_py: Path to a setup.py script
99+
:param env: Environment variables to set before running setup.py
100+
:return: Dictionary of data describing the package.
101+
:raise: RuntimeError if the setup script encountered an error
94102
"""
95-
mapping = {
96-
'setup_requires': 'build',
97-
'install_requires': 'run',
98-
'tests_require': 'test',
99-
}
100-
dependencies = {}
101-
for option_name, dependency_type in mapping.items():
102-
dependencies[dependency_type] = set()
103-
for dep in options.get(option_name, []):
104-
dependencies[dependency_type].add(
105-
create_dependency_descriptor(dep))
106-
return dependencies
103+
env_copy = os.environ.copy()
104+
if env is not None:
105+
env_copy.update(env)
106+
107+
conn_recv, conn_send = multiprocessing.Pipe(duplex=False)
108+
with conn_send:
109+
p = multiprocessing.Process(
110+
target=_get_setup_result_target,
111+
args=(os.path.abspath(str(setup_py)), env_copy, conn_send),
112+
)
113+
p.start()
114+
p.join()
115+
with conn_recv:
116+
result_or_exception_string = conn_recv.recv()
117+
118+
if isinstance(result_or_exception_string, dict):
119+
return result_or_exception_string
120+
raise RuntimeError(
121+
'Failure when trying to run setup script {}:\n{}'
122+
.format(setup_py, result_or_exception_string))
123+
124+
125+
def _get_setup_result_target(setup_py: str, env: dict, conn_send):
126+
"""
127+
Run setup.py in a modified environment.
128+
129+
Helper function for get_setup_metadata. The resulting dict or error
130+
will be sent via conn_send instead of returned or thrown.
131+
132+
:param setup_py: Absolute path to a setup.py script
133+
:param env: Environment variables to set before running setup.py
134+
:param conn_send: Connection to send the result as either a dict or an
135+
error string
136+
"""
137+
import distutils.core
138+
import traceback
139+
try:
140+
# need to be in setup.py's parent dir to detect any setup.cfg
141+
os.chdir(os.path.dirname(setup_py))
142+
143+
os.environ.clear()
144+
os.environ.update(env)
145+
146+
result = distutils.core.run_setup(
147+
str(setup_py), ('--dry-run',), stop_after='config')
148+
149+
# could just return all attrs in result.__dict__, but we take this
150+
# opportunity to filter a few things that don't need to be there
151+
conn_send.send({
152+
attr: value for attr, value in result.__dict__.items()
153+
if (
154+
# These *seem* useful but always have the value 0.
155+
# Look for their values in the 'metadata' object instead.
156+
attr not in result.display_option_names
157+
# Getter methods
158+
and not callable(value)
159+
# Private properties
160+
and not attr.startswith('_')
161+
# Objects that are generally not picklable
162+
and attr not in ('cmdclass', 'distclass', 'ext_modules')
163+
)})
164+
except BaseException:
165+
conn_send.send(traceback.format_exc())
107166

108167

109168
def create_dependency_descriptor(requirement_string):

colcon_core/task/python/build.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ async def build(self, *, additional_hooks=None): # noqa: D102
9797
'--build-directory', os.path.join(args.build_base, 'build'),
9898
'--no-deps',
9999
]
100-
if setup_py_data.get('data_files', []):
100+
if setup_py_data.get('data_files'):
101101
cmd += ['install_data', '--install-dir', args.install_base]
102102
self._append_install_layout(args, cmd)
103103
rc = await check_call(
@@ -142,7 +142,7 @@ def _undo_install(self, pkg, args, setup_py_data, python_lib):
142142
with open(install_log, 'r') as h:
143143
lines = [l.rstrip() for l in h.readlines()]
144144

145-
packages = setup_py_data.get('packages', [])
145+
packages = setup_py_data.get('packages') or []
146146
for module_name in packages:
147147
if module_name in sys.modules:
148148
logger.warning(
@@ -185,8 +185,8 @@ def _symlinks_in_build(self, args, setup_py_data):
185185
if os.path.exists(os.path.join(args.path, 'setup.cfg')):
186186
items.append('setup.cfg')
187187
# add all first level packages
188-
package_dir = setup_py_data.get('package_dir', {})
189-
for package in setup_py_data.get('packages', []):
188+
package_dir = setup_py_data.get('package_dir') or {}
189+
for package in setup_py_data.get('packages') or []:
190190
if '.' in package:
191191
continue
192192
if package in package_dir:
@@ -197,7 +197,7 @@ def _symlinks_in_build(self, args, setup_py_data):
197197
items.append(package)
198198
# relative python-ish paths are allowed as entries in py_modules, see:
199199
# https://docs.python.org/3.5/distutils/setupscript.html#listing-individual-modules
200-
py_modules = setup_py_data.get('py_modules')
200+
py_modules = setup_py_data.get('py_modules') or []
201201
if py_modules:
202202
py_modules_list = [
203203
p.replace('.', os.path.sep) + '.py' for p in py_modules]
@@ -208,7 +208,7 @@ def _symlinks_in_build(self, args, setup_py_data):
208208
.format_map(locals()))
209209
items += py_modules_list
210210
data_files = get_data_files_mapping(
211-
setup_py_data.get('data_files', []))
211+
setup_py_data.get('data_files') or [])
212212
for source in data_files.keys():
213213
# work around data files incorrectly defined as not relative
214214
if os.path.isabs(source):

colcon_core/task/python/test/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ def has_test_dependency(setup_py_data, name):
210210
False otherwise
211211
:rtype: bool
212212
"""
213-
tests_require = setup_py_data.get('tests_require', [])
213+
tests_require = setup_py_data.get('tests_require') or ()
214214
for d in tests_require:
215215
# the name might be followed by a version
216216
# separated by any of the following: ' ', <, >, <=, >=, ==, !=

test/test_package_identification_python.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ def unchanged_empty_descriptor(package_descriptor):
2727
assert package_descriptor.type is None
2828

2929

30-
@pytest.mark.xfail
3130
def test_error_in_setup_py(unchanged_empty_descriptor):
3231
setup_py = unchanged_empty_descriptor.path / 'setup.py'
3332
error_text = 'My hovercraft is full of eels'
@@ -51,7 +50,6 @@ def test_missing_setup_py(unchanged_empty_descriptor):
5150
extension.identify(unchanged_empty_descriptor)
5251

5352

54-
@pytest.mark.xfail
5553
def test_empty_setup_py(unchanged_empty_descriptor):
5654
extension = PythonPackageIdentification()
5755
(unchanged_empty_descriptor.path / 'setup.py').write_text('')
@@ -89,7 +87,6 @@ def test_re_identify_python_if_same_python_package(package_descriptor):
8987
assert package_descriptor.type == 'python'
9088

9189

92-
@pytest.mark.xfail
9390
def test_re_identify_python_if_different_python_package(package_descriptor):
9491
package_descriptor.name = 'other-package'
9592
package_descriptor.type = 'python'
@@ -180,7 +177,6 @@ def test_metadata_options(package_descriptor):
180177
assert options['packages'] == ['my_module']
181178

182179

183-
@pytest.mark.xfail
184180
def test_metadata_options_dynamic(package_descriptor):
185181
(package_descriptor.path / 'setup.py').write_text(
186182
'import setuptools; setuptools.setup()')

0 commit comments

Comments
 (0)