https://bugzilla.redhat.com/show_bug.cgi?id=2239811 Ben Beasley <code@xxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |code@xxxxxxxxxxxxxxxxxx --- Comment #4 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- I had started having a look at this yesterday, and I found a couple of additional things: ===== Issues ===== - While all of the files from which you remove shebangs have “main routines” and are able to be used as scripts (via e.g. 'python3 -m pynsgr.client' or 'python3 /path/to/pynsgr/client.py'), none of them has the executable bit set in the filesystem permissions even in the upstream git repository. The shebangs are therefore useless even to upstream; in the spirit of staying close to upstream, I suggest sending a PR to remove them and applying the PR as a patch. - The generated man pages are broken, e.g.: NSGR_JOB(1) User Commands NSGR_JOB(1) NAME nsgr_job - manual page for nsgr_job 1.0.3 DESCRIPTION Traceback (most recent call last): File "/builddir/build/BUILD‐ ROOT/python-pynsgr-1.0.3-1.fc40.x86_64//usr/bin/nsgr_job", line 5, in <module> from pynsgr.commands.nsgr_job import main ModuleNotFoundError: No module named 'pynsgr' nsgr_job 1.0.3 September 2023 NSGR_JOB(1) You must set the environment variable PYTHONPATH to '%{buildroot}%{python3_sitelib}' when invoking help2man. Once this is fixed, you don’t need --no-discard-stderr. While you are at it, consider using a shell loop to reduce repetition and make it immediately clear that both man pages are generated with the same options. Below, I have also fiddled with the formatting of the help2man options, but that is much deeper into the territory of personal preferences. install -d '%{buildroot}%{_mandir}/man1' for cmd in nsgr_job nsgr_submit do help2man \ --no-info \ --version-string=%{version} \ --help-option=-h \ --output="%{buildroot}%{_mandir}/man1/${cmd}.1" \ "%{buildroot}/%{_bindir}/${cmd}" done The result is still far from beautiful, but at least does contain useful text. ===== Notes (no change required for approval) ===== - In Issues, I requested that the shebang removal be upstreamed, which makes the following commentary useless to this package (but relevant in general): In the shebang removal, I would tend to pass all the files to a single sed invocation to avoid having to repeat the expression, and so that a person reading the spec file can immediately see that all files are transformed in exactly the same way. # remove shebangs sed -i '/^#!.*\/usr\/bin\/env.*python/ d' \ pynsgr/client.py \ pynsgr/commands/nsgr_submit.py \ pynsgr/commands/nsgr_job.py \ pynsgr/pyjavaproperties.py Furthermore, for shebang removal, I like matching on just '^#!', but requiring the match to be on the first line of the file; after all, if upstream changed the shebangs in non-executable modules to /usr/bin/python3, we would still want to remove them. # remove shebangs sed -i -r '1{/^#!/d}' \ pynsgr/client.py \ pynsgr/commands/nsgr_submit.py \ pynsgr/commands/nsgr_job.py \ pynsgr/pyjavaproperties.py -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2239811 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202239811%23c4 _______________________________________________ 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, report it: https://pagure.io/fedora-infrastructure/new_issue
