https://bugzilla.redhat.com/show_bug.cgi?id=2154546 --- Comment #2 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- (In reply to Carl George 🤠 from comment #1) > I understand the background in bug 2006555 and how you ended up on a doc PDF > file, but is it even worth it? I have my doubts that such a file would ever > be opened by any users of the package. The guidelines on shipping upstream > documentation call it a "should" requirement, not a "must". IMO the > bundling issues are justification to break with the "should" requirement. > Dropping the PDF will reduce the package size and the build time of all > future builds of the package (no generation and fewer build requirements). > I won't block the review on this but that's how I feel about it. If you do > feel strongly about including some form of documentation, how about just > using the docs/*.rst files directly? Your arguments against including the PDF documentation are all reasonable. On the other hand, I know how useful it can be to have proper documentation in the distribution in environments without easy Internet access. I would be willing to drop the documentation if pressed, but I think I would still prefer to keep it. I hadn’t considered packaging the documentation in source-only form, though. Normally that’s not an option because of things like sphinx.ext.autodoc. In this case, the .rst files are quite readable and contain everything that goes into the PDF. I’ll change the spec as follows, leaving the PDF build behind a build conditional for now. diff --git a/pythoncapi-compat.spec b/pythoncapi-compat.spec index 9818a15..0374c5d 100644 --- a/pythoncapi-compat.spec +++ b/pythoncapi-compat.spec @@ -1,8 +1,11 @@ # Sphinx-generated HTML documentation is not suitable for packaging; see # https://bugzilla.redhat.com/show_bug.cgi?id=2006555 for discussion. # -# We can generate PDF documentation as a substitute. -%bcond_without doc_pdf +# We can generate PDF documentation as a substitute. Since the .rst sources are +# complete (nothing like sphinx.ext.autodoc is used) and rather readable, +# however, we package them directly to reduce the build-time dependencies and +# produce a smaller package. +%bcond_with doc_pdf %global commit 3779f1221363b0f8c65c4f0ee24b36603c98547a %global snapdate 20221127 (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? y @@ -132,11 +135,13 @@ install -t '%{buildroot}%{_mandir}/man1' -D -p -m 0644 '%{SOURCE1}' # This primarily documents the script rather than the header. %doc README.rst +%if %{with doc_pdf} # This is a source for the PDF documentation, but is useful as a standalone # documentation file as well. %doc docs/changelog.rst -%if %{with doc_pdf} %doc docs/build/latex/pythoncapi_compat.pdf +%else +%doc docs/*.rst %endif > I don't see anything in the guidelines about commands in /usr/bin needing to > strip the file extension. On my system I see a few commands that end in > .py, and the only symlinks are for unversioned commands, not to strip the > extension. It's not a big deal, but I think it would be better to just ship > it with the same file name as upstream. If we do that and update the man > page to match, that will clear this rpmlint warning as well. > > pythoncapi-compat-tools.noarch: W: no-manual-page-for-binary > upgrade_pythoncapi.py You know, I considered this and sat on the fence for a while before adding the extensionless link. I don’t think either approach would be wrong. I’ll do as you suggest here. diff --git a/pythoncapi-compat.spec b/pythoncapi-compat.spec index 0374c5d..83a6faf 100644 --- a/pythoncapi-compat.spec +++ b/pythoncapi-compat.spec @@ -20,7 +20,7 @@ License: 0BSD URL: https://github.com/python/%{name} Source0: %{url}/archive/%{commit}/%{name}-%{commit}.tar.gz # Man page hand-written for Fedora in groff_man(7) format based on --help -Source1: upgrade_pythoncapi.1 +Source1: upgrade_pythoncapi.py.1 BuildRequires: gcc BuildRequires: gcc-c++ @@ -67,8 +67,7 @@ BuildArch: noarch %description tools %{common_description} -This package provides the command-line tool upgrade_pythoncapi.py (also -available as upgrade_pythoncapi, without the .py extension). +This package provides the command-line tool upgrade_pythoncapi.py. %package doc @@ -100,15 +99,8 @@ popd >/dev/null %install -# Install the header, which should be considered a header-only library. install -t '%{buildroot}%{_includedir}' -D -p -m 0644 pythoncapi_compat.h - -# Install the script as “upgrade_pythoncapi”, without the .py extension… -install -D -p upgrade_pythoncapi.py '%{buildroot}%{_bindir}/upgrade_pythoncapi' -# …but provide the “upgrade_pythoncapi.py” version as a symlink in case anyone -# has scripts that expect the same name as upstream. -ln -s upgrade_pythoncapi '%{buildroot}%{_bindir}/upgrade_pythoncapi.py' -# Install the man page for the script. +install -D -p -t '%{buildroot}%{_bindir}' upgrade_pythoncapi.py install -t '%{buildroot}%{_mandir}/man1' -D -p -m 0644 '%{SOURCE1}' @@ -125,9 +117,8 @@ install -t '%{buildroot}%{_mandir}/man1' -D -p -m 0644 '%{SOURCE1}' %files tools %license COPYING -%{_bindir}/upgrade_pythoncapi %{_bindir}/upgrade_pythoncapi.py -%{_mandir}/man1/upgrade_pythoncapi.1* +%{_mandir}/man1/upgrade_pythoncapi.py.1* %files doc diff --git a/upgrade_pythoncapi.1 b/upgrade_pythoncapi.py.1 similarity index 89% rename from upgrade_pythoncapi.1 rename to upgrade_pythoncapi.py.1 index 405b1a4..7e63454 100644 --- a/upgrade_pythoncapi.1 +++ b/upgrade_pythoncapi.py.1 @@ -1,10 +1,10 @@ -.TH UPGRADE_PYTHONCAPI "1" "December 2022" "" "User Commands" +.TH UPGRADE_PYTHONCAPI.PY "1" "January 2022" "" "User Commands" .SH NAME -upgrade_pythoncapi +upgrade_pythoncapi.py \(en upgrade C extension modules to newer Python C API .SH SYNOPSIS -.B upgrade_pythoncapi +.B upgrade_pythoncapi.py .RB [ \-h ] .RB [ \-o\ \fIOPERATIONS ] .RB [ \-q ] > I'll leave those two items up to you to decide on. Regardless the rest > looks good. Thanks for packaging this up. Package is approved. Thank you for the review! Here are the final spec file and SRPM I plan to import: Spec URL: https://music.fedorapeople.org/20230107/pythoncapi-compat-0^20221127git3779f12-1.fc37.src.rpm SRPM URL: https://music.fedorapeople.org/20230107/pythoncapi-compat.spec -- 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=2154546 _______________________________________________ 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
