Bug 424841 (ndisc6)

Summary: Review Request: ndisc6 - IPv6 diagnostic tools
Product: [Fedora] Fedora Reporter: Stjepan Gros <stjepan.gros>
Component: Package ReviewAssignee: Gwyn Ciesla <gwync>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, gwync, itamar, j, notting, sgros, tuju
Target Milestone: ---Flags: gwync: fedora-review+
dennis: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-02-06 21:02:17 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 Stjepan Gros 2007-12-14 10:36:19 UTC
Spec URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6.spec
SRPM URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6-0.9.3-1.fc8.src.rpm
Description:

This package gathers a few diagnostic tools for IPv6 networks:
- ndisc6, which performs ICMPv6 Neighbor Discovery in userland,
- rdisc6, which performs ICMPv6 Router Discovery in userland,
- rltraceroute6, yet another IPv6 implementation of traceroute,
- tcptraceroute6, a TCP/IPv6-based traceroute implementation,
- tracert6, a ICMPv6 Echo Request based traceroute,
- tcpspray6, a TCP/IP Discard/Echo bandwidth metter.

Comment 1 Jason Tibbitts 2007-12-21 05:58:03 UTC
This builds OK in mock on rawhide; rpmlint says the following:

  ndisc6.x86_64: E: setuid-binary /usr/bin/rltraceroute6 root 04755
  ndisc6.x86_64: E: setuid-binary /usr/bin/rdisc6 root 04755
  ndisc6.x86_64: E: setuid-binary /usr/bin/ndisc6 root 04755
  ndisc6.x86_64: E: non-standard-executable-perm /usr/bin/rltraceroute6 04755
  ndisc6.x86_64: E: non-standard-executable-perm /usr/bin/rdisc6 04755
  ndisc6.x86_64: E: non-standard-executable-perm /usr/bin/ndisc6 04755
Setuid binaries are troubling, expecially when the ipv4 counterparts aren't
setuid.  Has this code been properly audited?  Is it really necessary that
they be setuid?

  ndisc6.x86_64: W: symlink-should-be-relative
    /usr/share/man/man1/tcpspray6.1.gz /usr/share/man/man1/tcpspray.1.gz
  ndisc6.x86_64: W: symlink-should-be-relative
    /usr/share/man/man1/name2addr.1.gz /usr/share/man/man1/addr2name.1.gz
  ndisc6.x86_64: W: symlink-should-be-relative
    /usr/share/man/man8/tracert6.8.gz /usr/share/man/man8/rltraceroute6.8.gz
  ndisc6.x86_64: W: symlink-should-be-relative
    /usr/share/man/man8/tcptraceroute6.8.gz /usr/share/man/man8/rltraceroute6.8.gz
These should be relative symlinks; they don't really need need any pathname
information at all.

  ndisc6.x86_64: W: no-version-in-last-changelog
Changelog entries should be in one of the formats detailed in the Changelogs section of http://fedoraproject.org/wiki/Packaging/Guidelines

  ndisc6.x86_64: W: invalid-license GPL2+
I think you mean "GPLv2+"; see http://fedoraproject.org/wiki/Licensing

Comment 2 Stjepan Gros 2007-12-22 17:23:35 UTC
I fixed license, added version in Changelog file, and removed all setuid bits
(as a reslut binares are moved into sbin directory instead of bin). Finally, I
modified Makefile.am, regenerated Makefile.in and generated patch in order to
remove absolute pathnames in man page's symlinks. Is there some more
efficient/elegant way to do that?

Spec URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6.spec
SRPM URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6-0.9.3-2.fc8.src.rpm

Comment 3 Jason Tibbitts 2008-01-16 06:05:01 UTC
One important question before I assign this to myself: Is this your first
package for Fedora?  You haven't indicated that you need sponsorship, but I
don't see your name on the list of packagers.  What's your FAS ID?

Since I have this package open, I might as well run through my checklist.

Looking at the source, the License: tag should "GPLv2 or GPLv3".  They permit
only those versions, not "any later version".

The latest version now seems to be 0.9.5.  Did you want to update?

You can save yourself a little pain by using the %find_lang macro; at the end of
%install, add 
  %find_lang %{name}
and then change your files section to start with:
  %files -f %{name}.lang
and remove the two %lang(* lines from your %files list.  This will automatically
find all of the .po and related files and include just the right directories and
%lang tags for you.

* source files match upstream:
   13f238cc03e43dd05020755b3a5ec57d3cfa1eecfba71dc00157d26351afe718  
   ndisc6-0.9.3.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
X license field matches the actual license.
* license is open source-compatible.
* license text included in package.
? latest version is 0.9.5.
* BuildRequires are proper (none)
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   ndisc6 = 0.9.3-2.fc9
  =
   /bin/sh
   /usr/bin/perl
   perl(Getopt::Std)
   perl(strict)

* %check is not present; no test suite upstream.  No ipv6 here so I can't test 
   this.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 4 Stjepan Gros 2008-01-22 16:45:40 UTC
Yes, this is my first package. I obviously forgot to request sponsorship. Sorry.

For the rest, I updated to new version, changed revision, generated patch to
allow build on rawhide (builds perfectly on F8 without patch), and applied your
suggestion for language specific files.

I also cleaned up the patch (it doesn't patch Makefile.am and configure.ac
files). I don't know if it's good or not? Also, this version has a perl script
that it installs into /etc/rdnssd. Is it a good place for hook scripts?

Spec URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6.spec
SRPM URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6-0.9.5-1.fc8.src.rpm

Comment 5 Stjepan Gros 2008-03-15 10:19:37 UTC
Synchronized the package with upstream version. Patches are removed as original
package doesn't contain absolute paths anymore.

Spec URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6.spec
SRPM URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6-0.9.7-1.fc8.src.rpm


Comment 7 Gwyn Ciesla 2008-10-13 14:41:47 UTC
Starting official review, and I can sponsor you.  In the meantime, please complete some unofficial reviews for others, and post links here.

Comment 8 Gwyn Ciesla 2008-10-13 14:56:28 UTC
MUST: rpmlint must be run on every package. The output should be posted in the review.

SRPM:
ndisc6.src:76: W: macro-in-%changelog find_lang
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

FIX.

rpmlint on RPMs is clean.


- MUST: The package must be named according to the Package Naming Guidelines .

OK.

- MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption on Package Naming Guidelines .

OK.

- MUST: The package must meet the Packaging Guidelines .

OK.

- MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .

OK.

- MUST: The License field in the package spec file must match the actual license.

OK.

- MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.

Only includes v2, but OK.

- MUST: The spec file must be written in American English.

OK.

- MUST: The spec file for the package MUST be legible. If the reviewer is unable to read the spec file, it will be impossible to perform a review. Fedora is not the place for entries into the Obfuscated Code Contest (http://www.ioccc.org/).

OK.

- MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.

OK.

- MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.

OK.

- MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch needs to have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number should then be placed in a comment, next to the corresponding ExcludeArch line. New packages will not have bugzilla entries during the review process, so they should put this description in the comment until the package is approved, then file the bugzilla entry, and replace the long explanation with the bug number. The bug should be marked as blocking one (or more) of the following bugs to simplify tracking such issues: FE-ExcludeArch-x86 , FE-ExcludeArch-x64 , FE-ExcludeArch-ppc , FE-ExcludeArch-ppc64

OK.

- MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.

Running a mock build to test this.

- MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.

OK.

- MUST: Every binary RPM package which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. If the package has multiple subpackages with libraries, each subpackage should also have a %post/%postun section that calls /sbin/ldconfig. An example of the correct syntax for this is:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

NA.

- MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker.

NA.

- MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. Refer to the Guidelines for examples.

OK.

- MUST: A package must not contain any duplicate files in the %files listing.

OK.

- MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.

OK.

- MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ).

OK.

- MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines .

OK.  Consider using %{version} where you can, like in the Source0.  SAve yourself work later.

- MUST: The package must contain code, or permissable content. This is described in detail in the code vs. content section of Packaging Guidelines .

OK.

- MUST: Large documentation files should go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity)

NA.

- MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present.

OK.

- MUST: Header files must be in a -devel package.

NA.

- MUST: Static libraries must be in a -static package.

NA.

- MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability).

NA.

- MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.

NA.

- MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}

NA.

- MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec.

OK.

- MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. This is described in detail in the desktop files section of the Packaging Guidelines . If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.

NA.

- MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time.

OK.

- MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). See Prepping BuildRoot For %install for details.

OK.

- MUST: All filenames in rpm packages must be valid UTF-8.

OK.

Comment 9 Gwyn Ciesla 2008-10-13 15:11:47 UTC
BRs are good, so fix the macro in the changelog, show me some sample reviews, and we're good to approve and sponsor.

Comment 10 Gwyn Ciesla 2008-12-29 14:22:53 UTC
Ping?

Comment 11 Stjepan Gros 2008-12-29 18:25:49 UTC
I'm here, but I was busy doing my PhD and then came Christmas. I'll look into packages that need review to see if there are related ones.

Comment 12 Gwyn Ciesla 2008-12-29 18:33:45 UTC
I suppose that's a pretty good excuse. :)  Hope it went well.

Comment 13 Stjepan Gros 2008-12-29 22:07:40 UTC
Ok, I fixed the omission in the changelog section. The new spec and src.rpm files are available on the following URLs:

Spec URL: http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6.spec
SRPM URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/ndisc6/ndisc6-0.9.8-2.fc8.src.rpm

As for review of some packages, I will be hard to find one that's not already reviewed. :) Do you maybe have some suggestion?

Comment 14 Gwyn Ciesla 2008-12-30 14:05:51 UTC
Who says it can't already be in the process of being reviewed?  Just mention that it's practice and not binding.  Pick something you feel somewhat confident in, maybe in a language you have some experience with.

Comment 15 Gwyn Ciesla 2009-03-31 13:20:29 UTC
Ping?

Comment 16 Stjepan Gros 2009-04-02 07:40:10 UTC
Sorry for not letting you know, but I wont be able to do much until summer. Feel free to take this package.

Comment 17 Gwyn Ciesla 2009-04-02 12:52:51 UTC
Ok.  CC list, if I want to put in this package under the aegis of my own ownership, will it require a fresh review, or can I essentially self-approve this one?

Comment 18 Stjepan Gros 2009-11-15 09:24:34 UTC
What happened with this package? I don't see it appeared in rawhide or F12. This ticket is still open. Did anyone taken it?

Comment 19 Gwyn Ciesla 2009-11-16 13:30:25 UTC
I never got an answer to #17.  Are you back?

Comment 20 Stjepan Gros 2009-11-16 16:38:01 UTC
Actually, I'm here for some time already. But I thought that everything was settled with this entry, i.e. that you'll take it, so I didn't post anything to this thread?

Comment 21 Gwyn Ciesla 2009-11-16 16:45:04 UTC
Nope, it's a resounding nothing.  I've had a. . .full. . .personal life since post #17, so I've not touched it.  If you want to do the practice reviews, we can pick up where we left off.

Comment 22 Stjepan Gros 2009-11-16 17:17:24 UTC
Ok, I'll repackage ndisc6 (there is a new version) and submit it here and then we'll see what to do next.

Comment 23 Gwyn Ciesla 2009-11-16 18:14:08 UTC
Removed NEEDSPONSOR blocker.  I'll look over the new version when you post it, and approve it nothing new creeps up.

Comment 25 Gwyn Ciesla 2009-11-17 13:03:40 UTC
SRPM is 404.

Comment 27 Gwyn Ciesla 2009-11-17 14:14:27 UTC
This looks good too, APPROVED.

Comment 28 Stjepan Gros 2009-11-18 08:00:53 UTC
New Package CVS Request
=======================
Package Name: ndisc6
Short Description: IPv6 diagnostic tools
Owners: sgros
Branches: F-12
InitialCC: sgros

Comment 29 Itamar Reis Peixoto 2009-11-18 14:55:05 UTC
why not F-11 and EL-5 too ?

Comment 30 Stjepan Gros 2009-11-18 15:05:26 UTC
No particular reason, inertia primarily. I can request CVS branches for F-11 and EL-5 too, but first I'll have to look how that's done. :)

Comment 31 Gwyn Ciesla 2009-11-18 15:10:39 UTC
Similarly to #28.  Set the cvs flag to ?, and see this:

https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Comment 32 Stjepan Gros 2009-11-19 08:21:56 UTC
New Package CVS Request
=======================
Package Name: ndisc6
Short Description: IPv6 diagnostic tools
Owners: sgros
Branches: F-11 F-12 EPEL
InitialCC: sgros

Comment 33 Jason Tibbitts 2009-11-19 23:06:06 UTC
Your CVS request is not well-formed; there is no branch named "EPEL".  We can make either EL-4 or EL-5 branches (or both); please indicate which you would like in a corrected CVS request.

Comment 34 Stjepan Gros 2009-11-20 07:53:26 UTC
New Package CVS Request
=======================
Package Name: ndisc6
Short Description: IPv6 diagnostic tools
Owners: sgros
Branches: F-11 F-12 EL-5
InitialCC: sgros

Comment 35 Dennis Gilmore 2009-11-20 22:44:26 UTC
CVS Done

Comment 36 Fedora Update System 2009-11-23 12:15:43 UTC
ndisc6-0.9.9-1.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/ndisc6-0.9.9-1.el5

Comment 37 Fedora Update System 2009-11-23 12:18:43 UTC
ndisc6-0.9.9-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/ndisc6-0.9.9-1.fc11

Comment 38 Fedora Update System 2009-11-23 12:20:25 UTC
ndisc6-0.9.9-1.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/ndisc6-0.9.9-1.fc12

Comment 39 Fedora Update System 2009-11-25 15:04:28 UTC
ndisc6-0.9.9-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 40 Fedora Update System 2009-11-25 15:12:34 UTC
ndisc6-0.9.9-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 41 Fedora Update System 2009-12-10 04:01:02 UTC
ndisc6-0.9.9-1.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 42 Stjepan Gros 2010-02-06 21:02:17 UTC
Finally closing this ticket. Package for a long time in repositories.