Bug 455067 - Review Request: ferm - For Easy Rule Making
Review Request: ferm - For Easy Rule Making
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-11 15:53 EDT by Pavel Alexeev
Modified: 2010-04-01 17:01 EDT (History)
4 users (show)

See Also:
Fixed In Version: ferm-2.0.7-8.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-03-09 14:13:30 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pavel Alexeev 2008-07-11 15:53:32 EDT
Spec URL: http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
SRPM URL: http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-1.3.4-0.fc9.Hu.1.src.rpm

Hello. I'm pack ferm into rpm package and I would appreciate a review so that I can get it into Fedora repo.

Small note: I'm read naming guide and understand it, but in my own rpm-repository (http://hubbitus.net.ru/rpm) all my packages with changes made have portion of my release like ".Hu.<number>". This addon of release made to differ version from upstream packages.

Ferm description:
Ferm is a tool to maintain complex firewalls, without having
the trouble to rewrite the complex rules over and over again.
Ferm allows the entire firewall rule set to be stored in a separate
file, and to be loaded with one command. The firewall
configuration resembles structured programming-like language, which can
contain levels and lists.
Comment 1 Pavel Alexeev 2008-08-07 15:36:04 EDT
I am a new contributor and I am seeking a sponsor.
Comment 2 Mamoru TASAKA 2008-08-21 13:02:01 EDT
As it seems that the current latest version is 1.3.5, would you update srpm first?

Some notes:
- The name of spec file must be "ferm.spec"
- "ferm -" is redundant for Summary.
- "system/firewalls" is not a standard Group (please refer to
  $ rpmlint -I non-standard-group
- Please remove some meaningless release number suffix.
- The license tag "GPL" is not allowed on current Fedora packaging guidelines:
  https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
  https://fedoraproject.org/wiki/Licensing
- Source must be given by full URL:
  https://fedoraproject.org/wiki/Packaging/SourceURL
- "Requires: perl" is redundant. Usually perl (module) related dependencies are
  automatically detected and added to rebuilt binary rpms by rpmbuild.
- Move the lines:
-----------------------------------------------------------------
sed -i 's/PREFIX = /#PREFIX = /' config.mk
sed -i 's/MANDIR = /#MANDIR = /' config.mk
sed -i 's/DOCDIR = /#DOCDIR = /' config.mk
-----------------------------------------------------------------
  to %build or %prep (%prep is preferred for --short-circuit option)
- Please use macros. /usr should be %{_prefix}, for example:
  https://fedoraproject.org/wiki/Packaging/RPMMacros
- Now %defattr(-,root,root,-) is preferred.
Comment 3 Pavel Alexeev 2008-08-22 05:13:53 EDT
Fist, Mamoru Tasaka very thanks for review.

(In reply to comment #2)
> As it seems that the current latest version is 1.3.5, would you update srpm
> first?
Of course! Moreover now last version 2.0.2. I upgrade to it.

> Some notes:
> - The name of spec file must be "ferm.spec"
Ok.
> - "ferm -" is redundant for Summary.
Ok.
> - "system/firewalls" is not a standard Group (please refer to
>   $ rpmlint -I non-standard-group
Thanks. I'm change it to "Applications/System".
But another question is why rpmlint was silent??

> - Please remove some meaningless release number suffix.
Off course. But it is not meaningless for me :)
> - The license tag "GPL" is not allowed on current Fedora packaging guidelines:
>   https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
>   https://fedoraproject.org/wiki/Licensing
Ok, set it into GPLv2.
> - Source must be given by full URL:
>   https://fedoraproject.org/wiki/Packaging/SourceURL
Set to http://ferm.foo-projects.org/download/2.0/%{name}-%{version}.tar.gz
> - "Requires: perl" is redundant. Usually perl (module) related dependencies are
>   automatically detected and added to rebuilt binary rpms by rpmbuild.
I suppose that do not requires external perl modules. Requires: perl removed.
But I can't understand how it will be detected? It is based on standard Makefile, nor perl buildsystem.
> - Move the lines:
> -----------------------------------------------------------------
> sed -i 's/PREFIX = /#PREFIX = /' config.mk
> sed -i 's/MANDIR = /#MANDIR = /' config.mk
> sed -i 's/DOCDIR = /#DOCDIR = /' config.mk
> -----------------------------------------------------------------
>   to %build or %prep (%prep is preferred for --short-circuit option)
Ok. Moved to %prep section.
> - Please use macros. /usr should be %{_prefix}, for example:
>   https://fedoraproject.org/wiki/Packaging/RPMMacros
Done.
> - Now %defattr(-,root,root,-) is preferred.
Instead of %defattr(-,root,root)? Why?
Done.

http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-2.0.2-0.fc9.src.rpm
Comment 4 Mamoru TASAKA 2008-08-24 02:20:23 EDT
For 2.0.2-0:

* Release number
  - The minimum number which can be used for Release number is 1.
    0 is used only for pre-release tarballs:
    http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release
    (Of course, please change the Release number every time you modify
     your spec file)

* License
  - As far as I verified the source code, the license tag should
    be "GPLv2+".

* rpmlint issue
------------------------------------------------------
ferm.noarch: W: incoherent-version-in-changelog 2.0.2 2.0.2-0.fc10
------------------------------------------------------
  - The last entry of %changelog should contain full EVR (epoch-version-release)
    information like:
------------------------------------------------------
* Fri Aug 22 2008 Pavel Alexeev <Pahan [ at ] Hubbitus [ DOT ] spb [ dOt.] su> - 2.0.2-1
------------------------------------------------------

* Macros usage consistency
  - When using { on macros, please use { for all macros (except for some cases)
    consistently. You use %{_mandir} and %_sbindir , for example.

* Documents
  - Please also add the following files to %doc.
------------------------------------------------------
NEWS
------------------------------------------------------

(In reply to comment #3)
> (In reply to comment #2)
> > - "system/firewalls" is not a standard Group (please refer to
> >   $ rpmlint -I non-standard-group
> Thanks. I'm change it to "Applications/System".
> But another question is why rpmlint was silent??
  - On my system rpmlint warned about this.s

> > - "Requires: perl" is redundant. Usually perl (module) related dependencies are
> >   automatically detected and added to rebuilt binary rpms by rpmbuild.
> I suppose that do not requires external perl modules. Requires: perl removed.
> But I can't understand how it will be detected? It is based on standard
> Makefile, nor perl buildsystem.

  - This is detected by /usr/lib/rpm/perl.req. Try
------------------------------------------------------
$ rpm -ql ferm | /usr/lib/rpm/perl.req
------------------------------------------------------
    For example when perl.req finds the line like:
------------------------------------------------------
use Data::Dumper;
------------------------------------------------------
    (in /usr/sbin/import-ferm), rpmlint adds "Requires: perl(Data::Dumper) to
    binary rpms.
Comment 5 Mamoru TASAKA 2008-08-24 02:21:42 EDT
By the way:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 6 Pavel Alexeev 2008-08-24 20:26:41 EDT
(In reply to comment #4)
> For 2.0.2-0:
> 
> * Release number
>   - The minimum number which can be used for Release number is 1.
>     0 is used only for pre-release tarballs:
Hmmm, I'm lose sight of.
I assume now release shoud be 2 (1 should have been previous).

> * License
>   - As far as I verified the source code, the license tag should
>     be "GPLv2+".
Official site ( http://ferm.foo-projects.org/) says: "Licensed under the GPLv2"

> 
> * rpmlint issue
> ------------------------------------------------------
> ferm.noarch: W: incoherent-version-in-changelog 2.0.2 2.0.2-0.fc10
> ------------------------------------------------------
Ok, done.

> * Macros usage consistency
>   - When using { on macros, please use { for all macros (except for some cases)
>     consistently. You use %{_mandir} and %_sbindir , for example.
                                                                               
%_sbindir replaced by %{_sbindir}
Is there any guidelines about it or is it only by the aesthetic considerations?

> * Documents
>   - Please also add the following files to %doc.
> ------------------------------------------------------
> NEWS
> ------------------------------------------------------
Done.

> (In reply to comment #3)
> > (In reply to comment #2)
> > > - "system/firewalls" is not a standard Group (please refer to
> > >   $ rpmlint -I non-standard-group
> > Thanks. I'm change it to "Applications/System".
> > But another question is why rpmlint was silent??
>   - On my system rpmlint warned about this.s
You are using stable version of the rpmlint or rawhide? May be I can tune warning level somewhere?

$ rpm -q rpmlint
rpmlint-0.84-2.fc9.noarch

> 
> > > - "Requires: perl" is redundant. Usually perl (module) related dependencies are
> > >   automatically detected and added to rebuilt binary rpms by rpmbuild.
> > I suppose that do not requires external perl modules. Requires: perl removed.
> > But I can't understand how it will be detected? It is based on standard
> > Makefile, nor perl buildsystem.
> 
>   - This is detected by /usr/lib/rpm/perl.req. Try
> ...
Thank you very much for the explanation.
Comment 7 Pavel Alexeev 2008-08-24 20:31:31 EDT
(In reply to comment #5)
> By the way:
> 
> -------------------------------------------------------------
> NOTE: Before being sponsored:
> 
> This package will be accepted with another few work. 
> But before I accept this package, someone (I am a candidate) 
> must sponsor you.
It would be grateful if you become my sponsor.

> the person who want to be sponsored (like you) 
> are required to "show that you have an understanding 
> of the process and of the packaging guidelines" as is described
> on :
> http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
> 
> Usually there are two ways to show this.
> A. submit other review requests with enough quality.
> B. Do a "pre-review" of other person's review request
>    (at the time you are not sponsored, you cannot do
>    a formal review)
I like more the first way.
I'm have submitted several packages for review at this time (all waiting review):
php-pecl-runkit - https://bugzilla.redhat.com/show_bug.cgi?id=455226
php-pecl-parsekit - https://bugzilla.redhat.com/show_bug.cgi?id=455227
DivFix++ https://bugzilla.redhat.com/show_bug.cgi?id=458338
xls2csv https://bugzilla.redhat.com/show_bug.cgi?id=458866

Additionally, dependency from decision of Ankur Shrivastava in I may co-maintain axel - https://bugzilla.redhat.com/show_bug.cgi?id=454980 or not. By the way you may see my spec in the request.

Additionally, it is not formal review request (I'm not first submitter), but request of package sim - https://bugzilla.redhat.com/show_bug.cgi?id=411881 also created from my src.rpm and thus packages long time distributed from main website of sim (http://sim-im.org/wiki/Download)

Furthermore, I'm own small rpm-repository for Fedora: http://hubbitus.net.ru/rpm/ and planing submit more few package review requests shortly.

I do not have pre-review, but for that aim may also provide this bug-report https://bugzilla.redhat.com/show_bug.cgi?id=459124
Comment 9 Mamoru TASAKA 2008-08-25 03:42:56 EDT
For 2.0.2-2:

(In reply to comment #6)
> (In reply to comment #4) 
> > * License
> >   - As far as I verified the source code, the license tag should
> >     be "GPLv2+".
> Official site ( http://ferm.foo-projects.org/) says: "Licensed under the GPLv2"

  - Well I assume the site you quoted is using wrong license tag, because
    - We guess under what license the package is released by checking the whole
      codes in the tarball
    - Some files in the tarball declares explicitly the license is under GPLv2+
      (see files under doc/).
    (I guess the upstream are using "GPLv2" on the site with the meaning
     of "GPL version 2 and any later" :) )

    However as "GPLv2" is more strict than "GPLv2+", for now I accept "GPLv2"
    license tag. However please ask the upsteam which is correct.

> > * Macros usage consistency
> >   - When using { on macros, please use { for all macros (except for some cases)
> >     consistently. You use %{_mandir} and %_sbindir , for example.
> 
> %_sbindir replaced by %{_sbindir}
> Is there any guidelines about it or is it only by the aesthetic considerations?

  - Only cosmetic issue

> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > - "system/firewalls" is not a standard Group (please refer to
> > > >   $ rpmlint -I non-standard-group
> > > Thanks. I'm change it to "Applications/System".
> > > But another question is why rpmlint was silent??
> >   - On my system rpmlint warned about this.s
> You are using stable version of the rpmlint or rawhide? May be I can tune
> warning level somewhere?
> 
> $ rpm -q rpmlint
> rpmlint-0.84-2.fc9.noarch
  - rpmlint-0.84-2.fc10.noarch. Maybe the dependent packages are related.


Now:
- This package itself is okay 
- Your other review requests seem good to some extent.
  (There are some apparent issues which need fixing, however I don't guess I can have a time
   to review other review requests of yours as I am already reviewing 20 requests...
   I hope someone else will review other requests of yours.)

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

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".
As I found your name in FAS I am sponsoring you now.

If you want to import this package into Fedora 8/9, 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 10 Pavel Alexeev 2008-08-25 08:11:42 EDT
(In reply to comment #9)
>     However as "GPLv2" is more strict than "GPLv2+", for now I accept "GPLv2"
>     license tag. However please ask the upsteam which is correct.
I have sent mail to one of main developer with this question.

>   - rpmlint-0.84-2.fc10.noarch. Maybe the dependent packages are related.
Maybe, maybe. I using most packages form F9 stable repository at this time.

> 
> 
> Now:
> - This package itself is okay 
> - Your other review requests seem good to some extent.
>   (There are some apparent issues which need fixing, however I don't guess I
> can have a time
>    to review other review requests of yours as I am already reviewing 20
> requests...
>    I hope someone else will review other requests of yours.)
I hope too. Thank you once more for this review.


> Please follow the procedure written on:
> http://fedoraproject.org/wiki/PackageMaintainers/Join
> from "Install the Client Tools (Koji)".
> As I found your name in FAS I am sponsoring you now.
I have done this steps several times ago. Even in this present reviews I check it builds in koji.

My name in FAS is "hubbitus".

> If you want to import this package into Fedora 8/9, you also have
> to look at
> http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
Ok, thanks, this document I have not read yet.


> If you have questions, please ask me.
Thanks. I think the questions yet to be. What is preferred way to contact with you?
Comment 11 Mamoru TASAKA 2008-08-25 08:44:12 EDT
(In reply to comment #10)
> > Please follow the procedure written on:
> > http://fedoraproject.org/wiki/PackageMaintainers/Join
> > from "Install the Client Tools (Koji)".
> > As I found your name in FAS I am sponsoring you now.
> I have done this steps several times ago. Even in this present reviews I check
> it builds in koji.
> 
> My name in FAS is "hubbitus".

I am already sponsoring you. Please follow "Join" wiki again.
From what you say, I guess next step of yours is
"Add Package to CVS and Set Owner" (i.e. write CVS request on this bug)

> > If you have questions, please ask me.
> Thanks. I think the questions yet to be. What is preferred way to contact with
> you?
If the question is related to this review request, you can write it on
this bug. Otherwise feel free to mail to me.
Comment 12 Jason Tibbitts 2008-08-25 08:50:53 EDT
Might I add that "For Easy Rule Making" as a Summary really doesn't say anything about what this package does.  I would suggest including "firewall rule manager" or at least something about firewalls in the Summary.
Comment 13 Pavel Alexeev 2008-08-25 12:54:36 EDT
I got answer from Max Kellerman, one of upstream ferm developer:

> What is more true GPLv2 or "GPLv2 or any later version" (GPLv2+ in  
> > Fedora terminology)??
...
The home page is only 99.9% accurate, to keep it as short as possible
and readable - you know, nobody reads text in a web browser,
especially if it's longer than really needed  ;-) 

The only "real" licensing statement is the one in the source code,
which should be "GPLv2+" for you.

So, I'm change Licence to GPLv2+

http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-2.0.2-3.fc9.src.rpm

Now I reading and working on importing this package into Fedora 8/9.

@Jason Tibbitts

I have follow guidelines and got summary from upstream.
May be "For Easy firewall Rule Making"? Furthermore it is abbrivation as I understood.
Comment 14 Pavel Alexeev 2008-08-26 04:31:14 EDT
New Package CVS Request
=======================
Package Name: ferm
Short Description: For Easy Rule Making
Owners: hubbitus
Branches: F-8 F-9
InitialCC:
Comment 15 Kevin Fenzi 2008-08-26 19:20:08 EDT
cvs done.
Comment 16 Pavel Alexeev 2008-08-27 06:30:09 EDT
Guake 0.3.1 released will you update it?
Comment 17 Pavel Alexeev 2008-08-27 06:56:57 EDT
Sorry, last post was erroneous.
Comment 18 Fedora Update System 2008-08-28 09:33:36 EDT
ferm-2.0.2-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/ferm-2.0.2-3.fc9
Comment 19 Fedora Update System 2008-08-28 09:33:41 EDT
ferm-2.0.2-3.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/ferm-2.0.2-3.fc8
Comment 20 Mamoru TASAKA 2008-08-28 10:59:18 EDT
Now closing.
Comment 21 Fedora Update System 2008-09-05 08:20:48 EDT
ferm-2.0.2-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2008-09-10 02:46:28 EDT
ferm-2.0.2-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 23 Fedora Update System 2008-09-10 03:17:50 EDT
ferm-2.0.2-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Pavel Alexeev 2010-03-09 10:39:48 EST
Package Change Request
======================
Package Name: ferm
New Branches: EL-5
Owners: hubbitus
Comment 25 Mamoru TASAKA 2010-03-09 14:13:30 EST
You need not reopen the bug for CVS request.
Comment 26 Jason Tibbitts 2010-03-10 17:14:51 EST
CVS done.  And yes, we don't care if the ticket is open; we just look for the status of the fedora-cvs flag.
Comment 27 Pavel Alexeev 2010-03-11 05:31:00 EST
Thank you. And ok, I'll known in next times.
Comment 28 Fedora Update System 2010-03-11 06:18:37 EST
ferm-2.0.7-8.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/ferm-2.0.7-8.el5
Comment 29 Fedora Update System 2010-04-01 17:01:03 EDT
ferm-2.0.7-8.el5 has been pushed to the Fedora EPEL 5 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.