Spec URL: http://www.hinegardner.org/fedora-extras/nginx.spec SRPM URL: http://www.hinegardner.org/fedora-extras/nginx-0.5.15-1.src.rpm Description: Nginx [engine x] is an HTTP(S) server, HTTP(S) reverse proxy and IMAP/POP3 proxy server Rpmlint reports non-standard-uid and non-standard-gid for a couple of directories in the package. Part of the install adds an 'nginx' user, and the directories rpmlint reports are on are directories that should be owned by the 'nginx' user. I used the Pound-2.2-2.src.rpm as a model and it also creates a new 'pound' user and it has this same rpmlint error. I am also in need of a sponsor.
MD5 (nginx-0.5.15.tar.gz) = 651feea215c9d934542b4bae1711d921
I'd be happy to review this package (and sponsor you)... look for a full review in a bit.
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (BSD) See below - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 651feea215c9d934542b4bae1711d921 nginx-0.5.15.tar.gz 651feea215c9d934542b4bae1711d921 nginx-0.5.15.tar.gz.1 OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version Issues: 1. Not a blocker, just a suggestion, but you might want to change your Source0 line from: Source0: http://sysoev.ru/nginx/nginx-0.5.15.tar.gz to Source0: http://sysoev.ru/nginx/nginx-%{version}.tar.gz So you don't need to change it every release. 2. The upstream calls the license "BSD-Like"... should the License here be "BSD-Like" in the spec? 3. rpmlint says: E: nginx non-standard-uid /var/log/nginx nginx E: nginx non-standard-gid /var/log/nginx nginx E: nginx non-standard-uid /var/lib/nginx nginx E: nginx non-standard-gid /var/lib/nginx nginx Can be ignored. W: nginx strange-permission nginx.init 0775 You might make the init file in the src.rpm mode 644 and then install it with 755 perms? Not a blocker. W: nginx rpm-buildroot-usage %build export DESTDIR=%{buildroot} False positive. E: nginx configure-without-libdir-spec I see that you can't use %configure here, but you might add a comment about having to do so. W: nginx mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 1) Pick tabs or spaces, don't mix. 4. Should the /usr/share/nginx/html be instead marked as %doc? which would put them under /usr/share/doc/nginx-%{version}/ Or does it call those html files from the program? 5. The server doesn't start here after install... I get: 2007/03/22 20:36:50 [emerg] 8189#0: glob() "/usr/share/nginx/conf/mime.types" failed (2: No such file or directory) in /etc/nginx/nginx.conf:18
Oh, one final thing... this server listens on port 80 by default. That might conflict with any of the pile of webservers available. Should this listen on another port by default?
(In reply to comment #3) > 1. Not a blocker, just a suggestion, but you might want to change your Source0 > line from: > Source0: http://sysoev.ru/nginx/nginx-0.5.15.tar.gz > to > Source0: http://sysoev.ru/nginx/nginx-%{version}.tar.gz > > So you don't need to change it every release. Fixed. > 2. The upstream calls the license "BSD-Like"... should the License here > be "BSD-Like" in the spec? Fixed to: License: BSD-like > 3. rpmlint says: > > E: nginx non-standard-uid /var/log/nginx nginx > E: nginx non-standard-gid /var/log/nginx nginx > E: nginx non-standard-uid /var/lib/nginx nginx > E: nginx non-standard-gid /var/lib/nginx nginx > > Can be ignored. > > W: nginx strange-permission nginx.init 0775 > > You might make the init file in the src.rpm mode 644 and then install it with > 755 perms? Not a blocker. Fixed, nginx.int is 0644 in src.rpm and installs as 0755 > W: nginx rpm-buildroot-usage %build export DESTDIR=%{buildroot} > > False positive. > > E: nginx configure-without-libdir-spec > > I see that you can't use %configure here, but you might add a comment > about having to do so. Comments about the patches added. I will also be contacting upstream source to see if they want to accept the patches. > W: nginx mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 1) > > Pick tabs or spaces, don't mix. Fixed. > 4. Should the > /usr/share/nginx/html > be instead marked as %doc? which would put them under > /usr/share/doc/nginx-%{version}/ > Or does it call those html files from the program? Those files are served up by the program as default web pages and as default error pages. > 5. The server doesn't start here after install... I get: > > 2007/03/22 20:36:50 [emerg] 8189#0: glob() "/usr/share/nginx/conf/mime.types" > failed (2: No such file or directory) in /etc/nginx/nginx.conf:18 Patch made to the default installed nginx.conf to make sure all the paths are correct. nginx starts correctly now, so long as something else isn't listening on port 80 (by default)
(In reply to comment #4) > Oh, one final thing... this server listens on port 80 by default. > That might conflict with any of the pile of webservers available. > Should this listen on another port by default? Not really, this is a mainly a reverse proxy/load balancer server, so it will spread out the requests it gets to other servers. I would imagine anyone install nginx will not use the default configuration, it will require custom configuration depending upon their deployment architecture. Pound is a similar application and it is starting on port 80 by default.
Hi Jeremy, - zlib-devel is already required by openssl-devel - You can even drop the whole Requires line. rpm is smart enough to calculate that by itself. - Replace /usr/sbin/nginx in your %install section with %{_sbindir}/nginx - You're missing a forward slash in --lock-path=%{_localstatedir}lock/subsys/%{name} Nothing that should block this review, but I hope it helps.
(In reply to comment #7) > Hi Jeremy, > > - zlib-devel is already required by openssl-devel > - You can even drop the whole Requires line. rpm is smart enough to calculate that by itself. Unless someone objects, I'll leave these as they are. > - Replace /usr/sbin/nginx in your %install section with %{_sbindir}/nginx > - You're missing a forward slash in --lock-path=%{_localstatedir}lock/subsys/%{name} Both of these are now fixed > Nothing that should block this review, but I hope it helps. Thanks for the feedback, it always helps.
New spec and srpm fixing items from comment #3 and comment #7 Spec URL: http://www.hinegardner.org/fedora-extras/nginx.spec SRPM URL: http://www.hinegardner.org/fedora-extras/nginx-0.5.15-3.src.rpm
Looks like the standalone spec and the one in the src.rpm aren't quite the same (the standalone one has a added changelog entry for -3). You will want to use that spec when importing your package... All the blockers I see appear to be solved, so this package is APPROVED. I will go ahead and sponsor you... continue the process from: http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b Let me know if you have any questions!
New Package CVS Request ======================= Package Name: nginx Short Description: Robust, small and high performance http and reverse proxy server Owners: jeremy Branches: FC-5, FC-6 InitialCC
Package Change Request ====================== Package Name: nginx New Branches: EL-4 EL-5
cvs done.