search

bobfang1992 / pytomlpp

A python wrapper for tomlplusplus
https://bobfang1992.github.io/pytomlpp/pytomlpp.html
MIT License
86 stars 10 forks source link

Build manylinux wheels #3

Closed EpicWink closed 4 years ago

EpicWink commented 4 years ago

Build many-linux (2010) wheels in CI

PyPI requires uploaded wheels to be many-linux for distribution.

This PR incorporates #2 (so you might want to merge that in first).

PS: Travis CI is a hot piece of garbage. GitHub actions is so much easier to use. You might want to squash commits during the merge.

chaitan94 commented 4 years ago

You have beat me to it :sweat_smile:. I was working on the same, although I took a Github Actions and cibuildwheel based approach. I've recently set up such a workflow on one of my project https://github.com/adonmo/daylight/commit/ec413a9d8f10a024cbef8659362bd01b016d6654. Additionally, I refactored CMake build workflow to use a more modern pattern, avoiding the git submodules. That way I am able to build for macos (and possibly windows too - haven't tested this though). If you guys are interested I can raise a PR with my approach as well.

bobfang1992 commented 4 years ago

@EpicWink this is great. I was researching how to do this before but did not have any success. Let's drop Python 2.7 support and I will merge this :)

bobfang1992 commented 4 years ago

You have beat me to it 😅. I was working on the same, although I took a Github Actions and cibuildwheel based approach. I've recently set up such a workflow on one of my project adonmo/daylight@ec413a9. Additionally, I refactored CMake build workflow to use a more modern pattern, avoiding the git submodules. That way I am able to build for macos (and possibly windows too - haven't tested this though). If you guys are interested I can raise a PR with my approach as well.

Hi @chaitan94, can you wait until we got this merged? and then you can submit your change in a separate PR on top of this one? Does this make sense? Thanks.

bobfang1992 commented 4 years ago

@EpicWink @chaitan94 thanks for the contribution and advice. I will look into replacing Travis CI with Github Actions once we got both of your changes merged.

chaitan94 commented 4 years ago

Hi @chaitan94, can you wait until we got this merged? and then you can submit your change in a separate PR on top of this one? Does this make sense? Thanks.

Sure. I will push my changes to a forked repo and test there meanwhile. That way you guys can also have a look and compare. I can rebase it later after this gets merged.

EpicWink commented 4 years ago

@chaitan94 looking forward to it. I'd like to see how cibuildwheel works

MacOS, Windows and PyPy support would be required for pytompp to be used in packaging projects

chaitan94 commented 4 years ago

Added a PR: https://github.com/bobfang1992/pytomlpp/pull/4 can rebase later if required

sinoroc commented 4 years ago

If this is to be used in packaging, you'll likely need to support Python 2.7 until January 2021. If you don't have Python 2, some packaging libraries can't use your module.

If I am not mistaken for a dependency to be taken over in packaging projects (pip, etc.) it should be pure Python, because otherwise it can't be vendored in, is that right? Making the argument for Python 2 compatibility irrelevant.

EpicWink commented 4 years ago

@bobfang1992 the force-push you just made removed all of my signed commits (and replaced them with your own commits). Did you want me to force-push all of my original commits (which are all identical) to keep the signature (the verified icon next to the commits in GitHub)? This won't matter if you intend to squash the commits

EpicWink commented 4 years ago

If I am not mistaken for a dependency to be taken over in packaging projects (pip, etc.) it should be pure Python, because otherwise it can't be vendored in, is that right? Making the argument for Python 2 compatibility irrelevant.

For pip's specific design goals, yes. Other projects may distribute themselves differently, for example usually I install poetry via pip, and conda is a whole thing.

bobfang1992 commented 4 years ago

@EpicWink sorry I was just rebasing on #2, yeah please do, didn't expect to erase your commits.

bobfang1992 commented 4 years ago

@EpicWink seems adding py2 support is not trivial and I think instead of me working on this branch, we can do this:

Does this make sense? If you could remove py2.7 for now I will be merging it.

Thanks a lot!

EpicWink commented 4 years ago

Turns out I really want to squash those commits anyway. Should be good once the build finishes (however Python 2.7 is still broken)

bobfang1992 commented 4 years ago

@EpicWink okay. I will merge this and remove Python2.7 on master and will work on a separate branch to add it back. Thanks for all the effort.