Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Support for PEP517 build systems with new build style #26883

Closed
wants to merge 3 commits into from

Conversation

ahesford
Copy link
Member

@ahesford ahesford commented Dec 2, 2020

I have been told that PEP517 build systems are the way of the future for Python package building and installation, and setuptools will become (or is now) disfavored. python3-packaging is the first of our packages to drop setuptools and specifically require a PEP517 builder.

This is an attempt to support PEP517 builds in our python3-module build style. For now, the preferred (only?) way to do PEP517 builds is to rely on pip to do the work. Fortunately, because no PEP517 builder supports compiled extensions, we can avoid the pain of trying to force pip to behave with cross compilers (for now).

Use of the PEP517 build procedure in a template is enabled by setting python_pep517=yes. If this is adopted, we'll have to modify xlint as well.

pip can do a one-pass build and install, but I figured it was better to split into a build-wheel stage and an install-wheel stage so people can do ./xbps-src build and investigate the artifacts.

I am not thrilled with the use of globs in do_install when setting a default $python_pep517_wheel but, according to PEP 427 and its referenced PEP 425, the filename components I'm trying to match with the globs are not easily predicted. In any case, if this produces undesirable behavior in specific templates, the author can manually set that variable. Any other ideas are welcome.

Finally, the build process produces direct_url.json in the Python dist-info directory to comply with PEP 610, which replaces a simple version number in pip freeze output with a file:// URL pointing to the location of the wheel used for installation. (In our case, this will be /builddir/$wrksrc/$build_wrksrc/$python_pep517_wheel.) For distribution packages, I do not believe this is desirable. We can manually remove the file, for example in do_install, assuming the dist-info directory is predictable. Comments about doing this are welcome.

cc: any @void-linux/pkg-committers with a stake in Python packages

@@ -24,6 +30,11 @@ do_build() {
}

do_check() {
if [ -n "$python_pep517" ]; then
msg_warn "No standard test exists for PEP517 Python templates"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could try adding the pytest option to do_check, what do you think?

if command -v pytest >/dev/null; then \n pytest

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea, but I've gone one step further. Running python setup.py test complains about deprectation and suggests tox instead. Because that is the official recommendation, I try tox first if possible, otherwise try pytest if possible, and finally fall back to the old behavior for non-PEP517 builds.

I've modified a few templates as an example of how the change eliminates some redundancy.

If there is not significant debate about the PEP517 changes, I'll squash the two build-style/python3-module commits before merging. For now, I'll keep them apart so we can separate the two issues if need be.

Copy link
Member

@ericonr ericonr Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #25052 for some discussion about tox.

Btw, this PR closes that issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on opposition in the mentioned issue, I dropped tox from the build style and kept the preference for pytest over the deprecated setup.py test method.

Copy link
Member

@ericonr ericonr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to add to the Manual, as well :)

srcpkgs/python3-flit_core/template Outdated Show resolved Hide resolved
@ahesford ahesford force-pushed the packaging branch 2 times, most recently from f72fa4e to 195c58d Compare December 2, 2020 19:30
@Johnnynator
Copy link
Member

Why not do it as a seperate build_style? I would see it as a totally different build system and therefore it also should be in it's own build_style, especially since it already doesn't do it automatically but you need to manually set a var.

@ahesford
Copy link
Member Author

ahesford commented Dec 2, 2020 via email

@fosslinux
Copy link
Contributor

I agree with @Johnnynator

@ahesford
Copy link
Member Author

ahesford commented Dec 3, 2020

Comment added to update restriction on python-packaging, created a separate python3-pep517 build style and updated the manual. This simplified some things, and I moved the pytest adoption in python3-module to #26901 for separate consideration.

While doing a separate build style, I decided to allow $make_build_target to override the default $PWD target when necessary, and uesd $make_install_target instead of a new variable to select the wheel used for installation. This avoids the need for an xlint update.

@ahesford ahesford changed the title [RFC] Support for PEP517 build systems in python3-module [RFC] Support for PEP517 build systems with new build style Dec 3, 2020
@Chocimier
Copy link
Member

make_*_target needs documentation, as they are used a bit different than in make-based build styles.
Add new build style to build style scripts section too.
Please do not reformat existing manual sections when they stay with same content. Minimal, meaningful diffs are good.

@ahesford ahesford force-pushed the packaging branch 2 times, most recently from 901c4f9 to e056782 Compare December 6, 2020 05:53
@ahesford
Copy link
Member Author

ahesford commented Dec 6, 2020

I added the make_*_target behaviors to #optional_vars, added the new style to #build_scripts and cleaned up the diff in #pkgs_python.

Copy link
Member

@Chocimier Chocimier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more packages declaring build-backend builds with that, few fail, but that's not our fault.

Lgtm.

@ahesford
Copy link
Member Author

ahesford commented Dec 8, 2020

Closed in fe9c2d0.

@ahesford ahesford closed this Dec 8, 2020
@ahesford ahesford deleted the packaging branch December 8, 2020 20:58
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants