Bug 225909 - Merge Review: iputils
Merge Review: iputils
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 14:06 EST by Nobody's working on this, feel free to take it
Modified: 2008-01-17 03:02 EST (History)
7 users (show)

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


Attachments (Terms of Use)

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

http://cvs.fedora.redhat.com/viewcvs/devel/iputils/
Initial Owner: mbacovsk@redhat.com
Comment 1 Daniel Kopeček 2007-03-02 20:03:35 EST
(!!) MUST: rpmlint output:
**** Review message:
W: iputils no-url-tag
   - URL tag is missing, should be: http://www.tux.org/pub/net/ip-routing/
W: iputils redundant-prefix-tag
   - Line: 35 "Prefix: %{_prefix}" should be removed.
W: iputils buildprereq-use docbook-utils perl-SGMLSpm
W: iputils buildprereq-use glibc-kernheaders >= 2.4-8.19
W: iputils prereq-use chkconfig
W: iputils macro-in-%changelog files
   - Line: 404 - List /usr/sbin/rdisc in %files list.
     Should be: %%files
W: iputils mixed-use-of-spaces-and-tabs (spaces: line 131, tab: line 92)

********************
(!!) MUST: The package must meet the Packaging Guidelines.
**** Review message:
- Uses BuildPreReq instead of BuildRequires.
- Uses PreReq instead of Requires.
- BuildRoot: %{_tmppath}/%{name}-root
  Required value is: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

********************
(!!) SHOULD: Packager should query upstream for license text file.
**** Review message:
- License file is missing.
- Missing file "README.bonding" listed in %doc.
********************

(Sorry for the late answer)
Comment 2 Martin Bacovsky 2007-03-15 13:55:10 EDT
Everything should be fixed but "License file is missing". - I will ask upstream,
this package is bunch of utils from different sources so I'm not sure what
license they use. In spec is BSD, but I let them confirm that

I'm not sure what to do about rest of warnings from rpmlint you didn't mentioned
like W: iputils symlink-should-be-relative /usr/sbin/arping /sbin/arping

Current version is iputils-20070202-1.fc7, because I upgraded to new upstream.
Comment 3 Parag AN(पराग) 2007-08-31 01:21:33 EDT
rpmlint on binary rpm  gave me
W: iputils mixed-use-of-spaces-and-tabs (spaces: line 109, tab: line 70)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.

I: iputils checking
W: iputils symlink-should-be-relative /usr/share/man/man8/ping6.8.gz
/usr/share/man/man8/ping.8.gz
Absolute symlinks are problematic eg. when working with chroot environments.

W: iputils symlink-should-be-relative /usr/share/man/man8/tracepath6.8.gz
/usr/share/man/man8/tracepath.8.gz
Absolute symlinks are problematic eg. when working with chroot environments.

E: iputils setuid-binary /bin/ping6 root 04755
The file is setuid, this may be dangerous, especially if this 
file is setuid root.

E: iputils non-standard-executable-perm /bin/ping6 04755
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

E: iputils setuid-binary /bin/ping root 04755
The file is setuid, this may be dangerous, especially if this 
file is setuid root.

E: iputils non-standard-executable-perm /bin/ping 04755
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

W: iputils symlink-should-be-relative /usr/sbin/tracepath /bin/tracepath
Absolute symlinks are problematic eg. when working with chroot environments.

W: iputils symlink-should-be-relative /usr/sbin/ping6 /bin/ping6
Absolute symlinks are problematic eg. when working with chroot environments.

W: iputils symlink-should-be-relative /usr/sbin/arping /sbin/arping
Absolute symlinks are problematic eg. when working with chroot environments.

W: iputils symlink-should-be-relative /usr/sbin/tracepath6 /bin/tracepath6
Absolute symlinks are problematic eg. when working with chroot environments.

W: iputils incoherent-init-script-name rdisc
The init script name should be the same as the package name in lower case,
or one with 'd' appended if it invokes a process by that name.

Comment 4 Martin Nagy 2008-01-14 10:41:10 EST
Updated in CVS. rpmlint now gives me:
iputils.i686: E: setuid-binary /bin/ping6 root 04755
iputils.i686: E: non-standard-executable-perm /bin/ping6 04755
iputils.i686: E: setuid-binary /bin/ping root 04755
iputils.i686: E: non-standard-executable-perm /bin/ping 04755
- these are okay, we need root privileges
iputils.i686: W: symlink-should-be-relative /usr/sbin/tracepath /bin/tracepath
iputils.i686: W: symlink-should-be-relative /usr/sbin/ping6 /bin/ping6
iputils.i686: W: symlink-should-be-relative /usr/sbin/arping /sbin/arping
iputils.i686: W: symlink-should-be-relative /usr/sbin/tracepath6 /bin/tracepath6
- won't fix, because if i would change these to relative it would be dangerous
iputils.i686: W: incoherent-init-script-name rdisc
- this is alright (this package is a collection of utilities, which rdisc is
part of)

I also changed the character encoding for RELNOTES to utf8, and few symlinks.
I was also talking to Martin Bacovsky (previous maintainer) and he said that he
did query upstream for license files, but didn't receive no response.
Comment 5 Parag AN(पराग) 2008-01-14 22:45:34 EST
 1)MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines. 
 2) Follow initscript scriptlet as given at
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-b638e19c644263af59762a3154a60554a8303bb3
  missing Requires(preun): /sbin/service and Requires(postun): /sbin/service
 3)Wherever applicable please keep timestamp from files being copied from
upstream tarball. see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-0239576e441f9ef53d175c4aec8c12868dffb5ab
  4)Also, good to follow parallel make as given at
http://fedoraproject.org/wiki/Packaging/Guidelines#head-525c7d76890cb22df33b759c65c35c82bf434d2e

SHOULD:
   1)If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
Comment 6 Martin Nagy 2008-01-15 09:26:33 EST
Updated in CVS.
The missing require was added.
Timestamps are now preserved.
Make is now using %{?_smp_mflags} macro.
I also fixed rdisc's init script (it didn't remove the lock file on stopping the
service).
Comment 7 Parag AN(पराग) 2008-01-15 22:54:04 EST
1)You need to add following as you are calling service from %preun
Requires(preun): /sbin/service

2) Please add macros.
MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines. 

Still I can see SPEC have following lines which does not follow macros
install -cp rdisc               ${RPM_BUILD_ROOT}/sbin/
install -cp ping6               ${RPM_BUILD_ROOT}/bin/
install -cp tracepath           ${RPM_BUILD_ROOT}/bin/
install -cp tracepath6          ${RPM_BUILD_ROOT}/bin/

Also, No macro uses in %files.

See http://fedoraproject.org/wiki/Packaging/RPMMacros
Comment 8 Martin Nagy 2008-01-16 05:34:42 EST
OK, maybe I'm missing something, but I can't see any macros on the mentioned
page that would expand to /bin or /sbin, only to /usr/bin and /usr/sbin.
Comment 9 Parag AN(पराग) 2008-01-16 06:49:45 EST
oops. Extremely sorry my bad. I think I did heavy work today. need sleep now :)
But I hope following is really not in SPEC and need to there
Requires(preun): /sbin/service

APPROVED.
Comment 10 Martin Nagy 2008-01-17 03:02:53 EST
Commit & build done, thanks for the review.

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