https://bugzilla.redhat.com/show_bug.cgi?id=2003700 --- Comment #4 from Michal Schorm <mschorm@xxxxxxxxxx> --- I've went through the "Python Packaging Guidelines" chapter of "Fedora Packaging Guidelines" the best I could. However I require another packager to go through my findings and add a second opinion, since this is just the second Python package ever I dive into. I need the second pair of eyes. https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ --- 1/ BuildRequire python3-devel [NEED-2ND-OPINION] [NEED-2ND-OPINION] "Every package that uses Python (at runtime and/or build time) and/or installs Python modules MUST explicitly include BuildRequires: python3-devel in its .spec file, even if Python is not actually invoked during build time." Reviewer note: Your code: BuildRequires: python%{python3_pkgversion}-devel I believe the Guideline explicitly shows the code that is mandatory to be included. And it is without macros. _My understanding_ is that the pure following line MUST be used instead: BuildRequires: python3-devel 2/ Naming [OK] Package name: container-workflow-tool Canonical name: | rpm --eval "%{py_dist_name container-workflow-tool}" | container-workflow-tool Main importable module name: container_workflow_tool This package provides BOTH a tool (application) and the importable module. Currently the primarly function as I see it is to provide the application, so the following apply: "Packages that primarily provide applications, services or any kind of executables SHOULD be named according to the general Fedora naming guidelines (e.g. ansible)." Also: "Consider adding a virtual provide according to Library naming above (e.g. python3-PROJECTNAME), if it would help users find the package." Since the goal is _also_ to provide the importable module, it might be a good idea to add such virtual provide. 3/ Files to include [OK] [OK] "Packages MUST include the source file (*.py) AND the bytecode cache (*.pyc) for each pure-Python importable module." [OK] "The source files MUST be included in the same package as the bytecode cache." [OK] "Scripts that are not importable (typically ones in %{_bindir} or %{_libexecdir}) SHOULD NOT be byte-compiled." [OK] "The cache files are found in a __pycache__ directory and have an interpreter-dependent suffix like .cpython-39.pyc." 4/ Dist-info metadata [OK] [OK] "Each Python package MUST include Package Distribution Metadata conforming to PyPA specifications (specifically, Recording installed distributons)." [OK] "The metadata SHOULD be included in the same subpackage as the main importable module, if there is one." 5/ Explicit lists [FAIL] [OK] "Packages MUST NOT own shared directories owned by Python itself" [FAIL] "Packagers SHOULD NOT simply glob everything under a shared directory." Reviewer note: the package ship a single manual page. Please list the file explicitly, instead of: | %{_mandir}/man1/* 6/ PyPI parity [FAIL] [FAIL] "Every Python package in Fedora SHOULD also be available on the Python Package Index (PyPI)." Reviewer note: The attempt to make the project available via PyPI has yet not been made. [OK] "The command pip install PROJECTNAME MUST install the same package (possibly in a different version), install nothing, or fail with a reasonable error message." [FAIL] "If your package is not or cannot be published on PyPI, you can: * Ask Python SIG to block the name on PyPI for you" Reviewer note: It is unclear whether the upstream would like to maintain the project in PyPI, however to avoid future name collisions, it seems wise to me to block the name. If the name collision would happen, you (maintainer) would be force to change the package name AND you would have to try to convince upstream to rename its projects, since the names of the upstream project and the Fedora package SHOULD match. That is my understanding of the Python Naming Guidelines 7/ Provides and requirements [FAIL] | Requires | -------- | container-workflow-tool (rpmlib, GLIBC filtered): | /usr/bin/python3 | python3-container-workflow-tool | python3-container-workflow-tool (rpmlib, GLIBC filtered): | python(abi) | python3.10dist(gitpython) | python3.10dist(pyyaml) | python3.10dist(requests-kerberos) | Provides | -------- | container-workflow-tool: | container-workflow-tool | python3-container-workflow-tool: | python-container-workflow-tool | python3-container-workflow-tool | python3.10-container-workflow-tool | python3.10dist(container-workflow-tool) | python3dist(container-workflow-tool) [OK] "For any module intended to be used in Python 3 with import MODNAME, the package that includes it SHOULD provide python3-MODNAME, with underscores (_) replaced by dashes (-)." [OK] "For any FOO, a package that provides python3-FOO SHOULD use %py_provides or an automatic generator to also provide python-FOO and python3.X-FOO, where X is the minor version of the interpreter." [OK] "The provide SHOULD NOT be added manually: if a generator or macro is not used, do not add the python-FOO / python3.X-FOO provides at all." [OK] "Every Python package MUST provide python3dist(DISTNAME) and python3.Xdist(DISTNAME), where X is the minor version of the interpreter and DISTNAME is the Canonical project name corresponding to the Dist-info metadata." [OK] "This is generated automatically from the dist-info metadata. The provide SHOULD NOT be added manually: if the generator fails to add it, the metadata MUST be fixed." [FAIL] "Each Python package MUST explicitly BuildRequire python3-devel." Reviewer note: "BuildRequires: python%{python3_pkgversion}-devel" is used instead. [OK] "Packages MUST NOT have dependencies (either build-time or runtime) with the unversioned prefix python- if the corresponding python3- dependency can be used instead." [OK] "Packages SHOULD NOT have explicit dependencies (either build-time or runtime) with a minor-version prefix such as python3.8- or python3.8dist(" [OK] "Packages SHOULD NOT have an explicit runtime dependency on python3." [NEED-2ND-OPINION] "Packages MUST use the automatic Python run-time dependency generator." [OK] "Packages SHOULD use the opt-in build-dependency generator if possible." [OK] "The packager MUST inspect the generated requires for correctness. All dependencies MUST be resolvable within the targeted Fedora version." [OK] "Dependencies covered by the generators SHOULD NOT be repeated in the .spec file." [NEED-2ND-OPINION] The project README.md states that: | Requirements | ------------ | * python3 | * python3-GitPython | * python3-requests-kerberos | * fedpkg | * http://github.com/ficap/dhwebapi (installed using pip3 and requirements.txt) But I see those requirements neither generated nor explicitly specified. Shouldn't they be listed? 8/ Interpreter invocation [NEED-2ND-OPINION] [OK] "Shebang lines to invoke Python MUST use %{python3} as the interpreter." [OK] "Shebang lines to invoke Python SHOULD be #!%{python3} -%{py3_shebang_flags} and they MAY include extra flags." Reviewer note: macro "%pyproject_install" is used. | # grep -i -e "^#!" -r review-container-workflow-tool/rpms-unpacked/* | review-container-workflow-tool/rpms-unpacked/container-workflow-tool-1.0.0-1.fc36.noarch.rpm/usr/bin/cwt:#! /usr/bin/python3 -s [NEED-2ND-OPINION] "Every executable TOOL for which the current version of Python matters SHOULD also be invokable by python3 -m TOOL." "If the software doesn’t provide this functionality, packagers SHOULD ask the upstream to add it." Reviewer note: I don't know how to check if the current version of Python matters. 9/ Using Cython [OK] [OK] "packages MUST NOT use files pre-generated by Cython" 10/ Tests [FAIL] [FAIL] "If a test suite exists upstream, it SHOULD be run in the %check section. If that is not possible with reasonable effort, at least a basic smoke test (such as importing the packaged module) MUST be run in %check." Reviewer note: No test is currently run. Even thought the justification why the upstream test are not run is OK, it does NOT follow the guidelines. You MUST run atleast a basic smoke test. [OK] "In %check, packages SHOULD NOT run “linters”: code style checkers, test coverage checkers and other tools that check code quality rather than functionality." 11/ Source files from PyPI [OK] Reviewer note: None used. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2003700 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
