add setuptools to setup_requires

[rotu]

Due to issues discussed in #374, colcon-core setup requires setuptools 46.4.0 to install correctly.

[codecov]

Codecov Report

Merging #381 into master will decrease coverage by 0.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   80.01%   79.58%   -0.43%     
==========================================
  Files          56       57       +1     
  Lines        3272     3361      +89     
  Branches      543      559      +16     
==========================================
+ Hits         2618     2675      +57     
- Misses        615      640      +25     
- Partials       39       46       +7     

Impacted Files	Coverage Δ
colcon_core/package_identification/python.py	67.92% <0.00%> (-17.14%)	⬇️
colcon_core/location.py	100.00% <0.00%> (ø)
colcon_core/package_augmentation/python.py	84.37% <0.00%> (ø)
colcon_core/command.py	87.00% <0.00%> (+0.47%)	⬆️
colcon_core/task/python/build.py	37.70% <0.00%> (+3.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update caee5e6...458d9e8. Read the comment docs.

[dirk-thomas]

Wouldn't this fail in the case of packaging into a Debian package since the targeted Ubuntu distros don't have a new enough version?

[rotu]

Wouldn't this fail in the case of packaging into a Debian package since the targeted Ubuntu distros don't have a new enough version?

I don't know. On Debian, how do we resolve Python versioned dependencies anyway? I didn't think there was any way to cleanly map Python package dependencies to system dependencies, much less one that respects the version.

[dirk-thomas]

On Debian, how do we resolve Python versioned dependencies anyway?

A Debian package can only depend on other Debian packages. So the generated python3-colcon-core Debian package must run with the Debian package python3-setuptools available. Therefore the Debian package can't have a minimum version number for setuptools which exceeds the version available from Debian.

Why does the minimum version in install_requires also need to be bumped? I would expect that a newer version is only necessary when invoking the setup.py file of colcon-core but not when the package is later used.

If only setup_requires is added it needs to be checked that this change doesn't affect the generated deb.

[rotu]

By definition, the version in install_requires is the one setup.py needs to run appropriately. setup.py relies on a new version of setuptools in order to determine the package version, so attempting to run setup.py with a lesser version will result in failures.

I think it also makes sense to bump the install_requires version, since all colcon packages rely on newer setuptools, so I did. If you want to hold that back for some reason, I'm okay with reverting that change, provided all downstream colcon versions also update their setup_requires for correctness.

Because the Debian package installation does not use setup.py, yes, I don't think this will affect deb-based installation. But this package is published on PyPI. So on Debian if you pip install colcon-core, or install in an isolated virtual environment, or try to use this on a non-Debian platform, the setup_requires key is a must (at least until this packages switches to pep-518).

[dirk-thomas]

By definition, the version in install_requires is the one setup.py needs to run appropriately.

Isn't your point that this is what setup_requires is for - not install_requires?

I think it also makes sense to bump the install_requires version, since all colcon packages rely on newer setuptools, so I did.

The install_requires defines what is needed to use a package. It shouldn't be misused to declared what downstream packages need to be setup - that is what setup_requires in those downstream packages is for.

Because the Debian package installation does not use setup.py, yes, I don't think this will affect deb-based installation. But this package is published on PyPI. So on Debian if you pip install colcon-core, or install in an isolated virtual environment, or try to use this on a non-Debian platform, the setup_requires key is a must (at least until this packages switches to pep-518).

We are on the same page here. That is why setup_requires should be added. And if it would pose a problem for the Debian package it could be removed with a Debian patch file (like other dependencies already).

[rotu]

Isn't your point that this is what setup_requires is for - not install_requires?

Oops! Yes, I meant “By definition, the version in setup_requires is the one setup.py needs to run appropriately.”

We are on the same page here. That is why setup_requires should be added

And CI should be running with an older version of setuptools as well. Does colcon run properly under an older setuptools when installed with an newer one? Won’t package identification misidentify the version of packages in the colcon workspace?

[dirk-thomas]

Does colcon run properly under an older setuptools when installed with an newer one? Won’t package identification misidentify the version of packages in the colcon workspace?

A package might or might not use the attr syntax and if it does it should probably declare that it needs a new enough setuptools version. I don't think it is colcons responsibility to do that.

[rotu]

Backed out the change to install_requires. I still think it belongs there, but you're the project maintainer so it's your call.

[cottsay]

This field is deprecated in favor of PEP 517.