python-sh: add python-sh - #30378
Conversation
sh is a full-fledged subprocess replacement for Python 3.10+ and PyPy that allows you to call any program as if it were a function. Signed-off-by: Ilja Honkonen <[email protected]>
Formality Check: FailedWe checked this pull request against the contribution guidelines. Here is what needs your attention: 🛑 CRITICAL ERRORS
Tip Do not close this pull request to make corrections. Instead, modify your existing commits (e.g. Something broken? Consider reporting an issue. |
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 commit; 4 comments plus 1 nit, all on lang/python/python-sh/Makefile.
The one I would treat as a merge blocker is the missing host build-backend dependency — if sh 2.4.0 still declares poetry-core in [build-system], the package cannot build as written, and nothing on this PR has attempted a build yet. The hardcoded PKG_SOURCE_URL, the PKG_CPE_ID value and DEPENDS:=+python3 are packaging-quality points rather than build breakers, though the URL one will cause maintenance friction at the next version bump.
I had no network access to PyPI or NVD from this session, so the backend and CPE questions are genuinely open rather than rhetorical. The formality bot's findings are not repeated here.
Generated by Claude Code
| PKG_HASH:=a250aef68509ed93419c9a1d90b0647cd5cbe26107ba94d3717ef5b6d595ffd9 | ||
| PKG_SOURCE:=$(PYPI_NAME)-$(PKG_VERSION).tar.gz | ||
| PKG_SOURCE_URL:=https://files.pythonhosted.org/packages/e9/c8/137093889a8b8c9a66c9b46079cf2a0e155a8be027df212a4041e3b7b09c |
There was a problem hiding this comment.
Both of these are already derived by pypi.mk and the hardcoded URL is version-fragile. PKG_SOURCE?= / PKG_SOURCE_URL?= in pypi.mk:19-20 expand, for PYPI_NAME:=sh, to exactly sh-2.4.0.tar.gz and to https://files.pythonhosted.org/packages/source/s/sh — so the PKG_SOURCE line is a verbatim duplicate of the default, and the URL default is the version-independent form of the same download.
The e9/c8/1370… path is PyPI's per-file blake2 hash directory: it changes for every release, so every future version bump has to touch the URL as well, and a stale one fails with a 404 rather than a hash mismatch. lang/python/README.md:151 says both variables may be omitted when pypi.mk is included; only 5 of the 214 Makefiles under lang/python override PKG_SOURCE_URL, and all five do so because their source is not on PyPI at all.
| PKG_HASH:=a250aef68509ed93419c9a1d90b0647cd5cbe26107ba94d3717ef5b6d595ffd9 | |
| PKG_SOURCE:=$(PYPI_NAME)-$(PKG_VERSION).tar.gz | |
| PKG_SOURCE_URL:=https://files.pythonhosted.org/packages/e9/c8/137093889a8b8c9a66c9b46079cf2a0e155a8be027df212a4041e3b7b09c | |
| PKG_HASH:=a250aef68509ed93419c9a1d90b0647cd5cbe26107ba94d3717ef5b6d595ffd9 |
Generated by Claude Code
| PKG_LICENSE:=MIT | ||
| PKG_LICENSE_FILES:=LICENSE.txt | ||
| PKG_MAINTAINER:=Ilja Honkonen <[email protected]> | ||
| PKG_CPE_ID:=cpe:/a:python:python-sh |
There was a problem hiding this comment.
PKG_CPE_ID only does anything if the product string exists in the NVD CPE dictionary — a value that matches no entry makes the CVE tracking that consumes this field silently report nothing, which is worse than leaving the field out.
Two concerns with this value. First, the product part here is the OpenWrt PKG_NAME (python-sh), whereas the other cpe:/a:python:* entries in this directory use the upstream/PyPI project name: cpe:/a:python:openpyxl, cpe:/a:python:pillow, cpe:/a:python:requests — for PYPI_NAME:=sh that would be cpe:/a:python:sh. Second, I could not reach NVD from here to check whether any CPE entry exists for this project. Did you verify one does? If not, please drop the line — only 41 of the 214 packages under lang/python set PKG_CPE_ID at all, and omitting it is the normal outcome for a project with no NVD presence.
Generated by Claude Code
| PKG_MAINTAINER:=Ilja Honkonen <[email protected]> | ||
| PKG_CPE_ID:=cpe:/a:python:python-sh | ||
|
|
||
| include ../pypi.mk |
There was a problem hiding this comment.
No PKG_BUILD_DEPENDS is declared, which is a problem if sh does not build with setuptools. Py3Build/Compile/Default runs python -m build --no-isolation — with isolation off, whatever [build-system] requires the sdist declares has to be present on the host already; nothing fetches it on demand, so a missing backend is a hard build failure, not a slow path.
sh has used poetry-core as its build backend since the 2.x series. If that is still true for 2.4.0, this needs a line before the includes, the way python-rsa/Makefile:15 and python-bleak do:
PKG_BUILD_DEPENDS:=python-poetry-core/host
I could not reach PyPI from this session to read the 2.4.0 pyproject.toml, so could you confirm which backend it declares? Worth checking by hand rather than relying on CI here: the only checks that ran on this head commit are the three FormalityCheck jobs, so nothing has actually attempted to build the package.
Generated by Claude Code
| SUBMENU:=Python | ||
| TITLE:=Python process launching | ||
| URL:=https://github.com/amoffat/sh/ | ||
| DEPENDS:=+python3 |
There was a problem hiding this comment.
+python3 drags the entire split standard library onto the device for what is a single pure-Python module. lang/python/README.md:206-210 makes python3-light the default choice and reserves python3 for packages that need more than three of the separate stdlib packages; in this directory 95 Makefiles depend on python3-light against 25 on python3.
sh does import a fair amount (asyncio, logging, pty/termios, queue, …), so python3 may well turn out to be the right answer — but it should be the result of checking rather than the starting point. The tree ships a helper for exactly this (README:214-224):
make package/python-sh/{clean,configure} PY3=stdlib V=s
If the generated list is three or fewer, please switch to +python3-light plus those entries, in the style of python-rsa. Note the helper also flags imports from test/example files as false positives, so the raw output needs a read-through.
Generated by Claude Code
|
|
||
| PKG_LICENSE:=MIT | ||
| PKG_LICENSE_FILES:=LICENSE.txt | ||
| PKG_MAINTAINER:=Ilja Honkonen <[email protected]> |
There was a problem hiding this comment.
nit: this disagrees with the PR description, which answers Maintainer: with "not applicable" while the Makefile names you as PKG_MAINTAINER. Since a new package does need a maintainer, the Makefile line is presumably the accurate one — worth fixing the PR body so it does not read as an unmaintained submission.
Generated by Claude Code
📦 Package Details
Maintainer: not applicable
Description:
sh is a full-fledged subprocess replacement for Python 3.10+ and PyPy that allows you to call any program as if it were a function.
🧪 Run Testing Details
✅ Formalities