Bug 226161

Summary: Merge Review: mrtg
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Gwyn Ciesla <gwync>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: gwync, vcrhonek
Target Milestone: ---Flags: gwync: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-12-11 16:43:12 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:

Description Nobody's working on this, feel free to take it 2007-01-31 19:43:24 UTC
Fedora Merge Review: mrtg

http://cvs.fedora.redhat.com/viewcvs/devel/mrtg/
Initial Owner: mitr

Comment 1 Gwyn Ciesla 2008-09-16 16:21:58 UTC
rpmlint on SRPM:

mrtg.src:22: E: prereq-use vixie-cron
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

mrtg.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 44)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance.  Use either spaces or tabs for indentation, not both.

Fix.

mrtg.src: W: strange-permission filter-requires-mrtg.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

mrtg.src: W: strange-permission filter-provides-mrtg.sh 0755
A file that you listed to include in your package has strange permissions.
Usually, a file should have 0644 permissions.

What are these about?  If they're good this way, please document what they do in the spec.

rpmlint on RPMS:

mrtg.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/mrtg-2.16.2/contrib/whodo/GIFgraph/GIFgraph/samples/sample54.dat
mrtg.i386: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES

Tons of this all over the docs.  Fix.

mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualpri.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/hiperdsp.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualt1.pl "c:\perl\bin"
This script uses an incorrect interpreter.
mrtg.i386: E: wrong-script-interpreter /usr/share/doc/mrtg-2.16.2/contrib/cfgmaker_cisco/cfgmaker.cisco "/pkg/gnu/bin/perl"
This script uses an incorrect interpreter.

mrtg.i386: E: only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

mrtg.i386: W: dangerous-command-in-%trigger rm

Fix.

License tag should by GPLv2+.

Otherwise, full review looks good, no other blockers.

Comment 2 Gwyn Ciesla 2008-09-30 12:09:30 UTC
ccing current owner.

Comment 3 Gwyn Ciesla 2008-12-09 20:51:55 UTC
Ping?

Comment 4 Vitezslav Crhonek 2008-12-11 16:31:15 UTC
Hi,

(In reply to comment #1)
> rpmlint on SRPM:
> 
> mrtg.src:22: E: prereq-use vixie-cron
> The use of PreReq is deprecated. In the majority of cases, a plain Requires is
> enough and the right thing to do. Sometimes Requires(pre), Requires(post),
> Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

Fixed. Changed to plain Requires.

> 
> mrtg.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 44)
> The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
> annoyance.  Use either spaces or tabs for indentation, not both.
>

Fixed.

> 
> mrtg.src: W: strange-permission filter-requires-mrtg.sh 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.
> 
> mrtg.src: W: strange-permission filter-provides-mrtg.sh 0755
> A file that you listed to include in your package has strange permissions.
> Usually, a file should have 0644 permissions.
> 
> What are these about?  If they're good this way, please document what they do
> in the spec.

These files are used to filter perl provides/requires. I think it's not necessary to document them in spec, because it's often used and well known technique.
These files are also NOT listed to be included in final package.

For more info see:
https://fedoraproject.org/wiki/Packagin/Perl#External_filtering

> 
> rpmlint on RPMS:
> 
> mrtg.i386: W: wrong-file-end-of-line-encoding
> /usr/share/doc/mrtg-2.16.2/contrib/whodo/GIFgraph/GIFgraph/samples/sample54.dat
> mrtg.i386: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES
> 
> Tons of this all over the docs.  Fix.

I fixed only this one:
mrtg.x86_64: W: file-not-utf8 /usr/share/doc/mrtg-2.16.2/CHANGES

Because CHANGES is real documentation file. The rest is in /usr/share/doc/mrtg-2.16.2/contrib/* - just ideas for mrtg users over different operating systems. I feel it's right to make it available but untouched.

> 
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualpri.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/hiperdsp.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/TCH/dualt1.pl "c:\perl\bin"
> This script uses an incorrect interpreter.
> mrtg.i386: E: wrong-script-interpreter
> /usr/share/doc/mrtg-2.16.2/contrib/cfgmaker_cisco/cfgmaker.cisco
> "/pkg/gnu/bin/perl"
> This script uses an incorrect interpreter.

I have same reason as above to don't touch these files (often examples from other operating systems).

> 
> mrtg.i386: E: only-non-binary-in-usr-lib
> There are only non binary files in /usr/lib so they should be in /usr/share.

There are only perl modules in /usr/lib{64}/* and this it's AFAIK standard place for them and mrtg will search them here.

> 
> mrtg.i386: W: dangerous-command-in-%trigger rm
> 
> Fix.

This trigger is used when very old mrtg (2.9.17) is uninstalled. This warning isn't real problem... But it was added in Feb 2003, so I think it's time to remove it completely.

> 
> License tag should by GPLv2+.

License changed to GPLv2+.

> 
> Otherwise, full review looks good, no other blockers.

Changes are commited to the devel branch. Let me know if you have any comments or anything else, otherwise feel free to close this review.

Comment 5 Gwyn Ciesla 2008-12-11 16:43:12 UTC
Very good, thanks!

APPROVED