Spec URL: https://people.redhat.com/~jsnow/python-qemu-qmp.spec SRPM URL: python-qemu-qmp-0.0.1-1.fc36.src.rpm Description: The QEMU Monitor Protocol library is a QEMU Monitor Protocol (“QMP”) library written in Python, using asyncio. It is used to send QMP messages to running QEMU emulators. It requires Python 3.6+ and has no mandatory dependencies. This library can be used to communicate with QEMU emulators, the QEMU Guest Agent (QGA), the QEMU Storage Daemon (QSD), or any other utility or application that speaks QMP. Fedora Account System Username: jsnow This is my first attempt at packaging anything for Fedora at all, so I have plenty of doubts and there are surely things to fix; but "publish early, publish often" - here's attempt #1. Kashyap has been shepherding me through the process thus far. Future upstream builds of QEMU will need this library for the "check" phase as a builddep.
Hiya! Good start :-). A couple of small things: (1) I think you intended to remove the commented-out old macros # (pct)py3_build ... # (pct)py3_install Thanks for reworking the spec to use newer macros! (2) `rpmlint` complains about macro in comment: $> rpmlint python-qemu-qmp.spec ../SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm =================================================================================== rpmlint session starts =================================================================================== rpmlint: 2.2.0 configuration: /usr/lib/python3.10/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 2 python-qemu-qmp.spec:50: W: macro-in-comment %{pypi_name} python-qemu-qmp.spec:50: W: macro-in-comment %{pypi_name} ==================================================== 1 packages and 1 specfiles checked; 0 errors, 2 warnings, 0 badness; has taken 0.8 s ==================================================== (3) [INFO] The Koji scratch build succeeds; nice: ----------------------------------------------------------------------- $> koji build --scratch rawhide ../SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm Uploading srpm: ../SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm [====================================] 100% 00:00:00 96.37 KiB 232.47 KiB/sec Created task: 90346957 Task info: https://koji.fedoraproject.org/koji/taskinfo?taskID=90346957 Watching tasks (this may be safely interrupted)... 90346957 build (rawhide, python-qemu-qmp-0.0.1-1.fc36.src.rpm): free 90346957 build (rawhide, python-qemu-qmp-0.0.1-1.fc36.src.rpm): free -> open (buildvm-s390x-24.s390.fedoraproject.org) 90346963 rebuildSRPM (noarch): open (buildvm-s390x-24.s390.fedoraproject.org) 90346997 buildArch (python-qemu-qmp-0.0.1-1.fc37.src.rpm, noarch): open (buildvm-s390x-29.s390.fedoraproject.org) 90346963 rebuildSRPM (noarch): open (buildvm-s390x-24.s390.fedoraproject.org) -> closed 0 free 2 open 1 done 0 failed 90346997 buildArch (python-qemu-qmp-0.0.1-1.fc37.src.rpm, noarch): open (buildvm-s390x-29.s390.fedoraproject.org) -> closed 0 free 1 open 2 done 0 failed 90346957 build (rawhide, python-qemu-qmp-0.0.1-1.fc36.src.rpm): open (buildvm-s390x-24.s390.fedoraproject.org) -> closed 0 free 0 open 3 done 0 failed 90346957 build (rawhide, python-qemu-qmp-0.0.1-1.fc36.src.rpm) completed successfully ----------------------------------------------------------------------- (4) [TODO] We need to get someone more well-versed in licensing to review the license "RFC" comment you put in the spec. (5) Super nit: Usually, along with link to RPM spec, the review-request submitter also posts a link to the SRPM. But it's not a big deal. * * * Meanwhile, /me goes to run: $> fedora-review -m fedora-rawhide-x86_64 \ --rpm-spec -n ~/rpmbuild/SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm
Created attachment 1900632 [details] Output of `fedora-review` tool [Manual review upcoming, `fedora-review` tool output in attachment for the record.] It was invoked this way: $> fedora-review -m fedora-rawhide-x86_64 --rpm-spec \ -n ~/rpmbuild/SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm
So on this one: [ ]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib/python3.11/site-packages/qemu I checked with folks on #fedora-python, and the advice I got is the following: In most cases, directory co-ownership: e.g. `%dir %{python3_sitelib}/qemu` in all packages in the namespace) is appropriate for Python namespace package directories (Where "namespace" == the "qemu/" namespace in this case) And in our case, we can use the newer macro called "%pyproject_save_files" to resolve the review complaint: %install ... %pyproject_save_files qemu (The above macro only understands top-level packages and modules. But the emitted file list will co-own the top-level directory (i.e. /usr/lib/python3.11/site-packages/qemu) PS: The "%pyproject_save_files" macro is available on RHEL9, but _not_ on RHEL8. * * * Docs for file- and directory-ownership: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership ~
Note that: %install ... %pyproject_save_files qemu Only makes sense with: %files -n python3-%{pkg_name} -f %{pyproject_files} %doc README.rst %doc html %{_bindir}/qmp-shell %{_bindir}/qmp-shell-wrap I also highly recommend not listing BuildRequires like pip and setuptools manually, but using %pyproject_buildrequires instead.
(In reply to Miro Hrončok from comment #4) Hi, Miro. Thanks for the feedback. A quick question further below: > Note that: > > > %install > ... > %pyproject_save_files qemu > > > Only makes sense with: > > > > %files -n python3-%{pkg_name} -f %{pyproject_files} > %doc README.rst > %doc html > %{_bindir}/qmp-shell > %{_bindir}/qmp-shell-wrap Yep, noted. > I also highly recommend not listing BuildRequires like pip and setuptools > manually, but using %pyproject_buildrequires instead. Thanks for the macro. So that macro should be used this way, I presume? ... %generate_buildrequires %pyproject_buildrequires ... Also, I wonder if that should be "%pyproject_buildrequires -r", to generate requirements, or if just the plain "%pyproject_buildrequires" suffices.
-r stands for runtime. It generates the runtime requirement on buildtime (for tests -- which are mandatory but missing here BTW). It is on by default. "%pyproject_buildrequires" *is* "%pyproject_buildrequires -r".
Note that GPL-2.0-only is the SPDX expression that should now be used in the License tag. There are also parts of the spec that are IMHO needlessly obfuscating it: %global pypi_name qemu.qmp %global pkg_name qemu-qmp %global pypi_version 0.0.1 And parts of the spec that are deprecated and SHOULD not be used: %{?python_provide:%python_provide python3-%{pkg_name}} I highly recommend starting with https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file rather than "a file once created by pyp2rpm-3.3.8"
(In reply to Miro Hrončok from comment #7) > Note that GPL-2.0-only is the SPDX expression that should now be used in the > License tag. Actually probably: GPL-2.0-only AND LGPL-2.1-or-later (There is no "GPL eats LGPL" rule anymore.)
https://docs.fedoraproject.org/en-US/legal/license-field/#_no_effective_license_analysis
I'd separate the doc under a -doc subpackage. Also, some cleanup as the the python_provide that Miro mentioned would be fine. wrt pypi_name and pkg_name, given the difference between pkg and module name, lgtm, for version it may be included in the Version definition for clarity.
OK, thanks for the feedback. I've reworked the spec file to match the template Miro pointed out a bit more closely, but I kept the pkg_name and pypi_name macro definitions for now based on Alfredo's comment. I removed RFC comments that have been addressed in the discussion here. https://people.redhat.com/~jsnow/v2/python-qemu-qmp.spec https://people.redhat.com/~jsnow/v2/python-qemu-qmp-0.0.1-1.fc36.src.rpm I split out the docs into a subpackage. I didn't see this laid out in https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ so I took a wild stab at it. Remaining items to handle, as I understand them: TLDR: - License is fine despite rpmlint chirps, AFAIK - Need man pages for CLI scripts - Might need gpgverify (?) - Need to integrate tests into a check phase, but there are dep problems. At length: [ ]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU Library General Public License, Version 2.0", "GNU General Public License, Version 2", "*No copyright* GNU General Public License, Version 2 GNU Library General Public License v2 or later", "GNU Library General Public License v2 or later". 47 files have unknown license. Detailed output of licensecheck in /home/jsnow/src/fedora/jsnow-mk5/python-qemu-qmp/licensecheck.txt I suppose I'll need to update the individual files upstream with SPDX identifiers, but I can't really control that for the current version. All of the "unknown" files are intended to be licensed as LGPLv2+. One interesting piece is that the .tar.gz for the SDist on PyPI actually includes my .gitlab-ci.d files, one of which is licensed as GPLv2. I didn't really intend to distribute these files in the SDist, so I'll have to look into that. It's harmless for now, but could cause problems when I go to drop the remaining legacy GPLv2 code. Thought I'd mention it. Otherwise, I believe "GPL-2.0-only AND LGPL-2.0-or-later" to be the correct aggregate license for the software as presently bundled. The "LICENSE" file contains text that matches the 2.0 license that uses the "Library" wording and not the 2.1 "Lesser" wording. [ ]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. I assume a comment is OK. I added one. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). Rpmlint ------- Checking: python3-qemu-qmp-0.0.1-1.fc37.noarch.rpm python3-qemu-qmp-doc-0.0.1-1.fc37.noarch.rpm python-qemu-qmp-0.0.1-1.fc37.src.rpm python3-qemu-qmp.noarch: W: spelling-error %description -l en_US asyncio -> syncopation python3-qemu-qmp.noarch: W: invalid-license GPL-2.0-only python3-qemu-qmp.noarch: W: invalid-license LGPL-2.0-or-later python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell-wrap python3-qemu-qmp-doc.noarch: W: invalid-license GPL-2.0-only python3-qemu-qmp-doc.noarch: W: invalid-license LGPL-2.0-or-later python-qemu-qmp.src: W: spelling-error %description -l en_US asyncio -> syncopation python-qemu-qmp.src: W: invalid-license GPL-2.0-only python-qemu-qmp.src: W: invalid-license LGPL-2.0-or-later 3 packages and 0 specfiles checked; 0 errors, 10 warnings. 'asyncio' is the correct spelling. The version of rpmlint I have installed locally does not seem to like the SPDX identifiers, though I see that is a very recent requirement (July 2022). The manpage warning didn't come up for me on my first draft spec; I'll have to fix this upstream to generate manual pages for each binary tool. I think Sphinx can generate manual pages, but I haven't plumbed that yet. I'll work on this. (Did the old macros generate these ...?) [ ]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. I did sign my PyPI releases with my personal (jsnow@redhat) GPG key. I'll have to look into how to do this step. I also use an expiration on my key, so I worry that I am doing this step wrong. I can change my release process for 0.0.2 if necessary. (No idea what I am doing. EAFP.) [ ]: %check is present and all tests pass. "SHOULD". ... but I do have tests, so I will attempt to wire them up here. Upstream, these tests also include PyCQA tools, but I think those should be disabled here because they will only be a nuisance in this context if the goal is to just reproduce upstream tarballs verbatim. It will also balloon the builddeps quite a bit. I currently rely on avocado-framework >= 90, but I suppose 92.x (the "LTS" release) isn't packaged for F36 yet. I'll have to look into that, sorry. (Dogfood, yip yip!)
> wrt pypi_name and pkg_name, given the difference between pkg and module name, lgtm I meant the macros are not needed at all.
Thanks for the v2; some comments: (1) For `gpgverify`, Alfredo pointed to an example[a] from OpenStack: %{gpgverify} --keyring=%{SOURCE102} --signature=%{SOURCE101} --data=%{SOURCE0} Where: Source0: https://tarballs.openstack.org/%{pypi_name}/%{pypi_name}-%{upstream_version}.tar.gz Source101: https://tarballs.openstack.org/%{pypi_name}/%{pypi_name}-%{upstream_version}.tar.gz.asc Source102: https://releases.openstack.org/_static/%{sources_gpg_sign}.txt Perhaps we could borrow the same approach. @John: As upstream maintainer you need to provide the keyring and signature. [a] https://github.com/rdo-packages/oslo-cache-distgit/blob/rpm-master/python-oslo-cache.spec (2) On the "pypi_name" and "pkg_name" macros, FWIW, I personally prefer them as it feels clearer to define them in one place. (I find it distracting to see a dot one time and a dash another occurence while reading the RPM spec.) (3) `rpmlint` still whines about: $> rpmlint python-qemu-qmp.spec ../SRPMS/python-qemu-qmp-0.0.1-1.fc36.src.rpm ../RPMS/noarch/python3-qemu-qmp-* [...] python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell-wrap python3-qemu-qmp.noarch: W: invalid-license GPL-2.0-only python3-qemu-qmp.noarch: W: invalid-license LGPL-2.0-or-later python3-qemu-qmp-doc.noarch: W: invalid-license GPL-2.0-only python3-qemu-qmp-doc.noarch: W: invalid-license LGPL-2.0-or-later python-qemu-qmp.src: W: invalid-license GPL-2.0-only python-qemu-qmp.src: W: invalid-license LGPL-2.0-or-later I think your licensing comment in the spec is fine to address the warning. (4) The man-page for CLI scripts -- I didn't see that before either. I hope it's not too much work to convince Sphinx about it :-) (5) For %check, John is working with the upstream Avocado developers to get the 92.x version packaged.
(In reply to Miro Hrončok from comment #7) Hi, Miro I think v2 addresses most of your comments: > Note that GPL-2.0-only is the SPDX expression that should now be used in the > License tag. > > > There are also parts of the spec that are IMHO needlessly obfuscating it: > > %global pypi_name qemu.qmp > %global pkg_name qemu-qmp I see your point; but as noted in comment#13, I feel it's a tiny bit cleaner to define it once at the top. But if you insist to remove them, we could remove in v3 :-). > %global pypi_version 0.0.1 This is gone in v2. > And parts of the spec that are deprecated and SHOULD not be used: > > %{?python_provide:%python_provide python3-%{pkg_name}} And this too. > I highly recommend starting with > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > #_empty_spec_file rather than "a file once created by pyp2rpm-3.3.8" Sure. Please let us know if you have any glaring concerns in v2.
(In reply to John Snow from comment #11) > Remaining items to handle, as I understand them: > > TLDR: > > - License is fine despite rpmlint chirps, AFAIK Correct. rpmlint hasn't been updated to follow the new licensing guidelines. > - Need man pages for CLI scripts I'd recommend manpages for your users, but this is not a requirement. It's a SHOULD guideline, not a MUST. > - Might need gpgverify (?) You only need to use %gpgverify if upstream already provides signed tarballs. You can ignore this. > - Need to integrate tests into a check phase, but there are dep problems. What dependencies are missing? You should run tests if it's possible/practical. If not, you should at least use %pyproject_check_import. This serves as a basic smoke test by trying to import all public modules. > I suppose I'll need to update the individual files upstream with SPDX > identifiers, but I can't really control that for the current version. All of > the > "unknown" files are intended to be licensed as LGPLv2+. You definitely do not need to do that to get the package into Fedora. They are unknown, because the license scanner can't detect a license if there's no license header. That doesn't really mean anything. > One interesting piece is that the .tar.gz for the SDist on PyPI actually > includes my .gitlab-ci.d files, one of which is licensed as GPLv2.I didn't > really intend to distribute these files in the SDist, so I'll have to look > into > that. It's harmless for now, but could cause problems when I go to drop the > remaining legacy GPLv2 code. Thought I'd mention it. For Fedora, the only license that really matters is the license of the actual installed content. The License tag does not need to account for other files in the source tarball that aren't installed. > > [ ]: If the package is under multiple licenses, the licensing breakdown > must be documented in the spec. > > I assume a comment is OK. I added one. Yes, that is valid. > [x]: Rpmlint is run on all rpms the build produces. > Note: There are rpmlint messages (see attachment). > > Rpmlint > ------- > Checking: python3-qemu-qmp-0.0.1-1.fc37.noarch.rpm > python3-qemu-qmp-doc-0.0.1-1.fc37.noarch.rpm > python-qemu-qmp-0.0.1-1.fc37.src.rpm > python3-qemu-qmp.noarch: W: spelling-error %description -l en_US asyncio -> > syncopation Irrelevant. > python3-qemu-qmp.noarch: W: invalid-license GPL-2.0-only > python3-qemu-qmp.noarch: W: invalid-license LGPL-2.0-or-later You can ignore that, as I've explained. > python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell > python3-qemu-qmp.noarch: W: no-manual-page-for-binary qmp-shell-wrap Not a requirement, as I've explained. > python3-qemu-qmp-doc.noarch: W: invalid-license GPL-2.0-only > python3-qemu-qmp-doc.noarch: W: invalid-license LGPL-2.0-or-later See above. > python-qemu-qmp.src: W: invalid-license GPL-2.0-only > python-qemu-qmp.src: W: invalid-license LGPL-2.0-or-later See above. > (Did the old macros generate these ...?) No, they do not. > [ ]: %check is present and all tests pass. > > "SHOULD". ... but I do have tests, so I will attempt to wire them up here. Yes, you should do that if possible. > Upstream, these tests also include PyCQA tools, but I think those should be > disabled here because they will only be a nuisance in this context if the > goal > is to just reproduce upstream tarballs verbatim. Correct. The guidelines actually explicitly state that linting tools shouldn't be run in the RPM package. > I currently rely on avocado-framework >= 90, but I suppose > 92.x (the "LTS" release) isn't packaged for F36 yet. Yes, you'll have to edit the metadata to relax this requirement.
(In reply to Maxwell G from comment #15) > (In reply to John Snow from comment #11) > > - Need man pages for CLI scripts > > I'd recommend manpages for your users, but this is not a requirement. It's a > SHOULD guideline, not a MUST. The docs are there, I just need need to cajole Sphinx into writing a manpage for it. It'd be a shame to not have them accessible in a manner that users expect, I took the time to write them. Plus, I have a little bit of time while I wait for others to help unblock me on the dependency problem I mentioned, so I might have this for my next iteration. > > > - Might need gpgverify (?) > You only need to use %gpgverify if upstream already provides signed > tarballs. You can ignore this. I'm the upstream -- There's only one non-preview release, and I did indeed sign it. I've already added this step to my working version. (As an aside; being the upstream is why I am here trying to write the specfile, as a courtesy to the other QEMU downstream packagers who will wind up needing this package as a builddep for QEMU's own %check phase. I am realizing I went a little out of order and there are other criteria I need to fulfill to become a packager and own this package, but I am working on that process. I am a little waylaid because I am moving right now, so attention has been scattered and time has been short. I'm working on it. Feel free to gently remind me if I seem to have missed something obvious -- I probably did, in the chaos.) > > > - Need to integrate tests into a check phase, but there are dep problems. > > What dependencies are missing? You should run tests if it's > possible/practical. If not, you should at least use %pyproject_check_import. > This serves as a basic smoke test by trying to import all public modules. avocado-framework v90.0 or better. We use avocado upstream (in QEMU proper) and I was asked to try using it for my Python subproject instead of the more usual pytest to help dogfood that project, as well as to not add yet-another-testing-tool to the QEMU ecosystem. I'll add the smoke test for now, but see below for more on avocado. > > One interesting piece is that the .tar.gz for the SDist on PyPI actually > > includes my .gitlab-ci.d files, one of which is licensed as GPLv2.I didn't > > really intend to distribute these files in the SDist, so I'll have to look > > into > > that. It's harmless for now, but could cause problems when I go to drop the > > remaining legacy GPLv2 code. Thought I'd mention it. > > For Fedora, the only license that really matters is the license of the > actual installed content. The License tag does not need to account for other > files in the source tarball that aren't installed. > Good to know, thanks. (I think I still want to look into how to drop them from the SDist to begin with in the future if I can, but that's something for my research list.) > > I currently rely on avocado-framework >= 90, but I suppose > > 92.x (the "LTS" release) isn't packaged for F36 yet. > > Yes, you'll have to edit the metadata to relax this requirement. I can't - I am using asyncio features that only showed up in v90, and the tests would have to be rewritten substantially to target the older version. I couldn't find a configuration that works well across both versions, it's a behavior change I am relying on. Cleber Rosa (upstream maintainer for avocado-framework) tells me that fedora:latest carries a stream wherein avocado is kept at the bleeding edge, but that a non-modular package cannot rely upon a modular one as a dep. He is working on updating the normal package from Avocado LTS 82.0 to Avocado LTS 92.0, which will unblock me here. Thanks for the feedback! My action items right now are: - Help Cleber get the avocado package updated - Publish manpages for the CLI tools that I bundle. Will likely be rectified for upstream's v0.0.2. - Work through https://docs.fedoraproject.org/en-US/package-maintainers/Joining_the_Package_Maintainers/
> I can't - I am using asyncio features that only showed up in v90, and the tests would have to be rewritten substantially to target the older version. Indeed, I should've added "if compatible" to my suggestion.
(In reply to Kashyap Chamarthy from comment #14) > (In reply to Miro Hrončok from comment #7) > > Hi, Miro > > I think v2 addresses most of your comments: > > > Note that GPL-2.0-only is the SPDX expression that should now be used in the > > License tag. > > > > > > There are also parts of the spec that are IMHO needlessly obfuscating it: > > > > %global pypi_name qemu.qmp > > %global pkg_name qemu-qmp > > I see your point; but as noted in comment#13, I feel it's a tiny bit cleaner > to define it once at the top. But if you insist to remove them, we could > remove in v3 :-). Okay, Miro convinced me enough on IRC that we can do away with these two macros and fill in the literal values. His main arguments: IMHO in-spec defined macros should only be used for 1) values that happen to change during the package lifetime (e.g. `%version`) 2) values that are configurable in order to 12:54 < mhroncok> 09:33:17> build the package slightly differently (e.g. `%bcond_without tests`) 3) values that make it significantly DRY (e.g. `%summary` or `%_description` or `%build_procedure` of repeated) -- everyhing else is just making it more "clever" for bad reasons -- but this is my personal viewpoint only. @John: if you agree too, in v3, let's please do away with the "pypi_name" and "pkg_name" and fill in the literal values. [...]
Removing Miro's NEEDINFO. They've answered your question on IRC.
Some more comments: > Cleber Rosa (upstream maintainer for avocado-framework) tells me that fedora:latest carries a stream wherein avocado is kept at the bleeding edge, but that a non-modular package cannot rely upon a modular one as a dep. Yup, the implementation of modularity is... not the best. > He is working on updating the normal package from Avocado LTS 82.0 to Avocado LTS 92.0, which will unblock me here. That major version bump will only be able to happen in branched (f37) and rawhide (f38) according to the Updates Policy[1]. [1]: https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/ Keep in mind that you won't be able to push incompatible updates to this package in stable updates, either. Given that this is an alpha package, you'll have to take that into consideration. (people.redhat.com has an expired SSL cert. You might want to report that internally...) The specfile looks good for the most part. You need to include the license file and mark it with %license in %files for both subpackages. ``` %files -n python3-%{pkg_name} -f %{pyproject_files} %doc README.rst %license LICENSE LICENSE_GPL2 [...] %files -n python3-%{pkg_name}-doc %license LICENSE LICENSE_GPL2 %doc html ``` You should preserve the description for both subpackages and name the doc subpackage as `python-qemu-qmp-doc`. You can change ``` %package -n python3-%{pkg_name}-doc Summary: Documentation for %{pkg_name} %description -n python3-%{pkg_name}-doc Documentation for %{pkg_name} ``` to ``` %package doc Summary: Documentation for %{pkg_name} %description -n python3-%{pkg_name}-doc %_description This package provides documentation for python3-qemu-qmp. ``` You'll also need to change `%files -n python3-qemu-qmp-doc` to `%files doc`. More nitpicky comments: Consider removing *_name macros as discussed. > rm %{buildroot}%{_bindir}/qmp-tui I'm not a fan of this. Fedora packages should try to stay close to upstream projects[1], and this feels like a deviation from that. I'm happy to help you split it out into a subpackage if needed. However, if you still maintain that this shouldn't be packaged, I won't push hard on it. [1]: https://docs.fedoraproject.org/en-US/package-maintainers/Staying_Close_to_Upstream_Projects/
Also, I should be able to complete this review and sponsor you. Please send an introductory email to devel@ and do an informal review or two.
FWIW offthread I've also offered to sponsor jsnow, and be an implicit co-maintainer (as part of virtmaint-sig), since this will eventually be a qemu dependency
(In reply to Maxwell G from comment #20) > Some more comments: Thank you for your time and feedback, I appreciate it. My current goal is to fix as much as I can and ensure there are no more changes that might need to be made for v0.0.2 upstream to make this spec file workable. As I go, I'm learning a lot about the tooling and process so please pardon the temporary slowness. > > > Cleber Rosa (upstream maintainer for avocado-framework) tells me that fedora:latest carries a stream wherein avocado is kept at the bleeding edge, but that a non-modular package cannot rely upon a modular one as a dep. > > Yup, the implementation of modularity is... not the best. > > > He is working on updating the normal package from Avocado LTS 82.0 to Avocado LTS 92.0, which will unblock me here. > > That major version bump will only be able to happen in branched (f37) and > rawhide (f38) according to the Updates Policy[1]. > > [1]: https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/ > I think that's likely fine. QEMU 7.2 is the first candidate for needing this package, and that isn't expected until Nov/Dec. > Keep in mind that you won't be able to push incompatible updates to this > package in stable updates, either. Given that this is an alpha package, > you'll have to take that into consideration. Understood! (You called it an alpha package -- is that because you read my readme?) > > (people.redhat.com has an expired SSL cert. You might want to report that > internally...) Seems to have been fixed: "Not Before Wed, 10 Aug 2022 00:00:00 GMT" > > > The specfile looks good for the most part. > > You need to include the license file and mark it with %license in %files for > both subpackages. Oops. Lost these in an edit. Re-added in my working draft, thank you. > > ``` > %files -n python3-%{pkg_name} -f %{pyproject_files} > %doc README.rst > %license LICENSE LICENSE_GPL2 > [...] > > %files -n python3-%{pkg_name}-doc > %license LICENSE LICENSE_GPL2 > %doc html > ``` > > You should preserve the description for both subpackages and name the doc > subpackage as `python-qemu-qmp-doc`. You can change > > ``` > %package -n python3-%{pkg_name}-doc > Summary: Documentation for %{pkg_name} > %description -n python3-%{pkg_name}-doc > Documentation for %{pkg_name} > ``` > > to > > ``` > %package doc > Summary: Documentation for %{pkg_name} > > %description -n python3-%{pkg_name}-doc %_description > > This package provides documentation for python3-qemu-qmp. > ``` > > You'll also need to change `%files -n python3-qemu-qmp-doc` to `%files doc`. Ah, I see. OK, I've mostly implemented your suggestion, but I did spell out "python-qemu-qmp-doc" explicitly instead. (No strong reason, I guess I liked the visual parity? Will change if desired.) Is it common to reproduce the same description for the doc package? I picked a few at random and it didn't seem that way, but I did it anyway. > > More nitpicky comments: > > Consider removing *_name macros as discussed. I've done so in my working copy. I almost argued for keeping pypi_name, but I then found this blurb concerning pypi_source on the packaging guidelines: "For backward compatibility, the first argument is technically optional as well, but omitting it is deprecated. (It defaults to %srcname if defined, or to %pypi_name if defined, or to %name.)" So, I dropped both. > > > rm %{buildroot}%{_bindir}/qmp-tui > > I'm not a fan of this. Fedora packages should try to stay close to upstream > projects[1], and this feels like a deviation from that. I'm happy to help > you split it out into a subpackage if needed. However, if you still maintain > that this shouldn't be packaged, I won't push hard on it. > > [1]: > https://docs.fedoraproject.org/en-US/package-maintainers/ > Staying_Close_to_Upstream_Projects/ Understood. Currently, the TUI requires some packages that Fedora does not yet package. I'm reluctant to split out another Python package for QEMU before I'm fully finished with the first, so I think it's going to be a while before it's in fighting shape to bother splitting, packaging, documenting, etc. That said, it's there now, certainly -- would you find it preferable to just leave the script entry point installed, ignore missing manpage warnings, and allow the script to simply always return an error [1] ? On the other hand, and with regards to not violating user's expectations that upstream and downstream are not materially different, I've not documented or publicized this script via the docs [2], so I don't anticipate anyone will come looking for it here, so to speak. It was always intended as something you'd get when installing "qemu.qmp[tui]", not "qemu.qmp". And for right now, we only really need the core bits of the library to satisfy the QEMU build dependency. (I originally thought you could guard console script shims using extras, but have since learned that you cannot. I know this means that upstream it really ought to go into its own PyPI package, but I'm not ready for that just yet, for a few organizational reasons that need to be settled in qemu.git patches over the coming months. I will work towards eliminating the discrepancy eventually, some time prior to v1.0.) [1] https://gitlab.com/qemu-project/python-qemu-qmp/-/blob/main/qemu/qmp/qmp_tui.py#L40 [2] https://qemu.readthedocs.io/projects/python-qemu-qmp/en/latest/ Other items: - I tested my current draft with tests added alongside Cleber's new avocado build for f38. It works, but the avocado packages aren't properly ready/available yet. - gpgverify works, but I am using a temporary URL for the keyring during review here. Working on a better home for it. - I have patches upstream for manpage generation; that'll be for 0.0.2, which may as well be the first version we package anyway. In the interest of sharing my progress so you have something to look at: A draft specfile *without* manpages or the avocado dependency (It should work today as-is): https://people.redhat.com/~jsnow/v3a/python-qemu-qmp.spec A draft specfile based on what will be 0.0.2, with manpages and the avocado dependency (It won't work today as-is, but might be useful for critique): https://people.redhat.com/~jsnow/v3b/python-qemu-qmp.spec
(In reply to John Snow from comment #23) > (In reply to Maxwell G from comment #20) [...] > > > rm %{buildroot}%{_bindir}/qmp-tui > > > > I'm not a fan of this. Fedora packages should try to stay close to upstream > > projects[1], and this feels like a deviation from that. I'm happy to help > > you split it out into a subpackage if needed. However, if you still maintain > > that this shouldn't be packaged, I won't push hard on it. > > > > [1]: > > https://docs.fedoraproject.org/en-US/package-maintainers/ > > Staying_Close_to_Upstream_Projects/ > > Understood. Currently, the TUI requires some packages that Fedora does not > yet package. I'm reluctant to split out another Python package for QEMU > before I'm fully finished with the first, so I think it's going to be a > while before it's in fighting shape to bother splitting, packaging, > documenting, etc. FWIW, I think it is fine to remove 'qmp-tui' as of now in this spec file. Yes, Fedora packages "should try to stay close to upstream projects", but within reason. Once the related dependencies are in place, it can be included in a later build. [...]
(In reply to Cole Robinson from comment #22) > FWIW offthread I've also offered to sponsor jsnow, If you want to do the sponsoring in the end, that's fine with me, but I already started this review, so I might as well finish it :). > and be an implicit > co-maintainer (as part of virtmaint-sig), since this will eventually be a > qemu dependency That will work. (In reply to John Snow from comment #23) > (In reply to Maxwell G from comment #20) > > Some more comments: > > As I go, I'm > learning a lot about the tooling and process so please pardon the temporary > slowness. Don't worry! There's no time crunch here. > > Keep in mind that you won't be able to push incompatible updates to this > > package in stable updates, either. Given that this is an alpha package, > > you'll have to take that into consideration. > > Understood! > (You called it an alpha package -- is that because you read my readme?) Yes, I did :). > Ah, I see. OK, I've mostly implemented your suggestion, but I did spell out > "python-qemu-qmp-doc" explicitly instead. > (No strong reason, I guess I liked the visual parity? Will change if > desired.) I'd prefer `%package doc` and `%files doc`. > Is it common to reproduce the same description for the doc package? I picked > a few at random and it didn't seem that way, but I did it anyway. I'm not really sure what's more common, but yes, that's what I'd recommend. > I then found this blurb concerning pypi_source on the packaging guidelines: > "For backward compatibility, the first argument is technically optional as > well, but omitting it is deprecated. (It defaults to %srcname if defined, or > to %pypi_name if defined, or to %name.)" Yes, that's a good point that I sometimes forget. `%{pypi_source qemu.qmp}` would be best here. > > > rm %{buildroot}%{_bindir}/qmp-tui > > > > I'm not a fan of this. Fedora packages should try to stay close to upstream > > projects[1], and this feels like a deviation from that. I'm happy to help > > you split it out into a subpackage if needed. However, if you still maintain > > that this shouldn't be packaged, I won't push hard on it. > > > > [1]: > > https://docs.fedoraproject.org/en-US/package-maintainers/ > > Staying_Close_to_Upstream_Projects/ > > Understood. Currently, the TUI requires some packages that Fedora does not > yet package. What are the missing dependencies? At least looking at the tui extra, all three of those packages exist in Fedora.
@John: The "global_description" has a bit redundancy. What do you do think of this slightly rephrased text: The QEMU Monitor Protocol (QMP) library is a low-level Python library, written using "asyncio". It is useful for sending low-level commands (called "QMP commands") to control a running QEMU instance. It requires Python 3.6+ and has no mandatory dependencies. This library can be used to communicate with QEMU emulators, the QEMU Guest Agent (QGA), the QEMU Storage Daemon (QSD), or any other utility or application that speaks QMP.
(In reply to Maxwell G from comment #25) > (In reply to Cole Robinson from comment #22) > > FWIW offthread I've also offered to sponsor jsnow, > > If you want to do the sponsoring in the end, that's fine with me, but I > already started this review, so I might as well finish it :). Thanks for your time, again. I didn't want to *tell* Cole he was going to do it, and I just hadn't heard back from him when I started this process. > > > Keep in mind that you won't be able to push incompatible updates to this > > > package in stable updates, either. Given that this is an alpha package, > > > you'll have to take that into consideration. > > > > Understood! > > (You called it an alpha package -- is that because you read my readme?) > > Yes, I did :). > No better feeling than when someone reads your README :D The package is quite "stable" in the sense that a vendored version of this library has been in-use for the qemu.git test suite for over a year now (And is now being used aggressively in our third testing cycle -- it gets tested on many distros and BSDs and many versions thereof), but I wanted to leave myself ample room to make adjustments as I packaged it for PyPI, Fedora, etc. (I'm a coward: releasing a 1.0.0 out of the gate is too prideful that it begs for a fall.) The remaining GPL2 (only) code in legacy.py needs to be removed and replaced with a more intentionally designed synchronous interface, but the qemu.git test suite needs to be updated to use that new stuff first. So there are some "big" API changes planned for the near future, but mostly around legacy.py. (legacy.py implements a nearly identical API to the synchronous QMP library we had in-tree since 2010 or so. There are many projects on git that just copied it out of our tree verbatim, so I am fixing that.) The core of the library should be reasonably stable, but it's entirely possible that I'll want to shift things around a little to accommodate features needed for the sync interface -- to provide something that's more idiomatic and useful for synchronous paradigms instead of a glorified and hacky wrapper shim. That's the plan for 0.1.0. Once 0.1.z goes through a testing cycle or two for QEMU, I'll probably release the v1.0.0 -- so maybe another year or so. In practice, it usually takes about two releases for more esoteric platforms to phone home and inform us of problems. > > > Ah, I see. OK, I've mostly implemented your suggestion, but I did spell out > > "python-qemu-qmp-doc" explicitly instead. > > (No strong reason, I guess I liked the visual parity? Will change if > > desired.) > > I'd prefer `%package doc` and `%files doc`. > OK, no problem -- I'll do that on my local copy and have it ready when the dust has settled for avocado-framework. (I admit I tried it this way first, but goofed something else, so spelling it out was a debugging step that I just didn't... change.) > > > Is it common to reproduce the same description for the doc package? I picked > > a few at random and it didn't seem that way, but I did it anyway. > > I'm not really sure what's more common, but yes, that's what I'd recommend. > Fair enough! > > I then found this blurb concerning pypi_source on the packaging guidelines: > > "For backward compatibility, the first argument is technically optional as > > well, but omitting it is deprecated. (It defaults to %srcname if defined, or > > to %pypi_name if defined, or to %name.)" > > Yes, that's a good point that I sometimes forget. `%{pypi_source qemu.qmp}` > would be best here. > > > > > > rm %{buildroot}%{_bindir}/qmp-tui > > > > > > I'm not a fan of this. Fedora packages should try to stay close to upstream > > > projects[1], and this feels like a deviation from that. I'm happy to help > > > you split it out into a subpackage if needed. However, if you still maintain > > > that this shouldn't be packaged, I won't push hard on it. > > > > > > [1]: > > > https://docs.fedoraproject.org/en-US/package-maintainers/ > > > Staying_Close_to_Upstream_Projects/ > > > > Understood. Currently, the TUI requires some packages that Fedora does not > > yet package. > > What are the missing dependencies? At least looking at the tui extra, all > three of those packages exist in Fedora. ... Oh! *cough*. I won't say what version of Fedora I am running on my development machine, but I will inform you that urwid-readline does not exist there ;) ... I'll upgrade this weekend. Well. OK, I suppose I *could* make an RPM subpackage for it here and now, then, but it's *way* more actually-an-alpha than the rest of the library, so I admit to some reluctance to do it anyway ... I nervously want to clean house a little bit first upstream as I really didn't have plans on stuffing this half-finished TUI into Fedora yet. The problem might also resurface: my draft branches for improvements to the TUI involve an even more niche urwid plugin, and I am quite confident that's not in F36, at least. - Did you get a chance to look at the 2nd spec file I posted that includes the %check phase? I had a question on how best to set up the PYTHONPATH env var (et al). What I wrote appears to work so far as I have been able to test it, but it feels possibly brittle and/or overcomplicated.
> Did you get a chance to look at the 2nd spec file I posted that includes the %check phase? I started looking at it, but then my computer died and my comment got lost :(. I'll look at it again later.
``` PATH="%{buildroot}%{_bindir}:$PATH" \ PYTHONPATH="${PYTHONPATH:-%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}}" \ _PYTHONSITE="%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib}" \ PYTHONDONTWRITEBYTECODE=1 \ ``` That's mostly fine. If you'd like you can simplify it to ``` export PYTHONPATH=%{buildroot}%{python3_sitelib} export PYTHONDONTWRITEBYTECODE=1 export PATH="%{buildroot}%{_bindir}:${PATH}" avocado --config avocado.cfg run tests/*.py ``` You can remove the "${PYTHON:...}" parameter expansion. That's there in `%pytest` so a user can override the variable before invoking the the macro, which isn't needed here. You can also remove `%{buildroot}%{python_sitearch}` here, because the package doesn't install anything there. That's in the macro to make it work with both noarch and arched packages, but this package is noarch. ``` mkdir -p %{buildroot}%{_mandir}/man1 install -m 0644 man/*.1 %{buildroot}%{_mandir}/man1/ ``` You need to pass the `-p` argument to install to preserve mtimes. I prefer to combine the `mkdir` and `install` into one command, but that's optional: ``` install -Dpm 0644 man/*.1 -t %{buildroot}%{_mandir}/man1/ ``` Nitpicks: > %{pyproject_check_import -e qemu.qmp.qmp_tui} The `%pyproject_check_import -e qemu.qmp.qmp_tui` syntax is preferred here, but meh. ``` %package -n python3-qemu-qmp Summary: %{summary} %description -n python3-qemu-qmp %_description ``` The subpackage definitions would be more readable if you added a new line or two before `%description`.
Thank you for your review. OK; Draft V4: https://people.redhat.com/~jsnow/v4/python-qemu-qmp.spec Draft SRPM: https://people.redhat.com/~jsnow/v4/python-qemu-qmp-0.0.2.dev14+g7f9a044-1.fc36.src.rpm - Added gnupg2 builddep (Not sure why it was working before, but it's there now...) - Sorted BuildRequires alphabetically - Reworded description slightly to avoid some redundancy -- Kashyap: If you want to reword it more aggressively, please send me a MR on GitLab O:-) -- I agree it needs some more work, but I want to keep it in sync with upstream. - Added some newlines before `%description` - Use "%package doc" spelling for subpackage - Rephrase on qmp-tui exclusion comment (f36 added the missing dep, so don't blame that.) - Touch up install command for manpages to preserve timestamp and use single invocation - Remove RFC commend and shorten env vars around running avocado in %check - Update changelog timestamp and version If it looks OK, I'll cut v0.0.2 upstream and post a final draft with the final correct versions and URLs. Thanks, --js
This looks good! Feel free to cut a new release. Note that the tests fail with ``` + avocado --config avocado.cfg run tests/protocol.py Could not resolve references: tests/protocol.py ``` on f36, presumably due to the outdated avocado version. I didn't build it on f35, but I assume the same thing would happen there. We have three options to deal with this: 1. Only package this for f37 and f38/rawhide. 2. Disable tests on f35 and f36 and only run %pyproject_check_import. 3. Create a non-modular avocado compat package. If you change your mind about the qmp-tui thing, here's how you'd split it into a subpackage: ``` %package -n python3-qemu-qmp-tui Summary: Summary here Requires: python3-qemu-qmp = %{version}-%{release} Requires: python3dist(urwid) Requires: python3dist(pygments) Requires: python3dist(urwid) %description -n python3-qemu-qmp-tui %_description Subpackage specific description here [...] %files -n python3-qemu-qmp-tui # The licenses do not need to be included in this subpackage, # as it directly depends on the base package which includes them. %{_bindir}/qmp-tui ``` --- Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU Library General Public License, Version 2.0", "GNU General Public License, Version 2", "*No copyright* GNU General Public License, Version 2 GNU Library General Public License v2 or later", "GNU Library General Public License v2 or later". 51 files have unknown license. Detailed output of licensecheck in /home/gotmax/Sync/git- repos/packaging/fedora_rpms/review.repos/python-qemu-qmp/python-qemu- qmp/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local Python: [x]: Python eggs must not download any dependencies during the build process. [x]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Packages MUST NOT have dependencies (either build-time or runtime) on packages named with the unversioned python- prefix unless no properly versioned package exists. Dependencies on Python packages instead MUST use names beginning with python2- or python3- as appropriate. [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [x]: Reviewer should test that the package builds in mock. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in python3-qemu-qmp [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Rpmlint ------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://people.redhat.com/~jsnow/keys/FAEB9711A12CF475812F18F288A9064D183561EB.txt : CHECKSUM(SHA256) this package : 5df19d762b53e03e0058f6de8c841e96e950cea5699e48ec2593979ebe5c0da5 CHECKSUM(SHA256) upstream package : 5df19d762b53e03e0058f6de8c841e96e950cea5699e48ec2593979ebe5c0da5 https://people.redhat.com/~jsnow/qemu.qmp-0.0.2.dev14+g7f9a044.tar.gz.asc : CHECKSUM(SHA256) this package : 27b5918a7e43cc2eb095aa1e150996a113e26e71111499fb269a0e48d58826a2 CHECKSUM(SHA256) upstream package : 27b5918a7e43cc2eb095aa1e150996a113e26e71111499fb269a0e48d58826a2 https://people.redhat.com/~jsnow/qemu.qmp-0.0.2.dev14+g7f9a044.tar.gz : CHECKSUM(SHA256) this package : a5980b9ca171ec77d7828c744dcec82eb587f4da8ecd69f01cd03f005ef9c2b2 CHECKSUM(SHA256) upstream package : a5980b9ca171ec77d7828c744dcec82eb587f4da8ecd69f01cd03f005ef9c2b2 Requires -------- python3-qemu-qmp (rpmlib, GLIBC filtered): /usr/bin/python3 python(abi) python-qemu-qmp-doc (rpmlib, GLIBC filtered): Provides -------- python3-qemu-qmp: python-qemu-qmp python3-qemu-qmp python3.11-qemu-qmp python3.11dist(qemu-qmp) python3dist(qemu-qmp) python-qemu-qmp-doc: python-qemu-qmp-doc Generated by fedora-review 0.8.0 (e988316) last change: 2022-04-07 Command line :/usr/bin/fedora-review -prn python-qemu-qmp Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: Haskell, fonts, R, SugarActivity, C/C++, Perl, Java, PHP, Ocaml Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
(In reply to Maxwell G from comment #31) > If you change your mind about the qmp-tui thing, here's how you'd split it > into a subpackage: > > ``` > %package -n python3-qemu-qmp-tui > Summary: Summary here > Requires: python3-qemu-qmp = %{version}-%{release} > Requires: python3dist(urwid) > Requires: python3dist(pygments) > Requires: python3dist(urwid) > > %description -n python3-qemu-qmp-tui %_description > > Subpackage specific description here > > > [...] > > %files -n python3-qemu-qmp-tui > # The licenses do not need to be included in this subpackage, > # as it directly depends on the base package which includes them. > %{_bindir}/qmp-tui > ``` Looking at https://gitlab.com/qemu-project/python-qemu-qmp/-/blob/v0.0.1/setup.cfg#L61 there is no need to list the dependencies manually. You can do this instead: %pyproject_extras_subpkg -n python3-qemu-qmp tui %{_bindir}/qmp-tui Or, assuming you want custom description/summary/provides: %package -n python3-qemu-qmp+tui Summary: Summary here Requires: python3-qemu-qmp = %{version}-%{release} Provides: qmp-tui = %{version}-%{release} %description -n python3-qemu-qmp+tui %_description Subpackage specific description here [...] %files -n python3-qemu-qmp+tui %ghost %{python3_sitelib}/*.dist-info %{_bindir}/qmp-tui
(In reply to Maxwell G from comment #31) > This looks good! Feel free to cut a new release. > OK, will do. Might take a few days to get approvals, but there's nothing drastic on the table. Thanks for your time and attention! Apologies for stumbling a little blindly into this. I was overwhelmed. > Note that the tests fail with > > ``` > + avocado --config avocado.cfg run tests/protocol.py > Could not resolve references: tests/protocol.py > ``` > > on f36, presumably due to the outdated avocado version. I didn't build it on > f35, but I assume the same thing would happen there. We have three options > to deal with this: > > 1. Only package this for f37 and f38/rawhide. > 2. Disable tests on f35 and f36 and only run %pyproject_check_import. > 3. Create a non-modular avocado compat package. > Yeah, I assume it's because I require >= 90.0 for avocado. They added new asyncio test-running features especially for me around that version. I'm not keenly aware of precisely how it breaks otherwise - upstream I test on both v90 and "whatever the latest version is". (Aside: I use pipenv upstream to test against a frozen set of dependencies that I deemed to be our oldest supported. Is there a better way? pipenv is not really designed for this task and it's a bit cumbersome to use in that manner. Specifically, surgically updating exactly one dependency without updating others is difficult. I wish pip had an --oldest-possible flag to tweak the resolver...) I think that with the normal update policy, it might be too late to update avocado on f36 (afaiui) ... I think it's okay to just let this package be f37 and beyond. QEMU 7.2 is probably f38 material, though Cole would know best there. I assume f37 will be QEMU 7.1 (which still "vendors"* this library and will cut a final release this month or early next.) (We probably don't need to worry about QEMU 7.2 being packaged for f36 or any derivative thereof. I think. Maybe there are other considerations for RHEL or CentOS, but that's not our problem here today.) > If you change your mind about the qmp-tui thing, here's how you'd split it > into a subpackage: > > ``` > %package -n python3-qemu-qmp-tui > Summary: Summary here > Requires: python3-qemu-qmp = %{version}-%{release} > Requires: python3dist(urwid) > Requires: python3dist(pygments) > Requires: python3dist(urwid) > > %description -n python3-qemu-qmp-tui %_description > > Subpackage specific description here > > > [...] > > %files -n python3-qemu-qmp-tui > # The licenses do not need to be included in this subpackage, > # as it directly depends on the base package which includes them. > %{_bindir}/qmp-tui > ``` Thanks, I'll stash that for my notes. I don't think I'll change my mind for this first release, but it'll be useful to have on hand for v0.1.0. I promise I'll work to eliminate this weirdness ASAP. (qmp-tui will hopefully just replace qmp-shell entirely, but it's not ready to do so yet and isn't feature complete.) Thanks to Miro for his addendum as well. > > --- > > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > > > ===== MUST items ===== > > Generic: > [x]: Package successfully compiles and builds into binary rpms on at least > one supported primary architecture. > Note: Using prebuilt packages > [x]: Package is licensed with an open-source compatible license and meets > other legal requirements as defined in the legal section of Packaging > Guidelines. > [x]: License field in the package spec file matches the actual license. > Note: Checking patched sources after %prep for licenses. Licenses > found: "Unknown or generated", "GNU Library General Public License, > Version 2.0", "GNU General Public License, Version 2", "*No copyright* > GNU General Public License, Version 2 GNU Library General Public > License v2 or later", "GNU Library General Public License v2 or > later". 51 files have unknown license. Detailed output of licensecheck > in /home/gotmax/Sync/git- > repos/packaging/fedora_rpms/review.repos/python-qemu-qmp/python-qemu- > qmp/licensecheck.txt > [x]: License file installed when any subpackage combination is installed. > [x]: If the package is under multiple licenses, the licensing breakdown > must be documented in the spec. > [x]: Package contains no bundled libraries without FPC exception. > [x]: Changelog in prescribed format. > [x]: Sources contain only permissible code or content. > [-]: Package contains desktop file if it is a GUI application. > [-]: Development files must be in a -devel package > [x]: Package uses nothing in %doc for runtime. > [x]: Package consistently uses macros (instead of hard-coded directory > names). > [x]: Package is named according to the Package Naming Guidelines. > [x]: Package does not generate any conflict. > [x]: Package obeys FHS, except libexecdir and /usr/target. > [-]: If the package is a rename of another package, proper Obsoletes and > Provides are present. > [x]: Requires correct, justified where necessary. > [x]: Spec file is legible and written in American English. > [-]: Package contains systemd file(s) if in need. > [x]: Package is not known to require an ExcludeArch tag. > [x]: Large documentation must go in a -doc subpackage. Large could be size > (~1MB) or number of files. > Note: Documentation size is 10240 bytes in 1 files. > [x]: Package complies to the Packaging Guidelines > [x]: Package installs properly. > [x]: Rpmlint is run on all rpms the build produces. > Note: There are rpmlint messages (see attachment). > [x]: If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package is included in %license. > [x]: Package requires other packages for directories it uses. > [x]: Package must own all directories that it creates. > [x]: Package does not own files or directories owned by other packages. > [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT > [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the > beginning of %install. > [x]: Macros in Summary, %description expandable at SRPM build time. > [x]: Dist tag is present. > [x]: Package does not contain duplicates in %files. > [x]: Permissions on files are set properly. > [x]: Package must not depend on deprecated() packages. > [x]: Package use %makeinstall only when make install DESTDIR=... doesn't > work. > [x]: Package is named using only allowed ASCII characters. > [x]: Package does not use a name that already exists. > [x]: Package is not relocatable. > [x]: Sources used to build the package match the upstream source, as > provided in the spec URL. > [x]: Spec file name must match the spec package %{name}, in the format > %{name}.spec. > [x]: File names are valid UTF-8. > [x]: Packages must not store files under /srv, /opt or /usr/local > > Python: > [x]: Python eggs must not download any dependencies during the build > process. > [x]: A package which is used by another package via an egg interface should > provide egg info. > [x]: Package meets the Packaging Guidelines::Python > [x]: Package contains BR: python2-devel or python3-devel > [x]: Packages MUST NOT have dependencies (either build-time or runtime) on > packages named with the unversioned python- prefix unless no properly > versioned package exists. Dependencies on Python packages instead MUST > use names beginning with python2- or python3- as appropriate. > [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files > [x]: Binary eggs must be removed in %prep > > ===== SHOULD items ===== > > Generic: > [x]: Reviewer should test that the package builds in mock. > [-]: If the source package does not include license text(s) as a separate > file from upstream, the packager SHOULD query upstream to include it. > [x]: Final provides and requires are sane (see attachments). > [-]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in > python3-qemu-qmp > [?]: Package functions as described. > [x]: Latest version is packaged. > [x]: Package does not include license text files separate from upstream. > [-]: Package should compile and build into binary rpms on all supported > architectures. > [x]: %check is present and all tests pass. > [x]: Packages should try to preserve timestamps of original installed > files. > [x]: Buildroot is not present > [x]: Package has no %clean section with rm -rf %{buildroot} (or > $RPM_BUILD_ROOT) > [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. > [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file > [x]: Sources can be downloaded from URI in Source: tag > [x]: SourceX is a working URL. > [x]: Sources are verified with gpgverify first in %prep if upstream > publishes signatures. > [x]: Spec use %global instead of %define unless justified. > > ===== EXTRA items ===== > > Generic: > [x]: Rpmlint is run on all installed packages. > Note: There are rpmlint messages (see attachment). > > > Rpmlint > ------- > Cannot parse rpmlint output: > > > Rpmlint (installed packages) > ---------------------------- > Cannot parse rpmlint output: > > > Source checksums > ---------------- > https://people.redhat.com/~jsnow/keys/ > FAEB9711A12CF475812F18F288A9064D183561EB.txt : > CHECKSUM(SHA256) this package : > 5df19d762b53e03e0058f6de8c841e96e950cea5699e48ec2593979ebe5c0da5 > CHECKSUM(SHA256) upstream package : > 5df19d762b53e03e0058f6de8c841e96e950cea5699e48ec2593979ebe5c0da5 > https://people.redhat.com/~jsnow/qemu.qmp-0.0.2.dev14+g7f9a044.tar.gz.asc : > CHECKSUM(SHA256) this package : > 27b5918a7e43cc2eb095aa1e150996a113e26e71111499fb269a0e48d58826a2 > CHECKSUM(SHA256) upstream package : > 27b5918a7e43cc2eb095aa1e150996a113e26e71111499fb269a0e48d58826a2 > https://people.redhat.com/~jsnow/qemu.qmp-0.0.2.dev14+g7f9a044.tar.gz : > CHECKSUM(SHA256) this package : > a5980b9ca171ec77d7828c744dcec82eb587f4da8ecd69f01cd03f005ef9c2b2 > CHECKSUM(SHA256) upstream package : > a5980b9ca171ec77d7828c744dcec82eb587f4da8ecd69f01cd03f005ef9c2b2 > > > Requires > -------- > python3-qemu-qmp (rpmlib, GLIBC filtered): > /usr/bin/python3 > python(abi) > > python-qemu-qmp-doc (rpmlib, GLIBC filtered): > > > > Provides > -------- > python3-qemu-qmp: > python-qemu-qmp > python3-qemu-qmp > python3.11-qemu-qmp > python3.11dist(qemu-qmp) > python3dist(qemu-qmp) > > python-qemu-qmp-doc: > python-qemu-qmp-doc > > > > Generated by fedora-review 0.8.0 (e988316) last change: 2022-04-07 > Command line :/usr/bin/fedora-review -prn python-qemu-qmp > Buildroot used: fedora-rawhide-x86_64 > Active plugins: Generic, Shell-api, Python > Disabled plugins: Haskell, fonts, R, SugarActivity, C/C++, Perl, Java, PHP, > Ocaml > Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH [*] This library came from qemu.git, so it's not quite technically being vendored. I rewrote it in-tree, then forked it out. I'm in the process of modifying our configure/build system to just pull from PyPI when it can, but to tolerate offline builds and rely on system packages for mock builds. When we're happy with that, I'll axe the in-tree version. I'm planning this for 7.2.
Sorry for the delay, just finished moving :) v0.0.2 published upstream: https://pypi.org/project/qemu.qmp/ V5 spec: https://people.redhat.com/~jsnow/v5/python-qemu-qmp.spec V5 SRPM: https://people.redhat.com/~jsnow/v5/python-qemu-qmp-0.0.2-1.fc36.src.rpm Should be functionally equivalent to the v4, but with finalized and public URLs.
> Sorry for the delay, just finished moving :) No problem! I'm not sure if congratulations is the right thing to say to someone who has just moved, but let's go with that :). Thank you for incorporating all of my feedback and being patient. I have approved your package and will sponsor you into the packager group. Look out for an email from me about how to import your package into Fedora. Welcome! --- > (Aside: I use pipenv upstream to test against a frozen set of dependencies that I deemed to be our oldest supported. Is there a better way? pipenv is not really designed for this task and it's a bit cumbersome to use in that manner. Specifically, surgically updating exactly one dependency without updating others is difficult. I wish pip had an --oldest-possible flag to tweak the resolver...) One option is to just build the RPM package in your upstream CI against the versions of Fedora/CentOS Stream/whatever you wish to support. You can setup integration with COPR, use the Packit tool which seems to be gaining popularity, or some sort of homegrown solution. If you go with the third option, I think it's possible to run mock in Gitlab CI if you enable privileged mode for the container and pass `--isolation=simple` to mock.
(In reply to Maxwell G from comment #35) > > Sorry for the delay, just finished moving :) > > No problem! I'm not sure if congratulations is the right thing to say to > someone who has just moved, but let's go with that :). > > Thank you for incorporating all of my feedback and being patient. I have > approved your package and will sponsor you into the packager group. Look out > for an email from me about how to import your package into Fedora. Welcome! > Thank you for your patience in turn :)
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-qemu-qmp
FEDORA-2022-4211709a43 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-4211709a43
FEDORA-2022-4211709a43 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-2c94178162 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-2c94178162
FEDORA-2022-2c94178162 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-2c94178162 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-2c94178162 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-2c94178162 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.