Bug 515898 - Review Request: imapfilter - A flexible client side mail filtering utility for IMAP servers
Summary: Review Request: imapfilter - A flexible client side mail filtering utility fo...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-06 09:35 UTC by David Sommerseth
Modified: 2016-05-22 23:28 UTC (History)
7 users (show)

Fixed In Version: imapfilter-2.5-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-17 16:53:19 UTC
mtasaka: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description David Sommerseth 2009-08-06 09:35:57 UTC
Spec URL: http://people.redhat.com/dsommers/imapfilter.spec
SRPM URL: http://people.redhat.com/dsommers/imapfilter-2.0.10-1.fc11.src.rpm
Description: 
IMAPFilter is a mail filtering utility. It connects to remote mail servers
using the Internet Message Access Protocol (IMAP), sends searching queries
to the server and processes mailboxes based on the results. It can be used
to delete, copy, move, flag, etc. messages residing in mailboxes at the
same or different mail servers. The 4rev1 and 4 versions of the IMAP
protocol are supported.

This is just a pure packaging of the upstream imapfilter.

Comment 1 Susi Lehtola 2009-08-06 11:54:58 UTC
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/

Comment 2 David Sommerseth 2009-08-10 12:08:44 UTC
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.

Comment 3 David Sommerseth 2009-08-10 12:09:24 UTC
btw.  Spec URL now contains the new spec file

Comment 4 David Sommerseth 2009-08-10 17:42:33 UTC
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.

Comment 5 David Sommerseth 2009-08-10 17:43:22 UTC
(removed FE-NEEDSPONSOR)

Comment 6 Jason Tibbitts 2009-09-10 00:38:14 UTC
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.

Comment 7 David Sommerseth 2009-09-23 08:59:49 UTC
(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.

Comment 8 Jason Tibbitts 2009-09-23 17:37:09 UTC
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.

Comment 9 David Sommerseth 2009-09-24 08:34:22 UTC
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

Comment 10 Mamoru TASAKA 2009-10-12 19:30:25 UTC
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.

Comment 12 Mamoru TASAKA 2009-10-14 19:11:39 UTC
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.

Comment 13 Mamoru TASAKA 2009-10-23 14:16:20 UTC
ping?

Comment 14 David Sommerseth 2009-10-23 14:39:11 UTC
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!

Comment 15 Ian Weller 2009-11-14 22:02:03 UTC
ping?

Comment 16 David Sommerseth 2009-11-16 09:44:17 UTC
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.

Comment 17 Mamoru TASAKA 2009-11-16 12:24:04 UTC
Now I am sponsoring you. Please follow "Join" wiki again.

Comment 18 David Sommerseth 2009-11-16 13:36:38 UTC
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:

Comment 19 Mamoru TASAKA 2009-11-16 15:10:21 UTC
F21 is perhaps F12.

Comment 20 Jason Tibbitts 2009-11-16 17:57:32 UTC
CVS done (with F-11 and F-12 branches instead of "F11 F21").

Comment 21 Fedora Update System 2009-11-16 19:02:21 UTC
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

Comment 22 Fedora Update System 2009-11-16 19:02:27 UTC
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

Comment 23 Mamoru TASAKA 2009-11-17 16:53:19 UTC
Closing.

Comment 24 Fedora Update System 2009-12-01 04:29:36 UTC
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.

Comment 25 Fedora Update System 2009-12-04 23:58:25 UTC
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.

Comment 26 Andrea Veri 2012-02-24 11:22:23 UTC
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.

Comment 27 Gwyn Ciesla 2012-02-24 13:02:25 UTC
Git done (by process-git-requests).

Comment 28 Fedora Update System 2012-02-24 13:20:32 UTC
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

Comment 29 Fedora Update System 2012-02-27 11:44:57 UTC
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

Comment 30 Fedora Update System 2012-03-15 19:56:07 UTC
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.


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