Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 825415
Summary: | Review Request: opensmtpd - Free implementation of the server-side SMTP protocol as defined by RFC 5321 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Adrian Alves <aalves> | ||||||||
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> | ||||||||
Status: | CLOSED DUPLICATE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | unspecified | ||||||||||
Version: | rawhide | CC: | aalves, bobbypowers, denis, mrunge, notting, terje.rosten, tomspur | ||||||||
Target Milestone: | --- | ||||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2013-10-22 06:46:08 UTC | Type: | Bug | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Attachments: |
|
Description
Adrian Alves
2012-05-26 02:48:00 UTC
Adrian, could you please take some time and read the Fedora Packaging Guidelines? http://fedoraproject.org/wiki/Packaging:Guidelines This package is not following: http://fedoraproject.org/wiki/Packaging:Guidelines#No_Files_or_Directories_under_.2Fsrv.2C_.2Fopt.2C_or_.2Fusr.2Flocal Sigh, Adrian, would you please mind and try something like rpmlint or try to build your package under mock? Once you submitted your packages for review, you could also try to run e.g. fedora-review -b 825415 (to see this package fail to build) because of .... ./bootstrap: you need automake version 1.5 or later .... (In reply to comment #2) > Sigh, > > Adrian, would you please mind and try something like rpmlint or try to build > your package under mock? > > Once you submitted your packages for review, you could also try to run e.g. > fedora-review -b 825415 > (to see this package fail to build) > because of > .... > ./bootstrap: you need automake version 1.5 or later > .... Am using mock to build stuff. what u mean wit this: ./bootstrap: you need automake version 1.5 or later automake as a buildrequire or as a requiere? You did build that package in mock? That's astonishing: Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.sCYC5f + umask 022 + cd /builddir/build/BUILD + LANG=C + export LANG + unset DISPLAY + cd /builddir/build/BUILD + rm -rf opensmtpd + /usr/bin/gzip -dc /builddir/build/SOURCES/opensmtpd-201205220027.tar.gz + /usr/bin/tar -xf - + STATUS=0 + '[' 0 -ne 0 ']' + cd opensmtpd + /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w . + exit 0 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.akrw2O + umask 022 + cd /builddir/build/BUILD + cd opensmtpd + LANG=C + export LANG + unset DISPLAY + ./bootstrap ./bootstrap: you need automake version 1.5 or later RPM build errors: error: Bad exit status from /var/tmp/rpm-tmp.akrw2O (%build) Bad exit status from /var/tmp/rpm-tmp.akrw2O (%build) (that error message comes from build process using mock) (In reply to comment #1) > Adrian, could you please take some time and read the Fedora Packaging > Guidelines? > > http://fedoraproject.org/wiki/Packaging:Guidelines > > This package is not following: > > http://fedoraproject.org/wiki/Packaging: > Guidelines#No_Files_or_Directories_under_.2Fsrv.2C_.2Fopt.2C_or_.2Fusr. > 2Flocal Fixed the /usr/local path Spec URL: http://alvesadrian.fedorapeople.org/opensmtpd.spec SRPM URL: http://alvesadrian.fedorapeople.org/opensmtpd-201205220027-2.fc16.src.rpm Could you run rpmlint on the srpm/rpms and try to reduce the output? This is usually a good indicator of violating the guidlines, but not always. In this review request ALL output is at least SHOULD. This don't build in mock, ends with: + unset DISPLAY + ./bootstrap ./bootstrap: you need libtool As an addition to 825415#c1 : MUST No %config files under /usr. /usr/etc/mail for config files fails in several ways. (In reply to comment #7) > This don't build in mock, ends with: > > + unset DISPLAY > + ./bootstrap > ./bootstrap: you need libtool Added, check it now. Spec URL: http://alvesadrian.fedorapeople.org/opensmtpd.spec SRPM URL: http://alvesadrian.fedorapeople.org/opensmtpd-201205220027-3.fc16.src.rpm (In reply to comment #8) > As an addition to 825415#c1 : > MUST No %config files under /usr. > > /usr/etc/mail for config files fails in several ways. check my spec the only %config file goes to %config(noreplace) %{_sysconfdir}/mail/smtpd.conf it means /etc/mail/smtpd.conf A bit better, now it ends with: if test "cat" = "man"; then \ /bin/sed -e 's|/etc/mail/|/usr/etc/mail/|g' -e 's|/usr/libexec|/usr/libexec|g' -e 's|/var/run/smtpd.sock|/var/run/smtpd.sock|g' ${manpage} | gawk -f ./mdoc2man.awk > makemap.8.out; \ else \ /bin/sed -e 's|/etc/mail/|/usr/etc/mail/|g' -e 's|/usr/libexec|/usr/libexec|g' -e 's|/var/run/smtpd.sock|/var/run/smtpd.sock|g' ${manpage} > makemap.8.out; \ fi /bin/sed: can't read ./makemap.0: No such file or directory make[4]: *** [makemap.8.out] Error 2 make[4]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' make[3]: *** [install-exec-am] Error 2 make[3]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' make[2]: *** [install-am] Error 2 make[2]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' make[1]: *** [install-recursive] Error 1 make[1]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' make: *** [install-recursive] Error 1 error: Bad exit status from /var/tmp/rpm-tmp.qtP0Sa (%install) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.qtP0Sa (%install) Child return code was: 1' How should OpenSMTPD daemon be started? I don't see any sysv or systemd scripts included. Taken from your spec[1]: %files %config(noreplace) %{_sysconfdir}/mail/smtpd.conf %{_bindir}/* /usr/etc/mail/smtpd.conf ^^^^^^^^^^ What happens, when /etc/mail doesn't exist? Which package owns that dir? If you meant /etc/mail/smtpd.conf to be placed there, you need to require that package, which owns /etc/mail (imho currently sendmail) [1]: http://alvesadrian.fedorapeople.org/opensmtpd.spec Before we go any further here, you should run something like: fedora-review -b 825415 You'll get at least: Issues: [!]: MUST Package successfully compiles and builds into binary rpms on at least one supported primary architecture. See: http://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Support [!]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. Note: The package did not built BR could therefore not be checked or the package failed to build because of missing BR See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 [!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf is only needed if supporting EPEL5 Your package still doesn't compile under mock. I told you in comment #4, you should test that, you said it does and it turned out there were missing build requirements. I strongly suggest you read the output from fedora-review. If you don't understand something there, ask! (In reply to comment #11) > A bit better, now it ends with: > > if test "cat" = "man"; then \ > /bin/sed -e 's|/etc/mail/|/usr/etc/mail/|g' -e > 's|/usr/libexec|/usr/libexec|g' -e > 's|/var/run/smtpd.sock|/var/run/smtpd.sock|g' ${manpage} | gawk -f > ./mdoc2man.awk > makemap.8.out; \ > else \ > /bin/sed -e 's|/etc/mail/|/usr/etc/mail/|g' -e > 's|/usr/libexec|/usr/libexec|g' -e > 's|/var/run/smtpd.sock|/var/run/smtpd.sock|g' ${manpage} > makemap.8.out; \ > fi > /bin/sed: can't read ./makemap.0: No such file or directory > make[4]: *** [makemap.8.out] Error 2 > make[4]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' > make[3]: *** [install-exec-am] Error 2 > make[3]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' > make[2]: *** [install-am] Error 2 > make[2]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' > make[1]: *** [install-recursive] Error 1 > make[1]: Leaving directory `/builddir/build/BUILD/opensmtpd/src' > make: *** [install-recursive] Error 1 > error: Bad exit status from /var/tmp/rpm-tmp.qtP0Sa (%install) > RPM build errors: > Bad exit status from /var/tmp/rpm-tmp.qtP0Sa (%install) > Child return code was: 1' > > > How should OpenSMTPD daemon be started? I don't see any sysv or systemd > scripts included. I need help on build a systemd deamon for this not clue how to make one yet (In reply to comment #13) > I need help on build a systemd deamon for this not clue how to make one yet I think this is the smallest problem right now. Change the spec file so rpmlint is as reduced as reasonable and look, that it builds properly in mock. For instance, it's unusual to call ./configure directly, use the macro for that and many problems mentioned by Matthias will be gone my magic. But I'm afraid to get it properly building you need to patch it a bit... After that, we can have a look at adding systemd snippeds... (In reply to comment #14) > (In reply to comment #13) > > I need help on build a systemd deamon for this not clue how to make one yet > > I think this is the smallest problem right now. > > Change the spec file so rpmlint is as reduced as reasonable and look, that > it builds properly in mock. > For instance, it's unusual to call ./configure directly, use the macro for > that and many problems mentioned by Matthias will be gone my magic. > But I'm afraid to get it properly building you need to patch it a bit... > > After that, we can have a look at adding systemd snippeds... New release with all the fixes suggested by rpmlint, [adrian@fedora SPECS]$ rpmlint ../SRPMS/opensmtpd-201205220027-4.fc16.src.rpm opensmtpd.src: I: enchant-dictionary-not-found en_US 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Spec URL: http://alvesadrian.fedorapeople.org/opensmtpd.spec SRPM URL: http://alvesadrian.fedorapeople.org/opensmtpd-201205220027-4.fc16.src.rpm Now I will need assistance with systemd startup script https://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir: Packagers are highly encouraged to store libexecdir files in a package-specific subdirectory of %{_libexecdir}, such as %{_libexecdir}/%{name}. your files section contains: %{_libexecdir}/mail.local opensmtpd uses config in /etc/mail, which is owned by sendmail. Regarding /etc/mail, please take a look at https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function [root@sofja ~]# rpmquery -f /usr/bin/mailq sendmail-8.14.5-12.fc17.i686 [root@sofja ~]# rpmquery -f /usr/bin/newaliases sendmail-8.14.5-12.fc17.i686 [mrunge@sofja opensmtpd]$ repoquery -f /usr/sbin/makemap sendmail-0:8.14.5-12.fc17.i686 Looks like you're using filenames from sendmail which produces a conflict. https://fedoraproject.org/wiki/Packaging:Conflicts#Binary_Name_Conflicts - you didn't package the latest version - you definitely need to include the license file, I'd also include the readme. (as %doc) - finally: how are you going to handle openbsd-compat? It includes several files not originating from opensmtpd. https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries List of files and where they come from base64.{c,h} portable openssh basename.c portable openssh bsd-arc4random.c portable openssh bsd-closefrom.c portable openssh bsd-getpeereid.c portable openssh bsd-waitpid.{c,h} portable openssh daemon.c portable openssh defines.h portable openssh dirname.c portable openssh entropy.{c,h} portable openssh fgetln.c part of /usr/src/usr.bin/make/util.c fmt_scaled.c portable openssh fparseln.c part of /usr/src/libutil/fparseln.c getopt.c portable openssh imsg-buffer.c part of /usr/src/libutil/imsg-buffer.c imsg.{c,h} part of /usr/src/libutil/imsg.c includes.h portable openssh log.h mktemp.c portable openssh openbsd-compat.h setproctitle.c portable openssh Created attachment 797117 [details]
updated spec for 5.3.3p1
I've brushed off the spec and got it building with the latest 5.3.3p1 (portable) release (attached). It works well with the exception of manpage installation - some autotools macro goes wonk and insists on calling man cat. The spec I attached works around that - a proper fix would be better but I didn't have the patience to look into it this afternoon.
Matthais had a bunch of concerns - I'm interested in helping try to help sort through them and get this packaged, although I'm not a Fedora packager yet myself.
%{_libexecdir}/mail.local - didn't fix this yet, seems straightforward.
The updated spec no longer uses /etc/mail for its config file - it installs the single config in %{_sysconfdir}.
As for the file-name conflicts with sendmail - could we use /etc/alternatives for that? Seems like the straightforward way to do it, although if sendmail doesn't already alternative mailq, newaliases and makemap it will require a change to that package as well.
I can also easily add the licence and readme.
As for openbsd-compat - openssh contains the same lib/shim - my understanding is that it is part of the process the open* guys use to turn openssh/opensmtpd from the openBSD specific release -> portable release. I don't see anything in the openbsd packaging that removes it - so I assume they have an exception, it seems appropriate here as well?
Thanks!
Some comments: o in this line: %configure --prefix=/usr Remove --prefix=/usr , %configure sets --prefix for you. o Please add LICENSE, README.md and THANKS by using %doc macro in %files o I had to remove these: mv $RPM_BUILD_ROOT%{_mandir}/cat5 $RPM_BUILD_ROOT%{_mandir}/man5 mv $RPM_BUILD_ROOT%{_mandir}/cat8 $RPM_BUILD_ROOT%{_mandir}/man8 to build. o please do a koji scratch build: https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds o package will need to work with postfix, exim and sendmail, study these packages and add alternatives etc as needed. You can browse spec files here: http://pkgs.fedoraproject.org/cgit/$mta.git $mta=postfix,exim,sendmail o you can handle openbsd-compat the same way the openssh package do it please change License tag to fit. Created attachment 808309 [details]
opensmtpd-5.3.3p1 (snapshot)
I've been working on packaging "opensmtpd" for a while, in collaboration with core developers. Here is my current spec which has alternative support, pam, initscript, manpages, proper package dependencies and snapshots handling.
Currently, there is a small issue with "libexecdir" detection and usage, I think it will be resolved shortly according our recent conversation with developers (alternatively, a patch can be applied). The same with one missing manpage.
Comments, suggestions and improvements are welcome.
Nice improvements! Some comments: > %define prerelease 201310031101 Please use %global for %define. > BuildRequires: pam-devel Add extra space to align. > echo -e "\n Don't forget to call: /usr/sbin/alternatives --set mta /usr/sbin/sendmail.opensmtpd" > echo -e " if you want to switch to OpenSMTPD MTA immediately.\n" > echo -e "\n Don't forget to call: /usr/sbin/alternatives --set mta /usr/sbin/sendmail.sendmail" > echo -e " if you want to switch to Sendmail after OpenSMTPD MTA uninstall.\n" Remove these, rpm/yum is not interactive in such sense. However, you can add this info to a README.fedora file and/or in %description > %doc %{_mandir}/man5/*.5.gz > %doc %{_mandir}/man8/*.8.gz No need to mark as %doc, rpmbuild understand mans are docs. The only real problem left with this package now is conversion to systemd. Here are some pages to assist: https://fedoraproject.org/wiki/Packaging:Systemd http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd Created attachment 810722 [details] opensmtpd-5.3.3p1 (systemd) Opensmtpd specfile converted to systemd. Successfully built with mock, packages are tested on Fedora 19 (x86_64). Source RPM is available here: http://www.fateyev.com/RPMS/Fedora19/testing/SRPMS/ Binary packages: http://www.fateyev.com/RPMS/Fedora19/testing/x86_64/ There are small issues left: "libexecdir" detection and usage (currently `%{_libexecdir}/opensmtpd/smtpd` instead of `%{_libexecdir}/opensmtpd` is used during build), and a wrong path substitution in `smtpd.conf` (need to replace `/etc/opensmtpd/aliases` with `/etc/aliases` after package install). They're portable-specific issues and hopefully will be fixed shortly. Alternatively, I can write a small patch. *** This bug has been marked as a duplicate of bug 1021719 *** |