Bug 226161 - Merge Review: mrtg
Merge Review: mrtg
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jon Ciesla
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:43 EST by Nobody's working on this, feel free to take it
Modified: 2008-12-11 11:43 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-11 11:43:12 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:43:24 EST
Fedora Merge Review: mrtg

http://cvs.fedora.redhat.com/viewcvs/devel/mrtg/
Initial Owner: mitr@redhat.com
Comment 1 Jon Ciesla 2008-09-16 12:21:58 EDT
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 Jon Ciesla 2008-09-30 08:09:30 EDT
ccing current owner.
Comment 3 Jon Ciesla 2008-12-09 15:51:55 EST
Ping?
Comment 4 Vitezslav Crhonek 2008-12-11 11:31:15 EST
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 Jon Ciesla 2008-12-11 11:43:12 EST
Very good, thanks!

APPROVED

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