Add wheel build option

[Stormheg]

fixes #132

The primary purpose of this PR is to add an make bdist_wheel command to build a python wheel archive. This task is included in both the make publish and make publish-test commands and does not replace source distributions.

I've added a make clean-dist command to clean the dist folder. This command is ran before publishing to make sure we have a clean dist folder.

• Stay on point and keep it small so it can be easily reviewed. For example, try to apply any general refactoring separately outside of the PR.
• Consider adding unit tests, especially for bug fixes. If you don't, tell us why.
• not applicable.
• All new and existing tests pass, with 100% test coverage (make test-coverage)
• two failed tests but probably related to my dev environment (python 3.8.3, fedora 32)
  • FAIL: test_export_lxml_big_content_export (tests.test_exports.TestExportsLXML)
  • FAIL: test_export_lxml_entity_with_data-* (tests.test_exports.TestExportsLXML)
• Linting passes (make lint)
• check! ✅
• Consider updating documentation. If you don't, tell us why.
• List the environments / platforms in which you tested your changes.
• Python 3.8; Linux Fedora 32

[thibaudcolas]

Thank you for taking a stab at this @Stormheg!

This is working as expected for me, I’d rather still only have the single "build" command but I can make that change when merging this.

Before merging though – an important question from #132 was whether this change would be considered a breaking change or not. We’re still publishing as a source archive so I imagine that reduces the likelihood of issues, but I’d suspect lots of installers will default to the wheel if both options are available. Is that going to cause problems for some people?

Additionally I see Wagtail has this config for their wheel: https://github.com/wagtail/wagtail/blob/master/setup.cfg#L1-L2. Would we need this here as well?

[Stormheg]

Thanks for the thoughtful review @thibaudcolas!

This is working as expected for me, I’d rather still only have the single "build" command but I can make that change when merging this.

Refactored into a single build command 👍

Additionally I see Wagtail has this config for their wheel: https://github.com/wagtail/wagtail/blob/master/setup.cfg#L1-L2. Would we need this here as well?

I did some research on what that python-tag does exactly and found PEP 425. It is used to specify compatibility with python versions/implementations. Generated wheels already have py3 in their filename, indicating compatibility with Python 3 (and no Python 2).

To be honest, I don't think adding a python tag will add any significant value.

Before merging though – an important question from #132 was whether this change would be considered a breaking change or not. We’re still publishing as a source archive so I imagine that reduces the likelihood of issues, but I’d suspect lots of installers will default to the wheel if both options are available. Is that going to cause problems for some people?

I've tried wagtail bakery demo with a wheel version of this library and encountered no issues. But perhaps consider testing yourself and making a test release to PyPi?

Like you mentioned, installers will likely default to wheel versions of this library. I think there is a remote chance that causes issues in specific setups. Lets err on the side of caution, there is no harm done in making a major release 🙂

[thibaudcolas]

Thank you :)

I don’t think making a test release to PyPI will help much except confirming that it "works on my machine" – as you say it’s more about knowing about the remote chances, whether that’s old versions of pip, or other PyPI clients.

It’s very undesirable for this to be done as a major release if it could be done as a patch one, for a few reasons:

• Wagtail is already behind the latest major release of the exporter, so if this is only fixable with a major release then we won’t be able to fix this for Wagtail users without also updating Wagtail to address the other breaking changes, which will be much more work than just this PR and getting a release.
• This would also mean we’d only be able to fix this for people who install Wagtail 2.10 (or whichever version this is fixed in), whereas if it can be done as a patch release then anyone with a Wagtail install will be able to benefit from the change.

Researching this a bit, it looks like pip just does setup.py bdist_wheel without any further processing: https://pip.pypa.io/en/stable/reference/pip_install/#build-system-interface. I guess the next question is whether the project’s setup.py script would produce the same output regardless of the OS and Python version.

[thibaudcolas]

It looks like Django started publishing wheels in v1.6, but also backported the change to v1.5 and just did so on a patch update:

• v1.5.7, no wheel: https://pypi.org/project/Django/1.5.7/#files
• v1.5.8, wheel: https://pypi.org/project/Django/1.5.8/#files

This seems like as good of a sign as any that this should be appropriate.

[thibaudcolas]

It looks like it might be appropriate for us to actually upload wheels to existing releases – see pypa/packaging-problems#75. I’m still researching this though.

[thibaudcolas]

Thank you @Stormheg!