Bug 490438 - (rhn-client-tools) Review Request: rhn-client-tools - Support programs and libraries for Red Hat Network or Spacewalk
Review Request: rhn-client-tools - Support programs and libraries for Red Hat...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nigel Jones
Fedora Extras Quality Assurance
:
Depends On:
Blocks: F-Spacewalk spacewalk-koan rhncfg
  Show dependency treegraph
 
Reported: 2009-03-16 09:06 EDT by Miroslav Suchý
Modified: 2009-09-23 09:12 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-23 09:12:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dev: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Miroslav Suchý 2009-03-16 09:06:13 EDT
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 09:18:02 EDT
I'll start reviewing this package.
Comment 2 Miroslav Suchý 2009-03-24 07:27:59 EDT
ping?
Comment 3 Nigel Jones 2009-03-24 08:02:09 EDT
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 08:27:03 EDT
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-04 21:27:15 EDT
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 08:07:50 EDT
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 08:39:28 EDT
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 03:30:00 EDT
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 03:41:06 EDT
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-21 21:47:12 EDT
CVS done.

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