Fedora Merge Review: sendmail http://cvs.fedora.redhat.com/viewcvs/devel/sendmail/ Initial Owner: twoerner
I'll take a look at this one; I'll have initial comments tomorrow
I've run out of time for going through the review checklist today, so here for the time being are my thoughts on the rpmlint output: $ rpmlint sendmail-8.14.0-1.src.rpm W: sendmail summary-ended-with-dot A widely used Mail Transport Agent (MTA). => easy fix, remove dot W: sendmail invalid-license Sendmail => whilst rpmlint doesn't include the Sendmail license in its list of approved => licenses, the FSF includes sendmail in its directory of free software => (http://directory.fsf.org/sendmail.html) so this is OK W: sendmail no-url-tag => easy fix, add URL tag W: sendmail unversioned-explicit-provides smtpdaemon => normal for virtual provides, ignorable W: sendmail prereq-use /usr/sbin/alternatives => use of fine-grained dependencies is normally preferred, but in this case == the requirement is needed for a triggerpostun clause (as well as the post => and postun scripts) and as far as I know there is no way of specifying => fine-grained dependencies for trigger scripts, so this is OK W: sendmail unversioned-explicit-provides %{_sbindir}/sendmail W: sendmail unversioned-explicit-provides %{_bindir}/mailq W: sendmail unversioned-explicit-provides %{_bindir}/newaliases W: sendmail unversioned-explicit-provides %{_bindir}/rmail W: sendmail unversioned-explicit-provides %{_mandir}/man1/mailq.1.gz W: sendmail unversioned-explicit-provides %{_mandir}/man1/newaliases.1.gz W: sendmail unversioned-explicit-provides %{_mandir}/man5/aliases.5.gz => all OK as these files are provided in "alternatives" format W: sendmail prereq-use chkconfig >= 1.3 => OK, also needed for a trigger script W: sendmail prereq-use /usr/sbin/useradd /bin/mktemp fileutils gawk sed sh-utils => these should be replaced with the appropriate Requires tags for each => scriptlet; "fileutils" should probably be "coreutils" these days W: sendmail prereq-use fsl >= 1.2.0 => simple "Requires" should do? W: sendmail prereq-use openssl => not needed since rpm's auto-dependency finder picks up the library dep W: sendmail prereq-use /usr/sbin/saslauthd, openssl => "Requires: /usr/sbin/saslauthd" is sufficient W: sendmail prereq-use openldap, openssl => not needed W: sendmail prereq-use mysql => not needed E: sendmail hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/sendmail.sendmail => legitimate install of /usr/lib/sendmail for backward script compatibility E: sendmail hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/libmilter.a => false positive, the %install script is removing the library incorrectly => installed there by the upstream build script E: sendmail hardcoded-library-path in /usr/lib/sendmail.sendmail E: sendmail hardcoded-library-path in /usr/lib/sendmail.sendmail => legitimate install of /usr/lib/sendmail for backward script compatibility W: sendmail macro-in-%changelog preun W: sendmail macro-in-%changelog triggerpostun W: sendmail macro-in-%changelog post => easy fix, escape the macros in the changelog W: sendmail mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 239) => easy fix, expand the tabs => the here document for /etc/mail/access could be split off into a separate => source file and retain its tabs $ rpmlint sendmail-8.14.0-1.x86_64.rpm W: sendmail summary-ended-with-dot A widely used Mail Transport Agent (MTA). W: sendmail invalid-license Sendmail W: sendmail no-url-tag => already covered E: sendmail only-non-binary-in-usr-lib => rpmlint confused by a symlink to a binary elsewhere W: sendmail conffile-without-noreplace-flag /etc/rc.d/init.d/sendmail E: sendmail executable-marked-as-config-file /etc/rc.d/init.d/sendmail => initscript shouldn't be a conffile in the first place. I would argue that => any customizations should be possible by changing things in => /etc/sysconfig/sendmail. Failing that, use %config(noreplace) E: sendmail file-in-usr-marked-as-conffile /usr/lib64/sasl2/Sendmail.conf W: sendmail conffile-without-noreplace-flag /usr/lib64/sasl2/Sendmail.conf => This is where cyrus-sasl puts things, so not much choice about this E: sendmail zero-length /etc/mail/mailertable E: sendmail zero-length /etc/mail/domaintable E: sendmail zero-length /etc/mail/virtusertable => these files intentionally left blank, though perhaps comment lines could => be put in them, which would shut rpmlint up E: sendmail non-standard-executable-perm /usr/sbin/makemap 0555 E: sendmail non-standard-executable-perm /usr/sbin/smrsh 0555 => should be installed 0755 for debuginfo extraction anyway E: sendmail non-standard-gid /usr/sbin/sendmail.sendmail smmsp E: sendmail setgid-binary /usr/sbin/sendmail.sendmail smmsp 02755 E: sendmail non-standard-executable-perm /usr/sbin/sendmail.sendmail 02755 E: sendmail non-standard-dir-perm /var/spool/mqueue 0700 E: sendmail non-standard-uid /var/spool/clientmqueue smmsp E: sendmail non-standard-gid /var/spool/clientmqueue smmsp E: sendmail non-standard-dir-perm /var/spool/clientmqueue 0770 => non-standard but correct W: sendmail dangling-relative-symlink /usr/bin/newaliases.sendmail ../../usr/sbin/sendmail W: sendmail dangling-relative-symlink /usr/lib/sendmail.sendmail ../sbin/sendmail W: sendmail dangling-relative-symlink /usr/bin/mailq.sendmail ../../usr/sbin/sendmail W: sendmail dangling-relative-symlink /usr/bin/purgestat ../../usr/sbin/sendmail W: sendmail dangling-relative-symlink /usr/bin/hoststat ../../usr/sbin/sendmail => not dangling after alternatives in run in %post E: sendmail zero-length /var/log/mail/statistics => OK W: sendmail log-files-without-logrotate /var/log/mail => this directory contains only the statistics file, which doesn't need => rotating W: sendmail dangerous-command-in-%post cp => %post script is well-tested W: sendmail service-default-enabled /etc/rc.d/init.d/sendmail => listens only on localhost out of the box, so OK E: sendmail incoherent-subsys /etc/rc.d/init.d/sendmail sm-client E: sendmail incoherent-subsys /etc/rc.d/init.d/sendmail sm-client => two daemons running, both can't be coherent at the same time $ rpmlint sendmail-cf-8.14.0-1.x86_64.rpm W: sendmail-cf summary-ended-with-dot The files needed to reconfigure Sendmail. W: sendmail-cf invalid-license Sendmail W: sendmail-cf no-url-tag => already covered W: sendmail-cf no-documentation => perhaps label /usr/share/sendmail-cf/README as %doc ? => (and perhaps don't bother including /usr/share/sendmail-cf/cf/README at => all?) E: sendmail-cf non-executable-script /usr/share/sendmail-cf/sh/makeinfo.sh 0644 => the m4 snippet that uses this doesn't need it to be executable $ rpmlint sendmail-debuginfo-8.14.0-1.x86_64.rpm W: sendmail-debuginfo invalid-license Sendmail W: sendmail-debuginfo no-url-tag => already covered $ rpmlint sendmail-devel-8.14.0-1.x86_64.rpm W: sendmail-devel summary-ended-with-dot Extra development include files and development files. W: sendmail-devel invalid-license Sendmail W: sendmail-devel no-url-tag => already covered W: sendmail-devel no-documentation => how about: => %doc libmilter/docs/* $ rpmlint sendmail-doc-8.14.0-1.x86_64.rpm W: sendmail-doc summary-ended-with-dot Documentation about the Sendmail Mail Transport Agent program. W: sendmail-doc invalid-license Sendmail W: sendmail-doc no-url-tag => already covered
Created attachment 148135 [details] Patch addressing most rpmlint issues worth fixing
/usr/lib/sendmail is a dangling symlink, which should be owned by sendmail, too. Paul, if this should not be relevant for this review, I'll create a own bug report for it, because unowned dangling symlinks are worse. IMHO this is another MUST for approving.
No, /usr/lib/sendmail is generated by alternatives. It could also point to postfix or exim.
Any chance to own /usr/lib/sendmail somehow else?
I think adding: Provides: /usr/lib/sendmail would achieve the desired result. I think that is how sendmail "owns" /usr/sbin/sendmail, which is also managed by alternatives.
Created attachment 148408 [details] Patch implementing most of the following review suggestions Review to follow shortly.
Review: ======= - rpmlint output already covered in Comment #2 - package and spec naming OK - package generally meets guidelines - license is "Sendmail", not listed on FSF "list of licenses" page but sendmail *is* listed in the FSF directory of free software (http://directory.fsf.org/sendmail.html) - license correctly tagged in spec but license text needs to be packaged - spec written in English but legibility could be improved - sources match upstream - package builds OK on i386 and x86_64 in mock for rawhide - buildreqs OK - no locale data included - no shared libraries included - not relocatable - no directory ownership problems - no permissions problems - %clean section present and correct - macro usage is reasonably consistent - code, not content - large docs in -doc subpackage - docs don't affect runtime - header files and static libraries included in -devel subpackage (no upstream support for dynamic libraries) - no pkgconfig files - devel package has proper versioned dependency on main package - no libtool archives - not a GUI app, no desktop file needed - scriptlets are complex, but well-tested - all subpackages have proper versioned dependencies on main package Needs Work: * LICENSE file must be included as %doc in the main package I think. I'd also suggest moving FAQ, KNOWNBUGS, README, RELEASE_NOTES in the same way * non-standard buildroot %{_tmppath}/%{name}-root Guidelines now mandate: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * directory creation near the top of %install not using the same macros as later parts of %install Suggestions: * Group: for devel package should be Development/Libraries I think * %post script could return non-zero exit code - add "exit 0" at end? * symlinks for the sendmail-specific versions of files managed using alternatives should point to the sendmail-specific targets rather than the generic targets, i.e. /usr/bin/mailq.sendmail -> ../../usr/sbin/sendmail /usr/bin/newaliases.sendmail -> ../../usr/sbin/sendmail /usr/lib/sendmail.sendmail -> ../sbin/sendmail should instead be: /usr/bin/mailq.sendmail -> ../../usr/sbin/sendmail.sendmail /usr/bin/newaliases.sendmail -> ../../usr/sbin/sendmail.sendmail /usr/lib/sendmail.sendmail -> ../sbin/sendmail.sendmail * all of the alternatives-managed files should be "provided" by sendmail. Currently, the following are provided: %{_sbindir}/sendmail %{_bindir}/mailq %{_bindir}/newaliases %{_bindir}/rmail %{_mandir}/man1/mailq.1.gz %{_mandir}/man1/newaliases.1.gz %{_mandir}/man5/aliases.5.gz Also needed are: /usr/lib/sendmail %{_sysconfdir}/pam.d/smtp %{_mandir}/man8/sendmail.8.gz * general legibility improvements: - group the RPM tags at the top of the spec into general, build-time, and run-time sections - extra comments, particularly in %install - dispense with here documents - consistent indentation - replace initdir with prefdefined initrddir macro - use the "symlinks" program to fix up symlinks rather than the scripted routine that figures out how deep in the hierarchy a particular target directory is * removal of old cruft - the symbols _FFR_WORKAROUND_BROKEN_NAMESERVERS, _FFR_SMTP_SSL, _FFR_BLOCK_PROXIES, _FFR_UNSAFE_SASL, _FFR_MILTER_ROOT_UNSAFE no longer appear in the sendmail source and hence can be removed from the spec - the MySQL stuff in the spec appears to be there to support building with a patch maintained outside of the upstream sendmail source: http://www.palsenberg.com/index.php/plain/projects/sendmail_mysql_map_class This patch is not included in the Fedora package so why is the rest of the mysql support included? * addition of old cruft - if %{old_setup} is supported, include the aliases file so that just changing the macro value and rebuilding the spec will work. Otherwise, drop %{old_setup} altogether. * is it worth packaging /usr/share/sendmail-cf/cf/README ? * macro usage: - should /etc/alises be %{_sysconfdir}/aliases ? - should /etc/mail be %{_sysconfdir}/mail ? - should /etc/pam.d be %{_sysconfdir}/pam.d ? - should /etc/smrsh be %{_sysconfdir}/smrsh ? - should /var/spool be %{_localstatedir}/spool ? - files are installed to macro-ised directories in %install but the %files list has hardcoded directory names like /usr/bin * timestamps - I suggest trying to preserve all timestamps in upstream files that get packaged * scripted edits are done in %install using a mixture of perl and sed scripts, though they're all just straightforward search and replace changes. Best to just use sed for that. * the alternatives call in %post assumes that manpages are compressed using gzip, and will fail if for instance the buildsystem is configured to use bzip2 instead (or no compression). I think it's best to make the same assumptions everywhere, so I'd either (a) make the %post script very clever so it detects the manpage filenames (hard), or (b) hardcode the .gz extension for manpages in the %files list so that package build will fail on such systems. * Many of the %attr references in the %files list are redundant and could be removed. * The current %post script clobbers my access database every time I upgrade the sendmail package, because I have tweaked /etc/mail/Makefile to create the access.db from the concatenation of two files, /etc/mail/access and /etc/mail/access.shared. The %post script creates a new /etc/mail/access.db from /etc/mail/access, which loses the bulk of my entries. I would like to propose that if /usr/bin/make is installed, the /etc/mail/Makefile is used to generate the updated databases, else revert to using the original direct call to makemap. I'd also like to see the conditional build options (SASL1/SASL2/TLS etc.) reworked to use rpmbuild's --with/--without options so that alternative builds can be done straight from the SRPM without tweaking the spec file itself. Let's leave that for now though.
Review process has changed again; reviewer remains as assignee now.
Grr, reporter is nobody; whose bright idea was that?
Should add "Provides: MTA"; other packages that provide a MTA do that as well (exim, postfix, ssmtp, esmtp (bug filed, pending))
(In reply to comment #9) > * all of the alternatives-managed files should be "provided" by sendmail. Is it really sure? What is the point of providing the man pages? > %{_mandir}/man1/mailq.1.gz > %{_mandir}/man1/newaliases.1.gz > %{_mandir}/man5/aliases.5.gz > %{_mandir}/man8/sendmail.8.gz >
Providing the manpages means that 'rpm -qf /usr/share/man/man1/mailq.1.gz' etc. will return a meaningful result.
Uhm. For me a meaningfull result would be that that file isn't provided by any package. A file provide is useful if other packages may depend on it (like %_sbindir/sendmail), but in my opinion it would be better if the package didn't own the alternative symlinks when possible. Not to mention that, for other reduced functionality MTA like esmtp those man pages and the binaries are in the alternate system, even when they don't do anything, so I think it is even more wrong to provide them.
I think that if it's OK to provide %_sbindir/sendmail then it should be OK to provide the alternatives-based files too for consistency. Whilst there may well be other packages with dependencies on %_sbindir/sendmail, which isn't going to apply in the cases of manpages, the provides for the manpages cou;d still be useful for someone using say repoquery to find a manpage for a file so as to discover the file format etc. However, I don't see this as a blocker either way.
> E: sendmail hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/libmilter.a > => false positive, the %install script is removing the library incorrectly > => installed there by the upstream build script This is wrong, because /usr/lib/libmilter.a gets removed for a 64 bit build, because /usr/lib/libmilter.a has to be 32 bit here, but sendmail installs the 64 bit build. symlinks is not usable, because it would create realtive links outside of the buildroot. %{_localstatedir} ist not /var it is %{_prefix}/var, ergo /usr/var. /etc/aliases is not part of the sendmail package. ===> Please have a look at sendmail-8.14.1-1.
(In reply to comment #17) > > E: sendmail hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/libmilter.a > > => false positive, the %install script is removing the library incorrectly > > => installed there by the upstream build script > This is wrong, because /usr/lib/libmilter.a gets removed for a 64 bit build, > because /usr/lib/libmilter.a has to be 32 bit here, but sendmail installs the 64 > bit build. OK, but that's what I meant; the upstream installer puts it in /usr/lib and the spec puts it in /usr/lib64, so the upstream-installed one needs to be removed on x86_64. > symlinks is not usable, because it would create realtive links outside of the > buildroot. Are you sure? I tried the patch from Comment #8 before posting it and it looked to be doing the right thing to me. > %{_localstatedir} ist not /var it is %{_prefix}/var, ergo /usr/var. $ rpm --eval '%{_localstatedir}' /var > /etc/aliases is not part of the sendmail package. It should be if the package is supposed to work with %{old_setup} = yes (obviously not for current Fedora versions but for people trying to rebuild for older distros). I don't think %{old_setup} should be supported half-heartedly - either let it work properly or remove it altogether. > ===> Please have a look at sendmail-8.14.1-1. Where can I find that? It's not in my just-updated cvs checkout.
OK, agree now with _localstatedir: /usr/lib/rpm/macros: %_localstatedir %{_prefix}/var /usr/lib/rpm/redhat/macros: %_localstatedir /var I am always looking at /usr/lib/rpm/macros instead of /usr/lib/rpm/redhat/macros ... The old_setup switch is gone, also update support for sendmail < 8.12.0 with the corresponding triggers and post script tweaks. sendmail-8.14.1-1.1 is in rawhide with additional prereq replacements.
rpmlint output, ignoring things explained away in Comment #2. W: sendmail unversioned-explicit-provides MTA => I'm not convinced about the merits of "Provides: MTA" vs. "Provides: smtpdaemon", and the discussion on fedora-devel-list didn't seem to lead to any particular conclusion. It's harmless enough though so I'm not going to quibble about it. E: sendmail hardcoded-library-path in /usr/lib/sendmail => This comes from "Provides: /usr/lib/sendmail", is intentional and required for backwards compatibility (OK). W: sendmail unversioned-explicit-provides %{_sysconfdir}/pam.d/smtp W: sendmail unversioned-explicit-provides %{_mandir}/man8/sendmail.8.gz == Provides for alternatives-based files (OK). W: sendmail mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 232) => Should use either spaces or tabs consistently, not a mixture of the two. E: sendmail non-standard-executable-perm /usr/sbin/makemap 0555 => Permission should be 0755 to enable debuginfo extraction. E: sendmail non-readable /etc/aliases.db 0640 => This has appeared because the original package explicitly used %attr(0644... for this file. Harmless because (a) the file is %ghosted and (b) the permissions will be 0640 on the installed system anyway if the file is created properly. E: sendmail executable-marked-as-config-file /etc/rc.d/init.d/sendmail => Marking initscripts as %config is a contentious subject. I'm against it on the basis that configurability should be encapsulated in /etc/sysconfig/sendmail etc. but I can see the merits of the argument the other way too. Do you have a strong opinion on this or is this just an inheritance from older packaging? E: sendmail non-standard-executable-perm /usr/sbin/smrsh 0555 => Permission should be 0755 to enable debuginfo extraction. E: sendmail postin-without-chkconfig /etc/rc.d/init.d/sendmail => /sbin/chkconfig --add sendmail was removed from %post along with the stuff for upgrading from old versions, needs to be reinstated. Other comments: FFR_UNSAFE_SASL still present in sasl2 config - why? Why is the not-included mysql support stuff still in the spec? Any thoughts on my suggestion for the %post script near the end of Comment #9? Finally, the line containing "MSPQOWN=${nameuser}" in the spec has a trailing space on it. Please remove it, it's been annoying me for ages ;-)
The directory /usr/share/doc/sendmail-8.14.1 is no longer owned!
All done in package sendmail-8.14.1-2.
rpmlint looks OK now apart from: W: sendmail mixed-use-of-spaces-and-tabs (spaces: line 868, tab: line 2) some very minor cosmetic changes to the changelog can fix this The provides for "MTA" and smtpdaemon are well before the rest of the provides in the spec; it'd be good if they were all together. Requires(postun) doesn't need to include chkconfig Requires: openssl (with_tls, with_sasl, with_ldap) is redundant; autodetected library dependencies will pull in the correct package if sendmail is built against openssl. Similarly, Requires: openldap is redundant. %description doc says that "papers are provided in PostScript(TM) and troff formats" but it's now a PDF. The "touch" and initial "chmod" parts of "create db files" in %install are redundant because the files were created in the immediately preceding part of the spec, and with the correct permissions. Installation of %{maildir}/Makefile would be better grouped with the installation of the other files in %{maildir} above "create db files" rather than grouped with the initscript installation. Still no comment on using /etc/mail/Makefile for map rebuilds in %post? %attr(0755,root,root) for %{_initrddir}/sendmail in $files is redundant as those are the attributes it would have by default. So, mainly those are cosmetic changes and in general I'm happy with this. Any more comments from anyone else before I approve this one?
Because sendmail it basically the reference for all MTAs, I'd kindly ask you to take a look at the thread started by Ville Skyttä: https://www.redhat.com/archives/fedora-devel-list/2007-April/msg00377.html I have no objections at all for sendmail providing any or all of the three capabilities mentioned in that thread (/usr/sbin/sendmail, MTA, smtpdaemon), but maybe with this occasion we could define some sort of standard, defining the exact meaning of each capability. With that definitions at hand we could then file bugs and hopefully fix mdadm (see also https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226134), mailx and any other programs acting as MTA or having MTA requirements.
Warnung: Datei doppelt aufgelistet: /usr/share/sendmail-cf/README Warnung: Datei doppelt aufgelistet: /usr/share/sendmail-cf/sendmail.schema This means the files above are listed twice.
Created attachment 157873 [details] Patch fixing build issue in current rawhide The current package in CVS does not build in rawhide. This is because m4 in rawhide has recently been upgraded to version 1.4.9, and versions of m4 later than 1.4.8 return a non-zero exit code if a file to be include-d is not found. One of the patches applied in the Fedora package to the default submit.mc triggers this problem during the early part of %install and causes the package build to fail. The attached patch treats submit.{mc,cf} similarly to sendmail.{mc,cf} and fixes this problem.
Time to get this review moving again. Here's a refresher of where we stand: Current rpmlint state --------------------- Source Package: W: unversioned-explicit-provides MTA W: unversioned-explicit-provides smtpdaemon W: unversioned-explicit-provides server(smtp) => All Ok - these are valid virtual provides for which a version would be inappropriate W: unversioned-explicit-provides %{_sbindir}/sendmail W: unversioned-explicit-provides %{_bindir}/mailq W: unversioned-explicit-provides %{_bindir}/newaliases W: unversioned-explicit-provides %{_bindir}/rmail W: unversioned-explicit-provides /usr/lib/sendmail W: unversioned-explicit-provides %{_sysconfdir}/pam.d/smtp W: unversioned-explicit-provides %{_mandir}/man1/mailq.1.gz W: unversioned-explicit-provides %{_mandir}/man1/newaliases.1.gz W: unversioned-explicit-provides %{_mandir}/man5/aliases.5.gz W: unversioned-explicit-provides %{_mandir}/man8/sendmail.8.gz => Current guidelines on alternatives usage (https://fedoraproject.org/wiki/PackagingDrafts/UsingAlternatives, not yet written into the main guidelines but approved by FPC - see https://fedoraproject.org/wiki/Packaging:Minutes/20090414) says to %ghost these files rather than using explicit provides. (I see this is currently under discussion on fedora-packaging) E: hardcoded-library-path in /usr/lib/sendmail E: hardcoded-library-path in %{buildroot}/usr/lib E: hardcoded-library-path in %{buildroot}/usr/lib/sendmail.sendmail E: hardcoded-library-path in /usr/lib/sendmail.sendmail E: hardcoded-library-path in /usr/lib/sendmail.sendmail => legitimate install of /usr/lib/sendmail for backward script compatibility sendmail.src: W: mixed-use-of-spaces-and-tabs (spaces: line 964, tab: line 2) => easily fixed, and should be Main Binary Package: W: only-non-binary-in-usr-lib => rpmlint confused by a symlink to a binary elsewhere E: file-in-usr-marked-as-conffile /usr/lib64/sasl2/Sendmail.conf W: conffile-without-noreplace-flag /usr/lib64/sasl2/Sendmail.conf => This is where cyrus-sasl puts things, so not much choice about this at the moment W: non-standard-gid /usr/sbin/sendmail.sendmail smmsp E: setgid-binary /usr/sbin/sendmail.sendmail smmsp 02755 E: non-standard-executable-perm /usr/sbin/sendmail.sendmail 02755 E: non-standard-dir-perm /var/spool/mqueue 0700 W: non-standard-uid /var/spool/clientmqueue smmsp W: non-standard-gid /var/spool/clientmqueue smmsp E: non-standard-dir-perm /var/spool/clientmqueue 0770 => non-standard but correct W: log-files-without-logrotate /var/log/mail => This directory contains only the statistics file, which doesn't need rotating W: dangerous-command-in-%post chown => %post script is well-tested W: service-default-enabled /etc/rc.d/init.d/sendmail => Listens only on localhost out of the box, so OK E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client => Two daemons running, both can't be coherent at the same time E: zero-length /var/log/mail/statistics => Intended and OK E: executable-marked-as-config-file /etc/mail/make => Script needs to be editable for people that build their databases in non-standard ways Configuration Package: E: non-executable-script /usr/share/sendmail-cf/sh/makeinfo.sh 0644 => Doesn't need to be executable due to the way it's called; could shut => rpmlint up by removing the shellbang or making it executable though Documentation Package: W: file-not-utf8 /usr/share/doc/sendmail-8.14.3/contrib/etrn.0 => Should be recoded to UTF-8 like everything else in Fedora E: wrong-script-interpreter /usr/share/doc/sendmail-8.14.3/contrib/bounce-resender.pl "/usr/local/bin/perl" E: wrong-script-interpreter /usr/share/doc/sendmail-8.14.3/contrib/etrn.pl "/usr/local/bin/perl" E: wrong-script-interpreter /usr/share/doc/sendmail-8.14.3/contrib/smcontrol.pl "/usr/local/bin/perl" => Should be using %{__perl} instead Milter Runtime Library Package: W: no-documentation => OK, there isn't really any documentation appropriate to this package 7 packages and 0 specfiles checked; 18 errors, 25 warnings. A few other general comments ---------------------------- Please consider adding the patches from Bug #485426 to fix socket descriptor leaks to child processes. Requires: chkconfig for %postun isn't needed. Requires: openssl/openldap aren't needed - automatic library dependencies suffice. sendmail-doc and sendmail-cf could be noarch subpackages. The %attr(0755,root,root) for %{_initrddir}/sendmail isn't necessary because the initscript is installed with the correct permissions in the first place. %global is preferred over %define for the sort of use made in this package. The initscript sources /etc/rc.d/init.d/functions and hence the main package should have a dependency on initscripts. Adding this would then mean that the directory ownership of /etc/NetworkManager/dispatcher.d was unnecessary, as the initscripts package also owns that directory. Consider adding a "/" suffix to directories owned by the package in the %files lists, for clarity. The provides for "MTA" and smtpdaemon are well before the rest of the provides in the spec; it'd be good if they were all together. %description doc says that "papers are provided in PostScript(TM) and troff formats" but it's now a PDF. Is there any point to supporting with_sasl1? It's not been used since RHEL 3 and there are lots of things in the current package that would prevent it being rebuilt for RHEL 3 (e.g. it would need to include /etc/aliases again).
The /etc/mail/make script handles the authinfo database but not the msp-authinfo database (see http://www.sendmail.org/m4/msp.html). Adding this line after the one that handles the authinfo database works for me: test -f msp-authinfo && makedb msp-authinfo.db && chown smmsp:smmsp msp-authinfo{,.db} && chmod 640 msp-authinfo{,.db}
Sorry for taking so long. Most of this (hopefully all blockers) should be fixed in sendmail-8.14.3-9.fc13. I didn't include the msp-authinfo support as it requires special permissions and makemap fails when trying to regenerate the file. Thanks for the review.
What about sendmail-8.14.4-2.fc13? Still blockers?
Let's move it a little: [marca@caladan devel]$ rpmlint sendmail-8.14.4-3.fc13.x86_64.rpm sendmail.x86_64: W: only-non-binary-in-usr-lib > This should be fixed. sendmail.x86_64: E: file-in-usr-marked-as-conffile /usr/lib64/sasl2/Sendmail.conf > Probably can't fix sendmail.x86_64: W: conffile-without-noreplace-flag /usr/lib64/sasl2/Sendmail.conf > Use noreplace flag if user can lost settings. sendmail.x86_64: E: setgid-binary /usr/sbin/sendmail.sendmail smmsp 02755 sendmail.x86_64: E: non-standard-executable-perm /usr/sbin/sendmail.sendmail 02755 sendmail.x86_64: E: zero-length /var/log/mail/statistics sendmail.x86_64: E: executable-marked-as-config-file /etc/mail/make sendmail.x86_64: E: non-standard-dir-perm /var/spool/mqueue 0700 sendmail.x86_64: E: non-standard-dir-perm /var/spool/clientmqueue 0770 > Strange, but probably ok in this case. sendmail.x86_64: W: log-files-without-logrotate /var/log/mail > Couldn't be lograted? sendmail.x86_64: W: dangerous-command-in-%post chown sendmail.x86_64: W: service-default-enabled /etc/rc.d/init.d/sendmail sendmail.x86_64: E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client sendmail.x86_64: E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client sendmail.x86_64: W: service-default-enabled /etc/rc.d/init.d/sendmail > Ok in this case 1 packages and 0 specfiles checked; 9 errors, 6 warnings. [marca@caladan devel]$ rpmlint sendmail-8.14.4-3.fc13.src.rpm sendmail.src:59: W: unversioned-explicit-provides MTA sendmail.src:59: W: unversioned-explicit-provides smtpdaemon sendmail.src:59: W: unversioned-explicit-provides server(smtp) > Ok in this case. sendmail.src:60: W: unversioned-explicit-provides %{_sbindir}/sendmail sendmail.src:60: W: unversioned-explicit-provides %{_bindir}/mailq sendmail.src:60: W: unversioned-explicit-provides %{_bindir}/newaliases sendmail.src:61: E: hardcoded-library-path in /usr/lib/sendmail sendmail.src:61: W: unversioned-explicit-provides %{_bindir}/rmail sendmail.src:61: W: unversioned-explicit-provides /usr/lib/sendmail sendmail.src:62: W: unversioned-explicit-provides %{_sysconfdir}/pam.d/smtp sendmail.src:63: W: unversioned-explicit-provides %{_mandir}/man1/mailq.1.gz sendmail.src:63: W: unversioned-explicit-provides %{_mandir}/man1/newaliases.1.gz sendmail.src:64: W: unversioned-explicit-provides %{_mandir}/man5/aliases.5.gz sendmail.src:64: W: unversioned-explicit-provides %{_mandir}/man8/sendmail.8.gz > It was mentioned before, you can use ghost for these. sendmail.src:269: E: hardcoded-library-path in %{buildroot}/usr/lib sendmail.src:308: E: hardcoded-library-path in %{buildroot}/usr/lib/sendmail.sendmail sendmail.src:429: E: hardcoded-library-path in /usr/lib/sendmail.sendmail sendmail.src:478: E: hardcoded-library-path in /usr/lib/sendmail.sendmail > Ok in this case. 1 packages and 0 specfiles checked; 5 errors, 13 warnings.
? rpmlint on every package. OK name of package accords to the Naming Guidelines. OK spec file name match the base package %{name}. OK package meet the Packaging Guidelines. OK package has a good license. OK spec file is written in American English. OK spec file for the package must be legible. OK The sources used to build the package must match the upstream source. > 1b23d5000c8e7bfe82ec1a27f2f5fdc5 OK successful koji compilation. OK correct BuildRequires, Requires. - proper use of %find_lang macro. OK shared library must call ldconfig in %post and %postun. OK relocatable package must state this fact. OK package must own their directories. OK permissions on files must be set properly. OK %clean section with rm -rf %{buildroot}. OK consistent use of macros. OK package contains code, or permissable content. OK large documentation go in a -doc subpackage. OK %doc must not affect the runtime of the application. OK header files must be in a -devel package. - static libraries must be in a -static package. - pkgconfig(.pc) files must 'Requires: pkgconfig'. OK library files with a suffix (.so) must go in -devel. > in this case separate packages -milter OK usually devel packages must require the base package. OK Remove .la libtool archives. - GUI applications must include a %{name}.desktop file. OK %install section starts with rm -rf %{buildroot}. %defattr(-,root,root) Should be %defattr(-,root,root,-)
Thank you for review, let's move to sendmail-8.14.4-4.fc14, here are my comments: sendmail.x86_64: W: only-non-binary-in-usr-lib This should be fixed. > Caused by symlink /usr/lib/sendmail.sendmail. This approach is used accross MTAs. Fixing this warning requires changing all other MTAs - this is out of my scope. sendmail.x86_64: W: conffile-without-noreplace-flag /usr/lib64/sasl2/Sendmail.conf Use noreplace flag if user can lost settings. > Fixed. sendmail.x86_64: W: log-files-without-logrotate /var/log/mail Couldn't be lograted? > Nothing to rotate, there is only /var/log/mail/statistics with small internal statistics. sendmail.src:60: W: unversioned-explicit-provides %{_sbindir}/sendmail sendmail.src:60: W: unversioned-explicit-provides %{_bindir}/mailq sendmail.src:60: W: unversioned-explicit-provides %{_bindir}/newaliases sendmail.src:61: E: hardcoded-library-path in /usr/lib/sendmail sendmail.src:61: W: unversioned-explicit-provides %{_bindir}/rmail sendmail.src:61: W: unversioned-explicit-provides /usr/lib/sendmail sendmail.src:62: W: unversioned-explicit-provides %{_sysconfdir}/pam.d/smtp sendmail.src:63: W: unversioned-explicit-provides %{_mandir}/man1/mailq.1.gz sendmail.src:63: W: unversioned-explicit-provides %{_mandir}/man1/newaliases.1.gz sendmail.src:64: W: unversioned-explicit-provides %{_mandir}/man5/aliases.5.gz sendmail.src:64: W: unversioned-explicit-provides %{_mandir}/man8/sendmail.8.gz It was mentioned before, you can use ghost for these. > A little tricky to implement, but all are ghosted now. After these changes, it is possible that you encounter following new warning: sendmail.x86_64: W: read-error /etc/pam.d/smtp [Errno 2] No such file or directory: '/tmp/rpmlint.sendmail-8.14.4-4.fc14.x86_64.rpm.QncWlw//etc/pam.d/smtp' It is false positive, caused by bug in rpmlint, I reported it with patch to bugzilla (#570086).
I'd like to ACCEPT it if maintainer of this review don't mind.
Please be careful with the %ghosts. If there are more MTA packages installed, removing sendmail will remove the files as other MTAs don't own the files. I'm not sure if it's possible to make a clean upgrade to the new alternatives system. I posted about the problem on fedora-packaging list when the new system was introduced, but there were no solutions proposed.
Thank you for pointing this out Mirek. I think it is worth to keep this change as it is according to https://fedoraproject.org/wiki/Packaging:Alternatives I filled bugs addressing this issue accross all affected MTAs: esmtp #570797 ssmtp #570799 exim #570800 postfix #570801
A few minor nits I spotted this morning: * Can we make the -cf and -docs packages noarch? * Can we change %define to %global throughout? * The perl script interpreter fixup for contrib stuff could get done in %prep rather than %install * %description doc still says that "papers are provided in PostScript(TM) and troff formats" even though the only format packaged is PDF I'll have a last look next week when I have a bit more time.
Thank you Paul. It looks like the: sendmail.x86_64: E: file-in-usr-marked-as-conffile /usr/lib64/sasl2/Sendmail.conf can also be easily fixed. I will wait for Paul's comments and than implement all these in rawhide.
(In reply to comment #38) > Thank you Paul. > > It looks like the: > sendmail.x86_64: E: file-in-usr-marked-as-conffile > /usr/lib64/sasl2/Sendmail.conf > can also be easily fixed. How do you propose to fix this? Surely not by removing the %config? > I will wait for Paul's comments and than implement all these in rawhide. I hope to be able to take another look tomorrow.
It looks like the sasl is scanning the /usr/lib64/sasl2 for config and than tries the /etc/sasl2. From the spec file it looks like as a long time when this behaviour was implemented so I think it should be safe to simple move the config file (also with some check for custom config in old location, maybe in %prep). I will test this behaviour and if OK I will implement it.
Tested on F12 with config in /etc/sasl2 and the SASL2 worked as expected. From the CVS log it looks like this approach can be used for all systems from RHEL-5. Thus no need for config in /usr/lib64/sasl2.
Can you post the changes you made for this, up update CVS in devel?
I patched sendmail to build with libdd-5, rebuilt it with libdb-5 and switched to /etc/sasl2 config dir. The proposed changes are in CVS - sendmail-8.14.4-6.fc14. The sasl tries /usr/lib64/sasl2 and /etc/sasl2 config dirs in sequence and the first has higher priority. I add test for config in /usr/lib64/sasl2. If there exists config I move it to /etc/sasl2 replacing whatever is there (/etc/sasl2 has lower priority thus it wasn't used before and this approach shouldn't be a problem). I also fix timestamps. The config did not change between releases thus if not modified by user, the rpm verify should pass after upgrade. I used post section for this change, because if used in pre section, the old config was always replaced by file from the package (even with noreplace). Any comments/improvements are welcome.
Seems to work nicely apart from an annoying message at daemon startup: error: safesasl(/usr/lib64/sasl2/Sendmail.conf) failed: No such file or directory (this is with an F-12 build)
I can't see this message in rawhide. Originally I planned this feature for rawhide. I will check later with F12 for curiosity.
(In reply to comment #45) > I can't see this message in rawhide. Originally I planned this feature for > rawhide. > > I will check later with F12 for curiosity. I see the same thing on F-13 too. And it turns up in logwatch reports as well: --------------------- sendmail Begin (detail=3) ------------------------ SENDMAIL CONFIGURATION ---------------------- Warning: STARTTLS file errors: CRLFile missing SASL database Errors: In file /usr/lib64/sasl2/Sendmail.conf: No such file or directory: 2 Time(s) ... Of course it's not a problem if it doesn't happen on Rawhide.
Interesting, thank you for pointing this out, I will look on it. I tested only on rawhide. I also tested the SASL auth with various SASL configs and it worked as expected. What I did (on rawhide): # /etc/init.d/sendmail stop # /etc/init.d/sendmail start # echo test | sendmail root # sync # logwatch --range today --detail 10 --print --service All # grep -r sasl2 /var/log/* And I can not see this error on rawhide. I am also going to check the F12/F13. Please can you point me directly to place where this error is logged? And also for the F12/F13 is the SASL config working for you even in case of this error?
It's logged in /var/log/maillog at daemon startup: Jun 10 10:44:17 roary sendmail[18035]: gethostbyaddr(192.168.122.1) failed: 1 Jun 10 10:44:17 roary sendmail[18035]: error: safesasl(/usr/lib64/sasl2/Sendmail.conf) failed: No such file or directory Jun 10 10:44:17 roary sendmail[18036]: starting daemon (8.14.4): SMTP+queueing@01:00:00 Jun 10 10:44:17 roary sendmail[18036]: STARTTLS: ServerCertFile missing Jun 10 10:44:17 roary sendmail[18036]: started as: /usr/sbin/sendmail -bd -q1h Jun 10 10:44:17 roary sm-msp-queue[18045]: starting daemon (8.14.4): queueing@01:00:00 The SASL config still works for me (at least using CRAM-MD5 auth).
For me the error message doesn't appear after daemon restart but after every: # /usr/sbin/sendmail -O LogLevel=14 -bs -Am Thus it seems to be filtered out by default loglevel. Confirmed for F13 and also for rawhide. The good thing is that the functionality is not affected by this message. I will investigate the source.
Looks like the 'paranoia' safesasl check inside the sendmail code doesn't count with /etc/sasl2. Probably patching will be needed.
(In reply to comment #49) > For me the error message doesn't appear after daemon restart but after every: > > # /usr/sbin/sendmail -O LogLevel=14 -bs -Am > > Thus it seems to be filtered out by default loglevel. Confirmed for F13 and > also for rawhide. The good thing is that the functionality is not affected by > this message. I will investigate the source. Ah, right. Didn't occur to me to mention that I had: define(`confLOG_LEVEL',`13')dnl in my .mc file. Sorry about that.
Created attachment 423270 [details] Patch silencing complaint about non-existent %{_libdir}/sasl2/Sendmail.conf This patch skips the warning for missing SASL config file under %{_libdir}. A missing /etc/sasl2/Sendmail.conf still generates the error. Other errors (such as world-writable file) still generate the error too.
Thanks, well done. Integrated in sendmail-8.14.4-7.fc14.
Suggestions from comment 37 implemented in sendmail-8.14.4-8.fc14.
Any more blockers?
OK, here's another pass: rpmlint ======= sendmail.src:61: W: unversioned-explicit-provides MTA sendmail.src:61: W: unversioned-explicit-provides smtpdaemon sendmail.src:61: W: unversioned-explicit-provides server(smtp) => normal for virtual provides, ignorable sendmail.src:270: E: hardcoded-library-path in %{buildroot}/usr/lib sendmail.src:309: E: hardcoded-library-path in %{buildroot}/usr/lib/sendmail.sendmail sendmail.src:408: E: hardcoded-library-path in %{buildroot}/usr/lib/sendmail sendmail.src:445: E: hardcoded-library-path in /usr/lib/sendmail.sendmail sendmail.src:501: E: hardcoded-library-path in /usr/lib/sendmail.sendmail sendmail.src:518: E: hardcoded-library-path in /usr/lib/sendmail => legitimate install of /usr/lib/sendmail for backward script compatibility sendmail.x86_64: W: only-non-binary-in-usr-lib => false positive; there is a symlink to a real binary in /usr/sbin sendmail.x86_64: E: setgid-binary /usr/sbin/sendmail.sendmail smmsp 02755L sendmail.x86_64: E: non-standard-executable-perm /usr/sbin/sendmail.sendmail 02755L => needs to be setgid to function correctly sendmail.x86_64: E: zero-length /var/log/mail/statistics => needs to be present and empty, hence OK sendmail.x86_64: E: executable-marked-as-config-file /etc/mail/make => needs to be executable, needs to be customizable, hence OK sendmail.x86_64: E: non-standard-dir-perm /var/spool/mqueue 0700L sendmail.x86_64: E: non-standard-dir-perm /var/spool/clientmqueue 0770L => permissions for mail queues are correct sendmail.x86_64: W: log-files-without-logrotate /var/log/mail => statistics file here does not need rotating sendmail.x86_64: W: no-manual-page-for-binary hoststat sendmail.x86_64: W: no-manual-page-for-binary rmail.sendmail sendmail.x86_64: W: no-manual-page-for-binary purgestat => see below sendmail.x86_64: W: dangerous-command-in-%post chown => necessary to ensure correct permissions, which sendmail is fussy about sendmail.x86_64: W: service-default-enabled /etc/rc.d/init.d/sendmail => listens only on localhost out of the box, so OK sendmail.x86_64: E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client sendmail.x86_64: E: incoherent-subsys /etc/rc.d/init.d/sendmail sm-client => two daemons running, both can't be coherent at the same time sendmail-cf.noarch: E: non-executable-script /usr/share/sendmail-cf/sh/makeinfo.sh 0644L /bin/sh => the m4 snippet that uses this doesn't need it to be executable, hence OK sendmail-devel.x86_64: W: spelling-error %description -l en_US addons -> ad dons, ad-dons, Abaddon => could change it to add-ons but it would still fail the spell check sendmail-doc.noarch: W: spelling-error %description -l en_US troff -> toff, tr off, tr-off => whilst troff is spelt correctly, the assertion that documentation is provided in troff format is false; the %description should be re-written to reflect the actual contents of the package sendmail-milter.x86_64: W: no-documentation => perhaps move README.libmilter to this package? Review Checklist ================ - rpmlint mainly OK, as above - package and spec file naming OK - package meets guidelines - license is Sendmail, which is Fedora-approved and matches sources - upstream license file is packaged OK - spec file written in English and is legible - source tarball matches upstream - package builds fine in mock and koji on all supported architectures - buildreqs OK - no translations to worry about - ldconfig called properly in %post and %postun of milter package; no other packages contain shared libraries - no bundled system libraries - package not intended to be relocatable - directory ownership OK - no duplicate files - %defattr present and correct, all permissions OK - macro usage is consistent - code, not content - large docs moved to -doc subpackage - docs don't affect runtime - header files and libmilter.so symlink properly included in devel subpackage - no static or libtool libraries packaged - no pkgconfig files packaged - devel subpackage has proper dependency on main package - cf subpackage has proper dependency on main package - doc subpackage has proper dependency on main package - milter subpackage correctly does not depend on main package - not a GUI app, no desktop file needed - filenames all OK - package functions OK - scriptlets are sane - no file dependencies outside /bin, /sbin, and /usr/sbin - manpage coverage mostly complete Issues ====== The hoststat and purgestat commands are described in the main sendmail(8) man page, so it would be useful to add stub pages for them: %{_mandir}/man8/hoststat.8 and %{_mandir}/man8/purgestat.8 can just be: .so man8/sendmail.8 As for rmail, there is a manpage installed but it's not switched in and out using alternatives. Adding it to the alternatives group should resolve the rpmlint issue. %description for the doc subpackage needs rewriting. /usr/share/sendmail-cf/cf/README is rather useless to end users and doesn't really need packaging. The "touch" and initial "chmod" parts of "create db ghosts" in %install are redundant because the files were created in the immediately preceding part of the spec, and with the correct permissions. Some comments in the spec file about the purpose of each patch would be good, even though there's no upstream bug tracker or development mailing list to refer to. Is /usr/sbin/saslauthd really required? I use the auxprop method without needing saslauthd to run.
Well...but not everyone likes auxprop - I prefer saslauthd.
(In reply to comment #57) > Well...but not everyone likes auxprop - I prefer saslauthd. Fair enough, though I need to install cyrus-sasl-md5 manually to get my preferred config working so shouldn't that be a hard requirement too? ;-)
(In reply to comment #56) > sendmail-milter.x86_64: W: no-documentation > => perhaps move README.libmilter to this package? Adding LICENSE as %doc to this package will have made this one go away. However, there was no need for the addition of "License: Sendmail" to the milter subpackage as it was already inheriting that tag from the main package: $ rpm -q --qf "%{LICENSE}\n" sendmail-milter Sendmail
OK, thanks, the primary purpose of this update was to be compliant with the new Licensing Guidelines and not to kill the rpmlint warning (side effect ;). I will fix this and previously mentioned problems in new release.
(In reply to comment #58) > (In reply to comment #57) > > Well...but not everyone likes auxprop - I prefer saslauthd. > > Fair enough, though I need to install cyrus-sasl-md5 manually to get my > preferred config working so shouldn't that be a hard requirement too? ;-) In case we ship sasl2 config with pwcheck_method:saslauthd, I think the saslauthd in require is OK. But shouldn't we add cyrus-sasl-plain? It don't need shared secret and AFAIK the TLS/PLAIN is quite common setup thus the basic auth would work out of the box.
Please do not any dependency to cyrus-sasl-* because I've enough standalone installations, where sendmail is just sending out e-mails without any auth.
I think the status quo is OK, no need to change the requires, given that the shipped Sendmail-sasl2.conf is for saslauthd. Perhaps adding a README.auth with a quick howto (which packages to install, what to configure) for a few common auth scenarios would be better than adding a bunch of requires?
OK, thanks. Please check sendmail-8.14.4-10.fc15. >The "touch" and initial "chmod" parts of "create db ghosts" in %install are >redundant because the files were created in the immediately preceding part of >the spec, and with the correct permissions. Looks OK for me (line 395+). The previous parts are for sources, these are for .db. >%description for the doc subpackage needs rewriting. I did my best, but english is not my native language. Feel free to propose something more appropriate.
(In reply to comment #64) > >The "touch" and initial "chmod" parts of "create db ghosts" in %install are > >redundant because the files were created in the immediately preceding part of > >the spec, and with the correct permissions. > > Looks OK for me (line 395+). The previous parts are for sources, these are for > .db. Quite right. > >%description for the doc subpackage needs rewriting. > > I did my best, but english is not my native language. Feel free to propose > something more appropriate. I'd suggest this: This package contains the Sendmail Installation and Operation Guide (PDF), text files containing configuration documentation, plus a number of contributed scripts and tools for use with Sendmail. A couple of other quick points: * the "README.redhat" (newconfig patch) describes changes that happened for sendmail 8.12.x back in July 2002 and reflects what is now standard practice in Sendmail installations. I wouldn't have thought it was needed any more. * README.sendmail doesn't appear to contain anything of use to people installing the package. Robert, do you have any objections to this package being approved now?
Thanks. In sendmail-8.14.4-11.fc15: - README.libmilter was moved to milter subpackage - README.redhat was removed - doc subpackage description was updated I keep the README.sendmail file because: - In the future there can be added useful information. - Included instructions about debugging hooks can be useful. - It is installed in doc subpackage thus the regular user couldn't be confused.
OK, it's time I stopped nit-picking. There have been no blockers here for some time. Thanks for being a responsive maintainer Jaroslav. APPROVED.
Thanks for review.