Bug 509965 - Review Request: snmptt - SNMPTT (SNMP Trap Translator) is an SNMP trap handler written in Perl
Review Request: snmptt - SNMPTT (SNMP Trap Translator) is an SNMP trap handle...
Status: CLOSED DUPLICATE of bug 870615
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2009-07-07 03:41 EDT by Gary T. Giesen
Modified: 2012-10-28 02:58 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-04-24 14:49:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Gary T. Giesen 2009-07-07 03:41:56 EDT
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.
Comment 1 Susi Lehtola 2009-07-07 06:22:51 EDT
(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
Comment 2 Gary T. Giesen 2009-07-07 11:47:54 EDT
(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.
Comment 3 Ville Skyttä 2009-07-08 10:42:32 EDT
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.
Comment 4 Susi Lehtola 2009-07-08 10:54:22 EDT
(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).
Comment 5 Alex Burger 2009-07-23 19:00:19 EDT
The bugs reported by Ville Skyttä have been fixed in SNMPTT 1.3 which is currently in beta.
Comment 6 Gary T. Giesen 2009-07-23 19:05:49 EDT
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.
Comment 7 Gary T. Giesen 2009-07-28 14:49:33 EDT
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.
Comment 8 Ville Skyttä 2009-09-29 12:37:18 EDT
(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.
Comment 9 Ville Skyttä 2009-11-14 16:34:06 EST
Gary: ping?
Comment 11 Gary T. Giesen 2009-11-14 16:39:05 EST
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.
Comment 12 Ville Skyttä 2009-11-16 12:36:37 EST
(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.
Comment 13 Ville Skyttä 2009-11-16 12:40:42 EST
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.
Comment 14 Ville Skyttä 2010-06-06 05:24:18 EDT
Gary: ping?  See comment 12 and comment 13.
Comment 15 Vadym Chepkov 2010-10-05 08:23:46 EDT
snmptt came out of betta almost a year ago.
Was this project abandoned?
Comment 16 Ville Skyttä 2010-10-05 12:05:02 EDT
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...
Comment 17 Ville Skyttä 2011-03-06 15:56:03 EST
Gary: do you plan to continue this review?
Comment 18 Ville Skyttä 2011-09-08 13:44:33 EDT
Removing myself as the assignee due to lack of progress and feedback.
Comment 19 Volker Fröhlich 2012-01-09 08:28:39 EST
I guess we could close this as stalled review, as of:

http://fedoraproject.org/wiki/Policy_for_stalled_package_reviews#Submitter_not_responding
Comment 20 Mattia Verga 2012-01-21 14:34:06 EST
No response from the submitter, closing.
Comment 21 Volker Fröhlich 2012-04-16 18:54:56 EDT
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
Comment 22 Volker Fröhlich 2012-04-20 10:23:13 EDT
I just noticed, I uploaded the wrong SRPM. The spec is fine.
Comment 23 Jason Tibbitts 2012-04-24 14:49:44 EDT
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.
Comment 24 Andrew Colin Kissa 2012-10-27 06:14:39 EDT

*** This bug has been marked as a duplicate of bug 870615 ***

Note You need to log in before you can comment on or make changes to this bug.