Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225909 - Merge Review: iputils
Summary: Merge Review: iputils
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:06 UTC by Nobody's working on this, feel free to take it
Modified: 2008-01-17 08:02 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-01-17 02:48:03 UTC
panemade: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:06:46 UTC
Fedora Merge Review: iputils

http://cvs.fedora.redhat.com/viewcvs/devel/iputils/
Initial Owner: mbacovsk@redhat.com

Comment 1 Daniel Kopeček 2007-03-03 01:03:35 UTC
(!!) 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 17:55:10 UTC
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 05:21:33 UTC
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 15:41:10 UTC
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-15 03:45:34 UTC
 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 14:26:33 UTC
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-16 03:54:04 UTC
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 10:34:42 UTC
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 11:49:45 UTC
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 08:02:53 UTC
Commit & build done, thanks for the review.


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