If one tries to build py-rencode with cython installed, it fails during the configure phase. Excerpt below shown when py27-cython-0.28.2 was installed. py27-cython-0.29 is similar (line number differences in traceback). ======================= ===> Configuring for py27-rencode-1.0.6 Traceback (most recent call last): File "<string>", line 1, in <module> File "setup.py", line 53, in <module> ext_modules = cythonize(ext_modules) File "/usr/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 897, in cythonize aliases=aliases) File "/usr/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 777, in create_extension_list for file in nonempty(sorted(extended_iglob(filepattern)), "'%s' doesn't match any files" % filepattern): File "/usr/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 102, in nonempty raise ValueError(error_msg) ValueError: 'rencode/rencode.pyx' doesn't match any files *** Error code 1 Stop. make: stopped in /usr/ports/converters/py-rencode =======================
Investigation summary: 1) ports r482774 removed USE_PYTHON=cython (inadvertently, due to (2), or because the package successfully built without cython) 2) upstream setup.py does not declare cython as a dependency 3) upstream dev-requirements.txt does declare it as a dependency (noop for setuptools/pip based installs as they use setup.py:*_requires) 4) upstream setup.py conditionally (when importable/found) uses Cython (.pyx file) for the build, falling back to a setuptools build_ext (.c file) build 5) The 1.0.6 PyPI sdist (only) ships with a .c file 6) Upstream commit 5c928f14567fabc9efb8bbb8ac5e0eef03c61541 via issue #25 [1] adds a pyx file (not yet released). 7) setup.py declares 'sdist requires cython module to generate c' file, appearing to indicate it (cython) is a development dependency, prior to, and for producing, the relevant source distribution (which we use). In my opinion, a cython build (and the dependency on cython) it not necessary (particularly given (5) and (7)), even after #25 is released and the conditional code to build with cython should not be included in setup.py, at least not as a default case, or without an explicit request by the user to use cython, as an optional build method, for sdist consumers (most downstream packagers, and setuptools/pip users) [1] https://github.com/aresch/rencode/commit/5c928f14567fabc9efb8bbb8ac5e0eef03c61541 [2] https://github.com/aresch/rencode/pull/25
A commit references this bug: Author: koobs Date: Wed Nov 28 05:51:50 UTC 2018 New revision: 486079 URL: https://svnweb.freebsd.org/changeset/ports/486079 Log: converters/py-rencode: Fails to configure if cython is installed ports r482774 removed USE_PYTHON=cython, likely due to it (cython) not being explicitly declared as a dependency in setup.py:*_requires. However, setup.py conditionally builds with cython if it is installed, but the 1.0.6 source distribution (sdist) does not contain a .pyx file to build with. This leads to a configure/build error when cython is installed: ValueError: 'rencode/rencode.pyx' doesn't match any files Upstream commit 5c928f14567fabc9efb8bbb8ac5e0eef03c61541 [1] via issue #25 [2] adds the required .pyx file to the sdist, which technically addresses the "build with cython from the sdist" issue, but does not fundamentally resolve the higher-level question: why build with cython when a C source pre-processed by cython has already been produced for, and is contained in, the source distribution. A cython build (and the dependency on cython) does not appear to be necessary, nor intended [3][4], nor recommended [5], even after #25 is released, for sdist consumers. The conditional code to build with cython should not be included in setup.py, at least not as a default case, or without an explicit request by the user to use cython, as an optional build method, for sdist consumers (most downstream packagers and setuptools/pip users [6]). This change removes the conditional check for Cython, leaving the standard setuptools build_ext (with the packaged .c) file, as the build method. While I'm here: - Canonicalise COMMENT (match seutp.py:description) - Add test target with post-patch target to make the tests dir a module usable by a setup.py test (via test_suite directive) target. - Honour CFLAGS: Remove forced (appended) -O3 arg from setup.py [1] https://github.com/aresch/rencode/commit/5c928f14567fabc9efb8bbb8ac5e0eef03c61541 [2] https://github.com/aresch/rencode/pull/25 [3] setup.py: "Error: sdist requires cython module to generate `.c` file." [4] dev-requirements.txt:Cython [5] https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html#distributing-cython-modules [6] https://github.com/aresch/rencode/issues/11 PR: 233561 Reported by: John Hein <z7dr6ut7gs snkmail com> Approved by: koobs (python) Changes: head/converters/py-rencode/Makefile head/converters/py-rencode/files/ head/converters/py-rencode/files/patch-setup.py
Committed, thank you for the report John
Thanks for the fix, koobs. I got as far as hard-coding cythonize=False in setup.py locally. But your commit (and the excellent explanation herein) is far more satisfying. Was this raised upstream? I didn't get around to it yet (because I had not done the analysis that you did and did not understand the history). As you observed there are some inconsistencies (in both docs and code) that could be addressed upstream.
(In reply to John Hein from comment #4) You're welcome John I didn't raise the issue explicitly, *but*, when commit log messages contain git(hub) issue/pr/commit references, as were included here, references are added as comments to those respective GitHub objects, which I also believe authors and subscribers are notified of. I intentionally add as many GitHub references to my commits for this reason, so upstreams are implicitly notified of activity at (FreeBSD) downstream, thereby raising awareness. The detailed commit log message is also designed also as much for upstream, as it is for us :) I'll probably send a PR upstream fixing this and a few packaging issues at some stage, if they haven't already been fixed or improved in the meantime due to the extra notifications :)