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 ***