Bug 500437 - Review Request: cnetworkmanager - Command-line client for NetworkManager
Review Request: cnetworkmanager - Command-line client for NetworkManager
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-12 13:16 EDT by Kevin Fenzi
Modified: 2009-06-15 22:46 EDT (History)
7 users (show)

See Also:
Fixed In Version: 0.8.4-2.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-03 17:08:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jochen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Kevin Fenzi 2009-05-12 13:16:01 EDT
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 13:30:05 EDT
Scratch build at: http://koji.fedoraproject.org/koji/taskinfo?taskID=1350918
Comment 2 Jochen Schmitt 2009-05-12 14:05:42 EDT
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 14:48:11 EDT
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 14:57:25 EDT
(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 15:19:24 EDT
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 15:39:44 EDT
(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 07:50:06 EDT
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 14:19:02 EDT
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 17:02:12 EDT
>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 07:53:02 EDT
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 11:03:41 EDT
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 11:06:19 EDT
(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 12:42:10 EDT
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 14:10:14 EDT
The blocker is the issue reported on comment #12
Comment 15 Milos Jakubicek 2009-05-17 14:38:47 EDT
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 14:43:13 EDT
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 00:06:32 EDT
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 00:26:13 EDT
cvs done.
Comment 19 Fedora Update System 2009-05-18 01:09:23 EDT
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 01:18:40 EDT
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-18 22:03:07 EDT
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-18 22:03:39 EDT
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 13:44:08 EDT
Please push the packages to stable and close this bug.
Comment 24 Kevin Fenzi 2009-06-03 17:08:26 EDT
Done.
Comment 25 Fedora Update System 2009-06-15 22:02:53 EDT
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-15 22:46:05 EDT
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.