Bug 812674

Summary: Review Request: gnome-nettool - Graphical front-ends to various networking command-line tools
Product: [Fedora] Fedora Reporter: Kalev Lember <kalevlember>
Component: Package ReviewAssignee: Richard Shaw <hobbes1069>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hobbes1069, notting, package-review
Target Milestone: ---Flags: hobbes1069: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: gnome-nettool-3.2.0-3.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-05-02 04:44:20 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 Kalev Lember 2012-04-15 20:49:59 UTC
Spec URL: http://kalev.fedorapeople.org/gnome-nettool.spec
SRPM URL: http://kalev.fedorapeople.org/gnome-nettool-3.2.0-1.fc17.src.rpm
Description:
GNOME Nettool is a set of front-ends to various networking command-line
tools, like ping, netstat, ifconfig, whois, traceroute, finger.

Comment 1 Richard Shaw 2012-04-18 18:32:45 UTC
I'll take this one.

Quick spec review:
1. %global is preferred over %define[1].

2. I've never seen desktop-file-validate put in %check. I think that's usually for "make test" type unit testing to kill builds that fail their tests even if they build. I usually put it in %install. 

Otherwise looks good. Now off the building...

[1] http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Comment 2 Richard Shaw 2012-04-18 18:52:34 UTC
Ok, I couldn't build for rawhide. It seemed to bomb out during building the language files:

xsltproc -o gnome-nettool-ru.omf --stringparam db2omf.basename gnome-nettool --stringparam db2omf.format 'docbook' --stringparam db2omf.dtd "-//OASIS//DTD DocBook XML V4.1.2//EN" --stringparam db2omf.lang ru --stringparam db2omf.omf_dir "/usr/share/omf" --stringparam db2omf.help_dir "/usr/share/gnome/help" --stringparam db2omf.omf_in "/builddir/build/BUILD/gnome-nettool-3.2.0/help/gnome-nettool.omf.in"  `/usr/bin/pkg-config --variable db2omf gnome-doc-utils` ru/gnome-nettool.xml || { rm -f "gnome-nettool-ru.omf"; exit 1; }
http://www.oasis-open.org/docbook/xml/4.1.2/dbpoolx.mod:3408: parser warning : PEReference: %tbl.table.att; not found
                %tbl.table.att;
                               ^

which I guess is just a warning, but then it gets a whole bunch of these:
xsltproc -o gnome-nettool-zh_CN.omf --stringparam db2omf.basename gnome-nettool --stringparam db2omf.format 'docbook' --stringparam db2omf.dtd "-//OASIS//DTD DocBook XML V4.1.2//EN" --stringparam db2omf.lang zh_CN --stringparam db2omf.omf_dir "/usr/share/omf" --stringparam db2omf.help_dir "/usr/share/gnome/help" --stringparam db2omf.omf_in "/builddir/build/BUILD/gnome-nettool-3.2.0/help/gnome-nettool.omf.in"  `/usr/bin/pkg-config --variable db2omf gnome-doc-utils` zh_CN/gnome-nettool.xml || { rm -f "gnome-nettool-zh_CN.omf"; exit 1; }
http://www.oasis-open.org/docbook/xml/4.1.2/ent/iso-lat1.ent:1: parser error : Content error in the external subset
HTTP/1.1 200 OK
^
http://www.oasis-open.org/docbook/xml/4.1.2/ent/iso-lat1.ent:1: validity error : All markup of the conditional section is not in the same entity
HTTP/1.1 200 OK

Comment 3 Kalev Lember 2012-04-18 19:28:40 UTC
In your build, the XML validation step with xsltproc is failing for some reason. It is accessing the network to download the DTD files and is getting errors with the downloaded files.

My local build completes without warnings, but I suspect it's because I have docbook-style-xsl installed. In any case, it's not a problem with this package; I guess gnome-doc-utils-stylesheets should get a dep on docbook-style-xsl for local validation.

I did koji scratch builds for f17 and rawhide and xsltproc is only complaining about the lack of network access there and the builds are successful:
"ru/gnome-nettool.xml:8: warning: failed to load external entity /.../"

f17: http://koji.fedoraproject.org/koji/taskinfo?taskID=4002389
rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=4002392

I'll look into fixing gnome-doc-utils to add the missing dep, but this shouldn't block this package as the builds are fine in koji.

Comment 4 Kalev Lember 2012-04-18 20:23:53 UTC
Fixed the missing dep in gnome-doc-utils-0.20.10-2.fc17:
http://pkgs.fedoraproject.org/gitweb/?p=gnome-doc-utils.git;a=commitdiff;h=b1193134d

Comment 5 Richard Shaw 2012-04-18 20:28:38 UTC
Ok, no showstopper there. Can you comment on comment 1? (that sounds redundant :)

Comment 6 Kalev Lember 2012-04-18 20:46:38 UTC
(In reply to comment #1)
> I'll take this one.
> 
> Quick spec review:
> 1. %global is preferred over %define[1].

Fixed.


> 2. I've never seen desktop-file-validate put in %check. I think that's usually
> for "make test" type unit testing to kill builds that fail their tests even if
> they build. I usually put it in %install. 

There's a section in the packaging guidelines which explicitly says that it's fine to place it in either %check or %install. In any case, we want the build to fail if the validation fails, what's the whole point of using desktop-file-validate.

"one MUST run /.../ desktop-file-validate (in %check or %install)"

http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage


* Wed Apr 18 2012 Kalev Lember <kalevlember> - 3.2.0-2
- Use %%global instead of %%define (#812674)

Spec URL: http://kalev.fedorapeople.org/gnome-nettool.spec
SRPM URL: http://kalev.fedorapeople.org/gnome-nettool-3.2.0-2.fc17.src.rpm

Scratch build for rawhide with new gnome-doc-utils in buildroot:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4003063

gnome-doc-utils update if you want to try and give karma:
https://admin.fedoraproject.org/updates/gnome-doc-utils-0.20.10-2.fc17

Comment 7 Richard Shaw 2012-04-18 20:51:05 UTC
(In reply to comment #6)
> > 2. I've never seen desktop-file-validate put in %check. I think that's usually
> > for "make test" type unit testing to kill builds that fail their tests even if
> > they build. I usually put it in %install. 
> 
> There's a section in the packaging guidelines which explicitly says that it's
> fine to place it in either %check or %install. In any case, we want the build
> to fail if the validation fails, what's the whole point of using
> desktop-file-validate.
> 
> "one MUST run /.../ desktop-file-validate (in %check or %install)"

Hmmm... never noticed that before! 

 
> Scratch build for rawhide with new gnome-doc-utils in buildroot:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=4003063
> 
> gnome-doc-utils update if you want to try and give karma:
> https://admin.fedoraproject.org/updates/gnome-doc-utils-0.20.10-2.fc17

I'll try it in an F17 mock build (since I was using rawhide).

Comment 8 Richard Shaw 2012-04-18 20:52:08 UTC
Never mind :) The SRPM was fc17, the build was 18.

Comment 9 Richard Shaw 2012-04-18 21:11:08 UTC
Ok, a couple of problems:

3. Have you reported the bad FSF addresses upstream?

gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/info.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/info.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/util-mii.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/main.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/traceroute.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/utils.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/scan.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/lookup.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/finger.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/netstat.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/finger.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/lookup.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/netstat.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/ping.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/whois.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/ping.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/callbacks.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/nettool.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/whois.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/nettool.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/gn-combo-history.h
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/traceroute.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/scan.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/gn-combo-history.c
gnome-nettool-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/gnome-nettool-3.2.0/src/utils.c

4. I can't install the packages in a F17 chroot because the programs are in /bin still, not /usr/bin... Perhaps you need an %if conditionals for the requires for usrmove?

Comment 10 Kalev Lember 2012-04-18 21:24:54 UTC
(In reply to comment #9)
> Ok, a couple of problems:
> 
> 3. Have you reported the bad FSF addresses upstream?

No, I don't want to report such bugs without providing a patch and I don't have the energy right now to do one for gnome-nettool.


> 4. I can't install the packages in a F17 chroot because the programs are in
> /bin still, not /usr/bin... Perhaps you need an %if conditionals for the
> requires for usrmove?

You probably have an old mock cache or something. All these paths resolve fine:
$ repoquery -q --whatprovides /usr/bin/dig /usr/bin/netstat /usr/bin/nmap /usr/bin/ping /usr/bin/ping6 /usr/bin/pinky /usr/bin/traceroute /usr/bin/traceroute6 /usr/bin/whois
bind-utils-32:9.9.0-1.fc17.x86_64
bind-utils-32:9.9.0-1.fc17.x86_64
nmap-2:5.51-3.fc17.x86_64
iputils-0:20101006-14.fc17.x86_64
iputils-0:20101006-14.fc17.x86_64
coreutils-0:8.15-6.fc17.x86_64
jwhois-0:4.0-29.fc17.x86_64
whois-0:5.0.15-1.fc17.x86_64
whois-0:5.0.15-1.fc17.x86_64

Comment 11 Richard Shaw 2012-04-19 14:19:53 UTC
Weird... I did a mock --scrub=all but still didn't work for me. I'm not going to worry about holding up the review for it.

Comment 12 Richard Shaw 2012-04-19 14:32:40 UTC
Ok, I have one minor optional nit-pick. It's kind of stupid but the freedesktop spec indirectly requires at least a 48x48 icon file even if you have an SVG. This package only provides up to a 32x32. You could ask upstream or use imagemagick convert to create one of the SVG. I've done this on the fly in the spec file for previous packages but then you have the extra BRs of ImageMagick. 

+: OK
-: must be fixed
=: should be fixed (at your discretion)
?: Question or clairification needed
N: not applicable

MUST:
[+] rpmlint output: shown in comment.
[+] follows package naming guidelines
[+] spec file base name matches package name
[+] package meets the packaging guidelines
[+] package uses a Fedora approved license: GPLv2+
[+] license field matches the actual license.
[+] license file is included in %doc: COPYING
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum matches (18b97581aee0d297d9ec5df60b550dc6)
[+] package builds on at least one primary arch: Tested F16/17 x86_64
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[+] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[+] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[+] package contains a desktop file, uses desktop-file-install/validate
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[+] query upstream for license text
[N] description and summary contains available translations
[+] package builds in mock
[+] package builds on all supported arches: Tested x86_64
[?] package functions as described: Not tested
[+] sane scriptlets
[N] subpackages require the main package
[N] placement of pkgconfig files
[+] file dependencies versus package dependencies
[N] package contains man pages for binaries/scripts

*** APPROVED ***

Comment 13 Kalev Lember 2012-04-19 16:44:23 UTC
Thanks Richard!

I'll talk to upstream about the icons, good catch.

Comment 14 Kalev Lember 2012-04-19 16:45:49 UTC
New Package SCM Request
=======================
Package Name: gnome-nettool
Short Description: Graphical front-ends to various networking command-line tools
Owners: kalev
Branches: f17
InitialCC:

Comment 15 Gwyn Ciesla 2012-04-19 17:14:54 UTC
Unretired devel and f15, please take ownership in pkgdb, then file a Package
Change request for f16 and f17.

Comment 16 Kalev Lember 2012-04-19 17:34:52 UTC
Package Change Request
======================
Package Name: gnome-nettool
New Branches: f17
Owners: kalev

Thanks Jon. I'm not interested in the f15 and f16 branches, but took ownership for devel and here's the change request to create the f17 branch.

Comment 17 Gwyn Ciesla 2012-04-19 17:53:06 UTC
Git done (by process-git-requests).

Comment 18 Fedora Update System 2012-04-20 05:17:41 UTC
gnome-nettool-3.2.0-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/gnome-nettool-3.2.0-2.fc17

Comment 19 Kalev Lember 2012-04-20 16:50:45 UTC
(In reply to comment #9)
> 4. I can't install the packages in a F17 chroot because the programs are in
> /bin still, not /usr/bin... Perhaps you need an %if conditionals for the
> requires for usrmove?

Turns out you were right here all along.

Apparently in addition to working with remote repos, repoquery also uses local rpm database for depsolving, sometimes (!). And the local rpm database somehow handles the /bin -> /usr/bin symlink, so that it can resolve symlinked directories in already installed packages.

$ rpm -qf /bin/netstat
net-tools-1.60-134.20120127git.fc17.x86_64
$ rpm -qf /usr/bin/netstat
net-tools-1.60-134.20120127git.fc17.x86_64

Watch the inconsistency here, can't find provides for '/usr/bin/netstat' when it's the only thing I'm quering:

$ repoquery -q --whatprovides /usr/bin/netstat
$ repoquery -q --whatprovides /bin/netstat
net-tools-0:1.60-134.20120127git.fc17.x86_64

... but works when '/usr/bin/dig' is also specified on the command line:

$ repoquery -q --whatprovides /usr/bin/dig /usr/bin/netstat 
bind-utils-32:9.9.0-1.fc17.x86_64
bind-utils-32:9.9.0-1.fc17.x86_64

Fixed in http://pkgs.fedoraproject.org/gitweb/?p=gnome-nettool.git;a=commitdiff;h=661fcbdf

Comment 20 Fedora Update System 2012-04-21 21:03:42 UTC
Package gnome-nettool-3.2.0-3.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing gnome-nettool-3.2.0-3.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-6322/gnome-nettool-3.2.0-3.fc17
then log in and leave karma (feedback).

Comment 21 Fedora Update System 2012-05-02 04:44:20 UTC
gnome-nettool-3.2.0-3.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.