colcon / colcon-python-setup-py

Extension for colcon to support Python packages with the metadata in the setup.py file
http://colcon.readthedocs.io
Apache License 2.0
2 stars 7 forks source link

use distutils.core.run_setup to extract more complete package information #22

Closed dirk-thomas closed 4 years ago

dirk-thomas commented 4 years ago

This implements the patch proposed in colcon/colcon-core#215 in this package instead.

colcon-ros needs to be updated to use the new API instead of the now deprecated API.

@rotu I used your name as the author of the commit since this is basically just a port of your proposed patch. Let me know if you want me to change it.

rotu commented 4 years ago

Thank you. I approve :-)

dirk-thomas commented 4 years ago

This is still failing for me:

AttributeError: Can't get attribute 'run_setup_py' on <module 'colcon_python_setup_py.package_identification.python_setup_py' from '.../colcon_python_setup_py/package_identification/python_setup_py.py'>
rotu commented 4 years ago

I think either def run_setup_py needs to be in another file or the ProcessPool should be created after the module is fully imported (instead of during). I prefer the latter, in a file with minimal imports.

rotu commented 4 years ago

I can give this some attention tomorrow

dirk-thomas commented 4 years ago

With this patch I am unable to cleanly abort a running build using Ctrl-C. I would suspect the usage of multiprocessing to be the cause for it. That is probably why the current extension uses just a subprocess and prints the repr() of the result and then evaluates the string to retrieve the data.

rotu commented 4 years ago

Shoot. I suppose having a process pool with module lifespan is fraught with problems... adjusting the code

dirk-thomas commented 4 years ago

Just for the record: #24 targets this branch.

dirk-thomas commented 4 years ago

@rotu Please see an alternative PR #30 which uses distutils.core.run_setup() but doesn't switch the logic to use multiprocessing.

dirk-thomas commented 4 years ago

@kaptenkece I don't know what you want say with your comment. Please clarify.