GrahamDumpleton / wrapt

A Python module for decorators, wrappers and monkey patching.
BSD 2-Clause "Simplified" License
2.04k stars 230 forks source link

Migrate CI to GitHub Actions and port project to setuptools #179

Closed althonos closed 3 years ago

althonos commented 3 years ago

Hi Graham, saw your tweet and figured I would give a hand. I don't think I'm the best fit to maintain the whole thing (mostly because I'm not a wrapt user), but I can at least help with GitHub Actions and setuptools. Here's what this PR does:

Switch to setuptools

I moved the project metadata to setup.cfg (the setuptools configuration file), and made a minimal setup.py that does the following:

Another advantage of setup.cfg is that it can centralize the configuration of several other tools, so I put the configuration for coverage (originally in .coveragerc) and for tox (originally in tox.cfg) in there as well.

Migrate to GitHub Actions

I added two workflows:

GrahamDumpleton commented 3 years ago

This looks awesome and still trying to understand everything that is going on.

Based on test results in your fork, seems test harnesses need to be updated still to cope with changes to classmethod binding in Python 3.9, an issue I reported back in 2013, but which was only fixed in 3.9 even though I supplied a patch at the time when first reported.

Anyway, checking to see against your test results what else awaits before merge it. I suspect there is another issue hiding in there which is breaking synchronised decorator in Python 3.9, yet no one has reported an issue yet. :-(

naveen521kk commented 3 years ago

Using https://cibuildwheel.readthedocs.io/en/stable/ for building wheels seems to be a nice idea rather than doing things manully.

althonos commented 3 years ago

@naveen521kk : True, I'll have a look! I used my own repo as a template for the workflows, and couldn't get cibuildhweels to work properly then because my extension was a bit on the heavy side (building and linking numerical-heavy code), but here it may work, i'll toy with it!

GrahamDumpleton commented 3 years ago

Sorry for delay on further comment, been a hectic week.

Would there be an issue with restoring the tox.ini file so someone can still do local testing on their own computer?

althonos commented 3 years ago

No problem, I'm caught up in a lot of things as well, I know the feeling.

With the current setup, you can still run tox in the project folder to do testing locally!

GrahamDumpleton commented 3 years ago

But it would only do it for my current Python version, and I would need to set environment variables to test different variants wouldn't I. In other words, have lost the matrix for different Python versions with different environment variable settings as tox.ini file defined it. Just want to make sure am not missing anything as the equivalent now only seems to exist under a GitHub action. If having the tox.init file present conflicts with other things then fine, but just so I know that if test manually I need to know to do each variant myself.

althonos commented 3 years ago

The tox-gh-actions section only controls what happens in the GitHub Actions workflow, it does the opposite of what you described (it prevents tox from running the entire matrix). If you run tox you still get to test with all locally available Python versions (on my system, Python 2.7, Python 3.9, and PyPy):

$ cd ~/Code/wrapt
$ tox
...
_____________________________ summary_____________________________
  py27-without-extensions: commands succeeded
  py27-install-extensions: commands succeeded
  py27-disable-extensions: commands succeeded
ERROR:  py34-without-extensions: InterpreterNotFound: python3.4
ERROR:  py34-install-extensions: InterpreterNotFound: python3.4
ERROR:  py34-disable-extensions: InterpreterNotFound: python3.4
ERROR:  py35-without-extensions: InterpreterNotFound: python3.5
ERROR:  py35-install-extensions: InterpreterNotFound: python3.5
ERROR:  py35-disable-extensions: InterpreterNotFound: python3.5
ERROR:  py36-without-extensions: InterpreterNotFound: python3.6
ERROR:  py36-install-extensions: InterpreterNotFound: python3.6
ERROR:  py36-disable-extensions: InterpreterNotFound: python3.6
ERROR:  py37-without-extensions: InterpreterNotFound: python3.7
ERROR:  py37-install-extensions: InterpreterNotFound: python3.7
ERROR:  py37-disable-extensions: InterpreterNotFound: python3.7
ERROR:  py38-without-extensions: InterpreterNotFound: python3.8
ERROR:  py38-install-extensions: InterpreterNotFound: python3.8
ERROR:  py38-disable-extensions: InterpreterNotFound: python3.8
ERROR:   py39-without-extensions: commands failed
ERROR:   py39-install-extensions: commands failed
ERROR:   py39-disable-extensions: commands failed
  pypy-without-extensions: commands succeeded
GrahamDumpleton commented 3 years ago

I am obviously missing something because I can't see how that local run can work with all the variations when there is no tox.ini file to say what the variations are.

Other than that point, everything else looks good at this point and so will move on to merging it, or working out how to check out your branch, and actually playing with it. Thanks for your continued help.

althonos commented 3 years ago

I am obviously missing something because I can't see how that local run can work with all the variations when there is no tox.ini file to say what the variations are.

Ah, I see what you mean! Actually, what used to be in the tox.ini is still there, it's just that I centralized it to setup.cfg since tox supports having the configuration there as well, there variations are located there: https://github.com/althonos/wrapt/blob/feat-setuptools/setup.cfg#L72-L98

Other than that point, everything else looks good at this point and so will move on to merging it, or working out how to check out your branch, and actually playing with it. Thanks for your continued help.

No problem, glad I could be of help!

On a final note: after merging, you'll have one final step to do on your side to enable publishing the wheels directly to PyPI after a successful packaging workflow: create an environment named PyPI in the Settings of the GitHub repo, and add a variable named PYPI_API_TOKEN there containing a PyPI token with upload permissions for wrapt.

GrahamDumpleton commented 3 years ago

Okay, wasn't aware of tox using setup.cfg.

As to API token, saw that. Am contemplating whether though to modify that a bit so if sees a .devN on end of tag, that it pushes up packages to the test.pypi.org instance so have a validation step for building wheels. That way could tag with a .devN when in develop with wheels going there to test.