Package: sshguard 2.4.0-8 spec: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard/fedora-rawhide-x86_64/01039802-sshguard/sshguard.spec srpm: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard/fedora-rawhide-x86_64/01039802-sshguard/sshguard-2.4.0-8.fc32.src.rpm rpmlint (f30, x64_86): no errors, some warnings I consider false positives I'm also looking for a sponsor. About the package: sshguard detects & blocks brute-force attempts on common services by monitoring logs. Not as flexible as fail2ban, but extremely easy to configure and supports pretty much any IP blocking backend one might think of.
Hello, I'm a Fedora packager and I'll do this review. I'll test it and I'll post my findings no later than overmorrow.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= 1) The package does not own '/usr/libexec/sshguard' directory, but it should. 2) Systemd X SysVinit The package has an option to not build sysvinit stuff. Which it fine - we want systemd files in Fedora instead. However the condition on the first 3 lines is broken. On Fedora it will expand to "0 <= 6", which is always TRUE, so it will always pack sysvinit stuff over systemd stuff. 2.1) Fix the condition 2.2) Logrotate is used only with sysvinit and not with systemd - is that an intention? 2.3) When the package is build with systemd service instead of sysvinit script, do you think it still worth to ship the example '/usr/share/doc/sshguard/sshguard.service' file, even if it's nearly the same as the actual service file? 2.4) Please note, that the base package contains systemd service file, but the package does not require systemd. Thus it can end up in a state, when it is installed, but systemd is not. If the main functionality remains even without systemd, it's fine. If the functionality depends on the service, you need to fix the package requirements. Please check if it's OK. Also note that the e.g. teh firewalld subpackage depends on firewalld which depends on systemd ... so in that case the systemd will be pulled in. 2.5) The systemd service contains e.g. "After=firewalld.service". If the service is not present or not started, this won't have any effect. Thus you can end up with sshguard service running but firewalld service not running; and an error message in the systemd journal: "sshguard[1518]: sshg-fw-firewalld: Could not initialize firewall" Check if that's OK. 3) I'd suggest to have every changelog entry (each header) separated by a newline, to have it consistent with both itself and all other pkgs in Fedora. 4) I saw two bundled libraries that I suspect they are bundled, can you please confirm? * simclist library? * Fowler/Noll/Vo Hash (fnv) library ? They are mentioned here too: https://bitbucket.org/sshguard/sshguard/src/036efe21bd46122fde9d3d85aa71ee72b4c8d7e4/COPYING And i see them amongst the debugsource files, so they were used during the build process. If they are packed in Fedora, those packages MUST be used instead. If they are not packaged in Fedora, you MUST pack them too. Only in very special cases, when neither of those two steps above are a good solution, you may bundle it. But in such case you MUST mark that the package provide those bundles. ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [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]: 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. "sshguard is available under the terms of the OpenBSD license, which is based on the ISC License." The "OpenBSD" license is not specified here: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses, so it need aditional check. [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/sshguard [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/libexec/sshguard [x]: %build honors applicable compiler flags or justifies otherwise. [!]: 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. [?]: 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 6 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]: 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 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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [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 ===== 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. [!]: Final provides and requires are sane. Reviewer note: NOT sane until packed with SysVinit [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in sshguard-iptables , sshguard-firewalld , sshguard-nftables [?]: Package functions as described. [X]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [?]: Scriptlets must be sane, if used. [x]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. https://koji.fedoraproject.org/koji/taskinfo?taskID=37927883 [x]: %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]: 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. [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. Generated by fedora-review 0.7.3 (44b83c7) last change: 2019-09-18 Command line :/usr/bin/fedora-review -n sshguard Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, SugarActivity, R, Python, PHP, fonts, Ocaml, Perl, Haskell Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Please take a look at the issues I uncovered. After you fix them, I will do the review again, adding the rest of the review and attachements.
Hi Michal, thanks for the review & detailed feedback. Issues: ======= 1) The package does not own '/usr/libexec/sshguard' directory Reply: Fixed. 2) systemd vs sysvinit 2.1) Fix the condition Reply: I swear, one day I will wtite a flawless rpm macro ... I reverted it to checking for el6, specifically. I think the package will also work on older RHELs, but I have never checked. 2.2) Logrotate is used only with sysvinit and not with systemd - is that an intention? Reply: Yes. Usually, sshguard expects to run with systemd and logs via journald. I only have it log to a file on SysVInit. 2.3) When the package is build with systemd service instead of sysvinit script, do you think it still worth to ship the example [...] file [...]? Reply: You're right. I don't remember why I switched to creating my own service file. I changed it back to using upstream's example. 2.4) Please note, that the base package contains systemd service file, but the package does not require systemd. [...] Reply: Fixed. It should require systemd unless run on a sysvinit system. 2.5) The systemd service contains e.g. "After=firewalld.service". If the service is not present or not started, this won't have any effect. [...] Check if that's OK. Reply: I'd say it's OK, but that is debatable. The After= lines are really just there to ensure proper ordering IF the backend is present. If people use the backend-subpackages, the appropriate service will be pulled in as a dependency. If they're not, they will in any case have to configure sshguard themselves, including which backend to use, so I think it's fair to assume they'll also install that backend, or if not, understand what went wrong. Please advise. 3) I'd suggest to have every changelog entry (each header) separated by a newline [...]. Reply: Fixed. 4) I saw two bundled libraries that I suspect they are bundled, can you please confirm? Reply: Yes, that seems to be the case, I overlooked that. I don't think they're in Fedora, I'll package them separately if necessary and get back to you. Should I open a new review request for each and link to them from here? spec correcting {1-3): https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard-testing/fedora-rawhide-x86_64/01042441-sshguard/sshguard.spec diff: https://gitlab.com/lcts-rpm/sshguard/compare/sshguard-2.4.0-8...master src package: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard-testing/fedora-rawhide-x86_64/01042441-sshguard/sshguard-2.4.0-8.git.5.9154ef5.fc32.src.rpm I'll make a new package release once I've taken care of (4).
2.5) Personally, I see as the most important having an usefull error message on a expected and easy-to-find place. The program provides such message, so I'm absolutelly fine with that. I primarly wanted to find out if that's expected. 4) > Should I open a new review request for each and link to them from here? If you would pack them to Fedora, definitelly yes - it needs a standard new package review. I took a look at the libraries. I grepped SPECfiles of all of the pcakges in Rawhide; tarball of just those SPECs can be found here: http://src.fedoraproject.org/repo/rpm-specs-latest.tar.xz (Mentioning just in case you'd find it handy later in your packager life) The 'fnv' library is packed in Fedora only as a rust version, which won't help us much. https://src.fedoraproject.org/rpms/rust-fnv However ... as I'm strolling through the author's pages (http://www.isthe.com/chongo/tech/comp/fnv/#FNV-reference-source), I see the latest version: fnv-5.0.3.tar.gz [updated: 2012 May 20] so it's not upated much often :) In this case, I'd be fine with bundling it; even though it's a MUST to properly mark it as bundled in the SPEC. With a short justification. There is another package which bundles the 'simclist' library: https://src.fedoraproject.org/rpms/pcsc-lite-acsccid/blob/master/f/pcsc-lite-acsccid.spec#_22 It's not clear from the SPEC, but in its pcakage review, the reason is stated as purpose not strong enough to pack it as a standalone package. UPDATE: * I've contacted it't maintainer and ve uncovered, there's were more packages which were missing the mention of the bundling. Since the 'simclist' is not updated from 2010, I'm fine to bundle it too. http://mij.oltrelinux.com/devel/simclist/#downloadinstall http://mij.oltrelinux.com/devel/simclist/simclist-1.5-changes.txt This should be reconsidered once more packages that would require it would appear. UPDATE: * The latest upstream version is 1.5, however there are project on the internet (and in Fedora and Debian too), which has version 1.6, that appeared ... somewhere. Hopefully as a typo, but it may be worth deeper investigation and eventually need to ask upstream to release a bumped version to keep the sync. -- Mark the bundles correctly for both packages. After that I'll re-do the review. But it looks promising now :)
*** Bug 1260845 has been marked as a duplicate of this bug. ***
Hello Robert-AndrΓ© Mauchin. I belive this BZ doesn't deserve "FE-DEADREVIEW" status since it is days old and actively worked on. Haven't you meant different BZ? I did set the "FE-NEEDSPONSOR" as the reporter stated that he is looking for a sponsor. -- I removed the "FE-DEADREVIEW" flag. If you set it intentionaly to this BZ, please provide further explanation. Thank you
(In reply to Michal Schorm from comment #7) > I belive this BZ doesn't deserve "FE-DEADREVIEW" status since it is days old > and actively worked on. > Haven't you meant different BZ? > I did set the "FE-NEEDSPONSOR" as the reporter stated that he is looking for > a sponsor. > -- > I removed the "FE-DEADREVIEW" flag. > If you set it intentionaly to this BZ, please provide further explanation. > Thank you Hi Michal, I believe DEADREVIEW was accidentally copied from the duplicate bug (my original submission from 2015 that languished without reviewers for years). Clearing it on this one is correct. Thanks Michal and Christopher for taking this on. Best, Conrad
Sorry I don't know why DEADREVIEW was copied from the duplicate.
Nevermind :)
Thanks for sniffing around for fnv/simclist, also for setting FE-NEEDSPONSOR, I forgot that. I've added fnv & simclist as bundled provides. spec: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard/fedora-rawhide-x86_64/01042870-sshguard/sshguard.spec diff 2.4.0-8 -> 2.4.0-9: https://gitlab.com/lcts-rpm/sshguard/compare/sshguard-2.4.0-8...sshguard-2.4.0-9 src.rpm: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard/fedora-rawhide-x86_64/01042870-sshguard/sshguard-2.4.0-9.fc32.src.rpm fnv: sshguard doesn't bundle the entire library, only one specific 32bit hash function. Is it still OK to use bundled(fnv)? simclist: The 1.6 version of simclist most likely comes from the author's page on sourceforge [1], it was released in 2011, so similarly ancient. I'll contact the dev to verify. [1] http://freshmeat.sourceforge.net/projects/simclist
Alright, I went through the review again. This time I also checked & compared the RPM built by upstream. ----- 1) Requires & Provides Once it started building for systemd correctly, the Requires and Provides are sane now. Checked in a directory with only resulting rpms: # for i in `ls -1`; do echo -e "\t$i" ; rpm -q --requires $i ; echo " " ; done # for i in `ls -1`; do echo -e "\t$i" ; rpm -q --provides $i ; echo " " ; done I won't paste it whole here, because I don't see the point in that. Just the highlights: Requires for sshguard-2.4.0-9.fc32.x86_64.rpm systemd Requires for sshguard-firewalld-2.4.0-9.fc32.x86_64.rpm firewalld sshguard Requires for sshguard-iptables-2.4.0-9.fc32.x86_64.rpm iptables-services sshguard Requires for sshguard-nftables-2.4.0-9.fc32.x86_64.rpm nftables sshguard Provides of sshguard-2.4.0-9.fc32.x86_64.rpm bundled(fnv) = 5.0.2 bundled(simclist) = 1.4.4 1.1) However, when checking up the RPM package built by the sshguard upstream, the sshguard package also requires: coreutils diffutils fillup grep openssh The first four probabbly as a dependencies of the shell scripts. Please check if they are needed and if yes, add those requires. 1.2) One more suggestion: since you are using systemd in scriptlets, you should also use: %{?systemd_requires} in the main package, which is macro that expands to: Requires(post): systemd Requires(preun): systemd Requires(postun): systemd That will guarantee, the systemd will be there also at the time the scriptlets run, not just when the package is installed on the system. ----- 2) Files & file ownership All files are now owned by the package as they should be. 2.1) However, the upstream package has better structured docs. usr βββ share βββ doc Β Β βββ packages Β Β βββ sshguard Β Β βββ CHANGELOG.rst Β Β βββ doc Β Β βΒ Β βββ sshguard.8 Β Β βΒ Β βββ sshguard.8.rst Β Β βΒ Β βββ sshguard-setup.7 Β Β βΒ Β βββ sshguard-setup.7.rst Β Β βββ examples Β Β βΒ Β βββ net.sshguard.plist Β Β βΒ Β βββ sshguard.conf.sample Β Β βΒ Β βββ sshguard.service Β Β βΒ Β βββ whitelistfile.example Β Β βββ README.rst I'd follow them and add an "examples" folder 2.2) What concerns me, the upstream package also owns βββ var βββ lib βββ sshguard βββ db "/var/lib/<package>" holds "State data for packages and subsystems (optional)." Please check, if the package need to save some data like this and if it really needs theese directories. If yes, create and add them. 2.3) Also the config file from the upstream upstream package is IMHO more helpful, describing more options. I'd love to see it in Fedora too. /etc/sshguard.conf | #### OPTIONS #### | # Block attackers when their cumulative attack score exceeds THRESHOLD. | # Most attacks have a score of 10. (optional, default 30) | THRESHOLD=30 | | # Block attackers for initially BLOCK_TIME seconds after exceeding THRESHOLD. | # Subsequent blocks increase by a factor of 1.5. (optional, default 120) | BLOCK_TIME=120 | | # Remember potential attackers for up to DETECTION_TIME seconds before | # resetting their score. (optional, default 1800) | DETECTION_TIME=1800 | | # Size of IPv6 'subnet to block. Defaults to a single address, CIDR notation. (optional, default to 128) | IPV6_SUBNET=128 | | # Size of IPv4 subnet to block. Defaults to a single address, CIDR notation. (optional, default to 32) | IPV4_SUBNET=32 | | #### EXTRAS #### | # !! Warning: These features may not work correctly with sandboxing. !! | | # Full path to PID file (optional, no default) | PID_FILE="/run/sshguard.pid" | | # Colon-separated blacklist threshold and full path to blacklist file. | # (optional, no default) | BLACKLIST_FILE="90:/var/lib/sshguard/db/blacklist.db" | | # IP addresses listed in the WHITELIST_FILE are considered to be | # friendlies and will never be blocked. | WHITELIST_FILE="/etc/sshguard/whitelist" 2.4) And the config also point at the whitelist, which holds nice description and example of what and how to configure there. Also a very nice possible feature for this Fedora package. /etc/sshguard/whitelist | # comment line (a '#' as very first character) | # a single ip address | #1.2.3.4 | # address blocks in CIDR notation | #127.0.0.0/8 | #10.11.128.0/17 | #192.168.0.0/24 | # hostnames | #rome-fw.enterprise.com | #hosts.friends.com | # 2.5) Also, you pack a file "/usr/share/doc/sshguard/net.sshguard.plist", which is IMHO for MacOS and thus is completely irrelevant to Fedora (thus shouldn't be there): https://fileinfo.com/extension/plist Please verify my finding and if I'm correct, don't pack the file. 2.6) Please, prefix the directory you pack (line 193) with "%dir", like: %dir %{_libexecdir}/%{name} ----- 3) > simclist: The 1.6 version of simclist most likely comes from the author's page on sourceforge [1], it was released in 2011, so similarly ancient. > I'll contact the dev to verify. That would be awesome. ----- From my POV the packages becomes slowly more or less ready to be included in Fedora. However beacuse the review isn't just about package GO/NOGO, but also it should show your understanding of the packager job and packaging guidelines (and ability to comply with them) - especcialy if you are looking for a sponsorship - I won't grant you the "fedora-review +" ACK yet. Let's take a look at what I found this time. 4) Also can you share some script (, setup or instructions), how do you test the package works as expected - blocking the attack over ssh? I'd be interested :) It could be then added to the Fedora Continuous Integration Testing (https://docs.fedoraproject.org/en-US/ci/), once the package would get into Fedora
Sorry for the delay, work intruded. Also, thanks again for the time & effort you put into this. - Just to clarify, afaik the packages linked on the sshguard website are not built/maintained by upstream, though the debian package might be derived from their 'debian' branch. - Regarding 1.1, is there something similar to Arch's base-devel group, i.e. a set of packages that forms a standard build environment and is thus assumed to be present at buildtime and thus not listed explicitly? Anything using a configure script is likely to require coreutils and diffutils for example, but I don't usually see them as builddeps. spec: https://gitlab.com/lcts-rpm/sshguard/raw/sshguard-2.4.0-10/sshguard.spec diff: https://gitlab.com/lcts-rpm/sshguard/compare/sshguard-2.4.0-9...sshguard-2.4.0-10 srpm: https://copr-be.cloud.fedoraproject.org/results/lcts/sshguard/fedora-rawhide-x86_64/01045896-sshguard/sshguard-2.4.0-10.fc32.src.rpm 1.1) Additional dependencies. fixed. coreutils - 'echo' is used in /usr/sbin/sshguard diffutils - 'diff' and 'cmp' are used at buildtime (in 'configure' and 'ylwrap', respectively), see question above fillup - not needed. opensuse-specific dependency, I don't know what they use it for. grep - grep is used by /usr/sbin/sshguard openssh - not needed. People might in fact want to monitor e.g. ftp on a host without (a need for) ssh so I'd say it's bad to include this as a dependency. If ssh is missing/not running, the configs if unmodified will cause sshguard to monitor an empty log, which it is perfectly happy to do 1.2) %{?systemd_requires} fixed - nice, learned something new. 2.1) the upstream package has better structured docs. fixed: %{_pkgdocdir}/sshguard/ |- CONTRIBUTING.rst |- README.rst |- examples/ |- sshguard.conf.sample |- whitelistfile.example 2.2) /var/lib/sshguard/db ownership - used by the blacklist feature in the opensuse package, but not expected or created by sshguard on its own - not upstream's suggested path for the blacklist, they put it directly in /var/lib/sshguard/. According to the spec .../db is an opensuse default - see 2.4 2.3) better config file fixed - I adapted examples/sshguard.conf.sample, which is extensively commented 2.4) white/blacklisting I decided to set up the package so that it is prepared for this feature, but leaves it disabled. That way, a user can simply uncomment the relevant lines in the config file without further configuration - added /etc/sshguard.whitelist containing only commented instructions for use - added /var/lib/sshguard as package-owned dir - preset blacklist file to /var/lib/sshguard/blacklist, which will be created by sshguard if bl is enabled. It's not owned by the package, so RPM will delete the folder if the user didn't enable it, but retain the blacklist file if they did 2.5) net.sshguard.plist correct, removed 2.6) prefix the directory you pack with "%dir" fixed 3) simclist I haven't heard back yet. 4) CI testing It's a bit convoluted right now: - 'make check' already checks that all log parsing / pattern matching is working as expected - GitLab CI (config: https://gitlab.com/lcts-rpm/sshguard/blob/master/.gitlab-ci.yml) tests installation and basic functionality - set up minimal docker installations of the various official CentOS & Fedora images on docker hub - build & install the (sub-)package(s) - test that systemctl/service start|stop|status works without issues - lint the packages - sshguard-testing repo on copr tests that the build also works on other architectures - I have a tiny cloud server that uses those test builds and checks that the package is safe to use (for 'checks', read: it installs & uses the test build and I test if I can lock myself out etc.) GitLab CI is triggered by commits, test builds on copr by commits to master, test server updates via dnf-automatic. It should not be too difficult to integrate all that into a single pipeline - I only use copr+gitlab ci because the latter only does x86_64. Simulating an attack programmatically is certainly possible. If ssh is available, one could probably just try to log into localhost with fake credentials to trigger a block. I'll have to test that a bit. I'll have a look at fedora's CI ... it uses ansible, so that's a plus already ...
In Fedora, we have a "minimal buildroot". Set of packages that are always present for any build. Currectly, it consist of ~150 packages and you can check them e.g. in mock, when you init a rawhide environment, then you shell inside and run "rpm -qa". I'm, however, not aware of any "minimal root". You can install your package alone, by e.g. "dnf install --releasever="30" --installroot="/tmp/empty_test_area/" sshguard-2.4.0-10.fc32.x86_64.rpm". It will pull in tree of the dependencies you specified, but I wouldn't trust that all of the runtime dependecies your package need will be magically pulled in. Let's say: 'mkfs' a very usefull and standard utility. If you'd depend on the feeling, all mkfs binaries are from the same package, you'd be mistaken. Just run "rpm -qf /usr/sbin/mkfs*" to see yourself. Last real-world example, why knowing your dependencies pays off: Time to time, your dependency tree changes, as the packages you depend on develop or dies. Suddenly, your package stops building. Searching for why uncover, that your dependency used zlib. You used it also for your package. But because you haven't specified it as a buildrequires, and the package you depend on changed in a way it doesn't need zlib anymore, it won't successfuly build anymore. --- I like how the package looks now. I'm granting you the "fedora-review +". I also believe, you are worthy of the packager status, but since I'm not a Sponsor, I can't grant it to you. Yoou need to find a sponsor. If you'd have any questions, feel free to ask me. Also I recommend Fedora IRC channels and mailing lists, where you can find help, answers, guidance and explanations.
OK, that makes sense, re: dependencies. Thanks for your extensive review, I'll start looking for sponsors. I'll let the people who bundle simclist know about the version status once I get hold of the dev. Let me know if you want to be kept up-to-date as well.
> # simclist is a small library not worth splitting into its own paclage, and has not typo ;) > Requires: systemd > # for systemd service installation support > BuildRequires: systemd You only need systemd-rpm-macros, which is much more lightweight. > # for scriptlets > %{?systemd_requires} The guidelines changed on that a while back, and this is only necessary when using some other systemd tools like systemd-tmpfiles or systemd-sysusers, which you are not. Please remove. [https://pagure.io/packaging-committee/pull-request/890] > 2.2) Logrotate is used only with sysvinit and not with systemd - is that an intention? > Reply: Yes. Usually, sshguard expects to run with systemd and logs via journald. I only have it log to a file on SysVInit. Thank you ;) Logrotate has no place in a modern distro, IMO. In general, logging to a text file from a daemon has no place. DNF or anaconda logging to a text file during installation is different, because such events only happen occasionally, and systemd-journald might not be available. But anything which logs normally and constantly during runtime should not use text files. -- You should run 'fedpkg request-repo' and 'fedpkg request-branch' now.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/sshguard
> typo ;) Fixed. > You only need systemd-rpm-macros, which is much more lightweight. Thanks. I've changed the deps for f30+, f29 & epel still ship those macros as part of the systemd package, unless I'm very much mistaken. > > # for scriptlets > > %{?systemd_requires} > > The guidelines changed on that a while back, and this is only necessary when using > some other systemd tools like systemd-tmpfiles or systemd-sysusers, which you are > not. Please remove. [https://pagure.io/packaging-committee/pull-request/890] Done. > You should run 'fedpkg request-repo' and 'fedpkg request-branch' now. Done, it all seems to be working fine, though I haven't triggered a build yet. https://src.fedoraproject.org/rpms/sshguard/blob/master/f/sshguard.spec