Spec URL: https://cottsay.fedorapeople.org/python-flake8-quotes/python-flake8-quotes.spec SRPM URL: https://cottsay.fedorapeople.org/python-flake8-quotes/python-flake8-quotes-3.3.1-1.fc38.src.rpm Description: This package adds flake8 warnings with the prefix Q0: - Q000: Remove bad quotes - Q001: Remove bad quotes from multiline string - Q002: Remove bad quotes from docstring - Q003: Change outer quotes to avoid escaping inner quotes Fedora Account System Username: cottsay Target branches: f36 f37 epel9 Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=94030757 Thanks!
This is an informal review as I'm not a sponsor 1. Define the description as global variable, then use it for both overall description and package description, please take the fedora packaging guide example spec file [a] as reference 2. Use Source instead of Source0, it's more modern and packaging guide have updated to use it, more info could be found https://pagure.io/packaging-committee/pull-request/1157 [a] https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file
Wayne, neither of the things you pointed out is an actual rule. However, I agree those are good suggestions. Could you please follow https://docs.fedoraproject.org/en-US/packaging-guidelines/ReviewGuidelines/ and/or use https://pagure.io/FedoraReview/ ? ----- Scott, I have 3 questions reading the spec: 1. Is "MIT" a SPDX identifier or the old legacy "Callaway" identifier? 2. Would you consider *not* using %srcname at least in URL, for easier copy-paste?
s/I have 3 questions/I have 2 questions/ Sorry about the confusion.
Thanks for the engagement on this review, folks. > Define the description as global variable... Eh, I'll bite. Implemented in the latest revision. > Use Source instead of Source0... This I'm less keen on. I don't believe a change like this provides value alone, and will only cause more changes if an additional source file becomes necessary. Not a hill I'm willing to die on, just a preference. > Is "MIT" a SPDX identifier or the old legacy "Callaway" identifier? It happens to be both. "Modern Style with sublicense"[1]. > Would you consider *not* using %srcname at least in URL... Sure, implemented in the latest revision. I'll make this process change in all of my reviews moving forward. Spec URL: https://cottsay.fedorapeople.org/python-flake8-quotes/python-flake8-quotes.spec SRPM URL: https://cottsay.fedorapeople.org/python-flake8-quotes/python-flake8-quotes-3.3.1-2.fc38.src.rpm [1] https://fedoraproject.org/wiki/Licensing:MIT?rd=Licensing/MIT#Modern_Style_with_sublicense
Here is the result with run fedora-review combined with manual review. I don't find more issues. The %changelog part of update with spec change does follow the fedora guide on https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs The package is build with wheel so I've marked following as not applicable: [-]: Python eggs must not download any dependencies during the build process. [-]: A package which is used by another package via an egg interface should provide egg info. @Miro please help guide on this. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== Generic: [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", "*No copyright* MIT License". [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. [x]: 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. [-]: 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 successfully compiles and builds into binary rpms on at least one supported primary architecture. [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: [-]: Python eggs must not download any dependencies during the build process. [-]: 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]: 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). [?]: Package functions as described. [?]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [?]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [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]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 1 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.0 s
> The package is build with wheel so I've marked following as not applicable: > [-]: Python eggs must not download any dependencies during the build > process. Read this as: "Nothing can download any dependencies during the build process." > [-]: A package which is used by another package via an egg interface should > provide egg info. Read this as: "...should provide dist-info/egg-info metadata"
(In reply to Miro Hrončok from comment #6) > > The package is build with wheel so I've marked following as not applicable: > > [-]: Python eggs must not download any dependencies during the build > > process. > > Read this as: "Nothing can download any dependencies during the build > process." Ok, as check with the setup.py and the build.log it does not download extra dependencies during build, so update to PASS: [x]: Python eggs must not download any dependencies during the build process. > > > [-]: A package which is used by another package via an egg interface should > > provide egg info. > > Read this as: "...should provide dist-info/egg-info metadata" The dist-info is provided after build, so update to PASS: [x]: A package which is used by another package via an egg interface should provide egg info.
> Rpmlint > ------- > Cannot parse rpmlint output: Could you please run rpmlint manually on the spec, srpm and built rpm?
It's also a good practice to review the provides, requires and buildrequires, especially considering many are autogenerated.
My bad, missing the last few parts with copy paste which include Requires and Provides: Source checksums ---------------- https://github.com/zheller/flake8-quotes/archive/3.3.1/flake8-quotes-3.3.1.tar.gz : CHECKSUM(SHA256) this package : 64167f280031a5cce5499487a94ff023a058a70a7d9aba31d27f58e91b6703b8 CHECKSUM(SHA256) upstream package : 64167f280031a5cce5499487a94ff023a058a70a7d9aba31d27f58e91b6703b8 Requires -------- python3-flake8-quotes (rpmlib, GLIBC filtered): python(abi) python3.11dist(flake8) Provides -------- python3-flake8-quotes: python-flake8-quotes python3-flake8-quotes python3.11-flake8-quotes python3.11dist(flake8-quotes) python3dist(flake8-quotes) Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -b 2141871 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: Java, C/C++, Ocaml, SugarActivity, PHP, R, Perl, fonts, Haskell Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH I have run rpmbuild locally and it succeed, here is the log: setting SOURCE_DATE_EPOCH=1668470400 Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.dN66EZ Executing(%generate_buildrequires): /bin/sh -e /var/tmp/rpm-tmp.0G6C02 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.X7hlY1 Processing /home/waynesun/rpmbuild/BUILD/flake8-quotes-3.3.1 Preparing metadata (pyproject.toml): started Preparing metadata (pyproject.toml): finished with status 'done' Building wheels for collected packages: flake8-quotes Building wheel for flake8-quotes (pyproject.toml): started Building wheel for flake8-quotes (pyproject.toml): finished with status 'done' Created wheel for flake8-quotes: filename=flake8_quotes-3.3.1-py3-none-any.whl size=8638 sha256=2c088bfe5711b98eb5d8fc45ddd56bc3abc210c82eb1e7b01938a997d216a9f0 Stored in directory: /home/waynesun/.cache/pip/wheels/ba/fa/fb/6251ec887acf8f57fe5cae69d95710dcf940d7231e67de3b9c Successfully built flake8-quotes Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.1ins2z Using pip 22.2.2 from /home/waynesun/.local/lib/python3.10/site-packages/pip (python 3.10) Looking in links: /home/waynesun/rpmbuild/BUILD/flake8-quotes-3.3.1/pyproject-wheeldir Processing ./pyproject-wheeldir/flake8_quotes-3.3.1-py3-none-any.whl Installing collected packages: flake8_quotes Successfully installed flake8_quotes-3.3.1 removed '/home/waynesun/rpmbuild/BUILDROOT/python-flake8-quotes-3.3.1-2.fc36.x86_64/usr/lib/python3.10/site-packages/flake8_quotes-3.3.1.dist-info/RECORD' removed '/home/waynesun/rpmbuild/BUILDROOT/python-flake8-quotes-3.3.1-2.fc36.x86_64/usr/lib/python3.10/site-packages/flake8_quotes-3.3.1.dist-info/REQUESTED' Bytecompiling .py files below /home/waynesun/rpmbuild/BUILDROOT/python-flake8-quotes-3.3.1-2.fc36.x86_64/usr/lib/python3.10 using python3.10 Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.emPAbF ============================= test session starts ============================== platform linux -- Python 3.10.8, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 rootdir: /home/waynesun/rpmbuild/BUILD/flake8-quotes-3.3.1 plugins: cov-3.0.0 collected 28 items test/test_checks.py ..................... [ 75%] test/test_docstring_checks.py .... [ 89%] test/test_docstring_detection.py ... [100%] ============================== 28 passed in 0.27s ============================== Processing files: python3-flake8-quotes-3.3.1-2.fc36.noarch Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.47noa6 Provides: python-flake8-quotes = 3.3.1-2.fc36 python3-flake8-quotes = 3.3.1-2.fc36 python3.10-flake8-quotes = 3.3.1-2.fc36 python3.10dist(flake8-quotes) = 3.3.1 python3dist(flake8-quotes) = 3.3.1 Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 Requires: python(abi) = 3.10 python3.10dist(flake8) Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/waynesun/rpmbuild/BUILDROOT/python-flake8-quotes-3.3.1-2.fc36.x86_64 Wrote: /home/waynesun/rpmbuild/SRPMS/python-flake8-quotes-3.3.1-2.fc36.src.rpm Wrote: /home/waynesun/rpmbuild/RPMS/noarch/python3-flake8-quotes-3.3.1-2.fc36.noarch.rpm Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.j5KmKC
Unfortunately, fedora-review does not show buildrequires and strips important version information from provides. No need to build the package locally, fedora-review does that (in mock). I see nothing wrong here. So, let's approve the package. I will now do that on your behalf, until you are sponsored.
Thanks very much for the reviews!
FEDORA-2022-e754f3f2c6 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-e754f3f2c6
FEDORA-2022-e754f3f2c6 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-e7949ce462 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-e7949ce462
FEDORA-2022-4ca6fddec0 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-4ca6fddec0
FEDORA-EPEL-2022-45d75b9197 has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-45d75b9197
FEDORA-2022-4ca6fddec0 has been pushed to the Fedora 36 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-4ca6fddec0 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-4ca6fddec0 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2022-45d75b9197 has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-45d75b9197 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-e7949ce462 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-e7949ce462 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-e7949ce462 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-e7949ce462 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-4ca6fddec0 has been pushed to the Fedora 36 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2022-45d75b9197 has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.