Spec URL: http://labs.linuxnetz.de/bugzilla/sip-redirect.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/sip-redirect-0.1.2-1.src.rpm sip-redirect is a tiny SIP redirect server written in Perl. It is IPv4 and IPv6 capable, but the IPv6 support is optional. The RFC 3261 was the base for this simple and very configurable implementation. There is neither TCP nor multicast support programmed in.
Can't download sources from your main site w/o registration. It's no go for Fedora.
Peter, that's a thing which can be changed. If changed, will it switch to a "yes go"? ;-)
I agree that the source should if at all possible be obtainable without registration of any kind. I can't really fathom why you would want to restrict downloads like that. Get a fedorahosted or use your fedorapeople space if you're really concerned that too many people might download your software. The %pre scriptlet is missing the final "exit 0" line. I don't understand why you would need to touch and chown the log directory in %post. Why not just put it in the %files list normally?
My package does not fit 2nd must of https://fedorahosted.org/web/en/terms and never will do so. It's not a log directory, it's a log file. How else to get a empty log file, which fits "rpm -V" etc.? %ghost doesn't create files, just keeps attributes. %config isn't IMHO an option for a log file. MySQL afaik does handing of the log file the same way.
I'll review it.
Notes: * Please, provide full path to the source tarball. E.g. Source: http://ftp.robert-scheck.de/linux/%{name}/%{name}-%{version}.tar.gz instead of Source: %{name}-%{version}.tar.gz * Please, add empty %build section to make rpmlint happy :) * Use %{_initrddir} instead of %{_sysconfdir}/rc.d/init.d BTW useful list of rpm-macros: https://fedoraproject.org/wiki/Packaging/RPMMacros Other things seems sane. Consider applying changes according to these advices, and I'll made formal review.
(In reply to comment #6) > * Please, provide full path to the source tarball. E.g. > Source: http://ftp.robert-scheck.de/linux/%{name}/%{name}-%{version}.tar.gz > instead of > Source: %{name}-%{version}.tar.gz Really? See comment #1 again and AFAIK nirik complained at others about such things. > * Please, add empty %build section to make rpmlint happy :) I will do so, yes. > * Use %{_initrddir} instead of %{_sysconfdir}/rc.d/init.d > BTW useful list of rpm-macros: > https://fedoraproject.org/wiki/Packaging/RPMMacros AFAIK %{_initrddir} is obsolete since RPM 4.5.90 and %{_initrdir} is only available since RPM 4.5.90 (or the other way round). To avoid nasty if/else hackings, I'm always and everywhere using the less macrofied way in that single case (yes, I've e.g. EPEL packages). > Other things seems sane. Consider applying changes according to these advices, > and I'll made formal review. Before putting a new package there, can we clarify the points above and have a short talk regarding comment #3? Current situation is, that the sip-redirect needs to have a writeable log file, which I only can get with touching, as a %config isn't the right deal and %ghost doesn't create the file. Am I missing something or do you have a better idea?
If your source package has a upstream full URL, you should use it so people can find your upstream version. If that URL requires some kind of registration or the like there is nothing prohibiting that in the guidelines that I know of. You may get an email from my source checker about it being unable to check your source, but it can be blacklisted from checking in the cases like this. Personally I would rather see the full URL, even if it can't be downloaded from there without some clicking.
Ping? What are we currently lacking? The first two points from comment #6 are just a minor change, the third one I would like to avoid for the given reason.
Thanks for woke me up, Robert :) Let's summarize things * issue with registration may be ignored (however I still think that all sources, listed in Fedora's spec-files should be accessible w/o restrictions of any kind) * my first two remarks are cosmetic and you promised to change your spec acordingly * I age with you about %{_initrdir} issue - my last remark must be ignored About touch and chmod for log files - I don't think that there are considerably more effective techniques to create zero-length log-file if it does not exists than your approach (and MySQL maintainer's one, respectively). OK, assuming that you fix spec-file according to all suggestions, here is my REVIEW: - rpmlint is not silent: [petro@Sulaco noarch]$ rpmlint sip-redirect-0.1.2-1.noarch.rpm sip-redirect.noarch: W: non-standard-uid /var/log/sip-redirect sip sip-redirect.noarch: W: non-standard-gid /var/log/sip-redirect sip sip-redirect.noarch: E: non-root-user-log-file /var/log/sip-redirect sip sip-redirect.noarch: E: non-root-group-log-file /var/log/sip-redirect sip sip-redirect.noarch: W: dangerous-command-in-%post chown sip-redirect.noarch: E: incoherent-subsys /etc/rc.d/init.d/sip-redirect sip-redirect} 1 packages and 0 specfiles checked; 3 errors, 3 warnings. [petro@Sulaco noarch]$ + The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package meets the Packaging Guidelines. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines . + The License field in the package spec file matches the actual license. + The spec file is written in American English. + The spec file for the package is legible. - The sources used to build the package must match the upstream source, as provided in the spec URL. I cannot verify it since I cannot dl files w/o registration. + The package successfully compiles and builds into binary rpms on at least one supported architecture (ppc). + No extra build dependencies + The package doesn't create additional directories. + The package does not contain any duplicate files in the %files listing. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). + The package consistently uses macros, as described in the macros section of Packaging Guidelines . + The package contains code, or permissable content. + No large documentation files + Every file, the package includes as %doc, does not affect the runtime of the application. + The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). + All filenames in the packages are valid UTF-8. Warnings and errors from rpmlint may be ignored in this case, so the package is APPROVED
Peter, thanks for doing the review. I'll of course do the changes from previous comments as I already said before importing into CVS. New Package CVS Request ======================= Package Name: sip-redirect Short Description: Tiny IPv4 and IPv6 SIP redirect server written in Perl Owners: robert Branches: EL-4 EL-5 F-8 F-9 InitialCC: Cvsextras Commits: no
CVS Done
Package: sip-redirect-0.1.2-2.fc10 Tag: dist-f10-updates-candidate Status: complete Built by: robert Package: sip-redirect-0.1.2-2.fc8 Tag: dist-f8-updates-candidate Status: complete Built by: robert Package: sip-redirect-0.1.2-2.fc9 Tag: dist-f9-updates-candidate Status: complete Built by: robert 686 (sip-redirect): Build on target fedora-5-epel succeeded. 687 (sip-redirect): Build on target fedora-4-epel succeeded.
sip-redirect-0.1.2-2.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/sip-redirect-0.1.2-2.fc8
sip-redirect-0.1.2-2.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.
sip-redirect-0.1.2-2.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
sip-redirect-0.1.2-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/sip-redirect-0.1.2-2.fc10
sip-redirect-0.1.2-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.