Bug 500437 - Review Request: cnetworkmanager - Command-line client for NetworkManager
Summary: Review Request: cnetworkmanager - Command-line client for NetworkManager
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jochen Schmitt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-12 17:16 UTC by Kevin Fenzi
Modified: 2009-06-16 02:46 UTC (History)
7 users (show)

Fixed In Version: 0.8.4-2.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-06-03 21:08:26 UTC
jochen: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Kevin Fenzi 2009-05-12 17:16:01 UTC
Spec URL: http://www.scrye.com/~kevin/fedora/cnetworkmanager/cnetworkmanager.spec
SRPM URL: http://www.scrye.com/~kevin/fedora/cnetworkmanager/cnetworkmanager-0.8.4-1.fc11.src.rpm
Description: 

Cnetworkmanager is a command-line client for NetworkManager, intended
to supplement or replace the GUI applets.

rpmlint says: 

cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager.conf
cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager-06.conf

Which I think can both be ignored.

Comment 1 Kevin Fenzi 2009-05-12 17:30:05 UTC
Scratch build at: http://koji.fedoraproject.org/koji/taskinfo?taskID=1350918

Comment 2 Jochen Schmitt 2009-05-12 18:05:42 UTC
Good:
+ Basename of SPEC files matches with package name
+ Package name fullfill naming guildelines
+ Package contains no subpackages
+ Package contains valid license tag
+ License tag has GPLv2+ as an valid OSS license
+ Package contains verbain copy of the license text
+ URL tag contains URL of project home page
+ Could download source tar ball from upstream with spectool -g
+ Tar ball matches with upstream source
(md5sum: 8f3eccaeff900cd68e19785f13c813ab)
+ Local build works fine
+ Koji scratch build works fine
+ Rpmlint is quite on source rpm
+ %doc stanza is small, so we need no extra subpackage
+ Files has proper file permissions
+ %files stanza doesn't contains duplicate entries
+ All package files are owned by this package
+ No packaged files belong to another package
+ Package contains proper %Changelog


Bad:
- Please replace:


%build
make PREFIX=/usr

%install
rm -rf $RPM_BUILD_ROOT
make install DESTDIR=$RPM_BUILD_ROOT PREFIX=/usr sysconfdir=/etc

by:

%build
make PREFIX=%{_prefix}

%install
rm -rf $RPM_BUILD_ROOT
make install DESTDIR=$RPM_BUILD_ROOT PREFIX=%{_prefix} sysconfdir=%{_sysconfdir}

- No smp enabled build

- Please replace

%{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
%{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf

%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf

- Package contains a source file with has a public domain copyright note
  in the header. License tag should be 'GPLv2+ and Public Domain'

- Rpmlint has warning on binary rpm:

rpmlint cnetworkmanager-0.8.4-1.fc10.noarch.rpm
cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager.conf
cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager-06.conf

- Package python-gobject2 doesn't exist in Fedora.

If you want to introduced this missing package in Fedora, please set a blocker bug to this review.

Comment 3 Kevin Fenzi 2009-05-12 18:48:11 UTC
Thats for the review!

>Bad:
>- Please replace:
>
>
>%build
>make PREFIX=/usr
>
>%install
>rm -rf $RPM_BUILD_ROOT
>make install DESTDIR=$RPM_BUILD_ROOT PREFIX=/usr sysconfdir=/etc
>
>by:
>
>%build
>make PREFIX=%{_prefix}
>
>%install
>rm -rf $RPM_BUILD_ROOT
>make install DESTDIR=$RPM_BUILD_ROOT PREFIX=%{_prefix}
>sysconfdir=%{_sysconfdir}

Done. 

>- No smp enabled build

The build step is a single sed call. 
I will add smp if you insist, but it really makes no sense at all. 

>- Please replace
>
>%{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
>%{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf
>
>%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
>%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf

I don't think these are supposed to be config files. 
See gdm, NetworkManager, ConsoleKit, etc. None of them mark them as config. 
Users should not be editing these. It's just a bug that they are under /etc/ I guess. 

>- Package contains a source file with has a public domain copyright note
>  in the header. License tag should be 'GPLv2+ and Public Domain'

Ah, good point. Fixed. 

>
>- Rpmlint has warning on binary rpm:
>
>rpmlint cnetworkmanager-0.8.4-1.fc10.noarch.rpm
>cnetworkmanager.noarch: W: non-conffile-in-etc
>/etc/dbus-1/system.d/cnetworkmanager.conf
>cnetworkmanager.noarch: W: non-conffile-in-etc
>/etc/dbus-1/system.d/cnetworkmanager-06.conf

Which I think can be ignored. 

>- Package python-gobject2 doesn't exist in Fedora.
>
>If you want to introduced this missing package in Fedora, please set a blocker
>bug to this review.

Oops. That should be pygobject2. Fixed. 

Spec URL: http://www.scrye.com/~kevin/fedora/cnetworkmanager/cnetworkmanager.spec
SRPM URL: http://www.scrye.com/~kevin/fedora/cnetworkmanager/cnetworkmanager-0.8.4-2.fc11.src.rpm

Comment 4 Jochen Schmitt 2009-05-12 18:57:25 UTC
(In reply to comment #3)

> The build step is a single sed call. 
> I will add smp if you insist, but it really makes no sense at all. 
> 
> >- Please replace
> >
> >%{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
> >%{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf
> >
> >%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager.conf
> >%config(noreplace) %{_sysconfdir}/dbus-1/system.d/cnetworkmanager-06.conf
> 
> I don't think these are supposed to be config files. 
> See gdm, NetworkManager, ConsoleKit, etc. None of them mark them as config. 
> Users should not be editing these. It's just a bug that they are under /etc/ I
> guess. 

I have done a look ok gdm and have to disagree with you. On gdm you see 
%config in fromt of the files which an in %{_sysconfigdir}

Comment 5 Kevin Fenzi 2009-05-12 19:19:24 UTC
Odd. I was told by the gdm maintainer that they were not config files. ;) 

I only looked at ConsoleKit and gdm. 

I guess we can run this by the packaging folks and see what they suggest.

Comment 6 Jochen Schmitt 2009-05-12 19:39:44 UTC
(In reply to comment #5)
> Odd. I was told by the gdm maintainer that they were not config files. ;) 

I have look in the devel branch of gdm and shows, that the files in %{_sysconfdir}
was prefixed with %config.

Comment 7 Bastien Nocera 2009-05-13 11:50:06 UTC
dbus ".conf" files aren't config files. The only thing you'll manage to do is break the setup. Don't mark those as config files.

Comment 8 Jochen Schmitt 2009-05-13 18:19:02 UTC
Bood:
+ License tag has proper value GPLv2+ and Public Domain
+ Build on Koji works fine
* Local build works fine
+ Rpmlint is quite for source rpm
+ Local install and uninstall works fine

Bad:
- Application crashed when they is started without arguments:
cnetworkmanager
cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
org.freedesktop.DBus.Error.ServiceUnknown: The name org.freedesktop.NetworkManager was not provided by any .service files
NetworkManager is not running

No Blocker:
- Rpmlint complints on binary rpm:
 rpmlint cnetworkmanager-0.8.4-2.fc10.noarch.rpm
cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager.conf
cnetworkmanager.noarch: W: non-conffile-in-etc /etc/dbus-1/system.d/cnetworkmanager-06.conf
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

After a discussion I will accept this

Comment 9 Kevin Fenzi 2009-05-13 21:02:12 UTC
>Bad:
>- Application crashed when they is started without arguments:
>cnetworkmanager
>cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
>org.freedesktop.DBus.Error.ServiceUnknown: The name
>org.freedesktop.NetworkManager was not provided by any .service files
>NetworkManager is not running

Do you have NetworkManager running? 
It works fine here, but I think it's expecting NetworkManager to be running. 

I can report that as a bug upstream.

Comment 10 Martin Vidner 2009-05-14 11:53:02 UTC
Hello from upstream :) I am glad to see my software packaged, thanks!

Re comment 8:
> Bad:
> - Application crashed when they is started without arguments:
> cnetworkmanager
> cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
> org.freedesktop.DBus.Error.ServiceUnknown: The name
> org.freedesktop.NetworkManager was not provided by any .service files
> NetworkManager is not running

If it had crashed you would have got a python backtrace. This is expected behavior. I admit that quoting the entire DBus error makes it less readable.

Comment 11 Jochen Schmitt 2009-05-14 15:03:41 UTC
Crash is properly not the right word, but I have got an error message which I can't understand:

$ cnetworkmanager
cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
org.freedesktop.DBus.Error.ServiceUnknown: The name org.freedesktop.NetworkManager was not provided by any .service files
NetworkManager is not running

Comment 12 Matthew Miller 2009-05-14 15:06:19 UTC
(In reply to comment #11)
> Crash is properly not the right word, but I have got an error message which I
> can't understand:
> 
> $ cnetworkmanager
> cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
> org.freedesktop.DBus.Error.ServiceUnknown: The name
> org.freedesktop.NetworkManager was not provided by any .service files
> NetworkManager is not running  

Ignore the middle two lines -- they're unnecessarily-verbose error output. Pretend it just says this:

$ cnetworkmanager
cnetworkmanager 0.8.4 - Command Line Interface for NetworkManager
Error: NetworkManager is not running  


This isn't a packaging problem -- it's an improvement that could be made upstream.

Comment 13 Kevin Fenzi 2009-05-17 16:42:10 UTC
Hey Jochen: What blockers for approval remain? Let me know and I will try and get them fixed up. Thanks again for reviewing.

Comment 14 Jochen Schmitt 2009-05-17 18:10:14 UTC
The blocker is the issue reported on comment #12

Comment 15 Milos Jakubicek 2009-05-17 18:38:47 UTC
Jochen, the problem described in comment #12 is really not related to packaging (I wouldn't even call it a bug, if you have a cli for NM and you don't run NM, it is obvious that you get an error "NM is not running":), please don't delay the review because of this -- thank you very much in advance...!

Comment 16 Jochen Schmitt 2009-05-17 18:43:13 UTC
OK, I will APPROVE your package. But in may be nice, if you can create a new release of the application which show a better error message in this case bacause the error message which I have got is not very helpful for an user.

Comment 17 Kevin Fenzi 2009-05-18 04:06:32 UTC
Thanks Jochen!

New Package CVS Request
=======================
Package Name: cnetworkmanager
Short Description: Command-line client for NetworkManager
Owners: kevin
Branches: devel F-11 F-10
InitialCC:

Comment 18 Kevin Fenzi 2009-05-18 04:26:13 UTC
cvs done.

Comment 19 Fedora Update System 2009-05-18 05:09:23 UTC
cnetworkmanager-0.8.4-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/cnetworkmanager-0.8.4-2.fc11

Comment 20 Fedora Update System 2009-05-18 05:18:40 UTC
cnetworkmanager-0.8.4-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/cnetworkmanager-0.8.4-2.fc10

Comment 21 Fedora Update System 2009-05-19 02:03:07 UTC
cnetworkmanager-0.8.4-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update cnetworkmanager'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-5090

Comment 22 Fedora Update System 2009-05-19 02:03:39 UTC
cnetworkmanager-0.8.4-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update cnetworkmanager'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-5093

Comment 23 Jochen Schmitt 2009-06-03 17:44:08 UTC
Please push the packages to stable and close this bug.

Comment 24 Kevin Fenzi 2009-06-03 21:08:26 UTC
Done.

Comment 25 Fedora Update System 2009-06-16 02:02:53 UTC
cnetworkmanager-0.8.4-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 26 Fedora Update System 2009-06-16 02:46:05 UTC
cnetworkmanager-0.8.4-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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