This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 825415 - Review Request: opensmtpd - Free implementation of the server-side SMTP protocol as defined by RFC 5321
Review Request: opensmtpd - Free implementation of the server-side SMTP proto...
Status: CLOSED DUPLICATE of bug 1021719
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-25 22:48 EDT by Adrian Alves
Modified: 2013-10-22 02:46 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-22 02:46:08 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
updated spec for 5.3.3p1 (2.51 KB, text/x-rpm-spec)
2013-09-13 00:07 EDT, Bobby Powers
no flags Details
opensmtpd-5.3.3p1 (snapshot) (5.78 KB, text/x-rpm-spec)
2013-10-05 18:35 EDT, Denis Fateyev
no flags Details
opensmtpd-5.3.3p1 (systemd) (6.67 KB, text/x-rpm-spec)
2013-10-10 15:25 EDT, Denis Fateyev
no flags Details

  None (edit)
Description Adrian Alves 2012-05-25 22:48:00 EDT
Spec URL: http://alvesadrian.fedorapeople.org/opensmtpd.spec
SRPM URL: http://alvesadrian.fedorapeople.org/opensmtpd-201205220027-1.fc16.src.rpm
Description: OpenSMTPD is a FREE implementation of the server-side SMTP protocol as defined by RFC 5321, with some additional standard extensions. It allows ordinary machines to exchange e-mails with other systems speaking the SMTP protocol. Started out of dissatisfaction with other implementations, OpenSMTPD nowadays is a fairly complete SMTP implementation. OpenSMTPD is primarily developed by Gilles Chehade, Eric Faurot and Charles Longeau; with contributions from various OpenBSD hackers. OpenSMTPD is part of the OpenBSD Project. The software is freely usable and re-usable by everyone under an ISC license. 
Fedora Account System Username: alvesadrian
Comment 1 Terje Røsten 2012-05-26 03:38:25 EDT
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
Comment 2 Matthias Runge 2012-05-29 05:14:44 EDT
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
....
Comment 3 Adrian Alves 2012-05-29 06:00:20 EDT
(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?
Comment 4 Matthias Runge 2012-05-29 06:03:21 EDT
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)
Comment 5 Adrian Alves 2012-05-30 20:41:04 EDT
(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
Comment 6 Thomas Spura 2012-05-31 05:09:56 EDT
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.
Comment 7 Terje Røsten 2012-05-31 13:42:54 EDT
This don't build in mock, ends with:

+ unset DISPLAY
+ ./bootstrap
./bootstrap: you need libtool
Comment 8 Matthias Runge 2012-06-01 02:36:49 EDT
As an addition to 825415#c1 :
MUST No %config files under /usr. 

/usr/etc/mail for config files fails in several ways.
Comment 9 Adrian Alves 2012-06-01 22:46:45 EDT
(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
Comment 10 Adrian Alves 2012-06-01 22:48:18 EDT
(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
Comment 11 Terje Røsten 2012-06-02 03:54:24 EDT
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.
Comment 12 Matthias Runge 2012-06-02 04:06:25 EDT
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!
Comment 13 Adrian Alves 2012-06-09 20:43:47 EDT
(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
Comment 14 Thomas Spura 2012-06-12 05:23:04 EDT
(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...
Comment 15 Adrian Alves 2012-06-12 21:36:35 EDT
(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
Comment 16 Matthias Runge 2012-06-17 16:45:00 EDT
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
Comment 17 Bobby Powers 2013-09-13 00:07:36 EDT
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!
Comment 18 Terje Røsten 2013-09-15 07:55:34 EDT
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.
Comment 19 Denis Fateyev 2013-10-05 18:35:29 EDT
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.
Comment 20 Terje Røsten 2013-10-07 14:27:07 EDT
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
Comment 21 Denis Fateyev 2013-10-10 15:25:18 EDT
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.
Comment 22 Matthias Runge 2013-10-22 02:46:08 EDT

*** This bug has been marked as a duplicate of bug 1021719 ***

Note You need to log in before you can comment on or make changes to this bug.