koodaamo / tnefparse

a TNEF decoding library written in python, without external dependencies
GNU Lesser General Public License v3.0
49 stars 37 forks source link

GitHub Action build for Python 2.7 broken #59

Closed jugmac00 closed 4 years ago

jugmac00 commented 4 years ago

https://github.com/koodaamo/tnefparse/runs/657046886

Looks like mock has to be pinned to <= 3.0.5 as newer versions are Py3 only.

jugmac00 commented 4 years ago

@petri It took me a while, but I found the reason.

The short version: setup_requires is the problem. It is usually used to download packages, which are necessary already at install time, ie if you use e.g. yaml in your setup.py and need to parse it or so...

setup_requires has a fallback to use easy_install to download packages. easy_install is flawed and does not take care of the Python versions and downloads mock in the latest, Python 3 only version - as a side effect of installing pytest-console-scripts - not from the pip install mock comand ( you can easily try this out in a Python 2 virtualenv).

Turns out, setup_requires is not necessary at all.

When debugging this problem, I also noticed the usage of python setup.py test which uses the tests_require keyword from setup.py - turns out this is deprecated. https://setuptools.readthedocs.io/en/latest/setuptools.html But anyway - pytest is installed and could be used there, too.

We could change that to extras_require={'test': ['compressed_rtf',],}, - but actually, I do not think this is semantically correct - as compressed_rtf is used by the tnefparse app, not in the tests. So I understand an installed compressed_rtf offering an additional feature of tnefparse.

So it could be something like extras_require={'full': ['compressed_rtf',],}, or similar. Which could be also used for tests then.

Also, the users of the package then could install the "full" version of tnefparse via pip install tnefparse[full]. Which could need some documentation.

If you are ok with all these changes, I'd create a new pull request for the Python 2.7 issue, and new issues and PRs for the test/pytest/extras things.

P.S.: During investigation, I found some other issues - but I`ll create separate issues for them (ie. python-runner is deprecated, gh-action is not triggered for pull requests...).