Only require 'distro' on linux (as it is linux specific)#3087
Only require 'distro' on linux (as it is linux specific)#3087tardyp merged 3 commits intobuildbot:masterfrom
Conversation
master/setup.py
Outdated
| 'autobahn ' + autobahn_ver, | ||
| 'PyJWT', | ||
| 'distro' | ||
| 'distro;platform_system==="Linux"' |
There was a problem hiding this comment.
I wasn't aware of this construct. Is this supported by all pip/setuptools versions?
Does that work with wheels?
There was a problem hiding this comment.
This was defined in https://www.python.org/dev/peps/pep-0508/. Based on this post https://discourse.numenta.org/t/setup-py-error-invalid-environment-marker/1298/3 it's supported by:
- setuptools: 20.2.2 +
- wheel: 0.24.0 +
- pip: 8.1.2 +
There was a problem hiding this comment.
That is a much newer version of setuptools than currently required.
There was a problem hiding this comment.
If this is critical, the alternative would be something like
import platform
...
if platform.system() == 'Linux':
setup_args['install_requires'].append('distro')
If you prefer this, I can update the pull request.
There was a problem hiding this comment.
updated to use a more conservative approach to accomplish the same
There was a problem hiding this comment.
Problem with that approach is that it does not work with wheels. As I generate the universal wheels on my mac, it wont include distro anymore in the wheels deps.
I would be more in favor of the combination, which would test setup tools version :-/
There was a problem hiding this comment.
Can you give me an example of what your suggested approach would look like?
There was a problem hiding this comment.
import platform
from distutils.version import LooseVersion
import setuptools
...
if LooseVersion(setuptools.version.__version__) >= LooseVersion("20.2.2"):
setup_args['install_requires'].append('distro;platform_system==="Linux')
elif platform.system() == 'Linux':
setup_args['install_requires'].append('distro')
Codecov Report
@@ Coverage Diff @@
## master #3087 +/- ##
=======================================
Coverage 88.29% 88.29%
=======================================
Files 314 314
Lines 32927 32927
=======================================
Hits 29072 29072
Misses 3855 3855
Continue to review full report at Codecov.
|
|
@grembo Hope you don't mind, in order to move faster I did edit your PR last commit to include the proposed change. Let me know if this works for you (and your BSD port) |
|
@tardyp Sorry for the late response, this is ok with me. I'll change/remove the patch to the port on the next release, so it's all good. |
4 participants
This is a trivial/obvious fix.