Bug 490438 (rhn-client-tools) - Review Request: rhn-client-tools - Support programs and libraries for Red Hat Network or Spacewalk
Summary: Review Request: rhn-client-tools - Support programs and libraries for Red Hat...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: rhn-client-tools
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nigel Jones
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F-Spacewalk spacewalk-koan rhncfg
TreeView+ depends on / blocked
 
Reported: 2009-03-16 13:06 UTC by Miroslav Suchý
Modified: 2009-09-23 13:12 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-23 13:12:55 UTC
Type: ---
Embargoed:
dev: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Miroslav Suchý 2009-03-16 13:06:13 UTC
SRPM: http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools-0.4.22-1.src.rpm
SPEC: http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools.spec
Description:
Red Hat Network Client Tools provides programs and libraries to allow your
system to receive software updates from Red Hat Network or Spacewalk.

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1243552

Rpmlint is silent on SRC.RPM, but produce 2 warnings on rpm, which I would like to discuss.

rpmlint rhn-client-tools-0.4.22-1.noarch.rpm
rhn-client-tools.noarch: E: incoherent-logrotate-file /etc/logrotate.d/up2date
This name of log file and it configuration file is used for long time. So I do not if the change will not confuse existing users.

rpmlint rhn-setup-gnome-0.4.22-1.noarch.rpm
rhn-setup-gnome.noarch: W: no-documentation
This package contains only libraries, which if presented, then rhn_check from package rhn-setup will run with nice GUI. Otherwise it will run in ncurses. I really do not know what to put there as documentation since documentation is included in rhn-setup package.

Comment 1 Nigel Jones 2009-03-16 13:18:02 UTC
I'll start reviewing this package.

Comment 2 Miroslav Suchý 2009-03-24 11:27:59 UTC
ping?

Comment 3 Nigel Jones 2009-03-24 12:02:09 UTC
Okay, sorry about taking so long for the first glance...

At the moment I see two issues against the current guidelines:

1. Missing .desktop files

I know for a fact that Red Hat Enterprise Linux 3-5 has a GUI for 'rhn_register' and based on what I'm seeing I'm fairly sure you haven't taken it away, so per https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files it really needs a .desktop file so users can find it in the menus.

2. Python module locations

I'm not sure how set in stone this is, but w/ our Python Guidelines, python modules are prefered to be in %{python_sitelib}/modulename/foo.* (https://fedoraproject.org/wiki/Packaging/Python#System_Architecture) would moving these libraries cause any issues with rhn_register etc?

As per our discussion on IRC, I also mentioned about the ownership of a couple of directories...

/etc/sysconfig/rhn/allowed-actions/
/etc/sysconfig/rhn/allowed-actions/script/

Should these be owned by rhn-client-tools or is there going to be another package that provides the rhncfg packages that check for files under these directories?

Comment 4 Miroslav Suchý 2009-03-30 12:27:03 UTC
Updated SPEC: http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools.spec
SRPM: http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools-0.4.23-1.src.rpm

I added .desktop file (it is my first .desktop, hope I done it correctly).
I added allowed-actions ownership.

And about the python location... I asked and got these response:
Well as the guidelines say, we have our python *modules* (eg: rhnlib) in python_sitelib. I don't really consider our server/client code to be a utility module that belongs to sitelib and as far as I see its placed right in /usr/share/. Of course its before my time this decision was made, but I think its the right one. We should not have code in sitelib unless its a module that could be used generically. Anyway, thats my take. All they have to do is add that path to their PYTHONPATH. I don't think python guidelines enforce all the code to be in python-sitelibs anywhere. 
 -- Prad

Comment 6 Michael Stahnke 2009-05-05 01:27:15 UTC
I agree with Prad's comments on Python locations.  You really wouldn't reuse this code, so it belongs in /usr/share as an application not a development tool.

Builds in mock for EL5.  I am currently unable to check rawhide due current technical difficulties on my side :)

I am in favor of changing the name of the log file and logrotate file.  I realize legacy systems are using up2date for the log name, but since that tool has been phased out, I think the naming of the log file should be also; however this is a package review, and not a developer/design review....


The package looks good to me, but I'd like Nigel to look at it once more.

Comment 7 Nigel Jones 2009-05-14 12:07:50 UTC
Okay, couple of things here that I've just noticed:

 * Icon Cache: https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
   - Without the icon cache no Icon appears in the menus (ouch)
 * .desktop file
   - the menu entry lands on my F10 box in System->Preferences with the name 'rhn_register' - this isn't very descriptive imo
   - I'd also say it's more Administration not Preferences

A .desktop file that solves this would be something like:

----
[Desktop Entry]
Encoding=UTF-8
Name=Update Registration
GenericName=Register for software updates from Spacewalk/Satellite/Red Hat Network
Comment=Register to Spacewalk/Satellite/Red Hat Network.
Exec=rhn_register
Icon=up2date
Terminal=false
Type=Application
Categories=System;Settings;X-Red-Hat-Base;
----

This makes it appear as "Update Registration" to be honest you may want to call this "RHN Registration".

 * Broken deps
   - rhn-client-tools depends on yum-rhn-plugin which will apparently be a circular dependency when it gets put forward to review (this is bad)
   - rhn-setup depends on rhnsd which I can't seem to find

Comment 8 Miroslav Suchý 2009-09-01 12:39:28 UTC
Sorry for long delays, been quiet busy...
http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools.spec
http://miroslav.suchy.cz/fedora/rhn-client-tools/rhn-client-tools-0.7.1-1.src.rpm

I changed .desktop according your suggestion.
I move dependecy on yum-rhn-plugin to rhn-check, where it should be.

But I'm not sure about rhn-setup and rhnsd.
rhn-setup is subpackage of rhn-client-tools, rhnsd is standalone package whose review will follow shortly. rhnsd require rhn-check (again subpackage of rhn-client-tools). So I can not submit review till rhn-client-tools are accepted. I'm not sure how to fix this. And I would like to avoiding to merging those two packages.

Comment 9 Nigel Jones 2009-09-21 07:30:00 UTC
Hi Miroslav,

Everything looks good now, I'm going to give this the big fedora‑review+ and approve the package.

While nothing is blocking you at this stage from requesting CVS/Submitting a Build I will mention that because of the dependencies between SRPMs that you may want to wait before building until they are approved so you don't get broken dependency e-mails everywhere.

Comment 10 Miroslav Suchý 2009-09-21 07:41:06 UTC
New Package CVS Request
=======================
Package Name: rhn-client-tools
Short Description: Support programs and libraries for Red Hat Network or Spacewalk
Owners: msuchy
Branches: F-11, F-12
InitialCC:

Comment 11 Jason Tibbitts 2009-09-22 01:47:12 UTC
CVS done.


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