Bugzilla (bugzilla.redhat.com) will be under maintenance for infrastructure upgrades and will not be available on July 31st between 12:30 AM - 05:30 AM UTC. We appreciate your understanding and patience. You can follow status.redhat.com for details.
Bug 879365 - Review Request: system-config-network - network administration tool
Summary: Review Request: system-config-network - network administration tool
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bill Nottingham
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-11-22 17:08 UTC by Nils Philippsen
Modified: 2014-03-17 03:32 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-01-09 16:16:54 UTC
Type: ---
notting: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Nils Philippsen 2012-11-22 17:08:06 UTC
Spec URL: https://pkgs.fedoraproject.org/cgit/system-config-network.git/plain/system-config-network.spec?h=f17
SRPM URL: http://mirror2.hs-esslingen.de/fedora/linux/updates/17/SRPMS/system-config-network-1.6.5-1.fc17.src.rpm
Description: This is the network configuration tool,
supporting Ethernet, Wireless, TokenRing, ADSL, ISDN and PPP.
Fedora Account System Username: nphilipp

NB: This is to resurrect the TUI side of system-config-network and the spec file above is as it was previously. I plan to rip out the GUI side or otherwise disable it, and merge the -tui subpackage back into the main package.

Comment 1 Bill Nottingham 2012-11-27 16:10:53 UTC
MUST items:
- Package meets naming and packaging guidelines - OK
- Spec file matches base package name. - OK
- Spec has consistant macro usage. - OK
- Meets Packaging Guidelines. - OK
- License - GPLv2+, via code
- License field in spec matches - OK
- License file included in package  - OK
- Spec in American English - OK
- Spec is legible. - OK
- Sources match upstream md5sum: 
0751d4bd7e59c32626ba92fb3c3ff13d9207792b555b2720c61f35eda048ddb5  system-config-network-1.6.5.tar.bz2

? Can't find upstream.

- Package needs ExcludeArch - N/A
- BuildRequires correct - ***

glibc-devel and gcc doesn't need to be explicitly listed
In fact, this doesn't compile anything, so they're possibly wrong.
The version on the python req can likely be removed

- Spec handles locales/find_lang - OK
- Package is code or permissible content. - OK
- Doc subpackage needed/used. - N/A
- Packages %doc files don't affect runtime. - ***

What happens if the help isn't installed? Perhaps it shouldn't be %doc.

- Headers/static libs in -devel subpackage. - N/A

- Package is a GUI app and has a .desktop file - OK... until it's removed.

- Package compiles and builds on at least one arch.  - OK (tested x86_64)
- Package has no duplicate files in %files. - OK
- Package doesn't own any directories other packages own. - OK
- Package owns all the directories it creates. - OK
- No rpmlint output.

$ rpmlint system-config-network-tui-1.6.5-1.fc18.noarch.rpm 
system-config-network-tui.noarch: E: explicit-lib-dependency python-iwlib

OK.

system-config-network-tui.noarch: W: self-obsoletion redhat-config-network-tui <= 1.6.5 obsoletes redhat-config-network-tui = 1.6.5

Weird. Shouldn't it be <, not <= ?

system-config-network-tui.noarch: W: no-manual-page-for-binary system-config-network-cmd
system-config-network-tui.noarch: W: no-manual-page-for-binary system-config-network
system-config-network-tui.noarch: W: no-manual-page-for-binary system-config-network-tui
1 packages and 0 specfiles checked; 1 errors, 4 warnings.

Warnings, OK.

$ rpmlint system-config-network-1.6.5-1.fc18.noarch.rpm 
system-config-network.noarch: W: obsolete-not-provided rp3

Hah. Should just remove this.

system-config-network.noarch: W: self-obsoletion redhat-config-network <= 1.6.5 obsoletes redhat-config-network = 1.6.5

See above.

system-config-network.noarch: W: no-manual-page-for-binary system-control-network
system-config-network.noarch: W: no-manual-page-for-binary system-config-network-gui

OK.

- final provides and requires are sane: ***

Requires:

initscripts >= 0:5.99

I think we don't need this version any more.

python

Given we have /usr/bin/python in requires (and rpm-python, and dbus-python, and ...), this is superfluous.

Provides:

internet-config = 0.40-2.1
isdn-config = 0.18-10.70.1
netcfg = 2.36-3p.1
netconf = 0.1-1.1
netconfig = 0.8.24-1.2.2.1.1

These are all *ancient*. Are they still required to be provided?

SHOULD Items:

- Should build in mock. - OK (tested x86_64)
- Should build on all supported archs - didn't check
- Should function as described.  - didn't check
- Should have sane scriptlets. - OK (none)
- Should have subpackages require base package with fully versioned depend. - OK
- Should have dist tag - OK
- Should package latest version -OK, I guess

Suggestions:
1) Don't mark the help as %doc
2) Change the obsoletes from <= to <
3) Remove assorted obsoletes provides/requires

Comment 2 Nils Philippsen 2012-12-13 15:47:52 UTC
Sorry for the late reply, but my attempts to get rid of autofoo weren't too fruitful, in the end I scrapped that approach and changed the spec file to only build the GUI optionally and disabled that for F-18 and later.

(In reply to comment #1)
> MUST items:
> - Package meets naming and packaging guidelines - OK
> - Spec file matches base package name. - OK
> - Spec has consistant macro usage. - OK
> - Meets Packaging Guidelines. - OK
> - License - GPLv2+, via code
> - License field in spec matches - OK
> - License file included in package  - OK
> - Spec in American English - OK
> - Spec is legible. - OK
> - Sources match upstream md5sum: 
> 0751d4bd7e59c32626ba92fb3c3ff13d9207792b555b2720c61f35eda048ddb5 
> system-config-network-1.6.5.tar.bz2
> 
> ? Can't find upstream.

I've uploaded the new 1.6.6 tarball to fedorahosted and put the complete source URL into the spec file.

> - Package needs ExcludeArch - N/A
> - BuildRequires correct - ***
> 
> glibc-devel and gcc doesn't need to be explicitly listed
> In fact, this doesn't compile anything, so they're possibly wrong.

Likely. Not even Harald could tell my why they were in. Removed.

> The version on the python req can likely be removed

Ditto.

> - Spec handles locales/find_lang - OK
> - Package is code or permissible content. - OK
> - Doc subpackage needed/used. - N/A
> - Packages %doc files don't affect runtime. - ***
> 
> What happens if the help isn't installed? Perhaps it shouldn't be %doc.

They're only used in the GUI, i.e. removed %doc and moved to GUI-only parts which are disabled.

> - Headers/static libs in -devel subpackage. - N/A
> 
> - Package is a GUI app and has a .desktop file - OK... until it's removed.

Yes :-). Disabled/removed.

> - Package compiles and builds on at least one arch.  - OK (tested x86_64)
> - Package has no duplicate files in %files. - OK
> - Package doesn't own any directories other packages own. - OK
> - Package owns all the directories it creates. - OK
> - No rpmlint output.
> 
> $ rpmlint system-config-network-tui-1.6.5-1.fc18.noarch.rpm 
> system-config-network-tui.noarch: E: explicit-lib-dependency python-iwlib
> 
> OK.
> 
> system-config-network-tui.noarch: W: self-obsoletion
> redhat-config-network-tui <= 1.6.5 obsoletes redhat-config-network-tui =
> 1.6.5
> 
> Weird. Shouldn't it be <, not <= ?

I removed that ancient cruft.

> system-config-network-tui.noarch: W: no-manual-page-for-binary
> system-config-network-cmd
> system-config-network-tui.noarch: W: no-manual-page-for-binary
> system-config-network
> system-config-network-tui.noarch: W: no-manual-page-for-binary
> system-config-network-tui
> 1 packages and 0 specfiles checked; 1 errors, 4 warnings.
> 
> Warnings, OK.
> 
> $ rpmlint system-config-network-1.6.5-1.fc18.noarch.rpm 
> system-config-network.noarch: W: obsolete-not-provided rp3
> 
> Hah. Should just remove this.

Ditto.

> system-config-network.noarch: W: self-obsoletion redhat-config-network <=
> 1.6.5 obsoletes redhat-config-network = 1.6.5
> 
> See above.
> 
> system-config-network.noarch: W: no-manual-page-for-binary
> system-control-network
> system-config-network.noarch: W: no-manual-page-for-binary
> system-config-network-gui
> 
> OK.
> 
> - final provides and requires are sane: ***
> 
> Requires:
> 
> initscripts >= 0:5.99
> 
> I think we don't need this version any more.

Removed version.

> python
> 
> Given we have /usr/bin/python in requires (and rpm-python, and dbus-python,
> and ...), this is superfluous.

Yes.

> 
> Provides:
> 
> internet-config = 0.40-2.1
> isdn-config = 0.18-10.70.1
> netcfg = 2.36-3p.1
> netconf = 0.1-1.1
> netconfig = 0.8.24-1.2.2.1.1
> 
> These are all *ancient*. Are they still required to be provided?

I removed them along with the ancient obsoletes from above.

> SHOULD Items:
> 
> - Should build in mock. - OK (tested x86_64)
> - Should build on all supported archs - didn't check
> - Should function as described.  - didn't check
> - Should have sane scriptlets. - OK (none)
> - Should have subpackages require base package with fully versioned depend.
> - OK
> - Should have dist tag - OK
> - Should package latest version -OK, I guess
> 
> Suggestions:
> 1) Don't mark the help as %doc
> 2) Change the obsoletes from <= to <
> 3) Remove assorted obsoletes provides/requires

Find the changed spec file and SRPM (from a scratch build) at:
https://pkgs.fedoraproject.org/cgit/system-config-network.git/plain/system-config-network.spec?id=a583832940351fa56bc3b9e4b8c1440b451ac957
http://kojipkgs.fedoraproject.org//work/tasks/6656/4786656/system-config-network-1.6.6-1.fc19.src.rpm

Comment 3 Bill Nottingham 2012-12-13 21:06:31 UTC
That's a little goofy how you conditionalized the build, but it seems allowable. You should obsolete system-config-network-tui in the new package, though.

Comment 4 Nils Philippsen 2012-12-17 15:55:03 UTC
(In reply to comment #3)
> That's a little goofy how you conditionalized the build, but it seems
> allowable.

Heh, I can unroll the stuff if you like that better :-).

> You should obsolete system-config-network-tui in the new package, though.

Ugh, right. Here are the updated spec file and SRPM:

https://pkgs.fedoraproject.org/cgit/system-config-network.git/plain/system-config-network.spec?id=330c2d90ef7e4e2e2bd9d14f9f1fc377054a1736
http://kojipkgs.fedoraproject.org//work/tasks/6988/4796988/system-config-network-1.6.8-1.fc19.src.rpm

Comment 5 Bill Nottingham 2012-12-17 18:47:35 UTC
%description
This is %{?with_gui:the GUI of }the network configuration tool,
supporting Ethernet, Wireless, TokenRing, ADSL, ISDN and PPP.

%if %{with gui}
%package tui
Summary: Network Administration Tool TUI
Group: Applications/System
%endif
Requires: initscripts
Requires: usermode
Requires: rpm-python
Requires: newt-python
Requires: pciutils
Requires: usermode
Requires: dbus-python
Requires: python-ethtool
Requires: python-iwlib

This construct is causing the Requires: for the TUI to be added to the description, not the package - the conditional requires need to be in the main body of the spec.

Comment 6 Nils Philippsen 2012-12-18 10:32:11 UTC
(In reply to comment #5)
> This construct is causing the Requires: for the TUI to be added to the
> description, not the package - the conditional requires need to be in the
> main body of the spec.

Right. Here are spec file/SRPM with %package/%description sections in a working order:

https://pkgs.fedoraproject.org/cgit/system-config-network.git/plain/system-config-network.spec?id=77b0bce03d5044ae6f416bb0b003f09e8c58298e
http://kojipkgs.fedoraproject.org//work/tasks/9269/4799269/system-config-network-1.6.9-1.fc19.src.rpm

Comment 7 Bill Nottingham 2012-12-18 17:19:27 UTC
Looks good. Approved.

Comment 8 Nils Philippsen 2012-12-19 09:59:59 UTC
Package Change Request
======================
Package Name: system-config-network
New Branches: f18 devel
Owners: nphilipp

Bring (TUI parts of) system-config-network out of retirement for functionality not yet provided by nmcli.

Comment 9 Gwyn Ciesla 2012-12-19 12:20:30 UTC
Git done (by process-git-requests).


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