[Bug 1265685] New: Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Bug ID: 1265685
           Summary: Review Request: sylfilter -- a Bayesian spam filter
                    for mailers
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

This is a review request for sylfilter.

here are the files for the packages:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-0.1.fc22.src.rpm

SylFilter is a generic message filter library and command-line tools.
SylFilter provides a Bayesian filter which is very popular as a spam filtering
algorithm.  
SylFilter is also internationalized and can be applied to any languages.

SylFilter library provides simple but powerful C APIs and can be used from C
programs.

SylFilter can be used as a command-line tool inside a junk filter mail program
similar to major tools such as bogofilter and bsfilter etc.

For further

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #1 from Globe Trotter  ---
Updated spec and SRPM files are here:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-1.fc22.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Michael Schwendt  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #2 from Michael Schwendt  ---
Not blocking bug 177841 means that the ticket has been visible only in the
queue of NEW reviews: http://fedoraproject.org/PackageReviewStatus/NEW.html

That nobody has commented on it implies that nobody from the community is
interested in it.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #3 from Ranjan Maitra  ---
Updated spec and SRPM files are here:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-1.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #4 from Ranjan Maitra  ---
Fedora Account System Username: maitra

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Ralf Corsepius  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Ralf Corsepius  ---
Ranjan, your "rant" on users@ has provoked me enough to add some 
informal comments on this package (based on *0.8-1 from comment#1):

* *.spec is cluttered with rpm anachronisms, 
  which should not be used in new *.spec:
- Remove %clean
- Remove "rm -rf $RPM_BUILD_ROOT" from %install
- Remove BuildRoot
- Remove %defattr

* Packages should not be installed using %makeinstall, but be using DESTDIR,
e.g. use %make_install instead.

* Package wastes resources on building a static library.
- Append --disable-static to %configure
- Remove "rm ..../*.a"

* AFAIS, this package seems to bundle a library (libsylpheed) which already is
in Fedora (package sylpheed). You should be using the unbundled version.
(%configure --with-libsylph=sylpheed --with-libsylph-dir=/usr)

* COPYING is duplicated in %doc and %license. Remove the copy from %doc.

* The package should be split into a *-devel and non-devel subpackages.

(In reply to Ranjan Maitra from comment #3)
> Updated spec and SRPM files are here:
>
> SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
> SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-1.fc23.src.rpm

Increment the %release each time you change something in your *spec. Not
incrementing Release avoidably complicates reviews.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #6 from Ranjan Maitra  ---
Thanks! I tried this, but now I get errors, indeed lots of it. Can I send you
personal e-mail?

Btw, my communication with Hiroyuki led me to believe that if I had sylpheed
installed as a binary, then I should use --with-libsylph=builtin.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #7 from Ralf Corsepius  ---
(In reply to Ranjan Maitra from comment #6)
> Thanks! I tried this, but now I get errors, indeed lots of it. Can I send
> you personal e-mail?

I don't have much spare time left today, and can't promise, but I can try to do
so, likely tomorrow.
> Btw, my communication with Hiroyuki led me to believe that if I had sylpheed
> installed as a binary, then I should use --with-libsylph=builtin.

No, unless sysfilter is crappily designed, there should not be any technical
reason for bundling.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #8 from Ranjan Maitra  ---
Here is the updated spec and rpm file. They compile. They do not compile if I
remove

% clean

and if I replace

--with-libsylph=sylpheed

in place of

--with-libsylph=builtin

Updated spec and SRPM files are here:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-.fc23.src.rpm

Not sure how to split a package into itself and a devel but is that important?

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #9 from Ranjan Maitra  ---
Never mind. I was able to correct it some more. Updated files are now here. (I
think that I was supposed to remmove the entire %clean section).

I now recall why I prefer builtin. This makes it independent of sylpheed. If it
is not there, it can not be used. But someone may want to use sylfilter which
is fast and lightweight using a different e-mailer. Indeed, I do so myself,
usning procmail with sylfilter and also sylpheed with sylfilter, the first to
do an initial processing and the second to classify mails that have not been
correctly classified.  Any suggestions as to how to handle this?

Btw, the new files are here (sorry for the typo in the previous post):

Updated spec and SRPM files are here:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-2.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #10 from Ralf Corsepius  ---
(In reply to Ranjan Maitra from comment #8)
> Not sure how to split a package into itself and a devel but is that
> important?

Yes. This is a MUSTFIX.

Somewhat oversimplyfied nutshell: 
* Move %includedir/*.h, %libdir/*.so and devel docs into *-devel
* Move %bindir/*, %libdir/*.so.* and user docs into <main>

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #11 from Ralf Corsepius  ---
(In reply to Ranjan Maitra from comment #9)
> I now recall why I prefer builtin. This makes it independent of sylpheed.

That's short-sighted, but explaining this would be beyond the scope of a
review.
> If
> it is not there, it can not be used.

Correct. Taking care about this is called "packaging".

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #12 from Ranjan Maitra  ---
(In reply to Ralf Corsepius from comment #10)
> (In reply to Ranjan Maitra from comment #8)
> > Not sure how to split a package into itself and a devel but is that
> > important?
>
> Yes. This is a MUSTFIX.
>
> Somewhat oversimplyfied nutshell:
> Move %includedir/*.h, %libdir/*.so and devel docs into *-devel
> Move %bindir/*, %libdir/*.so.* and user docs into <main>

OK I will look into this. Still not sure about the value but will do.

(In reply to Ralf Corsepius from comment #11)
> (In reply to Ranjan Maitra from comment #9)
> > I now recall why I prefer builtin. This makes it independent of sylpheed.
>
> That's short-sighted, but explaining this would be beyond the scope of a
> review.
> > If
> > it is not there, it can not be used.
>
> Correct. Taking care about this is called "packaging".

But then if sylpheed is not there, then it will be pulled in even if the user
does not use the e-mailer, but only the filter. Is that a good move? It does
not affect me personally because I use both sylpheed and sylfilter.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #13 from Michael Schwendt  ---
> But then if sylpheed is not there, then it will be pulled in even
> if the user does not use the e-mailer,

It would be perfectly imaginable to have the "sylpheed" src.rpm put libsylph
files into separate "libsylph" and "libsylph-devel" subpackages. It would not
be the first application package to split off a library like that.

Just because that hasn't been done so far is no reason not to do it as soon as
the first libsylph API users appear in the package collection.
> Btw, my communication with Hiroyuki led me to believe that if
> I had sylpheed installed as a binary, then I should use
> --with-libsylph=builtin.

That sounds unusual. If libsylph is available as a shared lib plus API headers,
why not reuse it then?

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #14 from Upstream Release Monitoring  ---
maitra's scratch build of sylfilter-0.8-2.fc23.src.rpm for f23 completed
http://koji.fedoraproject.org/koji/taskinfo?taskID=12027082

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #15 from Ralf Corsepius  ---
Any update?

Open issues with *-2:
* [MUSTFIX] Not building against external sylpheed
- Missing "BuildRequires: sylpheed-devel"
- Pass --with-libsylph=sylpheed instead of --with-libsylph=builtin
  to %configure

* [MUSTFIX] no *-devel subpackage

* [MUSTFIX] *.la are not removed.
Add
rm ${RPM_BUILD_ROOT}%{_libdir}/*.la
to %install

* The DESTDIR=... is redundant in 
"%make_install DESTDIR=$RPM_BUILD_ROOT"
"%make_install" already adds DESTDIR

* Useless define: %define  ver 0.8

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #16 from Ranjan Maitra  ---
(In reply to Ralf Corsepius from comment #15)
> Any update?
>
> Open issues with *-2:
> [MUSTFIX] Not building against external sylpheed
> Missing "BuildRequires: sylpheed-devel"
> Pass --with-libsylph=sylpheed instead of --with-libsylph=builtin
> to %configure

Why is this a MUSTFIX? Building against sylpheed will mean requiring the
package for installation. It reduces the package size from 91k to 46k which is
not much, however, requiring sylpheed for someone who does not intend to use
sylpheed for this purpose would increase his/her pulled-in RPM size by 7.8M
(which is what sylpheed's RPM is).

As I said, it does not affect me personally, but is this something we want to
do?

Nevertheless, I have done what you have suggested. Not sure if it is a good
idea.
> [MUSTFIX] no *-devel subpackage

Will have to fix this: first have to find how to do this.
> [MUSTFIX] *.la are not removed.
>
> Add
> rm ${RPM_BUILD_ROOT}%{_libdir}/*.la
> to %install

done (after %make_install)
> The DESTDIR=... is redundant in
>
> "%make_install DESTDIR=$RPM_BUILD_ROOT"
> "%make_install" already adds DESTDIR
> Useless define: %define  ver 0.8

I don't understand this one.

New files uploaded:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-3.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #17 from Ralf Corsepius  ---
(In reply to Ranjan Maitra from comment #16)
> (In reply to Ralf Corsepius from comment #15)
> > Any update?
> >
> > Open issues with *-2:
> > [MUSTFIX] Not building against external sylpheed
> > Missing "BuildRequires: sylpheed-devel"
> > Pass --with-libsylph=sylpheed instead of --with-libsylph=builtin
> > to %configure
>
> Why is this a MUSTFIX?

To put it bluntly: because bundling is harmful, stupid and dumb.

Less bluntly: bundling renders packages unmaintainable, vulnerable and causes
bloat.
> Building against sylpheed will mean requiring the
> package for installation. It reduces the package size from 91k to 46k which
> is not much, however, requiring sylpheed for someone who does not intend to
> use sylpheed for this purpose would increase his/her pulled-in RPM size by
> 7.8M (which is what sylpheed's RPM is).

Size-wise bundling causes bloat because each statically linked packages re-adds
the same libraries over and over again.

What causes the bloat in case of sylpheed is the libs* not having been split
out into a separate package and them being packaged together with docs and
binaries. If this is of real concern to you, you can consider to file an RFE
and request a split out of the libs.
> > [MUSTFIX] no *-devel subpackage
>
> Will have to fix this: first have to find how to do this.

Coarse (Incomplete) outline:

<snip>
% package devel
Summary: Development files for sylfilter
Requires: sylfilter%{?_isa} = %{version}-%{release}

%description devel
Development files for sylfilter
[...]
%files
%doc README
%license COPYING
%{_bindir}/sylfilter
%{_libdir}/libsylfilter.so.*

%files devel
%{_libdir}/libsylfilter.so
%{_includedir}/sylfilter
</snip>

I hope tot be able have a look into *-3, tomorrow.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #18 from Ranjan Maitra  ---
I have made the changes suggested by splitting sylfilter and making a
sylfilter-devel file. Please review and comment:

New files uploaded:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-4.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #19 from Ranjan Maitra  ---
Raif, Any luck finding time to look at this? Thanks!

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Christopher Meng  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #20 from Christopher Meng  ---
(In reply to Ranjan Maitra from comment #16)
> > Useless define: %define  ver 0.8
>
> I don't understand this one.

Please don't make use of any useless macros, you can also define a macro 'nam'
and value it like Name: %{nam} and so on.

However, these are just useless, keeping your spec simple but powerful is the
key that sponsor will basically notice your work. Keep it in a whirl will make
people think if you really know how to package. There is no luck for people
don't understand suggestions, especially useful ones.

Writing specs is not programming, you don't need to reinvent any wheels to
demonstrate that you are skilled (And from comments I don't think you are
skilled).

------------------------------- My review -------------------------------

1. Summary:  A generic message filter library and command-line tools

I guess you are not an native English speaker, please correct the sentence.
Hence please revise %description.

2. Requires: sylpheed

This is often superfluous. Generally, RPM will pick up it automatically.

3. %__make %_smp_mflags

Please keep in one style: %{XYZ} instead of %XYZ

Also try using %{make_build} if you want.

4. %changelog
* Fri Dec 11 2015 Ranjan Maitra <X> 0.8-4
- initial rebuild for Fedor (to include the devel file)
* Wed Dec 02 2015 Ranjan Maitra <X> 0.8-3
- initial rebuild for Fedor (to make dependent on sylpheed, as per  comments
from Ralf Corsepius <Y>)
* Tue Dec 01 2015 Ranjan Maitra <X> 0.8-2
- initial rebuild for Fedora (to take care of comments from Ralf Corsepius <Y>)
* Tue Sep 22 2015 Ranjan Maitra <X> 0.8-1
- initial build for Fedora

i. Please do not invoke any other people's email address unless there is a
strong reason you must do. For such simple anti spam trick, email address
collector will easily crack it.

ii. Please leave a blank line per entry.

iii. Please FIX the language, like 'Fedor'.

5. Check deps in -devel package to see if any explicit requires are needed.
(Like, you build package with sqlite-devel and glib2-devel and while installing
sylfilter-devel, RPM won't pick up sqlite-devel or glib2-devel)

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #21 from Ranjan Maitra  ---
(In reply to Christopher Meng from comment #20)
> (In reply to Ranjan Maitra from comment #16)
> > > Useless define: %define  ver 0.8
> >
> > I don't understand this one.
>
> Please don't make use of any useless macros, you can also define a macro
> 'nam' and value it like Name: %{nam} and so on.

Done.
> Writing specs is not programming, you don't need to reinvent any wheels to
> demonstrate that you are skilled (And from comments I don't think you are
> skilled).

Not much fear of that.
> ------------------------------- My review -------------------------------
> Summary:  A generic message filter library and command-line tools
>
> I guess you are not an native English speaker, please correct the sentence.
> Hence please revise %description.

I was using the sylfilter site's description earlier. Rephrased.
> Requires: sylpheed
>
> This is often superfluous. Generally, RPM will pick up it automatically.

Why so? If sylpheed is not installed, then sylfilter has to be built
differently.
> %__make %_smp_mflags
>
> Please keep in one style: %{XYZ} instead of %XYZ
>
> Also try using %{make_build} if you want.

Done.
> %changelog
> Fri Dec 11 2015 Ranjan Maitra <X> 0.8-4
> initial rebuild for Fedor (to include the devel file)
> Wed Dec 02 2015 Ranjan Maitra <X> 0.8-3
> initial rebuild for Fedor (to make dependent on sylpheed, as per  comments
>
> from Ralf Corsepius <Y>)
> Tue Dec 01 2015 Ranjan Maitra <X> 0.8-2
> initial rebuild for Fedora (to take care of comments from Ralf Corsepius
>
> <Y>)
> Tue Sep 22 2015 Ranjan Maitra <X> 0.8-1
> initial build for Fedora
>
> i. Please do not invoke any other people's email address unless there is a
> strong reason you must do. For such simple anti spam trick, email address
> collector will easily crack it.

Done. Removed all e-mail addresses.
> ii. Please leave a blank line per entry.

Done.
> iii. Please FIX the language, like 'Fedor'.

Done! And rpmlint did not picj this up?
> Check deps in -devel package to see if any explicit requires are needed.
>
> (Like, you build package with sqlite-devel and glib2-devel and while
> installing sylfilter-devel, RPM won't pick up sqlite-devel or glib2-devel)

Yes, fixed.

New files posted at:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-5.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #22 from Dan Horák  ---
> > Requires: sylpheed
> >
> > This is often superfluous. Generally, RPM will pick up it automatically.
>
> Why so? If sylpheed is not installed, then sylfilter has to be built
> differently.

no, rpmbuild adds automatically dependencies on shared libraries used in the
resulting rpm. So there will be always a dependency on libsylph no matter in
which package or rpm libsylph will live.

If sylfilter is useful without the sylpheed GUI client, we can move the
libsylph library into own subpackage.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #23 from Ranjan Maitra  ---
(In reply to Dan Horák from comment #22)
> > > Requires: sylpheed
> > >
> > > This is often superfluous. Generally, RPM will pick up it automatically.
> >
> > Why so? If sylpheed is not installed, then sylfilter has to be built
> > differently.
>
> no, rpmbuild adds automatically dependencies on shared libraries used in the
> resulting rpm. So there will be always a dependency on libsylph no matter in
> which package or rpm libsylph will live.
>
> If sylfilter is useful without the sylpheed GUI client, we can move the
> libsylph library into own subpackage.

Sylfilter is useful without the sylpheed GUI client, but does this not mean
that sylpheed will have to be rebuilt and be disruptive to an existing mature
package (the mailer?).

Btw, someone is trying to package libsylph separately, FWIW:
https://bugzilla.redhat.com/show_bug.cgi?id=1288927

New files removing requirement of sylpheed posted at:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-6.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #24 from Dan Horák  ---
(In reply to Ranjan Maitra from comment #23)
> (In reply to Dan Horák from comment #22)
> > > > Requires: sylpheed
> > > >
> > > > This is often superfluous. Generally, RPM will pick up it automatically.
> > >
> > > Why so? If sylpheed is not installed, then sylfilter has to be built
> > > differently.
> >
> > no, rpmbuild adds automatically dependencies on shared libraries used in the
> > resulting rpm. So there will be always a dependency on libsylph no matter in
> > which package or rpm libsylph will live.
> >
> > If sylfilter is useful without the sylpheed GUI client, we can move the
> > libsylph library into own subpackage.
>
> Sylfilter is useful without the sylpheed GUI client, but does this not mean
> that sylpheed will have to be rebuilt and be disruptive to an existing
> mature package (the mailer?).

There is nothing disruptive on introducing a new subpackage in an exiting
package. The library file is already there, it will just move to new rpm.
> Btw, someone is trying to package libsylph separately, FWIW:
> https://bugzilla.redhat.com/show_bug.cgi?id=1288927

I don't understand why someone wants to introduce 8 years old code when an
up-to-date version is already available in the distro ...
> New files removing requirement of sylpheed posted at:
>
> SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
> SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-6.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #25 from Ranjan Maitra  ---
(In reply to Dan Horák from comment #24)
> (In reply to Ranjan Maitra from comment #23)
> > (In reply to Dan Horák from comment #22)
> > > > > Requires: sylpheed
> > > > >
> > > > > This is often superfluous. Generally, RPM will pick up it automatically.
> > > >
> > > > Why so? If sylpheed is not installed, then sylfilter has to be built
> > > > differently.
> > >
> > > no, rpmbuild adds automatically dependencies on shared libraries used in the
> > > resulting rpm. So there will be always a dependency on libsylph no matter in
> > > which package or rpm libsylph will live.
> > >
> > > If sylfilter is useful without the sylpheed GUI client, we can move the
> > > libsylph library into own subpackage.
> >
> > Sylfilter is useful without the sylpheed GUI client, but does this not mean
> > that sylpheed will have to be rebuilt and be disruptive to an existing
> > mature package (the mailer?).
>
> There is nothing disruptive on introducing a new subpackage in an exiting
> package. The library file is already there, it will just move to new rpm.

OK, but I guess then I am stuck till this happens. I will containing using my
local rpm which has been serving me well for the past 5 years.
> > Btw, someone is trying to package libsylph separately, FWIW:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1288927
>
> I don't understand why someone wants to introduce 8 years old code when an
> up-to-date version is already available in the distro ...

I agree and have not quite understood the rationale.
> > New files removing requirement of sylpheed posted at:
> >
> > SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
> > SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-6.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #26 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Ralf Corsepius from comment #17)
> (In reply to Ranjan Maitra from comment #16)
> > (In reply to Ralf Corsepius from comment #15)
> > > Open issues with *-2:
> > > [MUSTFIX] Not building against external sylpheed
> > > Missing "BuildRequires: sylpheed-devel"
> > > Pass --with-libsylph=sylpheed instead of --with-libsylph=builtin
> > > to %configure
> >
> > Why is this a MUSTFIX?
>
> To put it bluntly: because bundling is harmful, stupid and dumb.
>
> Less bluntly: bundling renders packages unmaintainable, vulnerable and
> causes bloat.

To expand a bit on this, because Ranjan is a new packager, and is most likely
not aware of the long history of this topic ;):

Bundling used to be totally forbidden, with exceptions reluctantly granted
by the Fedora Packaging Committee. This policy was recently relaxed [1,2],
but bundling is still best avoided [3]. When the bundled library is already
packaged, it is required to use the system-wide copy:
"All packages whose upstreams allow them to be built against system libraries
must be built against system libraries."

The old policy [4] explains why bundling is bad. Please note that those
considerations are still valid, and the policy was only relaxed because
people were tired of fighting with unreasonable upstreams and massive
amounts of bundled code in some projects.

[1] https://fedorahosted.org/fesco/ticket/1483
[2] https://fedorahosted.org/fesco/ticket/1491
[3]
https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication...
[4]
https://fedoraproject.org/w/index.php?title=Packaging:No_Bundled_Libraries&a...
> > Building against sylpheed will mean requiring the
> > package for installation. It reduces the package size from 91k to 46k which
> > is not much, however, requiring sylpheed for someone who does not intend to
> > use sylpheed for this purpose would increase his/her pulled-in RPM size by
> > 7.8M (which is what sylpheed's RPM is).
>
> Size-wise bundling causes bloat because each statically linked packages
> re-adds the same libraries over and over again.

With today's disks, extra 8MB in files on disk is really unimportant.
Even if sylpheed is installed, nothing is run by default, so apart from
a bit of disk space this doesn't cause any problems. So even if the libs*
are *not* split out, it would be totally fine for sylfilter package to pull
in sylpheed.

Of course it is nice to split out libs* and avoid this bit
of bloat, but it's not that much of an issue, and no reason to hold up
the review. Currently sylpheed.rpm provides libsylph-0.so.1()(64bit),
and rpm will automatically generate a dependency on this in sylfilter.rpm.
If/when libs* are split out, this dependency will be satisfied by the
libs* subpackage. That's all.

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841 (FE-NEEDSPONSOR)     |

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #27 from Dan Horák  ---
The sylpheed-libs package has been introduced in sylpheed-3.4.3-3.fc24

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #28 from Ranjan Maitra  ---
Any news on this review? Thanks!

[Bug 1265685] Review Request: sylfilter -- a Bayesian spam filter for mailers

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #29 from Zbigniew Jędrzejewski-Szmek  ---
rpmlint is quite helpful:
sylfilter.i686: W: devel-file-in-non-devel-package /usr/lib/libsylfilter.so

.so is packaged in both subpackages. It should be present only in the -devel
supbackage. Common trick is to %{_libdir}/libsylfilter.so.* in the main
subpackage.

sylfilter-devel.i686: W: tag-in-description C Requires:
sylfilter-devel.i686: W: tag-in-description C Requires:
You should move the Requires tag above %description because they're being
interpreted as the part of the text.

/usr/lib/libsylfilter.la should be removed after installation
[https://fedoraproject.org/wiki/Packaging:Guidelines#StaticLibraries].

I looked over all the other comments, and it seems everything that other
reviews noted was fixed. It looks very clean now.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: sylfilter   |Review Request: sylfilter -
                   |-- a Bayesian spam filter   |A generic message filter
                   |for mailers                 |library and command-line
                   |                            |tools

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #30 from Ranjan Maitra  ---
Thank you for the detailed comments and suggested fixes - they have been
incorporated. Here are the results from rpmlint:

$ rpmlint  sylfilter-0.8-7.fc23.src.rpm 
sylfilter.src: W: spelling-error %description -l en_US bogofilter -> bogometer
sylfilter.src: W: spelling-error %description -l en_US bsfilter -> filterer,
filter
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

New files posted:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-7.fc22.src.rpm

Please let me know if additional corrections and information are needed.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #31 from Ranjan Maitra  ---
Corrected and reposted links (sorry):

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-7.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #32 from Zbigniew Jędrzejewski-Szmek  ---
? I don't see fixes for the things in comment #29 in the latest spec file.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #33 from Ranjan Maitra  ---
Perhaps I misunderstood your comments, but here are the changes that were made
in response to #29.
> .so is packaged in both subpackages. It should be present only in the -devel supbackage. Common trick is to %{_libdir}/libsylfilter.so.* in the main subpackage.

In the spec file: I did (see in particular the fifth line from here):
....
%files
%doc README
%{_bindir}/sylfilter
%{_libdir}/libsylfilter.*
%{_libdir}/libsylfilter.so.*
%license COPYING

%files devel
%{_libdir}/libsylfilter.so
%{_includedir}/sylfilter
> You should move the Requires tag above %description because they're being interpreted as the part of the text.

My spec file now goes like this:

%package devel
Summary: Development files for sylfilter
Requires: sylfilter%{?_isa} = %{version}-%{release}

%description devel
Development files for sylfilter
Requires: sqlite-devel
Requires: glib2-devel

%description
This is SylFilter, a generic message filter library, and some command-line
tools
that provide a Bayesian filter which is very popular as a spam filtering
algorithm.

Is this not what was suggested? Many thanks!

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #34 from Ranjan Maitra  ---
Sorry,  I think I found my error: new files uploaded

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-8.fc23.src.rpm

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #35 from Zbigniew Jędrzejewski-Szmek  ---
What about the .la file?

One more thing:
the %changelog is nearly useless.

*Tue Mar 22 2016 Ranjan Maitra 0.8-7
- initial rebuild for Fedora addresses comments in BZ 1265685
→ this tell me nothing. It should say "Fix %%description" or whatever.

* Thu Jan 7 2016 Ranjan Maitra 0.8-6
- initial rebuild for Fedora (removing requirement of sylpheed)
→ this should just say: "Remove requirement on sylpheed"

* Thu Jan 7 2016 Ranjan Maitra 0.8-5
- initial rebuild for Fedora (to fix stylistic suggestions per Christopher Meng
→ this should just say: "Fix style issues"
(or "Implement stylistic suggestions", if we want to be gramatically correct)

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #36 from Ranjan Maitra  ---
Sorry, I will work on a more descriptive %changelog spec file.

How does one remove the .la file?

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #37 from Ranjan Maitra  ---
Sorry, I will work on a more descriptive %changelog spec file.

How does one remove the .la file after installation? The webpage does not seem
to me to be clear. It only says remove after installation.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #38 from Zbigniew Jędrzejewski-Szmek  ---
(In reply to Ranjan Maitra from comment #37)
> How does one remove the .la file after installation? The webpage does not
> seem to me to be clear. It only says remove after installation.

At the end of the %install section add:
rm %{buildroot}%{_libdir}/*.la
(adjust path as necessary)

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #39 from Ranjan Maitra  ---
Thanks! It appears to work. New files posted:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-9.fc23.src.rpm

However, I seem to be getting a warning in my rpmlint now and I can't figure
out where this third warning came from. Very perplexing!

$ rpmlint sylfilter-0.8-9.fc23.src.rpm 
sylfilter.src: W: spelling-error %description -l en_US bogofilter -> bogometer
sylfilter.src: W: spelling-error %description -l en_US bsfilter -> filterer,
filter
sylfilter.src:70: W: macro-in-%changelog %description
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Zbigniew Jędrzejewski-Szmek  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST
              Flags|fedora-review?              |fedora-review+

--- Comment #40 from Zbigniew Jędrzejewski-Szmek  ---
In comments and changelog percent signs must be escaped by doubling:
- removes %description to after Requires
→
- move %%description after Requires

%files
...
%{_libdir}/libsylfilter.*
%{_libdir}/libsylfilter.so.*

Everything which is matched by the second glob is also matched by the first
glob. You only need the second line.

And as discussed before: there's no need to say "addresses comments of Zbigniew
Jędrzejewski-Szmek". I understand that the intent is to give due credit, but we
try to keep changelogs short. You can just remove those lines.

Package is APPROVED. Please fix up those minor issues in the initial upload.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #42 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/sylfilter

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #41 from Ranjan Maitra  ---
New files uploaded for the record:

SPEC: http://maitra.public.iastate.edu/Fedora/sylfilter.spec
SRPM: http://maitra.public.iastate.edu/Fedora/sylfilter-0.8-10.fc23.src.rpm

Will proceed with package approval.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #43 from Fedora Update System  ---
sylfilter-0.8-10.fc23 has been submitted as an update to Fedora 23.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-3e49eba720

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #45 from Fedora Update System  ---
sylfilter-0.8-10.fc24 has been submitted as an update to Fedora 24.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-8c3ce917d1

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #44 from Fedora Update System  ---
sylfilter-0.8-10.el7 has been submitted as an update to Fedora EPEL 7.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-70cf974005

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #46 from Fedora Update System  ---
sylfilter-0.8-10.fc22 has been submitted as an update to Fedora 22.
https://bodhi.fedoraproject.org/updates/FEDORA-2016-18c55e72fc

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #48 from Fedora Update System  ---
sylfilter-0.8-10.fc22 has been pushed to the Fedora 22 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-18c55e72fc

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #47 from Fedora Update System  ---
sylfilter-0.8-10.el7 has been pushed to the Fedora EPEL 7 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-70cf974005

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #49 from Fedora Update System  ---
sylfilter-0.8-10.fc23 has been pushed to the Fedora 23 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-3e49eba720

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #50 from Fedora Update System  ---
sylfilter-0.8-10.fc24 has been pushed to the Fedora 24 testing repository. If
problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-8c3ce917d1

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #51 from Fedora Update System  ---
sylfilter-0.8-10.fc24 has been pushed to the Fedora 24 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2016-04-02 11:55:51

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #52 from Fedora Update System  ---
sylfilter-0.8-10.fc23 has been pushed to the Fedora 23 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #53 from Fedora Update System  ---
sylfilter-0.8-10.fc22 has been pushed to the Fedora 22 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #54 from Fedora Update System  ---
sylfilter-0.8-10.fc23 has been pushed to the Fedora 23 stable repository. If
problems still persist, please make note of it in this bug report.

[Bug 1265685] Review Request: sylfilter - A generic message filter library and command-line tools

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1265685

--- Comment #55 from Fedora Update System  ---
sylfilter-0.8-10.el7 has been pushed to the Fedora EPEL 7 stable repository. If
problems still persist, please make note of it in this bug report.