Spec URL: https://download.copr.fedorainfracloud.org/results/carlwgeorge/reviews/fedora-rawhide-x86_64/09671295-spacer/spacer.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/carlwgeorge/reviews/fedora-rawhide-x86_64/09671295-spacer/spacer-0.5.0-1.fc44.src.rpm Description: A CLI utility for adding spacers when command output stops. Fedora Account System Username: carlwgeorge
Copr build: https://copr.fedorainfracloud.org/coprs/build/9671315 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2402625-spacer/fedora-rawhide-x86_64/09671315-spacer/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
This looks good! I suggested packaging README.md as documentation, and I suggested removing duplicate terms from the License expression and writing it across multiple lines instead of one very long line. I also mentioned that you might want to drop the i686 package under https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval. However, none of these blocks the review, so the package is APPROVED as-is. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated The spec file was generated with rust2rpm using --path, simplifying the review. Using rust2rpm --path is deprecated and will be removed in a future version. That does not mean that there is anything wrong with using it now. Compared to the original rust2rpm output: - rpmautospec macros are expanded - the License field has been populated based on the output of %cargo_license_summary - the Source has been filled in - the description has ben de-macro-ized - unnecessary %autosetup arguments have been removed: -n spacer-%{version} -p1 - the file README.md is not packaged as documentation ===== Issues ===== - I think README.md looks useful, and you should package it as documentation (add "%doc README.md" to the %files). - While the License field is formally correct, it would be easier to read, modify, and audit if you removed duplicate terms – (MIT OR Apache-2.0) appears more than once, and it is also redundant with (Apache-2.0 OR MIT) – and wrote it with one term per line, like this: License: %{shrink: MIT AND Apache-2.0 AND ISC AND Unicode-DFS-2016 AND (0BSD OR MIT OR Apache-2.0) AND (Apache-2.0 OR MIT) AND (Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT) AND (MIT OR Zlib OR Apache-2.0) AND (Unlicense OR MIT) } Here I have chosen Fabio Valentini’s usual ordering: the source license term first, then simple single-element terms in alphabetical order, then disjunctive subexpressions in alphabetical order, with duplicate terms removed. The actual order does not matter, except that a sensible order makes it easier to audit for correctness when the %cargo_license_summary output changes. Under https://docs.fedoraproject.org/en-US/legal/license-field/#_special_rules_for_or_expressions, which says “if all the license operands of the OR sub-expression also appear in the license expression outside the OR sub-expression, then you can eliminate the OR sub-expression,” you are technically allowed to drop the (Apache-2.0 OR MIT) AND line from the above expression. I don’t like this rule, and I don’t recommend using it – I think that it takes us too far from the simplicity of “just write down all the licenses” for no significant benefit – but it is permitted. ===== Notes ===== - A man page is always desirable for a command-line tool, https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages, but it’s by no means required to find a way to furnish one. I tested out help2man on spacer and found that it wasn’t able to properly escape Unicode output, so I don’t recommend using it here. - You may choose to go ahead and drop i686 support. It’s best not to do this in Rust libraries when it can be avoided, since they may be pulled into new dependency trees at any time, but for a pure application package like this there is no reason to bother building an i686 package that will never be used. # https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval ExcludeArch: %{ix86} ===== 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", "MIT License". 6 files have unknown license. Detailed output of licensecheck in /home/ben/fedora/review/2402625-spacer/licensecheck.txt [x]: %build honors applicable compiler flags or justifies otherwise. [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]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [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]: The License field must be a valid SPDX expression. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: 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). [x]: Package functions as described. (Tests pass.) [x]: 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. [x]: Package should compile and build into binary rpms on all supported architectures. https://koji.fedoraproject.org/koji/taskinfo?taskID=138064992 [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [-]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define autorelease(e:s:pb:n) %{?-p:0.}%{lua: OK: this an rpmautospec macro expansion, and the advice to never used %define is dubious anyway, https://pagure.io/packaging-committee/issue/1449. [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. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: spacer-0.5.0-1.fc44.aarch64.rpm spacer-0.5.0-1.fc44.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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 rpmlintrc: [PosixPath('/tmp/tmpzb6g0haa')] checks: 32, packages: 2 spacer.aarch64: W: no-manual-page-for-binary spacer spacer.aarch64: W: no-documentation 2 packages and 0 specfiles checked; 0 errors, 2 warnings, 7 filtered, 0 badness; has taken 0.3 s Rpmlint (debuginfo) ------------------- Checking: spacer-debuginfo-0.5.0-1.fc44.aarch64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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 rpmlintrc: [PosixPath('/tmp/tmp4pukfj51')] checks: 32, packages: 1 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 5 filtered, 0 badness; has taken 0.6 s Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.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: 32, packages: 2 spacer.aarch64: W: no-manual-page-for-binary spacer spacer.aarch64: W: no-documentation 2 packages and 0 specfiles checked; 0 errors, 2 warnings, 9 filtered, 0 badness; has taken 0.2 s Source checksums ---------------- https://github.com/samwho/spacer/archive/v0.5.0/spacer-0.5.0.tar.gz : CHECKSUM(SHA256) this package : 310b08c538c04bae779a1c4786430e974801e8880a4c5256dc0877bc82b61af0 CHECKSUM(SHA256) upstream package : 310b08c538c04bae779a1c4786430e974801e8880a4c5256dc0877bc82b61af0 Requires -------- spacer (rpmlib, GLIBC filtered): ld-linux-aarch64.so.1()(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) rtld(GNU_HASH) Provides -------- spacer: spacer spacer(aarch-64) Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2402625 Buildroot used: fedora-rawhide-aarch64 Active plugins: Generic, Shell-api Disabled plugins: SugarActivity, fonts, Perl, C/C++, R, Ocaml, Java, Python, Haskell, PHP Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
The Pagure repository was created at https://src.fedoraproject.org/rpms/spacer
Thanks Ben for the review! I implemented the License and ExcludeArch adjustments as requested. I'll pass on the README.md file for now, as I don't think it provides any value over the program's built-in `--help` output.
FEDORA-2025-e52df2a9ce (spacer-0.5.0-1.fc43) has been submitted as an update to Fedora 43. https://bodhi.fedoraproject.org/updates/FEDORA-2025-e52df2a9ce
FEDORA-2025-9a661873e7 (spacer-0.5.0-1.fc44) has been submitted as an update to Fedora 44. https://bodhi.fedoraproject.org/updates/FEDORA-2025-9a661873e7
FEDORA-2025-9a661873e7 (spacer-0.5.0-1.fc44) has been pushed to the Fedora 44 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2025-e52df2a9ce has been pushed to the Fedora 43 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-e52df2a9ce \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-e52df2a9ce See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-e52df2a9ce (spacer-0.5.0-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report.