Spec URL: http://dirtypackets.net/software/rpm/snmptt/snmptt.spec SRPM URL: http://dirtypackets.net/software/rpm/snmptt/snmptt-1.2-1.src.rpm Description: SNMPTT (SNMP Trap Translator) is an SNMP trap handler written in Perl This is a new package, I am a new packager (this is my third package submitted for review) and I require a sponsor.
(You don't need the NEEDSPONSOR tag since I've already agreed to sponsor you.) - Drop "SNMPTT (SNMP Trap Translator) is" from the summary. - Change the Requires: /sbin/chkconfig to Requires: chkconfig. - The character set conversion should be iconv -f ISO-8859-1 -t UTF-8 ChangeLog > ChangeLog.utf8 && \ touch -r ChangeLog ChangeLog.utf8 && \ mv ChangeLog{.utf8,} - Drop the exit 0 from %build. - %{_initdir} should be %{_initddir}. What's the idea behind the following?? %if 0%{?rhel} > 5 || 0%{?fedora} > 9 install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initdir}/snmptt %else install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initrddir}/snmptt %endif - You can use install to create directories as well with "install -d /path/to/dir" - Use the standard script to create the user https://fedoraproject.org/wiki/Packaging/UsersAndGroups - Same thing for the initscript stuff https://fedoraproject.org/wiki/Packaging/SysVInitScript
(In reply to comment #1) > (You don't need the NEEDSPONSOR tag since I've already agreed to sponsor you.) > > - Drop "SNMPTT (SNMP Trap Translator) is" from the summary. > Fixed. > - Change the Requires: /sbin/chkconfig to Requires: chkconfig. Fixed. > - The character set conversion should be > iconv -f ISO-8859-1 -t UTF-8 ChangeLog > ChangeLog.utf8 && \ > touch -r ChangeLog ChangeLog.utf8 && \ > mv ChangeLog{.utf8,} > I had actually used the convention here: http://fedoraproject.org/wiki/PackageMaintainers/Packaging_Tricks#Convert_encoding_to_UTF-8 But I'll use your method in the future. I've corrected the spec > - Drop the > exit 0 > from %build. Fixed. > > - %{_initdir} should be %{_initddir}. What's the idea behind the following?? > %if 0%{?rhel} > 5 || 0%{?fedora} > 9 > install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initdir}/snmptt > %else > install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initrddir}/snmptt > %endif > > This was a typo here. (I guess that's what I get for writing a package at 3am) It should have been: %if 0%{?rhel} > 5 || 0%{?fedora} > 9 install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initddir}/snmptt %else install -D -p -m 0755 snmptt-init.d %{buildroot}%{_initrddir}/snmptt %endif Since, according to https://fedoraproject.org/wiki/Packaging/RPMMacros %{_initddir} is deprecated, I was trying to have use %{_initrddir} on platforms that support it. Let me know which way you prefer I package (ie. just use %{_initrddir} all the time or use the conditional version above) > - You can use install to create directories as well with "install -d > /path/to/dir" > I had original done it that way, but changed it to make it easier to read. I've changed it back and I'll use 'install -d' going forward > - Use the standard script to create the user > https://fedoraproject.org/wiki/Packaging/UsersAndGroups > Fixed > - Same thing for the initscript stuff > https://fedoraproject.org/wiki/Packaging/SysVInitScript Fixed. I'll post the update spec/SRPM as soon as I hear back from you on the init.d script issue above.
Jussi, if you're reviewing this, could you assign the bug to yourself? And BTW, FE-NEEDSPONSOR is still set... Anyway, I packaged this a few months ago too, but haven't really used much at all. In case you wish to compare and/or borrow something, the SRPM is at http://scop.fedorapeople.org/packages/snmptt-1.2-0.fc10.src.rpm I suggest at least looking at the patches and TODO comments in the specfile. I've also forwarded the patches upstream in early April and he said he'd look into them when he finds time. A copy of the mail I sent which contains some comments about the patches and other things is at http://scop.fedorapeople.org/packages/snmptt.txt, in particular the issue with world writable log files. FWIW, I'd personally just use %{_initrddir} for all distro versions.
(In reply to comment #3) > Jussi, if you're reviewing this, could you assign the bug to yourself? And > BTW, FE-NEEDSPONSOR is still set... I don't think I'll review this, there are a lot of people waiting to be sponsored and I have quite many reviews open still. Dropped FE-NEEDSPONSOR (must have forgotten to drop it in the last comment).
The bugs reported by Ville Skyttä have been fixed in SNMPTT 1.3 which is currently in beta.
Fantastic, I'll wait for that before we proceed. I was going to take a crack at some of them but have been busy with other packages, but if upstream it taking care of it that's great news.
Decided to get the package out there so it can get some testing (and I'll upgraded it to 1.3 when released). Spec URL: http://dirtypackets.net/software/rpm/snmptt/snmptt.spec SRPM URL: http://dirtypackets.net/software/rpm/snmptt/snmptt-1.3-0.1.beta2.src.rpm rpmlint output: (Complains about non-standard UID but I doubt that's a blocker) rpmlint -i snmptt-1.3-0.1.beta2.fc11.noarch.rpm snmptt.noarch: W: non-standard-uid /var/spool/snmptt snmptt A file in this package is owned by a non standard user. Standard users are: root, bin, daemon, adm, lp, mail, news, uucp, gopher, ftp, pkiuser, squid, pvm, named, postgres, mysql, nscd, rpcuser, rpc, netdump, vdsm, rpm, ntp, mailman, gdm, xfs, mailnull, apache, wnn, smmsp, puppet, tomcat, ldap, frontpage, nut, beagleindex, tss, piranha, prelude-manager, snortd, condor, pegasus, webalizer, haldaemon, vcsa, avahi, tcpdump, privoxy, sshd, radvd, arpwatch, fax, nocpulse, desktop, dbus, jonas, clamav, sabayon, polkituser, postfix, majordomo, quagga, exim, distcache, radiusd, hsqldb, dovecot, ident, nobody, nfsnobody. snmptt.noarch: W: non-standard-gid /var/spool/snmptt snmptt A file in this package is owned by a non standard group. Standard groups are: root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, cdrom, mail, news, uucp, gopher, ftp, man, pkiuser, dialout, floppy, games, slocate, utmp, squid, pvm, named, postgres, mysql, nscd, rpcuser, console, rpc, tape, netdump, utempter, kvm, rpm, ntp, video, dip, mailman, gdm, xfs, pppusers, popusers, slipusers, mailnull, apache, wnn, smmsp, puppet, tomcat, lock, ldap, frontpage, nut, beagleindex, tss, piranha, prelude-manager, snortd, audio, condor, wine, pegasus, webalizer, haldaemon, vcsa, avahi, realtime, tcpdump, privoxy, sshd, radvd, shadow, arpwatch, fax, nocpulse, desktop, dbus, jonas, clamav, screen, quaggavt, sabayon, polkituser, wbpriv, postfix, postdrop, majordomo, quagga, exim, distcache, radiusd, hsqldb, dovecot, ident, nobody, users, nfsnobody. snmptt.noarch: W: non-standard-uid /var/log/snmptt snmptt A file in this package is owned by a non standard user. Standard users are: root, bin, daemon, adm, lp, mail, news, uucp, gopher, ftp, pkiuser, squid, pvm, named, postgres, mysql, nscd, rpcuser, rpc, netdump, vdsm, rpm, ntp, mailman, gdm, xfs, mailnull, apache, wnn, smmsp, puppet, tomcat, ldap, frontpage, nut, beagleindex, tss, piranha, prelude-manager, snortd, condor, pegasus, webalizer, haldaemon, vcsa, avahi, tcpdump, privoxy, sshd, radvd, arpwatch, fax, nocpulse, desktop, dbus, jonas, clamav, sabayon, polkituser, postfix, majordomo, quagga, exim, distcache, radiusd, hsqldb, dovecot, ident, nobody, nfsnobody. snmptt.noarch: W: non-standard-gid /var/log/snmptt snmptt A file in this package is owned by a non standard group. Standard groups are: root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, cdrom, mail, news, uucp, gopher, ftp, man, pkiuser, dialout, floppy, games, slocate, utmp, squid, pvm, named, postgres, mysql, nscd, rpcuser, console, rpc, tape, netdump, utempter, kvm, rpm, ntp, video, dip, mailman, gdm, xfs, pppusers, popusers, slipusers, mailnull, apache, wnn, smmsp, puppet, tomcat, lock, ldap, frontpage, nut, beagleindex, tss, piranha, prelude-manager, snortd, audio, condor, wine, pegasus, webalizer, haldaemon, vcsa, avahi, realtime, tcpdump, privoxy, sshd, radvd, shadow, arpwatch, fax, nocpulse, desktop, dbus, jonas, clamav, screen, quaggavt, sabayon, polkituser, wbpriv, postfix, postdrop, majordomo, quagga, exim, distcache, radiusd, hsqldb, dovecot, ident, nobody, users, nfsnobody. 1 packages and 0 specfiles checked; 0 errors, 4 warnings.
(In reply to comment #7) > Spec URL: http://dirtypackets.net/software/rpm/snmptt/snmptt.spec > SRPM URL: > http://dirtypackets.net/software/rpm/snmptt/snmptt-1.3-0.1.beta2.src.rpm I'd like to review this but the .spec is still at 1.2 and the SRPM URL gives a 404 Not Found.
Gary: ping?
Updated URLs: http://giesen.fedorapeople.org/snmptt/snmptt.spec http://giesen.fedorapeople.org/snmptt/snmptt-1.3-0.1.beta2.fc11.src.rpm
Still waiting for a 1.3 from the author, I'll ping him. In the meantime, the packaging shouldn't change from 1.3b2 to 1.3 release so I think a review is still worthwhile.
(In reply to comment #10) > http://giesen.fedorapeople.org/snmptt/snmptt-1.3-0.1.beta2.fc11.src.rpm Version 1.3 was released yesterday it seems. Anyway here's a partial review from skimming the above beta2 specfile, will complete the review when the package has been updated to 1.3: - Summary isn't very helpful wrt. what the package does. In my package I used "SNMP Trap Translator" which isn't perfect but IMO slightly better the current one. - %description doesn't actually describe snmptt but snmptrapd. In my snmptt package I had this: SNMPTT (SNMP Trap Translator) is an SNMP trap handler written in Perl for use with the Net-SNMP / UCD-SNMP snmptrapd program. It can be used to translate trap output from snmptrapd to more descriptive and human friendly form, supports logging, invoking external programs, and has the ability to accept or reject traps based on a number of parameters. - A number of installed files that contain hardcoded paths are installed using macros. This is a non-blocker as far as this review is concerned, however I'd recommend either using those hardcoded paths in the specfile or implementing something to replace those hardcoded paths in installed files with the expansions of macros. - snmptthandler is installed as %{_sbindir}/snmptthandler, %{_bindir}/snmpttconvert, and %{_bindir}/snmpttconvertmib which doesn't look right to me. - %post and %preun are not guarded for non-zero exit status See my old package in comment #3, it has fixes/improvements for all of the above.
One more thing: I shipped snmptt-net-snmp-test in my snmptt package. I'm not sure if I ever used it but it is referred to in upstream docs so I'd just like to confirm if you omitted it on purpose or if it was an oversight.
Gary: ping? See comment 12 and comment 13.
snmptt came out of betta almost a year ago. Was this project abandoned?
If by "project" you mean this package review: as far as I'm concerned, no it's not abandoned. I've just been waiting for a response to comments 12 and 13 for almost a year now...
Gary: do you plan to continue this review?
Removing myself as the assignee due to lack of progress and feedback.
I guess we could close this as stalled review, as of: http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews#Submitter_not_responding
No response from the submitter, closing.
Not ready yet, but updated to use systemd and the final 1.3 version: http://www.geofrogger.net/review/snmptt-1.3-1.fc16.src.rpm http://www.geofrogger.net/review/snmptt.spec
I just noticed, I uploaded the wrong SRPM. The spec is fine.
Why is this open but marked DEADREVIEW? I see no response from the submitter since sometime in 2009. If someone else wants to submit this, please open your own review ticket.
*** This bug has been marked as a duplicate of bug 870615 ***