Spec URL: http://till.fedorapeople.org/review/postfix-logwatch.spec SRPM URL: http://till.fedorapeople.org/review/postfix-logwatch-1.36.12-1.tillf7.src.rpm Description: This is a postfix log analyzer for logwatch.
I'd be happy to review this package. Look for a full review here in a few.
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPLv2+) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: c5eda596bc3d142ff5cf6db7986e6459 postfix-logwatch-1.36.12.tgz c5eda596bc3d142ff5cf6db7986e6459 postfix-logwatch-1.36.12.tgz.1 OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane: SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have dist tag OK - Should package latest version Issues: 1. rpmlint says: postfix-logwatch.noarch: E: non-executable-script /etc/logwatch/scripts/services/postfix 0644 It's not clear to me if this does need to be executable, but if it was you could run it standalone, so that might be better? postfix-logwatch.noarch: W: conffile-without-noreplace-flag /etc/logwatch/scripts/services/postfix Should this have (noreplace) for upgrades to avoid messing up people's customizations?
Hi Kevin. I'm not sure how this has been packaged - postfix-logwatch runs either under logwatch (wherein, it does not need to be executable), and standalone where it does. In my new versions of the code, I have enabled perl's taint mode (via #!/usr/bin/perl -T) for the standalone mode, and taint mode will not work under logwatch. As a standalone package, taint mode works fine. I check postfix-logwatch into the logwatch cvs tree, so Fedora will automatically get postfix-logwatch (named simply postfix in /etc/logwatch/scripts/services/ directory) via logwatch updates. I would recommend that this package named "postfix-logwatch" be installed as the standalone program, which will a) be more up to date than that in logwatch, and b) be decoupled from logwatch's schedule, which as a project is becoming increasingly less active. I have some significant new features I'm about to release - if you could hold for a couple of days, I should be ready to release.
(In reply to comment #3) > I'm not sure how this has been packaged - postfix-logwatch runs either under > logwatch (wherein, it does not need to be executable), and standalone where it does. It is packaged to be used with logwatch. > I check postfix-logwatch into the logwatch cvs tree, so Fedora will > automatically get postfix-logwatch (named simply postfix in > /etc/logwatch/scripts/services/ directory) via logwatch updates. The main reason I built the package is to get this into Fedora EPEL, because the postfix script in logwatch in RHEL does not produce such a nice output. > I would recommend that this package named "postfix-logwatch" be installed as the > standalone program, which will a) be more up to date than that in logwatch, and > b) be decoupled from logwatch's schedule, which as a project is becoming > increasingly less active. These two benefits are no advantages of the standalone program, but only of this package at all, imho. When the script is run from logwatch it is more up to date and decoupled from logwatch's schedule, too. > I have some significant new features I'm about to release - if you could hold > for a couple of days, I should be ready to release. This can be done.
I understand the motivation. The version of logwatch has been historically ancient in RHEL releases. It may be not safe to assume that my current versions of postfix-logwatch will just work in older versions of logwatch. I'll make no guarantees there. I've also restructured the code into multiple perl modules, which will be shared with amavis-logwatch, and future parsers. One of the primary reasons I wrote postfix-logwatch, and added the standalone feature, was so that the script could be run from the command line, allowing more dynamic varieties of reports, which is not possible under logwatch without modifying the conf/services/postfix.conf file before each run. The standalone config file is likely to be different than that used for logwatch (in conf/services). The newer --config_file option allows using varieties of configuration files for specific reports, and command line options provide further customization on the fly (esp. with new, upcoming features). There is also a new man page; logwatch doesn't have module-specific man pages, so this is a bit of a divergence. As an "encouragement" to make the utility available in standalone mode, consider speed and performance. Logwatch is very clumsy and expensive in how it deals with log files - it copies the primary log file, and when archives are being searched, the archives as well, into a monolithic copy in the tmp directory. Performance on larger installations can be abysmal. I encourage using postfix-logwatch in standalone mode to avoid this excess. I'd prefer not to deal with any confusions regarding which version filter is in which version of logwatch. If this can be minimized, I'm ok with it. MrC
"may be not" => "may not be". (as an addendum, an upcoming new option dynamically reconfigures and builds several command line options on the fly, and adjusts how some config file variables are interpreted. This has more utility when running standalone).
I've updated postfix-logwatch with the new features indicated. The release is labeled 1.36.13pre5.
Whats the status here?
Sorry for the delay. Kevin, to whom was your question addressed? I'll be promoting the devel release to released this week. It will include a couple minor bug fixes in the pre5 release, and the man page will be completed.
Till. Mike, Kevin.. what's the status here? I'd like to see this package in EPEL :)
I let version 1.36.12.1 run on my CentOS machines for several weeks and nothing broke, therefore I would add the package as it is to EPEL.
This sounds excellent. Kevin, could you please continue the review ?
I'm ok with whatever choice is made here. But consider that 1.36.12 is fairly old, and these changes have been made since: 2008-01-14 (version: 1.36.13pre7) - New: Support ETRN rejects (option: RejectEtrn). - Fix: Include optional text in L1 output for header/body checks Hold messages. - Fix: Accept "unknown" as an IP in Connection rate limit messages. Thanks: Stefan Jakobs. - Fix: Improve MxError captures - Fix: Ignore "sql auxprop plugin ..." messages - Fix: Ignore more debug_peer_level=2 messages - Fix: Correct failure to ignore "connect to subsystem..." debug lines - Fix: Add missing RejectVerify to config file 2007-12-15 (version: 1.36.13pre6) - Fix: in BounceLocal and EnvelopeSenderDomains, set null domain and formatted host to '*unknown'. - Fix: Move postfix_warning before postfix_cleanup, as some cleanup warnings were caught as unmatched. - Fix: An Accepted message is now triggered by smtpd "client=..." and pickup "uid=..." log entries, instead of qmgr's "from=xxx, size=nnn, nrcpt=nnn" log lines. A message may have been accepted during a previous time period, but delivery delays result in multiple qmgr delivery attempts, resulting in over counting Accepted messages. Thanks: Stefan Jakobs - New: Bytes delivered is now broken down by Bytes sent via SMTP, LMTP, and forwarded, to be orthogonal with message counts. - Fix: Resent messages no longer reduce the number of messages accepted - this was a naive attempt at not counting, for example, messages released and requeued from a content filter's quarantine. 2007-11-14 (version: 1.36.13pre5) - New: Threshold limiting and Top N lists for every level in each Detail section. Every level in each section can now be limited with minimum count thresholds and top N lists. See the updated README file, the comments in the postfix-logwatch.conf file, and the new postfix-logwatch man page. Requested by: Pavel Urban - New: Rejects can now be categorized by reject reply code. A new option/variable "reject_reply_patterns" is a list of reject reply code regular expressions, which are used for categorizing rejects. This feature allows, for example, distinguishing 421 transmission channel closes from 45x errors. (eg. 450 mailbox unavailable, 451 local processing errors, 452 insufficient storage). The default list is: "5.. 4.. Warn" which creates three groups of rejects: permanent rejects, temporary failures, and reject warnings (as in warn_if_reject). Requested by: Noel Jones - New: postfix-logwatch man page created (net yet complete) - New: Support for all access(5) actions. See the "Level Limiter Options" section in the postfix-logwatch(1) man page, and "Common access control actions" in postfix-logwatch.conf. - New: Added envelope senders and envelope sender domains reports. These are disabled by default. Enable with level limiter options --envelopesenders 1 and --envelopesenderdomains 1 (or 2 to also see senders listed under domains). See also the postfix-logwatch.conf file. Suggested by: Brendan - Change: Uncomment all variables in the config file. This should help ensure the variables in the config file stay in sync with those used in the source. - Change: Merged sections SenderDelayNotification, DSNDelivered, and DSNUndelivered into NotificationSent, with sub-sections indicating the type of notification. This gives the total number of sent notifications in the Summary section, and the breakdown by type of notification in the Detailed section. - Change: Removed "msgs" prefix from several options: msgsdeferred, msgsdelivered, msgsforwarded, msgsresent, msgssent, and msgssentlmtp are now deferred, delivered, forwarded, resent, sent and sentlmtp. The old options are still usable. - Change: force --help output to 80 chars. - Change: Taint mode is now on by default in standalone mode. It is disabled upon installation in logwatch mode, as logwatch fails with taint mode enabled. - Change: Set the primary key in tlsserverconnect/tlsclientconnect options to type/cypher to reduce excessive level 2 output. - Change: Reported values that cannot be determined or are unavailable are prefixed with an asterisk (e.g. *unknown, *unspecified). - Change: Warn section title 'Warn action logged' changed simply to "Warned", to be consistent with other access/header_ and body_checks. The option is --warned, but --warn is still acceptable. - Fix: Significantly reduce memory footprint when detail < 5. - Fix: Usage and --help now correctly show only detail section level limiter options that are available. Previously, summary- only counts were also display as level limiter options. - Fix: "too many errors after DATA" and "timeout after DATA" may include a byte count, as in "(348 bytes)". Thanks: John Beaver - Fix: Ignore postgrey 'delayed ...' lines - Fix: Allow logwatch --debug option to pass into postfix-logwatch - Fix: configuration file reading code was not properly warning on non-existent files - Fix: "filter" actions were incremented on "redirect" actions - Fix: Give more room to percentiles in delays percentiles table. A 5 day delay is 432000.000 seconds, and the previous table width was not sufficient. - Fix: --show_sect_vars command line option inadvertently required an argument; the name has been shortened to --sect_vars/--nosect_vars and correctly no longer requires an argument. The longer names --[no]show_sect_vars work as well. Thanks: Noel Jones - Fix: Handle some unmatched connect to failures in section ConnectToFailure. Cleanup and consolidate several similar messages; add additional detail at level 3. - Fix: Increment postsuper Hold messages by the number of messages placed on hold. - Fix: Add "non-ESMTP response ..." messages to SmtpConversationError. First level is now the general SMTP error description. - Internal: Converted source to be package-based to allow code sharing with amavis-logwatch. The single-file executable is auto-generated from the packages. - Internal: internal gen_test_log function to create sample log data for testing. Reads '#TD' comments from within postfix-logwatch. 2007-10-16 (version: 1.36.13pre4) - Fix: Handle messages "SSL_connect error to example.com: 0" and "Cannot start TLS: handshake failure" by coercing into warnings Thanks: Rob Sterenborg 2007-10-15 (version: 1.36.13pre3) - Fix: Handle "postmaster" DSNs - Fix: Handle "discarding EHLO keywords:" (ignored) - Internal: create bounce subroutine to handle bounce messages 2007-10-05 (version: 1.36.13pre2) - New: Initial support for postgrey (http://postgrey.schweikert.ch/) Requested by: Sebastian Wolfgarten 2007-10-05 (version: 1.36.13pre1) - New: Support for policy-spf software postfix-policyd-spf-perl Requested by: Nicodemo P. and Rob Sterenborg I can bump the version to 1.36.13 final if necessary - its been operational for quite some time.
Till: Would you like me to take another look at the last posted package here, or would you like to spin up an updated one? Let me know and we can get this review moving...
Any news here?
Please add a link to a site for the latest version of the filter inline; I tried getting the latest postfix file from http://www2.logwatch.org:81/ but it's incompatible with the logwatch in v5.1. Hoping 5.1 gets a fix to stop barfing all over my amavisd and current-postfix log messages soon...
Chris: The latest versions are available at: http://www.mikecappella.com/logwatch Please try the devel version: 1.36.13pre7 I will have a pre8 version out this week, and if there are no issues, will promote to 1.36.13.
I am sorry, but I do not have the time to continue this. I will reopen this bug report, when I have, but maybe someone else wants to package this, which I do not want to block.
I'm reopening this as I have been in discussion with Karel Klic (logwatch maintainer) about removing the (old) postfix script from the logwatch package, and distributing Mike's up-to-date script in a new postfix-logwatch package. Mike, are you still there? Your website doesn't seem to work at the moment: http://www.mikecappella.com/logwatch
No longer blocks FE-DEADREVIEW.
I'm here. I've moved to sourceforge: http://logreporters.sourceforge.net/ https://sourceforge.net/projects/logreporters/
Richard: were you going to submit a new updated package here for review?
I will get to it. Just haven't had much time lately to work on it.
Why don't you open a new review on the package when you are ready to submit? In the mean time we can close this one.