Bug 169345
Summary: | Review Request: SEC - Simple Event Correlator | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Didier <d.bz-redhat> |
Component: | Package Review | Assignee: | John Mahowald <jpmahowald> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | jose.p.oliveira.oss, lists |
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://www.estpak.ee/~risto/sec/ | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-09-01 21:13:31 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Didier
2005-09-27 09:39:04 UTC
Is this your first package? If it is you'll need a sponsor. rpmlint complains about: W: sec incoherent-version-in-changelog 2.3.2-4 2.3.2-4%{_my_ext} E: sec non-standard-executable-perm /usr/share/man/man1/sec.1.gz 0744 W: sec log-files-without-logrotate /var/log/sec.log W: sec dangerous-command-in-%post rpm W: sec dangerous-command-in-%preun rpm W: sec dangerous-command-in-%postun rpm E: sec no-status-entry /etc/rc.d/init.d/sec E: sec no-chkconfig-line /etc/rc.d/init.d/sec Some things to look at: - release has an undeclared macro in it. Try "4%{?dist}" instead. - Group is tricky, Applications/Text works well too. - man page should be non executable - add status command and chkconfig line to init script Dangerous rpm commands are false alarms, they're commented out. Can't find any other Extras work from you, need sponsor. My apologies, John, as I have been swamped with work lately ; I don't like bitrot either. SEC has a quite stable release scheme, and no new versions have been released in the interim. I'll take this (sponsor, rpmlint-ing, etc.) up ASAP (probably early february) ; thanks for your time. I just got sponsorship rights and as such I'm taking a look at NEEDSPONSOR bugs, also John has become a sponsor some time ago too. So if you could be so kind to show some lifesigns and perhaps even create an updated version which addresses John's initial comments then we can get this moving forward, and hopefully welcome you as an FE contributer :) (ping) :) SEC got an update last month ; I'll repackage version 2.3.3 this week. (quite eager to contribute but, due to workload, the whole sponsoring thing was just a bit too much (camel's straw) to look into for the last few months) I'm looking forward to a 2.3.3 package to review then. SRPM upgraded to upstream 2.3.3 ; fixed spec-file in accordance with comment #1. http://www.dmbr.ugent.be/~didier/sec/sec-2.3.3-1.src.rpm http://www.dmbr.ugent.be/~didier/sec/sec.spec TIA for reviewing ! rpmlint: W: sec log-files-without-logrotate /var/log/sec.log W: sec dangerous-command-in-%post rpm W: sec dangerous-command-in-%preun rpm W: sec dangerous-command-in-%postun rpm E: sec no-chkconfig-line /etc/rc.d/init.d/sec W: sec incoherent-subsys /etc/rc.d/init.d/sec $prog The blocker here is the chkconfig line, you should change to something like "chkconfig: - 26 74" so it by default doesn't start in any runlevel. Missing dependancy on service for %postun (package initscripts) Missing dependancy on service for %preun (package initscripts) Missing dependancy on chkconfig for %post (package chkconfig) Missing dependancy on chkconfig for %preun (package chkconfig) You'll need to Require, say, /sin/service and /sbin/chkconfig Source0 doesn't download automatically, http://download.sourceforge.net/simple-evcorr/%{name}-%{version}.tar.gz would be nice. Updated sec.init default runlevel http://www.dmbr.ugent.be/~didier/sec/sec-2.3.3-2.src.rpm http://www.dmbr.ugent.be/~didier/sec/sec.spec Comment #8 : - John, could you please pm me your .rpmlintrc ? With rpmlint-0.76, I do not get any of the above warnings/errors. - Source0 : I get a '404 Not found' on the suggested http://download.sourceforge.net/simple-evcorr/sec-2.3.3.tar.gz ; my proposed http://prdownloads.sourceforge.net/simple-evcorr/sec-2.3.3.tar.gz does not yield any errors ... Didier, The pid file mentioned in the init script isn't created by default. It needs to be added to the sec.sysconfig file: -pid=/var/run/sec.pid The pidfile is needed to add a logrotate file like the following example: /etc/logrotate.d/sec -------------------- /var/log/sec { missingok notifempty sharedscripts postrotate /bin/kill -HUP `cat /var/run/sec.pid 2> /dev/null` 2> /dev/null || true endscript } -------------------- I still need to make a couple more logrotate tests to see if everything is working correctly and I also need to take a look at perl source code as some log lines appear to have been writen as '...\n' instead of "...\n". jpo The log problems appear to be related to the following line shellcmd /bin/echo -- "\n%t %s : $0\n" >> %int_logfile;\ in the file "001_init.sec" (In reply to comment #9) > - Source0 : > I get a '404 Not found' on the suggested > http://download.sourceforge.net/simple-evcorr/sec-2.3.3.tar.gz ; my proposed > http://prdownloads.sourceforge.net/simple-evcorr/sec-2.3.3.tar.gz does not yield > any errors ... > No but if you wget that you get a html page instead of the source. The above vewrsion is correct, yes sometiems it gives 404 as its a round robin dns to the mirrors and not all mirrors seem to carry all packages all of the time an other solutin is welcome, but for now we usually use the above form, the prdownloads form is wrong as it doesn't get you the source. Besides that I wonder where do we stand with this, are you waiting for a review of the version posted in comment #9, ifso could you first please address JPO's comments about the pid file and logrotate stuff and post a new version then I will be happy todo a review (I know I promised that earlier too, but I forgot to put myself on the CC-list so I missed your new versipon luckily John picked it up. About rpmlint that is AFAIK not .rpmmacros related are you sure you have the latest version of rpmlint (from the FE-devel branch)? Unfortunatly many comments were lost here due to the BZ crash. Summarising: -Chris Petersen <lists forevermore net> needs this for his work so he posted a much improved spec and srpm (Chris can you repost a link to these please). -I responded that that was very nice of him and that I understand he wanted some progress because he needed this for his work, but that this wasn't helping in getting Didier sponsored as Didier himself must show that he is able to create proper specs. -Some friendly discussion -End result, Didier was only interested (for now) in getting SEC into FE, Chris is going to get it into FE since he needs it to work and Didier well submit a new package review request with the FE_NEEDSPONSOR flag if / when he wants to contribute something else. As such I'm removing the FE_NEEDSPONSOR blocker since this is now a regular review (with Chris being the review submitter). Chris repeating myself: please repost the link to your latest SRPM and spec. wow, talk about loss of information.... SRPM: http://rpm.forevermore.net/sec/sec-2.3.3-4.src.rpm SPEC: http://rpm.forevermore.net/sec/sec.spec Still a few concerns from rpmlint, but I honestly don't know what the best way to fix them would be (or just leave things how they are). Haven't had a chance to bring any of it up in irc or on the mailing list. OK, release 4: rpmlint: W: sec non-conffile-in-etc /etc/sec/README W: sec non-conffile-in-etc /etc/sec/examples/pix-security.sec W: sec non-conffile-in-etc /etc/sec/examples/snortsam.sec W: sec non-conffile-in-etc /etc/sec/examples/syslog-ng.sec W: sec non-conffile-in-etc /etc/sec/examples/general.sec W: sec non-conffile-in-etc /etc/sec/examples/001_init.sec W: sec non-conffile-in-etc /etc/sec/examples/snort.sec W: sec non-conffile-in-etc /etc/sec/examples/pix-url.sec W: sec non-conffile-in-etc /etc/sec/examples/hp-openview.sec W: sec non-conffile-in-etc /etc/sec/examples/bsd-PHYSMOD.sec W: sec non-conffile-in-etc /etc/sec/examples/cvs.sec W: sec non-conffile-in-etc /etc/sec/examples/vtund.sec W: sec non-conffile-in-etc /etc/sec/examples/ssh.sec W: sec non-conffile-in-etc /etc/sec/examples/dameware.sec W: sec non-conffile-in-etc /etc/sec/examples/ssh-brute.sec W: sec non-conffile-in-etc /etc/sec/examples/clamav.sec W: sec non-conffile-in-etc /etc/sec/examples/labrea.sec W: sec non-conffile-in-etc /etc/sec/examples/mpd.sec W: sec non-conffile-in-etc /etc/sec/examples/bsd-MONITOR.sec W: sec non-conffile-in-etc /etc/sec/examples/bsd-USERACT.sec W: sec non-conffile-in-etc /etc/sec/examples/windows.sec W: sec non-conffile-in-etc /etc/sec/examples/dbi-example.sec W: sec non-conffile-in-etc /etc/sec/examples/amavisd.sec W: sec non-conffile-in-etc /etc/sec/examples/portscan.sec Non-executable files in your package are being installed in /etc, but is not marked %config. I'm inclined to ignore, they are samples. W: sec incoherent-subsys /etc/rc.d/init.d/sec $prog W: sec strange-permission sec.init 0755 W: sec mixed-use-of-spaces-and-tabs All minor. - package meets naming guidelines - package meets packaging guidelines - license (GPL) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream * However, you could have the URL point directly to the source: http://dl.sourceforge.net/sourceforge/simple-evcorr/%{name}-%{version}.tar.gz - package compiles on devel (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file APPROVED The package has been created and added to CVS. I apparently can't resolve the ticket, so someone else will have to close it for me. build successfull for devel... closing this bug for Chris. Package Change Request ====================== Package Name: sec New Branches: EL-5 F-7 cvs done. Note that there was already a F-7 branch for this package. |