search

ziglang / zig-pypi

The Zig programming language, packaged for PyPI
https://pypi.org/project/ziglang/
MIT License
148 stars 16 forks source link

make_wheels: implement PEP 517 #1

Closed FFY00 closed 3 years ago

FFY00 commented 3 years ago

Make make_wheels.py a Python build backend, as defined in PEP 517, allowing this project to be pip-installable from source.

FFY00 commented 3 years ago

I'd probably wait for https://github.com/pypa/packaging/pull/446.

FFY00 commented 3 years ago

I understand your sentiment. The main benefit would be that the project would be installable via pip when the --no-binary option is passed, it would also allow people to pip install . with this checked out without having to build all the wheels. It's good practice to release a source distribution for packages published on PyPI instead of only binary ones. Nightly builds would be nice, and this would make that much easier. It's an improvement that could be made in the future, but that we don't really need to do.

Ultimately, it's up to you. I don't really think the code this PR introduces is that much, it's mostly build_sdist, and some lines in build_wheel, the rest is just minor refactoring. I could split the refactoring to a separate commit if you want. FWIW, I would be happy to help out with maintenance of this if that is a concern to you. Anyway, please let me know what you think. If this is out of scope, that is perfectly fine :blush:

whitequark commented 3 years ago

It's good practice to release a source distribution for packages published on PyPI instead of only binary ones.

One reason this might not be a good idea is that we're using wheel's API (which is explicitly not public) and we override some of its internals. This is doesn't matter one bit if only final artifacts that are uploaded to PyPI, but it does matter for sdists.

Also--unless I'm missing something, does the sdist not explicitly specify the dependency on either wheel or libarchive-c? The former I think will be always installed with PEP517, the latter I don't think so?

whitequark commented 3 years ago

Overall, I feel like building wheels through PEP517 is a nice idea (even if I personally may not see much use of it), but I'm not convinced at all that I want to upload sdists that rely on internal wheel API and download artifacts via HTTP, since that's potentially a liability for people who use them in the future. The wheels aren't going to break, the sdists very well might.

FFY00 commented 3 years ago

One reason this might not be a good idea is that we're using wheel's API (which is explicitly not public) and we override some of its internals. This is doesn't matter one bit if only final artifacts that are uploaded to PyPI, but it does matter for sdists.

Oh, I didn't notice.

Also--unless I'm missing something, does the sdist not explicitly specify the dependency on either wheel or libarchive-c? The former I think will be always installed with PEP517, the latter I don't think so?

I forgot to commit the pyproject.toml, which specifies those dependencies.

Overall, I feel like building wheels through PEP517 is a nice idea (even if I personally may not see much use of it), but I'm not convinced at all that I want to upload sdists that rely on internal wheel API and download artifacts via HTTP, since that's potentially a liability for people who use them in the future. The wheels aren't going to break, the sdists very well might.

Makes sense. We could ship a sdist with all the required artifacts, but that doesn't seem great.

Anyway, I totally forgot to generate the metadata for the sdist anyway, so this PR would require a bit more complexity.

Closing this as out-of scope for now, if in the future we want to make this work with nightly builds or similar, it could be revisited if it makes sense.