Bug 255722
| Summary: | [meta] Mk/Uses/python.mk: PEP-517 (pyproject.toml) Support | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Yuri Victorovich <yuri> | ||||||||||||
| Component: | Individual Port(s) | Assignee: | freebsd-python (Nobody) <python> | ||||||||||||
| Status: | Closed FIXED | ||||||||||||||
| Severity: | Affects Many People | CC: | eduardo, flo, flying-sheep, grahamperrin, jeroen.pulles, lbartoletti, python, rhurlin, rigoletto, thierry, vishwin, yasu | ||||||||||||
| Priority: | Normal | Keywords: | feature, tracking | ||||||||||||
| Version: | Latest | Flags: | vishwin:
maintainer-feedback+
|
||||||||||||
| Hardware: | Any | ||||||||||||||
| OS: | Any | ||||||||||||||
| URL: | https://wiki.freebsd.org/Python/PEP-517 | ||||||||||||||
| See Also: |
https://reviews.freebsd.org/D36290 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268893 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268979 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268904 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269013 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269168 |
||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 265660, 265692, 265693, 268018, 268283, 264679, 266681, 268431 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Yuri Victorovich
2021-05-09 08:14:01 UTC
Hi Yuri, We have already some ports using pyproject.toml (sip related IIRC): - https://cgit.freebsd.org/ports/tree/devel/py-qt5-qscintilla2/Makefile#n32 - https://cgit.freebsd.org/ports/tree/graphics/py-python-poppler-qt5/Makefile#n35 But I agree with you, a variable for pyproject.toml wouldn't hurt. I created an account here to clear up some confusion: pyproject.toml doesn’t have anything to do with setuptools *or* sip. As of 6 years ago, it’s *the* standard way to specify how a python package is built, defined in https://www.python.org/dev/peps/pep-0517/#source-trees This means there’s two ways do define python package builds: This standard one and the legacy setup.py I described here how building an OS package from a python repo/sdist in a generic way could look: https://github.com/theislab/anndata/issues/561#issuecomment-835770254 (In reply to flying-sheep from comment #2) flying-sheep@web.de, Thank you for your comments. Our python.mk needs to be updated to reflect the new standard. Yuri Created attachment 235704 [details]
WIP.patch
Add a very early draft of USE_PYTHON=build feature.
Created attachment 235732 [details]
WIP-2.patch
Created attachment 235745 [details]
WIP-3.patch
Created attachment 235750 [details]
RC-1.patch
Created attachment 235751 [details]
RC-2.patch
Please see https://wiki.freebsd.org/Python/PEP-517 that I wrote up months ago but didn't have enough time to fully implement yet. tl;dr it's way more involved than USES_PYTHON=build (and should not be that) (In reply to Charlie Li from comment #9) Hi Charlie, Thanks for your comment. Current implementation is functionally almost identical to your description in https://wiki.freebsd.org/Python/PEP-517: * It is PEP-517 compliant. * It requires port maintainers to parse pyproject.toml and add DEPENDS lines accordingly (this is also done in the supplied examples). * It checks that requirements are met, and fails otherwise. Differences: * I used USE_PYTHON=build instead of USE_PYTHON=pep517: the user intent here is to build the project. IMO there's no need to put the standard name into Makefiles in many places. "build" is a lot easier to remember. * autoplist isn't currently supported. It isn't obvious that it should be. Benefits of this implementation: * It is minimalistic. It does exactly what is required with as little code as possible. * It uses only build/installer packages, a minimally required set of dependencies, and still provides the same functionality. Best, Yuri autoplist is supported, just a different way of doing it (and much less fragile). "Minimalistic" will not work here, because a major part of PEP-517 is completely deprecating the setup.py or otherwise distutils path in package management and standardising on wheels. Even though setuptools includes an internal copy of distutils, there is no guarantee of it still existing in the future. Thus, in terms of future-proofing, this cannot go into the framework without also converting the toolchain to setuptools-less bootstrapping. USE_PYTHON=build actually is misleading, because it's only one (minimalist) tool that only builds, not installs. In fact, py-build and py-installer didn't exist until after PEP-517 was accepted and (ensure)pip gained support. (In reply to Charlie Li from comment #11) Dear Charlie, distutils and setuptools aren't required ot be present in the suggested approach. They would only be needed if pyproject.toml would explicitly specify them as a dependency. "build" is a user intent, not a package name. It is such so that it can be easily remembered. > [...] in terms of future-proofing, this cannot go into the framework without also converting the toolchain to setuptools-less bootstrapping. This is a more global issue. setuptools and distutils aren't going anywhere regardless of what standards say, simply because there are so many projects that use them. This potential problem can be solved once it becomes a real problem. And it won't become a real problem any time soon. Yuri > distutils and setuptools aren't required ot be present in the suggested approach They are present by virtue of py-build and py-installer currently in the tree using distutils/setuptools to build themselves. installer is actually built wrong in this case since the specified build backend is flit, not setuptools (but works by coincidence). > "build" is a user intent, not a package name Still misleading, because the intent does not imply install to staging/wherever. > This potential problem can be solved once it becomes a real problem. And it won't become a real problem any time soon. It will become a real problem in our tree when PEP-517 support lands in the framework. Currently, when USES_PYTHON=distutils is specified, setuptools is an unconditional RUN_DEPENDS, which is unnecessary and wrong for PEP-517. (In reply to Charlie Li from comment #13) If you are implying that all instances of setuptools/distutils use would be removed form the ports tree for strict PEP-517 - this is a non-starter at the moment because of sheer number of non-compliant packages. To be clear, the current patch introduces a local PEP-517 compliance so that the projects that have already switched to PEP-517 can be ported. This opens the path forward to a lot of software to enter the ports tree. Strict global PEP-517 compliance is something else that can be gradually addressed at a later time. This patch doesn't present an obstacle to this. Yuri > If you are implying that all instances of setuptools/distutils use would be removed form the ports tree
Not what I meant, since setuptools is also itself a PEP-517 build backend.
The current patch is a hack at best, much like my phab reviews marked WIP. Functionality that USE_PYTHON=distutils provided that is available in PEP-517 (like autoplist) has to be provided. Additionally, the "toolchain" has to bootstrap itself through the framework. There is no gradual as far as the framework is concerned, but individual packages can go at their pace.
To further clarify, the best "testsuite" for the framework support is the (self-)bootstrap "toolchain" listed in the wiki.
(In reply to Charlie Li from comment #15) Charlie, You have probably misunderstood. The purpose of this patch is to allow individual packages to proceed at their own pace. Some of them currently can't enter the ports tree. This patch doesn't aim to enable global PEP-517 compliance, but it also doesn't stand in its way. If you mean that the whole tree should be converted to PEP-517 at once - this is not practically possible. But this is an entirely orthogonal matter to the patch in question. There is also no reason why this patch would prevent self-bootstrapping. Yuri > If you mean that the whole tree should be converted to PEP-517 at once - this is not practically possible.
Also not what I meant. I agree that individual ports/packages should proceed at their own paces.
Bottom line, this really cannot be anywhere near rushed and has to be thought through holistically, even though there exist prospective ports that need the framework. The self-bootstrapping bit is merely an example of required flexibility in the framework (which does not exist in this WIP), in their case to eliminate circular dependencies. I updated the wiki page to clarify on some design goals that need met.
Committed. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=600312a9118bc7b9c5bb7decbefe706f9046507b commit 600312a9118bc7b9c5bb7decbefe706f9046507b Author: Yuri Victorovich <yuri@FreeBSD.org> AuthorDate: 2022-08-21 19:46:39 +0000 Commit: Yuri Victorovich <yuri@FreeBSD.org> CommitDate: 2022-08-21 20:16:06 +0000 Mk/Uses/python.mk: Add USE_PYTHON=build to support pyproject.toml based projects USE_PYTHON=build supports PEP-517 at the level of individual ports. Global support (making PEP-517 be used for all ports) is outside of the scope of this patch. PR: 255722 Approved by: python (maintainer's timeout; 14 days) Differential Revision: https://reviews.freebsd.org/D36061 Mk/Uses/python.mk | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) This was not approved or timed out. Please back out as soon as practicable. @Yuri Please revert as soon as possible. Proposed change received feedback with additional design notes (comment 17, among others) provided to which no feedback from you was received. Not withstanding, committing under maintainer timeout (13 days) after having received review and comment, by way of non-response, is not within the spirit of the maintainer timeout guidelines (In reply to Kubilay Kocak from comment #21) Kubilay, > [...] committing under maintainer timeout (13 days) [...] 14 full days passed. > [...] after having received review and comment [...] I asked for specifics and there was nothing specific. What should be done in in such case? Let it sit dormant for another 6 months? python@ had a chance to disapprove the review and he didn't. Yuri A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=7d48381cbd686ee8d77871bc25cf1e926a314f85 commit 7d48381cbd686ee8d77871bc25cf1e926a314f85 Author: Charlie Li <vishwin@FreeBSD.org> AuthorDate: 2022-08-22 01:39:08 +0000 Commit: Charlie Li <vishwin@FreeBSD.org> CommitDate: 2022-08-22 01:39:08 +0000 Uses/python.mk: revert unapproved feature addition (and consumers) Change proposal was in discussion with open questions and additional documented design requirement [0] for submitter to review and provide feedback on, which was not provided. Submitter (and anyone else) is welcome to work with python@ to produce an appropriately reviewed feature. [0] https://wiki.freebsd.org/Python/PEP-517 With hat: python (vishwin, koobs) PR: 255722, 265660, 265692, 265693 Approved by: fluffy (mentor) Mk/Uses/python.mk | 48 +--------------- devel/Makefile | 1 - devel/py-hatch-vcs/Makefile (gone) | 23 -------- devel/py-hatch-vcs/distinfo (gone) | 3 - devel/py-hatch-vcs/pkg-descr (gone) | 4 -- devel/py-hatch-vcs/pkg-plist (gone) | 20 ------- misc/Makefile | 2 - misc/py-anndata/Makefile (gone) | 30 ---------- misc/py-anndata/distinfo (gone) | 3 - misc/py-anndata/pkg-descr (gone) | 4 -- misc/py-anndata/pkg-plist (gone) | 111 ------------------------------------ misc/py-hist/Makefile (gone) | 36 ------------ misc/py-hist/distinfo (gone) | 3 - misc/py-hist/pkg-descr (gone) | 6 -- misc/py-hist/pkg-plist (gone) | 69 ---------------------- 15 files changed, 1 insertion(+), 362 deletions(-) This is in my TODO list. ^Triage: Level up importance and classify This is still in python@'s purview, and is still not timed out. Certain bootstrap packages mentioned in the wiki page have finally converted to not using setuptools to build, so the "test suite" at the very least can be properly implemented and tested. Additionally, build has not been the only minimalist PEP-517 builder for some time now (Gentoo's portage uses a different package(s) that still comply with the standard) Note: the framework is to at least account for overridable variables needed to bootstrap devel/py-flit-core in a self-hosting manner, ie what review D34786 does. This includes the autoplist procedure, which may be better implemented using a proper CSV parser, perhaps in Python itself using the built-in csv library. (In reply to Charlie Li from comment #27) Do we really need autoplist? It is not difficult for a porter to run `make makeplist', and it is much more interesting to have a fixed pkg-plist: you can really see what will be installed, and you can check the diff (missing files) when upgrading a port. autoplist is necessary, not only because it is supported with distutils, but RECORD is the wheel specification PEP-427's stronger (as in nearly every entry is hashed, something we don't do yet) equivalent of our plist. You can already really see what gets installed and compare versions by diffing RECORD (though the hashes will differ) I do not know where this RECORD is available, but I doubt that it will let you know which files will be installed by a port before its installation. RECORD is always in .dist-info, which is metadata unconditionally installed for every wheel. autoplist currently only pulls from that file. RECORD was initially defined in PEP-376; the canonical standard is now maintained as https://packaging.python.org/en/latest/specifications/recording-installed-packages/ One remaining snag is ensuring entry-points (https://packaging.python.org/en/latest/specifications/entry-points/) is properly accounted for in autoplist, as these are installed outside SITELIBDIR. data_files from distutils/setuptools will not be supported. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=cc8a1878e0242055ab6a8c20d33654451f78720e commit cc8a1878e0242055ab6a8c20d33654451f78720e Author: Charlie Li <vishwin@FreeBSD.org> AuthorDate: 2022-11-16 23:31:40 +0000 Commit: Charlie Li <vishwin@FreeBSD.org> CommitDate: 2023-01-11 05:22:16 +0000 python.mk: introduce USE_PYTHON=pep517 for PEP-517 support USE_PYTHON=pep517 takes no arguments. Operation is similar to USE_PYTHON=distutils, although the build backend specified in pyproject.toml is to be specified in BUILD_DEPENDS explicitly. A usage guide and implementation primer is available at: https://wiki.freebsd.org/Python/PEP-517 With hat: python Approved by: fluffy (mentor) Co-authored by: yuri PR: 255722 Differential Revision: https://reviews.freebsd.org/D36290 CHANGES | 13 ++++++++++ Mk/Uses/python.mk | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 3 deletions(-) This is now committed, with proper working USE_PYTHON=autoplist support, so that migrations from USE_PYTHON=distutils are as seamless as possible. Do mind that you will have to add the build backend specified in pyproject.toml as an explicit BUILD_DEPENDS. Feel free to report any problems as they arise here or elsewhere. Working on updating audio/py-gtts (https://github.com/pndurette/gTTS) with: --- USE_PYTHON= pep517 concurrent autoplist [1] BUILD_DEPENDS= ${PYTHON_PKGNAMEPREFIX}wheel>0:devel/py-wheel@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}setuptools>0:devel/py-setuptools@${PY_FLAVOR} --- [1] pyproject.toml: --- [build-system] requires = ["setuptools>=61", "wheel"] build-backend = "setuptools.build_meta" --- Builds fine: --- (...) adding 'gTTS-2.3.0.dist-info/RECORD' removing build/bdist.freebsd-13.1-RELEASE-p5-amd64/wheel Successfully built gTTS-2.3.0-py3-none-any.whl --- And fails on stage: --- =======================<phase: stage >============================ ===== env: DEVELOPER_MODE=yes STRICT_DEPENDS=yes USER=nobody UID=65534 GID=65534 ===> Staging for py39-gtts-2.3.0 ===> py39-gtts-2.3.0 depends on package: py39-click>0 - found ===> py39-gtts-2.3.0 depends on package: py39-six>0 - found ===> py39-gtts-2.3.0 depends on package: py39-requests>0 - found ===> py39-gtts-2.3.0 depends on file: /usr/local/bin/python3.9 - found ===> Generating temporary packing list Traceback (most recent call last): File "/usr/local/lib/python3.9/runpy.py", line 197, in _run_module_as_main return _run_code(code, main_globals, None, File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code exec(code, run_globals) File "/usr/local/lib/python3.9/site-packages/installer/__main__.py", line 85, in <module> _main(sys.argv[1:], "python -m installer") File "/usr/local/lib/python3.9/site-packages/installer/__main__.py", line 73, in _main with WheelFile.open(args.wheel) as source: File "/usr/local/lib/python3.9/contextlib.py", line 119, in __enter__ return next(self.gen) File "/usr/local/lib/python3.9/site-packages/installer/sources.py", line 122, in open with zipfile.ZipFile(path) as f: File "/usr/local/lib/python3.9/zipfile.py", line 1248, in __init__ self.fp = io.open(file, filemode) FileNotFoundError: [Errno 2] No such file or directory: '/wrkdirs/usr/ports/audio/py-gtts/work-py39/gTTS-2.3.0/dist/gtts-2.3.0-*.whl' --- Any clues? (In reply to Nuno Teixeira from comment #36) > FileNotFoundError: [Errno 2] No such file or directory: > '/wrkdirs/usr/ports/audio/py-gtts/work-py39/gTTS-2.3.0/dist/gtts-2.3.0-*.whl' I have the same problem while trying to update my port devel/py-pyls-black: FileNotFoundError: [Errno 2] No such file or directory: '/poudriere/ports/default/devel/py-pyls-black/work-py39/pyls-black-0.4.7/dist/pyls-black-0.4.7-*.whl' In my case, it is a wrong(?) naming of the wheel file: pyls_black-0.4.7-*.whl instead of pyls-black-0.4.7-*.whl (underscore) Perhaps your problem is similar? So...PORTNAME has to match exactly the package name from CHEESESHOP/PYPI, which is how the wheel filename is derived. Unfortunately we have a lot of ports where PORTNAME doesn't match for various reasons. (In reply to Charlie Li from comment #38) Thanks for the explanation. For my port devel/py-pyls-black, the PYPI name (and the github name) is pyls-black[1][2] (with minus sign), but wheel seems to derive its name from setup.cfg entry in lines 20/21: [options.entry_points] pyls = pyls_black = pyls_black.plugin ^^^ ^^^ [1] https://pypi.org/project/pyls-black/ [2] https://github.com/rupert/pyls-black/ BTW: This also happens with other ports like textproc/py-whatthepatch ... Further discussion on wheel filenames at bug 268893. Should we start using pep517 whenever possible on each bugzilla updates sent by maintainers? e.g., an update that uses distutils, a committer can do some testing with pep517 and if ok upload a patch so maintainer could test it too? Thanks |