https://bugzilla.redhat.com/show_bug.cgi?id=2439289
--- Comment #4 from Ben Beasley <[email protected]> --- Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - Dist tag is present. OK: fedora-review seems to be confused about %autorelease. - Package does not use a name that already exists. Note: A package with this name already exists. Please check https://src.fedoraproject.org/rpms/biboumi See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Naming/#_conflicting_package_names OK: I acknowledge that this is a review for unretirement. - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in biboumi See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_scriptlets You need something like this: %post %systemd_post biboumi.service %preun %systemd_preun biboumi.service %postun %systemd_postun_with_restart biboumi.service https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd - This path should be written with the %_unitdir macro: %{_prefix}/lib/systemd/system/biboumi.service Write this instead: %{_unitdir}/biboumi.service https://docs.fedoraproject.org/en-US/packaging-guidelines/Systemd/#packaging_filesystem - Please document why you’re packaging a snapshot rather than the latest tagged release. I understand that there’s been a lull in maintainership and a change in upstream. Maybe a link to https://codeberg.org/poezio/biboumi/issues/3516 and perhaps a brief mention of why the changes in https://codeberg.org/poezio/biboumi/src/commit/bf958d0e34e7d8a39dccfcf3c64f72ef0db41f73/CHANGELOG.rst#version-10.0 are worth packaging an unreleased version. - This is incorrect. License: ZLIB The SPDX identifier is Zlib, not ZLIB. https://spdx.org/licenses/Zlib.html This was flagged by rpmlint: biboumi-debuginfo.x86_64: W: invalid-license ZLIB biboumi.x86_64: W: invalid-license ZLIB I found some files under other licenses: - biboumi/cmake/Modules/CodeCoverage.cmake is BSD-3-Clause - the other biboumi/cmake/Modules/*.cmake are LicenseRef-Fedora-Public-Domain These are all build-system files, so they do not affect the License tag, and they are all under acceptable licenses for distribution in the source RPM. However, strictly speaking, you should submit the public-domain files for validation and add them to public-domain-text.txt in fedora-license-data. (There is no doubt that “This file is in the public domain” will be found to be a proper public-domain dedication; this is a low-friction process.) See https://gitlab.com/fedora/legal/fedora-license-data/-/work_items/679 for a similar example. - Whenever possible, replace “make” with %make_build, which expands to something like: /usr/bin/make -O -j${RPM_BUILD_NCPUS} V=1 VERBOSE=1 https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make You can also simplify this: pushd %_vpath_builddir make popd It can be: %make_build -C %_vpath_builddir or, equivalently: %make_build --directory %_vpath_builddir Similarly, all of this: pushd doc sphinx-build . texinfo -b texinfo make man pushd texinfo makeinfo --docbook biboumi.texi popd popd could be: sphinx-build doc doc/texinfo -b texinfo %make_build -C doc man makeinfo --docbook -o doc/texinfo/biboumi.xml doc/texinfo/biboumi.texi or if you prefer long options: sphinx-build doc doc/texinfo --builder texinfo %make_build --directory doc man makeinfo --docbook --output doc/texinfo/biboumi.xml doc/texinfo/biboumi.texi And as with building the tests, pushd %_vpath_builddir make check popd can be: %make_build -C %_vpath_builddir check or, equivalently: %make_build --directory %_vpath_builddir check - Configuration files need to be marked as such. These: %{_sysconfdir}/biboumi/irc.gimp.org.policy.txt %{_sysconfdir}/biboumi/policy.txt should be: %config(noreplace) %{_sysconfdir}/biboumi/irc.gimp.org.policy.txt %config(noreplace) %{_sysconfdir}/biboumi/policy.txt https://docs.fedoraproject.org/en-US/packaging-guidelines/#_configuration_files This was flagged by rpmlint: biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/irc.gimp.org.policy.txt biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/policy.txt - Since the version number in CMakeLists.txt is 10.0, I think this should be treated as a 10.0 pre-release snapshot instead of a 9.0 post-release snapshot, i.e., Version: 10.0~%{commitdate}git%{shortcommit} or possibly Version: 10.0~dev^%{commitdate}git%{shortcommit} to be consistent with https://codeberg.org/poezio/biboumi/src/commit/bf958d0e34e7d8a39dccfcf3c64f72ef0db41f73/CMakeLists.txt#L7 See also https://pagure.io/packaging-committee/issue/1210. - Please pass the “-p” option to “install” anywhere you use it. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_timestamps You can also save a little boilerplate by asking install to create the target directory (“-D”). This part isn’t mandatory. Instead of: mkdir -p %{buildroot}%{_mandir}/man1 install -m644 doc/_build/man/biboumi.1 %{buildroot}%{_mandir}/man1 mkdir -p %{buildroot}%{_datadir}/help/en/biboumi install -m644 doc/texinfo/biboumi.xml %{buildroot}%{_datadir}/help/en/biboumi you can write: install -m644 -p -t %{buildroot}%{_mandir}/man1 -D doc/_build/man/biboumi.1 install -m644 -p -t %{buildroot}%{_datadir}/help/en/biboumi -D \ doc/texinfo/biboumi.xml - The service is configured to run as user “nobody” and group “biboumi”, but we need to ensure that these exist, or are created. The “nobody” user will already be present via soft-static allocation. https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_soft_static_allocation https://src.fedoraproject.org/rpms/setup/blob/f2d2aec19325d0200557c13e607d983e71154aec/f/uidgid#_208 However, you will need to take care of allocating the “biboumi” user. See the following links for documentation: https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation https://www.freedesktop.org/software/systemd/man/latest/sysusers.d.html - These documentation dependencies appear to be unnecessary, and can be removed: BuildRequires: python3dist(docutils) BuildRequires: python3-sphinx_rtd_theme - Dependencies that are discovered via pkgconfig or CMake should be specified by the appropriate pkgconfig(…) or cmake(…) virtual Provides rather than by naming the -devel package. https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/ So these remain unchanged, because they provide .pc files / pkgconfig(…), but the CMake build system doesn’t appear to use it. * botan3-devel * expat-devel This remains unchanged, because it doesn’t provide .pc files: * udns-devel This is supposed to be found using pkgconfig (FindPkgConfig / pkg_check_modules), if available, but the pkg_check_modules is for gcrypt instead of libgcrypt, so this approach fails and it falls back to manual detection. You can leave the BR unchanged; you might suggest to upstream that they check for a pkgconfig module named libgcrypt. * libgcrypt-devel These are found using pkgconfig (FindPkgConfig / pkg_check_modules), so they should be rewritten to reflect that: * libgcrypt-devel → pkgconfig(libgrcypt) * libidn-devel → pkgconfig(libidn) * libuuid-devel → pkgconfig(uuid) * libpq-devel → pkgconfig(libpq) * sqlite-devel → pkgconfig(sqlite3) * systemd-devel → pkgconfig(libsystemd) *HOWEVER*, it turns out that biboumi only uses libgcrypt as a fallback when botan3 is not found. Therefore, in practice, you should remove the BR on libgcrypt-devel/pkgconfig(libgcrypt) altogether. This is found using its CMake configs/targets, so it should be rewritten to reflect that: * catch-devel → cmake(Catch2) This is not needed because it duplicates libpq-devel / pkgconfig(libpq), and nothing needs the internals exposed by postgresql-private-devel (which actually provides postgresql-devel). Please remove it: * postgresql-devel This is kind of pointless, as it’s brought in via gcc-c++, and nearly every C and C++ program requires libc headers (and they generally do not carry an explicit BR for them). Maybe this was added because of the dependency on iconv, which is provided by glibc. I still think it makes sense to drop this, but it’s up to you. Keep it if you like it; it isn’t hurting anything. BuildRequires: glibc-devel - Consider installing the sample configuration file as documentation: %doc conf/biboumi.cfg The CHANGELOG.rst file would also be useful. Similarly, you might consider whether it’s worth installing the reStructuredText documentation sources as documentation, as in the upstream spec file. They are written cleanly enough that they could be useful as plain-text documentation without processing. %doc docs/*.rst - Please consider not building this for i686, since nothing will depend on it there. # https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval ExcludeArch: %{ix86} Besides, this fails to build on i686 due to a segfault in the tests. ==== Notes (no change required for approval) ==== - I suspect that this manual dependency is not really required. Requires: systemd After all, we do install a systemd service file, and we also link libsystemd. Dependency generators should suffice. - This is acceptable, %global shortcommit %(c=%{commit}; echo ${c:0:10}) but you may be interested to know that you can now do the same thing without a subshell: %global shortcommit %{sub %{commit} 1 10} - Since 3541.patch is exactly https://codeberg.org/poezio/biboumi/pulls/3541, I would be inclined to write Patch: https://codeberg.org/poezio/biboumi/pulls/3541.patch but whether this is an improvement is a matter of opinion. The upstream link in the comment already satisfies the packaging guidelines. - RPM now supports a %conf section. It has not been written into the packaging guidelines yet (see https://pagure.io/packaging-committee/issue/1159) but it’s semantically a better place to call %cmake. %conf %cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \ […] %build %cmake_build […] ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. 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. [!]: 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* zlib License", "BSD 3-Clause License", "*No copyright* Public domain". 280 files have unknown license. Detailed output of licensecheck in /home/ben/fedora/review/2439289-biboumi/licensecheck.txt Should be Zlib, not ZLIB [-]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/help/en(thorvg- doc, filesystem, python3-androguard, profanity-doc, python3-autodocsumm, python-x3dh-docs, python3-goocalendar, python3-tablib, libstrophe-doc, qxmpp-doc, python3-questionary, novelwriter-doc, bstring-doc, python3-wand, python3-sphinxcontrib- chapeldomain, python3-pyexiftool, python-twomemo-docs, python-slixmpp- doc, python3-cobalt, python3-colorspacious, python3-xeddsa, python3-pyjson5, python-backcall-doc, python3-doubleratchet, python3-bytecode, python3-junitparser, rauc-doc, manifold-doc, python3-oldmemo) This is an appropriate use of directory co-ownership. If /usr/share/help/*/ directories are really a standardized location for documentation, consider asking to have them added to the filesystem package. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries or specifies bundled libraries with Provides: bundled(<libname>) if unbundling is not possible. [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). (However, it shoud use %{_unitdir}.) [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. [!]: Requires correct, justified where necessary. I think that the explicit “Requires: systemd” should not be needed. [x]: Spec file is legible and written in American English. [!]: Package contains systemd file(s) if in need. The package contains a unit file for the service, but lacks a sysusers file to allocate the biboumi group. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines (except as noted) [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 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]: 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 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 1693 bytes in 1 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [!]: Uses parallel make %{?_smp_mflags} macro. See Issues. [!]: Sources can be downloaded from URI in Source: tag Note: Could not download Source0: https://codeberg.org/poezio/biboumi/archive/bf958d0e34e7d8a39dccfcf3c64f72ef0db41f73.tar.gz#/biboumi- bf958d0e34.tar.gz See: https://docs.fedoraproject.org/en-US/packaging- guidelines/SourceURL/ OK: this works with “spectool -g” [-]: 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. See Issues; please document why we need a snapshot. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments (The URL is fine; fedora-review is confused) [-]: 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. Tested with a scratch build on all architectures except s390x, due to the current s390x builder outage. Tested with a local mock build under qemu-user-static emulation for s390x. Everything was fine except i686; see Issues. [x]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. See Issues; please pass “-p” to “install.” [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]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [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: biboumi-9.0^20260113gitbf958d0e34-1.fc45.x86_64.rpm biboumi-9.0^20260113gitbf958d0e34-1.fc45.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.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 rpmlintrc: [PosixPath('/tmp/tmpx_uxxwfu')] checks: 32, packages: 2 biboumi.src: E: spelling-error ('doesn', '%description -l en_US doesn -> does, does n') biboumi.x86_64: E: spelling-error ('doesn', '%description -l en_US doesn -> does, does n') biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/irc.gimp.org.policy.txt biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/policy.txt biboumi.src: W: invalid-license ZLIB biboumi.x86_64: W: invalid-license ZLIB 2 packages and 0 specfiles checked; 2 errors, 4 warnings, 7 filtered, 2 badness; has taken 0.2 s Rpmlint (debuginfo) ------------------- Checking: biboumi-debuginfo-9.0^20260113gitbf958d0e34-1.fc45.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.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 rpmlintrc: [PosixPath('/tmp/tmp4eefyog5')] checks: 32, packages: 1 biboumi-debuginfo.x86_64: W: invalid-license ZLIB 1 packages and 0 specfiles checked; 0 errors, 1 warnings, 5 filtered, 0 badness; has taken 0.4 s Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.9.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 biboumi.x86_64: E: spelling-error ('doesn', '%description -l en_US doesn -> does, does n') biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/irc.gimp.org.policy.txt biboumi.x86_64: W: non-conffile-in-etc /etc/biboumi/policy.txt biboumi-debuginfo.x86_64: W: invalid-license ZLIB biboumi.x86_64: W: invalid-license ZLIB 2 packages and 0 specfiles checked; 1 errors, 4 warnings, 9 filtered, 1 badness; has taken 0.3 s Requires -------- biboumi (rpmlib, GLIBC filtered): libbotan-3.so.11()(64bit) libc.so.6()(64bit) libexpat.so.1()(64bit) libexpat.so.1(LIBEXPAT_1.0.0)(64bit) libexpat.so.1(LIBEXPAT_1.1.0)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libidn.so.12()(64bit) libidn.so.12(LIBIDN_1.0)(64bit) libpq.so.5()(64bit) libpq.so.5(RHPG_9.6)(64bit) libsqlite3.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.13)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libsystemd.so.0()(64bit) libsystemd.so.0(LIBSYSTEMD_209)(64bit) libudns.so.0()(64bit) libuuid.so.1()(64bit) libuuid.so.1(UUID_1.0)(64bit) rtld(GNU_HASH) systemd Provides -------- biboumi: biboumi biboumi(x86-64) Generated by fedora-review 0.11.0 (05c5b26) last change: 2025-11-29 Command line :/usr/bin/fedora-review -b 2439289 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: Perl, PHP, Ocaml, R, fonts, Haskell, Python, SugarActivity, Java Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2439289 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202439289%23c4 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
