Bug 598860 - Review Request: httpd-itk - MPM Itk for Apache HTTP Server
Summary: Review Request: httpd-itk - MPM Itk for Apache HTTP Server
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nikos Roussos
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-06-02 08:33 UTC by Pavel Alexeev
Modified: 2012-03-24 19:04 UTC (History)
5 users (show)

Fixed In Version: httpd-itk-2.2.22-5.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-03-16 21:22:38 UTC
Type: ---
Embargoed:
comzeradd: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Pavel Alexeev 2010-06-02 08:33:47 UTC
Spec URL: http://hubbitus.net.ru/rpm/Fedora13/httpd-itk/httpd-itk.spec
SRPM URL: http://hubbitus.net.ru/rpm/Fedora13/httpd-itk/httpd-itk-2.2.15-1.fc13.src.rpm
Description: This package contain mpm-itk which is an MPM (Multi-Processing Module) for the Apache web server. Mpm-itk allows you to run each of your vhost under a separate uid and gid — in short, the scripts and configuration files for one vhost no longer have to be readable for all the other vhosts.

In summary it is Apache module (opposite CGI solutions like suexec), fast and allow safely use non-thread-aware code software (like many PHP extensions f.e.)


Except spelling warnings about uid/gid words and similar there only two warning:
httpd-itk.src:41: W: unversioned-explicit-provides webserver
I think it is not problem, it is meta provide because it act as webserver.
httpd-itk.i686: W: no-manual-page-for-binary httpd.itk
There really no man page.

Koji builds:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2224333
http://koji.fedoraproject.org/koji/taskinfo?taskID=2224360
http://koji.fedoraproject.org/koji/taskinfo?taskID=2224345

P.S. Spec file formatted by tabs with 5 space width ( http://fedoraproject.org/wiki/PavelAlexeev/tabsize ). Please, do not start
review if it is a problem for you.

Comment 1 Pavel Alexeev 2010-07-28 14:06:33 UTC
New apache version: http://hubbitus.net.ru/rpm/Fedora13/httpd-itk/httpd-itk-2.2.16-2.fc13.src.rpm

Comment 3 Pavel Alexeev 2010-10-30 11:19:27 UTC
Update follow to upstream:
http://hubbitus.net.ru/rpm/Fedora13/httpd-itk/httpd-itk-2.2.17-3.fc13.src.rpm

Comment 4 Pavel Alexeev 2011-03-23 20:11:30 UTC
Update follow to Fedora upstream httpd:
http://hubbitus.info/rpm/Fedora13/httpd-itk/httpd-itk-2.2.17-4.fc13.src.rpm

Comment 5 Pavel Alexeev 2011-08-21 11:21:33 UTC
New version 2.2.19
http://hubbitus.info/rpm/Fedora15/httpd-itk/httpd-itk-2.2.19-1.fc15.src.rpm

Comment 6 Pavel Alexeev 2011-09-10 15:17:34 UTC
http://hubbitus.info/rpm/Fedora15/httpd-itk/httpd-itk-2.2.20-1.fc15.src.rpm

%changelog                                                                                                                                                                           
* Sat Sep 10 2011 Pavel Alexeev <Pahan> - 2.2.20-1                                                                                                                     
- Security upstream update

Comment 8 Ryan H. Lewis (rhl) 2011-12-24 02:15:40 UTC
Hi, links to working spec files + rpmlint -v on the .rpms please.

Comment 9 Ryan H. Lewis (rhl) 2011-12-24 02:19:59 UTC
... and if you fix your crazy file spacing to that of a normal fedora system. I'll undertake your review.

Comment 10 Pavel Alexeev 2012-01-01 20:57:14 UTC
I believe what provided below links are works, at least for src.rpm:
http://hubbitus.info/rpm/Fedora15/httpd-itk/httpd-itk-2.2.21-1.fc15.src.rpm
http://hubbitus.info/rpm/Fedora15/httpd-itk/httpd-itk.spec

I'll be glad if you take it, but what you are mean under "a normal fedora system"?? Could you please provide link to guideline or policy?

Comment 11 Ryan H. Lewis (rhl) 2012-01-02 20:16:32 UTC
"P.S. Spec file formatted by tabs with 5 space width (
http://fedoraproject.org/wiki/PavelAlexeev/tabsize ). Please, do not start
review if it is a problem for you."

Make it so that I don't need to do this. 

I should be able to view the spec file so that it is legible: http://fedoraproject.org/wiki/Packaging:Guidelines#Spec_Legibility

I'm not modifying my tabsize to deal with your spec.

Comment 12 Pavel Alexeev 2012-01-02 20:26:11 UTC
How I can known what tab size you are using? Now, today, tomorrow??

May be we start real work? Or do not start it.
I'm ready continue this discussion if you want, but I think mail list or private conversation, forum or jabber will be much more appropriate place for that.

Comment 13 Nikos Roussos 2012-02-14 21:32:22 UTC
Actually you're using tabs instead of spaces :) And in some cases you're mixing them. Use either spaces or tabs (spaces preferable). It's not against the policy, but it would make the spec far more readable.

Add some descriptive comments or/and upstream links on patches
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

You could add  build requires dependencies one per line. It's more readable.

Use the full length of a line for description, up to 80 characters.

I'm not sure it's a good idea to add an echo command on %install section. If you want to give some information to the user, better add a README.Fedora or add some info on description.

Comment 14 Pavel Alexeev 2012-02-18 15:19:30 UTC
Firstly thank you for the review, Nikos.

(In reply to comment #13)
> Actually you're using tabs instead of spaces :) And in some cases you're mixing
> them.
I believe it is not. Please say on what line and I'll fix it. Rpmlint silent about that.

> Add some descriptive comments or/and upstream links on patches
> https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

It there. I have slightly fix it and add URL on comment.
> 
> You could add  build requires dependencies one per line. It's more readable.
It then increase size of file which already not so small. I think it is not block.
> 
> Use the full length of a line for description, up to 80 characters.
There no lines longer already.

> I'm not sure it's a good idea to add an echo command on %install section. If
> you want to give some information to the user, better add a README.Fedora or
> add some info on description.
I have no any serious advantages of it, but it moved.

Also update to new version:
http://hubbitus.info/rpm/Fedora16/httpd-itk/httpd-itk.spec
http://hubbitus.info/rpm/Fedora16/httpd-itk/httpd-itk-2.2.22-1.fc16.src.rpm

Comment 15 Nikos Roussos 2012-02-21 09:09:30 UTC
(In reply to comment #13)
> Actually you're using tabs instead of spaces :) And in some cases you're mixing
> them.
I believe it is not. Please say on what line and I'll fix it. Rpmlint silent
about that.

For instance you use tabs on BuildRequires lines but spaces on "if" block at line 79.

> You could add  build requires dependencies one per line. It's more readable.
It then increase size of file which already not so small. I think it is not
block.

Both this and the above are definitely not blockers. It's just friendly suggestions to make the spec more readable.

> Use the full length of a line for description, up to 80 characters.
There no lines longer already.

I meant the opposite :) Some lines should be longer. For instance the first one on the description block.

>> Add some descriptive comments or/and upstream links on patches
> It there. I have slightly fix it and add URL on comment.

Nice. Please keep track of this in order to update your spec in a future release.

I'll do a format review asap.

Comment 16 Pavel Alexeev 2012-02-23 17:45:12 UTC
(In reply to comment #15)
> (In reply to comment #13)
> > Actually you're using tabs instead of spaces :) And in some cases you're mixing
> > them.
> I believe it is not. Please say on what line and I'll fix it. Rpmlint silent
> about that.
> 
> For instance you use tabs on BuildRequires lines but spaces on "if" block at
> line 79.

Yes, you are right there. Thank you. Fixed.
 
> I meant the opposite :) Some lines should be longer. For instance the first one
> on the description block.
Fixed.

http://hubbitus.info/rpm/Fedora16/httpd-itk/httpd-itk-2.2.22-2.fc16.src.rpm
http://hubbitus.info/rpm/Fedora16/httpd-itk/httpd-itk.spec

Comment 17 Nikos Roussos 2012-02-28 16:25:16 UTC
cp: cannot stat `README.Fedora': No such file or directory

You have to include README.Fedora on src.rpm and add it on SOURCES on spec. See a spec example on how to do it:
http://comzeradd.fedorapeople.org/specs/idjc.spec

Fix this and I think the package is ready for review and approval

Comment 19 Nikos Roussos 2012-03-06 12:50:54 UTC
It seems ok. Here is the review

+: OK
-: must be fixed
=: should be fixed (at your discretion)
?: Question or clarification needed
N: not applicable

MUST:
[+] rpmlint output: shown in comment: none
[+] follows package naming guidelines
[+] spec file base name matches package name
[+] package meets the packaging guidelines
[+] package uses a Fedora approved license: ASL 2.0
[+] license field matches the actual license.
[-] license file is included in %doc

Please include LICENSE on %doc (also README)

[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum matches
[+] package builds on at least one primary arch: Tested F16 x86_64
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[+] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[N] package contains a desktop file, uses desktop-file-install/validate
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[N] query upstream for license text
[=] description and summary contains available translations
[+] package builds in mock
[=] package builds on all supported arches: Tested x86_64
[+] package functions as described: 
[N] sane scriptlets
[N] subpackages require the main package
[N] placement of pkgconfig files
[+] file dependencies versus package dependencies
[=] package contains man pages for binaries/scripts

Have you considered adding man pages or are the same as httpd package?

Comment 20 Pavel Alexeev 2012-03-06 19:03:17 UTC
There nothing place in separate man. It's just apache MPM providing another inner security model. Installation note placed in rpm description. Each other help and utilities come from main httpd package which in dependencies.

Comment 21 Nikos Roussos 2012-03-06 19:41:03 UTC
See also the comment about license. You should include the LICENSE on %doc (and  README)

Comment 22 Pavel Alexeev 2012-03-06 19:47:34 UTC
LICENSE and README from apache? For what? It will be just duplication of that files in main required package.

Comment 23 Nikos Roussos 2012-03-06 20:01:56 UTC
Now I see what you mean :) You're right. My bad.

Package is *approved*. You may proceed with the SCM request.

Comment 24 Pavel Alexeev 2012-03-06 20:24:42 UTC
Nikos, thank you very much for the review!

New Package SCM Request
=======================
Package Name: httpd-itk
Short Description: MPM Itk for Apache HTTP Server
Owners: hubbitus
Branches: F16 F17 EL-5 EL-6
InitialCC:

Comment 25 Gwyn Ciesla 2012-03-06 21:18:44 UTC
Git done (by process-git-requests).

Comment 26 Fedora Update System 2012-03-08 12:41:24 UTC
httpd-itk-2.2.22-5.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/httpd-itk-2.2.22-5.el6

Comment 27 Fedora Update System 2012-03-08 13:14:40 UTC
httpd-itk-2.2.22-5.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/httpd-itk-2.2.22-5.fc16

Comment 28 Fedora Update System 2012-03-08 13:20:59 UTC
httpd-itk-2.2.22-5.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/httpd-itk-2.2.22-5.fc17

Comment 29 Fedora Update System 2012-03-08 16:15:38 UTC
Package httpd-itk-2.2.22-5.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing httpd-itk-2.2.22-5.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-3388/httpd-itk-2.2.22-5.fc17
then log in and leave karma (feedback).

Comment 30 Fedora Update System 2012-03-16 21:22:38 UTC
httpd-itk-2.2.22-5.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 31 Fedora Update System 2012-03-17 23:34:01 UTC
httpd-itk-2.2.22-5.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 32 Fedora Update System 2012-03-24 19:04:14 UTC
httpd-itk-2.2.22-5.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.