Spec URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.0.Final-1.fc14.src.rpm Description: mod_cluster is an httpd-based load balancer. Like mod_jk and mod_proxy, mod_cluster uses a communication channel to forward requests from httpd to one of a set of application server nodes. Unlike mod_jk and mod_proxy, mod_cluster leverages an additional connection between the application server nodes and httpd. The application server nodes use this connection to transmit server-side load balance factors and lifecycle events back to httpd via a custom set of HTTP methods, affectionately called the Mod-Cluster Management Protocol (MCMP). This additional feedback channel allows mod_cluster to offer a level of intelligence and granularity not found in other load balancing solutions. Scratch build for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=2614547
rpmlint output: *** mod_cluster.spec: 0 packages and 1 specfiles checked; 0 errors, 0 warnings. *** mod_cluster-1.1.0.Final-1.fc14.src.rpm: mod_cluster.src: W: name-repeated-in-summary C Mod_cluster mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 5 warnings. *** mod_cluster-1.1.0.Final-1.fc14.x86_64.rpm: mod_cluster.x86_64: W: name-repeated-in-summary C Mod_cluster mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 4 warnings. *** mod_cluster-debuginfo-1.1.0.Final-1.fc14.x86_64.rpm: mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Note: URL is accepted when accessed from Firefox.
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2884588 Some "first sight" remarks: _ No alpha allowed in version for "final" releases. You should use version "1.1.0" and adapt the source URL accordingly. _ Summary should be reworked, at least in compliance with rpmlint output. _ Last upstream version is now 1.1.1 _ There are some bundled software in the source tarball (httpd, apr, ssl, etc): although you explicitly avoid using them, I would advise to remove the corresponding subdirectories before configure: find srclib -mindepth 1 -maxdepth 1 ! -name mod_cluster -print0|xargs -0 -r rm -rf this will cause compilation failure whenever a bundled directory is used. _ You do not use RPM_OPT_FLAGS as a base for the compilation flags. _ There are compilation warnings that do indicate potential problems (format descriptor/value type mismatch). Consider writing a patch for those. _ You should include the license file srclib/mod_cluster/lgpl.txt as %doc.
Thank you for this review, I'll prepare a new src.rpm including your comments shortly!
Spec URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.1-1.fc14.src.rpm I fixed described issues. Re warnings - I'm not a C programmer, will contact mod_cluster developers to fix it in the future versions if that's ok. rpmlint output: $ rpmlint /var/lib/mock/fedora-14-x86_64/result/mod_cluster-1.1.1-1.fc14.x86_64.rpm mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 3 warnings. $ rpmlint /var/lib/mock/fedora-14-x86_64/result/mod_cluster-1.1.1-1.fc14.src.rpm mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 4 warnings. Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2900512 Thanks!
rpmlint output: *** mod_cluster.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. *** mod_cluster-1.1.1-1.fc14.src.rpm mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 4 warnings. *** mod_cluster-1.1.1-1.fc14.x86_64.rpm mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 3 warnings. *** mod_cluster-debuginfo-1.1.1-1.fc14.x86_64.rpm mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 1 warnings. +=OK, -=Not OK, X=Not applicable, ?=Not verifiable MUST Items: + rpmlint output OK (see above) + named and versioned according to the Package Naming Guidelines + spec file name matches base package name - dist tag is present and OK + complies with all the legal guidelines: + License: xyz valid, matches actual license + No known patent problems + No emulator, no firmware, no binary-only or prebuilt components + COPYING packaged as %doc + spec file is legible and written in american english + source matches upstream: MD5: 3d4fec37ecfd1e5ec33cd147525cfe5e SHA1: 2e42674ac8a5e43fba1b78a04b0087d9772e3afc SHA256: 368a1db30abd0d7c4910e677f7eed4a5c0b1ca2c661cfb76ec6dee820b6c0058 + latest version is being packaged + build root is correct + builds on at least one primary architecture X known non-working architectures are listed in ExcludeArch (BZ #) + no missing BuildRequires (builds in mock) X complies with translation/locale guidelines X ldconfig calls in %post and %postun for packages containing shared libraries + no shared libraries are added to the regular linker search paths + no duplicated system libraries + package not relocatable + owns the directories and files it creates + doesn't own and directory it shouldn't + no duplicate files in %files + permissions correct, defattr used correctly + macros used consistently + no non-code content X large documentation files are in a -doc subpackage + no %doc files required at runtime X header files are in a -devel subpackage X static libraries are in a -static subpackage X suffixed library files have a matching .so file in the -devel subpackage X pkgconfig files are in a -devel subpackage X -devel package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}" + no .la files X .desktop file present X desktop-file-install is used in %install and the .desktop file passes validation + all filenames are valid UTF-8 + complies with the FHS - proper changelog, tags, BuildRoot, BuildRequires, Summary, Description + no macros in Summary and Description + no non-UTF-8 characters - all relevant documentation included as %doc + compiler flags are appropriate (RPM_OPT_FLAGS are used) + %clean is present + no bundled software + debuginfo package is valid + no rpaths + complies to %config guidelines X complies with init script guidelines - no timestamp-clobbering file commands - _smp_mflags used X complies to web application guideline X %check is present and all test pass + final provides and requires are sane X no conflicts (installs properly) SHOULD Items: + license already included upstream X translations for description and summary are provided by upstream + package builds in mock ? submitter reports having successfully tested the package functionality X scriptlets are sane X subpackages other than -devel should require the base package using a fully versioned dependency + no file dependencies X package contains man pages Comments: _ Use %global instead of %define _ Release: 1%{dist} should be 1%{?dist} _ You must add your name before the e-mail address in the changelog records: http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs _ Since there is a docs directory in the source, there is probably something to package as %doc. It seems to be a docbook, thus some processing is needed to generate ready-to-read files for the doc package (i.e.: html). _ Use "cp -a" to preserve files timestamp. _ Use "make %{?_smp_mflags}" or explain why you don't. http://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make About the printf format warnings problem: in any case you should report it upstream. In the meantime, and since the review policy does not explicitly forbid a reviewer to do so, I can try to produce such a patch for you. Your opinion ?
Yes, please any help is appreciated. I mentioned these errors to developers yesterday, in a mail. I'll fix my mistakes in the meantime, waiting for your patch. Thank you!
Created attachment 483718 [details] Avoid most of the compilation warnings I compiled successfully with this patch on F14 i386 and x86_64. I still have one warning after applying this patch: I did not fix it because the fix might cause problems when compiling for M$W :-( Please note the following: 1) I did NOT test the resulting binaries (no infrastructure to do it here). I'm really expecting you can and would do it. 2) I'm sure about fixes dealing with output formats. 3) I'm only pretty confident about fixes for undefined variables: the way I defined them seems the most probable, but I'm only 95% sure. Anyway it cannot be worse than not defined them at all! 4) Other fixes are suppressions of unused code and variables: those should be 100% sure too.
Patrick, I fixed almost everything you found and applied your patch, works great. I have tested it locally, mod_cluster loads perfectly and mod_cluster-manager shows up without issues. I would like to test it more by the community in real-world scenarios (updates-testing). I've notified upstream developers about your patch. I haven't added the documentation because it requires maven3 to build. I failed building it with maven2. Thank you for your work! $ rpmlint mod_cluster.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint ./mod_cluster-1.1.1-2.fc14.x86_64.rpm mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 3 warnings. $ rpmlint ./mod_cluster-debuginfo-1.1.1-2.fc14.x86_64.rpm mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Spec URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.1-2.fc14.src.rpm Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2904647
> I would like to test it more by the community in real-world scenarios (updates-testing). Sure, I would too! > I haven't added the documentation because it requires maven3 to build. I failed building it with maven2. That's a very good reason ! Perhaps some comments telling it in the %build and %files section, as well as a commented dummy BuildRequires wouldn't be too much... APPROVED
Thank you for the review! I'll add the comments to final package. New Package SCM Request ======================= Package Name: mod_cluster Short Description: Apache HTTPD based load balancer Owners: goldmann Branches: f13 f14 f15 el6
Git done (by process-git-requests).
mod_cluster-1.1.1-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.el6
mod_cluster-1.1.1-2.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc14
mod_cluster-1.1.1-2.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc13
mod_cluster-1.1.1-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc15
Thank you for review and git process. Packages submitted to updates-testing.
mod_cluster-1.1.1-2.el6 has been pushed to the Fedora EPEL 6 testing repository.
mod_cluster-1.1.1-2.fc15 has been pushed to the Fedora 15 stable repository.
mod_cluster-1.1.1-2.fc14 has been pushed to the Fedora 14 stable repository.
mod_cluster-1.1.1-2.fc13 has been pushed to the Fedora 13 stable repository.
mod_cluster-1.1.1-2.el6 has been pushed to the Fedora EPEL 6 stable repository.