Bug 515898

First, be sure to read and understand
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets

Now, a few comments:

- Drop
 BuildRequires: glibc-devel gcc binutils make
as these are in the standard buildroot 
http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

- Drop the Requires: line altogether, they're automatically picked up by RPM.
http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

- Instead of
 ./configure -p %{_prefix} -b %{_bindir} -s %{_datadir}/imapfilter -m %{_mandir}
use
 %configure

- Drop
 mkdir -m 755 -p ${RPM_BUILD_ROOT}%{_defaultdocdir}/%{name}-%{version}/
 cp README LICENSE ${RPM_BUILD_ROOT}%{_defaultdocdir}/%{name}-%{version}/
as you already have listed the files in %doc. (The doc directory is removed, recreated and the files listed in %doc placed into it when %doc is processed.)

- Replace
 /usr/bin/
with %{_bindir}
 /usr/share/
with %{_datadir}
 /usr/share/man
with %{_mandir}

- You're not owning %{_datadir}/imapfilter, instead of
 /usr/share/imapfilter/account.lua
 /usr/share/imapfilter/auxiliary.lua
 /usr/share/imapfilter/common.lua
 /usr/share/imapfilter/deprecated.lua
 /usr/share/imapfilter/mailbox.lua
 /usr/share/imapfilter/message.lua
 /usr/share/imapfilter/options.lua
 /usr/share/imapfilter/regex.lua
 /usr/share/imapfilter/set.lua
you can just specify
 %{_datadir}/imapfilter/

Quick update.  I rearranged things according to suggestions.  I could not change the ./configure to use %configure, as it is not a autotools based configure script, just a normal shell script doing some basic stuff.

btw.  Spec URL now contains the new spec file

Found one issue regarding debuginfo packages, missing CFLAGS=-g during compilation.  I've contacted the upstream developer to see if we can get a patch into the configure scripts which adds the needed compiler flags via CFLAGS.  Awaiting feedback before I continue.

(removed FE-NEEDSPONSOR)

I see you removed the NEEDSPONSOR blocker, but I can't find your name in the account system.  It's not "davids" and a search on "Sommerseth" finds nothing.  What's your FAS ID?

I downloaded and unpacked the src.rpm linked above, but it doesn't seem to match the spec file linked above.  Whenever you make a change to a package under  review, you should increase Release:, generate a new package and update both that and the spec file.  Otherwise it quickly becomes difficult for any prospective review to keep things straight.  There are far more submissions than reviewers, so making it  difficult for reviewers is not a good idea.

(In reply to comment #6)
> I see you removed the NEEDSPONSOR blocker, but I can't find your name in the
> account system.  It's not "davids" and a search on "Sommerseth" finds nothing. 

Ahh sorry! I thought that Jussi Lehtola forgot to do that when answering.  My mistake!

> What's your FAS ID?

It should be dsommers

> I downloaded and unpacked the src.rpm linked above, but it doesn't seem to
> match the spec file linked above.  Whenever you make a change to a package
> under  review, you should increase Release:, generate a new package and update
> both that and the spec file.  Otherwise it quickly becomes difficult for any
> prospective review to keep things straight.  There are far more submissions
> than reviewers, so making it  difficult for reviewers is not a good idea.  

Again, I'm sorry for making a mess here.

Anyhow!  A new release if imapfilter has surfaced, so I've updated spec files and built a new src.rpm.  Hopefully things are more consistent now.

Spec URL: http://people.redhat.com/dsommers/imapfilter/imapfilter.spec
SRPM URL: http://people.redhat.com/dsommers/imapfilter/imapfilter-2.0.11-1.fc11.src.rpm

You made me aware of rpmlint in another bz ... so I did run rpmlint on spec file, src.rpm and binary rpms.  But there is one thing which I'm not able to fix ...

imapfilter.src:25: W: configure-without-libdir-spec
imapfilter.spec:25: W: configure-without-libdir-spec

I believe this is because the configure script is not autotools based.  I've made a comment about it in the .spec file.  I did however reduce this warning to only appear once in each file and not twice, by renaming the ./configure script.

Oh, for whatever reason you have privacy set so that a search for your name finds nothing.  I see the "dsommers" account is not in the packager group and I don't see any offer to sponsor you in this ticket, so I've readded the NEEDSPONSOR blocker.

One thing I note immediately is that this package does not use the proper set of compiler flags.  I see you forced "-g"; you really need to pass all of $RPM_OPT_FLAGS.

Don't worry about the "configure-without-libdir-spec" complaint; rpmlint just doesn't understand that your configure script isn't generated by autoconf.  There is no need to try and work around bogus warnings at all, and doing so often only adds needless crap to the spec.  The point is to address the rpmlint complaints in the review, even if it's just to note that the complaints are erroneous.

Thanks again for a very quick feedback!

I've added the $RPM_OPT_FLAGS as requested.  In addition I also removed the renaming of the configure script.  I hope I haven't missed anything else ...

Spec URL: http://people.redhat.com/dsommers/imapfilter/imapfilter.spec
SRPM URL: http://people.redhat.com/dsommers/imapfilter/imapfilter-2.0.11-2.fc11.src.rpm

Some notes:

* Using %versin macro
  - Using %{version} macro on Source0 is preferred because with this
    you probably won't have to change Source0 line when version is
    upgraded:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* CFLAGS
  - CFLAGS is still wrong (i.e. Fedora specific compilation flags are
    not correctly honored).
-----------------------------------------------------------------------
    47  + make -j4
    48  cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -Wall -O -DMAKEFILE_SHAREDIR='"/usr/share/imapfilter"' -I/usr/local/include   -c -o auth.o auth.c
    49  cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -Wall -O -DMAKEFILE_SHAREDIR='"/usr/share/imapfilter"' -I/usr/local/include   -c -o buffer.o buffer.c
-----------------------------------------------------------------------
    Fedora's default optimization level is "-O2" (as you can check
    by $ rpm --eval %optflags), which is replaced by the latter
    "-O".

* Installing documents
------------------------------------------------------------------------
mkdir -m 755 -p ${RPM_BUILD_ROOT}%{_defaultdocdir}/%{name}-%{version}/
cp README LICENSE ${RPM_BUILD_ROOT}%{_defaultdocdir}/%{name}-%{version}/
------------------------------------------------------------------------
  - These two lines do nothing and should be removed.
    ! build.log shows this:
------------------------------------------------------------------------
   120  + mkdir -m 755 -p /builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11/
   121  + cp README LICENSE /builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11/
.....
.....
   133  Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.JZOnjc
   134  + umask 022
   135  + cd /builddir/build/BUILD
   136  + cd imapfilter-2.0.11
   137  + DOCDIR=/builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11
   138  + export DOCDIR
   139  + rm -rf /builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11
   140  + /bin/mkdir -p /builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11
   141  + cp -pr README LICENSE /builddir/build/BUILDROOT/imapfilter-2.0.11-2.fc12.i386/usr/share/doc/imapfilter-2.0.11
   142  + exit 0
------------------------------------------------------------------------
      Especially see the lines 139-140.
      i.e. %doc foo (where foo is not a absolute path) means:
      - First clean up %buildroot%_defaultdocdir/<rpm_name>-%version completely
      - Then create %buildroot%_defaultdocdir/<rpm_name>-%version and copy the
        specified files into the directory.

Applied changes suggested by Mamoru Tasaka in comment #10.

Spec URL: http://people.redhat.com/dsommers/imapfilter.spec
SRPM URL: http://people.redhat.com/dsommers/imapfilter-2.0.10-3.fc11.src.rpm
Patch0 URL: http://people.redhat.com/~dsommers/imapfilter/imapfilter-configure.patch

Please post the correct URL of the srpm :)

Anyway:
- Now this package can be approved.
- Usually a person who needs sponsorship for packager group is
  requested to either submit another review request or do a pre-review
  of other person's review request as written on:
  http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

  Now you have another review request (bug 515230), which seems good
  to some extent from a quick glance.

------------------------------------------------------
  This package (imapfilter) is APPROVED by mtasaka
------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
After you request for sponsorship a mail will be sent to sponsor 
members automatically (which is invisible for you) which notifies 
that you need a sponsor. After that, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 10/11/12, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

ping?

Sorry!  I've been quite busy lately and haven't had time to look into this yet.  I've done a koji scratch build which looked good at first sight.  

http://koji.fedoraproject.org/koji/taskinfo?taskID=1747177

Unfortunately, I will be very busy the next two weeks or so, before I'll manage to continue.  It's far from forgotten ... I just to complete some more urgent work first.

I'll come back stronger as soon as possible!

ping?

I'm ready for some work on this package now, after some über hectic weeks.  Going to read through a lot of the documentation about sponsorship again, to refresh what I need to do now.

Now I am sponsoring you. Please follow "Join" wiki again.

New Package CVS Request
=======================
Package Name: imapfilter
Short Description: A flexible client side mail filtering utility for IMAP servers
Owners: dsommers
Branches: F11 F21
InitialCC:

F21 is perhaps F12.

CVS done (with F-11 and F-12 branches instead of "F11 F21").

imapfilter-2.0.11-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/imapfilter-2.0.11-3.fc11

imapfilter-2.0.11-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/imapfilter-2.0.11-3.fc12

Closing.

imapfilter-2.0.11-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

imapfilter-2.0.11-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: imapfilter 
New Branches: el6
Owners: dsommers averi
InitialCC: dsommers averi

Imapfilter is not (yet) available for EPEL, please create the required branch so we can import it there. David acked this request.

Git done (by process-git-requests).

imapfilter-2.2.2-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/imapfilter-2.2.2-4.el6

imapfilter-2.5-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/imapfilter-2.5-1.el6

imapfilter-2.5-1.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.