This is my first package and I am seeking a sponsor. * Spec URL: https://pagure.io/bees/raw/f40/f/bees.spec * SRPM URL: https://pagure.io/bees/blob/f40/f/rpmbuild/SRPM/bees-0.10-1.fc40.src.rpm * Description: bees is a block-oriented userspace deduplication agent designed for large btrfs filesystems. It is an offline dedupe combined with an incremental data scan capability to minimize time data spends on disk from write to dedupe. (https://github.com/Zygo/bees) * Fedora Account System Username: kylegospo This package is also hosted on COPR and has been built for F39, F40, and Rawhide. https://copr.fedorainfracloud.org/coprs/kylegospo/bees/ https://koji.fedoraproject.org/koji/taskinfo?taskID=121048064 https://koji.fedoraproject.org/koji/taskinfo?taskID=121048311 This package has a patch to fix the default build rules being too strict, a PR for which can be found at: https://github.com/Zygo/bees/pull/286 Reproducible: Always
Hi Kyle, welcome to Fedora! I'm happy to sponsor you, of course. I recommend joining the Fedora development list and introducing yourself (https://docs.fedoraproject.org/en-US/package-maintainers/Joining_the_Package_Maintainers/#join_the_important_mailing_lists) and also if you're not already on Matrix joining at least Fedora's development channel there. There's a web client at https://chat.fedoraproject.org/ and you can log in with your FAS account. Part of being a packager is being familiar with the packaging guidelines and reviewing other packager's submissions. Before I sponsor you into the packager group, could you perform a couple reviews on some recent package submissions that haven't been accepted yet? You can query Bugzilla on the Package Review component or look through the mailing list (https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org/) for recent submissions. Be sure to note in your review that it's not an "official" review, and post links here when you've completed them. If you've got any questions on the process don't hesitate to ask! Overall, the package looks very good, there's just a few minor things that need tweaking. Below is the output from the `fedora-review` tool with some notes from me in the "Issues" section. fedora-review is pretty helpful in giving reviewers a list of things to check, but I also use it when packaging to make sure I'm not missing anything. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - You've got the BuildRequires on systemd-rpm-macros, but we need to use them in the various scriptlets. This ships a systemd service template and I am actually not sure how the scriptlets work with those, so I'm going to investigate this and get back to you. For reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets - Package depends on btrfs-progs and util-linux at runtime according to https://github.com/Zygo/bees/blob/master/docs/install.md#dependencies. You'll need to add an explicit `Requires: ` for both. - rpmlint is upset about the length of the description, can you line wrap it to ~80 characters? - Is it possible to run the upstream tests in a %check section, or are the requirements difficult/impossible to meet in the build environment? Ideally, we run the tests, or a subset of them that do work in the build environment, but if that's not doable we should include a comment on the reason why. - In the %files section, you should own the bees directories in etc and libexec. Changing the entries to the following should work: %{_libexecdir}/bees %{_sysconfdir}/bees - The specfile in the srpm and at the URL are _slightly_ different (there's no trailing newline in one of them) ===== 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]: Header files in -devel subpackage, if present. [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. [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", "zlib License". 94 files have unknown license. Detailed output of licensecheck in /home/jcline/packaging/2299972-bees/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. [!]: Package requires other packages for directories it uses. Note: No known owner of /usr/libexec/bees, /etc/bees [!]: Package must own all directories that it creates. Note: Directories without known owners: /etc/bees, /usr/libexec/bees [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. [!]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: 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 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 2362 bytes in 1 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). [-]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: 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. [!]: %check is present and all tests pass. [x]: 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]: Fully versioned dependency in subpackages if applicable. [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: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL) [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. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/Zygo/bees/archive/refs/tags/v0.10.tar.gz : CHECKSUM(SHA256) this package : d100efbc6084f494400892ef53fa476fd6f201dba3b2fddee11ef90dd9d6111d CHECKSUM(SHA256) upstream package : d100efbc6084f494400892ef53fa476fd6f201dba3b2fddee11ef90dd9d6111d Requires -------- bees (rpmlib, GLIBC filtered): /usr/bin/bash libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libm.so.6()(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.15)(64bit) libstdc++.so.6(CXXABI_1.3.3)(64bit) libstdc++.so.6(CXXABI_1.3.7)(64bit) rtld(GNU_HASH) bees-debuginfo (rpmlib, GLIBC filtered): bees-debugsource (rpmlib, GLIBC filtered): Provides -------- bees: bees bees(x86-64) bees-debuginfo: bees-debuginfo bees-debuginfo(x86-64) debuginfo(build-id) bees-debugsource: bees-debugsource bees-debugsource(x86-64) Diff spec file in url and in SRPM --------------------------------- --- /home/jcline/packaging/2299972-bees/srpm/bees.spec 2024-07-25 16:58:29.649575466 -0400 +++ /home/jcline/packaging/2299972-bees/srpm-unpacked/bees.spec 2024-07-22 20:00:00.000000000 -0400 @@ -39,3 +39,3 @@ %changelog -%autochangelog \ No newline at end of file +%autochangelog Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2299972 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: SugarActivity, PHP, Java, fonts, Haskell, Perl, R, Python, Ocaml Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
I've done some looking around and the current systemd scriptlets don't totally support templates. What you'll want is: %post %systemd_post 'bees@.service' %preun %systemd_preun 'bees@*.service' %postun %systemd_postun_with_restart 'bees@*.service' However, the uninstall case currently doesn't handle the glob. This was supposed to be fixed in systemd 256 (https://github.com/systemd/systemd/blob/eef4cd51f94d837bd0e71512c831634a2902522d/NEWS#L689) but seems to still not work in this particular case. Still, with a little work on the systemd side the above will at some point soon work, so that's what I'd recommend instead of trying to work around the issue.
Thanks for the review and the warm welcome! I've added the systemd macros and the missing new line at the end of the spec file. * Spec URL: https://pagure.io/bees/raw/f40/f/bees.spec * SRPM URL: https://pagure.io/bees/blob/f40/f/rpmbuild/SRPM/bees-0.10-1.fc40.src.rpm * F40 Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=121265713 I am working on those reviews and will link them here asap. Let me know if anything else needs adjustment, and thanks again.
Copr build: https://copr.fedorainfracloud.org/coprs/build/7806541 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2299972-bees/fedora-rawhide-x86_64/07806541-bees/fedora-review/review.txt Found issues: - Systemd service file(s) in bees Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets Please know that there can be false-positives. --- 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.
The remaining requested changes have been made as well as some others. * Line breaks added to shorten the description * Git removed as a BuildDependency * localconf now used per project's makefile * BEES_VERSION corrected with prefixed v * Tests added in %check * Missing wildcard in post systemd step fixed * Folder name specified in %files without wildcard The tests are both built and ran in check, but I can adjust that so that they're built in the build step and only ran in the check step if that's incorrect. I also didn't see a macro for make test so please let me know if I'm missing something. * Spec URL: https://pagure.io/bees/raw/f40/f/bees.spec * SRPM URL: https://pagure.io/bees/blob/f40/f/rpmbuild/SRPM/bees-0.10-1.fc40.src.rpm * F40 Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=121334742
Created attachment 2043204 [details] The .spec file difference from Copr build 7806541 to 7810932
Copr build: https://copr.fedorainfracloud.org/coprs/build/7810932 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2299972-bees/fedora-rawhide-x86_64/07810932-bees/fedora-review/review.txt Found issues: - Systemd service file(s) in bees Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets Please know that there can be false-positives. --- 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.
Requested reviews can be found at: https://bugzilla.redhat.com/show_bug.cgi?id=2302539 https://bugzilla.redhat.com/show_bug.cgi?id=2302515 The 2nd one is partially complete due to an inability to build, but I will wrap that up when my questions are answered.
Spec review notes: > Source: %{url}/archive/refs/tags/v%{version}.tar.gz This should use the form "%{url}/archive/v%{version}/%{name}-%{version}.tar.gz" > cat <<EOF > localconf > BEES_VERSION=v%{version} > DEFAULT_MAKE_TARGET=all > LIBEXEC_PREFIX=%{_libexecdir}/%{name} > LIB_PREFIX=%{_libdir} > PREFIX=%{_prefix} > SYSTEMD_SYSTEM_UNIT_DIR=%{_unitdir} > EOF This should be in the "%conf" section in between %prep and %build. Cf. https://rpm-software-management.github.io/rpm/manual/spec.html#conf-since-rpm--418
Hi Neal! Thanks for the review, those changes are now applied. * Spec URL: https://pagure.io/bees/raw/f40/f/bees.spec * SRPM URL: https://pagure.io/bees/blob/f40/f/rpmbuild/SRPM/bees-0.10-1.fc40.src.rpm * F40 Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=121574583 Lastly, review at https://bugzilla.redhat.com/show_bug.cgi?id=2302515 is now complete.
Sorry for the delay getting back to this (and thanks Neal for the review as well)! There's only one thing I spot, which is that the /etc/bees directory is unowned[0]. You can mark it as part of the package by adding: %{_sysconfdir}/%{name}/ Everything else looks solid to me. Thanks for doing those reviews. My only notes are that the feedback from Ben in https://bugzilla.redhat.com/show_bug.cgi?id=2302515#c4 is spot on and in the future you'll want to do that. Finally, thanks for sticking with the process! Once you address the unowned directory, I'll give it one last look over, approve the package, and sponsor you into the packager group. [0] https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/
Hi Jeremy, that change is now completed! I'll be sure to thank Ben for the feedback as well. * Spec URL: https://pagure.io/bees/raw/f40/f/bees.spec * SRPM URL: https://pagure.io/bees/blob/f40/f/rpmbuild/SRPM/bees-0.10-1.fc40.src.rpm * F40 Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=121900587
Thanks for all your hard work here, I think this is ready to go. The next steps will be for me to add you to the packager group, and then importing the package to dist-git and building it for Rawhide. We can walk through that together and if you have any questions in the future I'm here to help.
The Pagure repository was created at https://src.fedoraproject.org/rpms/bees
FEDORA-2024-87701c5d16 (bees-0.10-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2024-87701c5d16
FEDORA-2024-87701c5d16 (bees-0.10-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.