Bug 433678 - Review Request: ricci - cluster and systems management agent
Review Request: ricci - cluster and systems management agent
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ryan McCabe
Fedora Extras Quality Assurance
: Reopened
Depends On: 433679
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-20 14:57 EST by Ryan McCabe
Modified: 2012-02-15 18:18 EST (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-02-15 18:18:46 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
andreas: fedora‑review-
kevin: fedora‑cvs+


Attachments (Terms of Use)
Patch to bring ricci specfile into line with Fedora requirements. (471 bytes, patch)
2008-03-26 16:56 EDT, Chris Feist
no flags Details | Diff

  None (edit)
Description Ryan McCabe 2008-02-20 14:57:05 EST
Spec URL: http://people.redhat.com/rmccabe/conga/ricci.spec
SRPM URL: http://people.redhat.com/rmccabe/conga/ricci-0.13.0-1.src.rpm
Description: ricci is the agent that runs on locally on systems managed by conga.

Conga is in RHEL4 and RHEL5, and more information about the project is at http://sourceware.org/cluster/conga/
Comment 1 Chris Feist 2008-03-26 15:20:39 EDT
Taking ownership to work on review.
Comment 2 Chris Feist 2008-03-26 16:54:40 EDT
rpmlint results:

ricci.i386: E: non-standard-uid /var/lib/ricci/certs/cacert.config ricci
ricci.i386: E: non-standard-gid /var/lib/ricci/certs/cacert.config ricci
ricci.i386: E: non-standard-uid /var/lib/ricci/certs ricci
ricci.i386: E: non-standard-gid /var/lib/ricci/certs ricci
ricci.i386: E: non-standard-gid /usr/libexec/ricci ricci
ricci.i386: E: non-standard-uid /var/lib/ricci/queue ricci
ricci.i386: E: non-standard-gid /var/lib/ricci/queue ricci
ricci.i386: E: non-standard-uid /var/lib/ricci/certs/clients ricci
ricci.i386: E: non-standard-gid /var/lib/ricci/certs/clients ricci
ricci.i386: E: non-standard-uid /var/lib/ricci ricci
ricci.i386: E: non-standard-gid /var/lib/ricci ricci
ricci.i386: E: non-standard-dir-perm /var/lib/ricci 0700
ricci.i386: E: non-standard-gid /usr/libexec/ricci/ricci-worker ricci
ricci.i386: E: explicit-lib-dependency libcap
ricci.i386: E: explicit-lib-dependency libxml2

The non-standard uid/gid errors are ok because a ricci user is created for the
daemon to run under.

The non-standard-dir-perm error is ok because only the ricci user should have
access to that directory.

I've fixed the explicit-lib-dependency errors in my patch that I'm attaching (as
well as a minor fix to the BuildRoot location.


Comment 3 Chris Feist 2008-03-26 16:56:12 EDT
Created attachment 299231 [details]
Patch to bring ricci specfile into line with Fedora requirements.
Comment 4 Chris Feist 2008-03-26 16:56:58 EDT
Ryan, if you make those changes and then post the locations of the fixed .spec &
src.rpm I can approve this package.
Comment 5 Ryan McCabe 2008-03-27 21:22:01 EDT
Thanks. Made those changes and posted an updated src.rpm and spec file.

Spec URL: http://people.redhat.com/rmccabe/conga/ricci.spec
SRPM URL: http://people.redhat.com/rmccabe/conga/ricci-0.13.0-2.src.rpm
Comment 6 Chris Feist 2008-03-28 14:46:05 EDT
Looks good, package is ready to go.
Comment 7 Ryan McCabe 2008-04-04 12:29:41 EDT
It's still blocked by bug #433679 (clustermon review), as ricci depends on
clustermon.
Comment 8 Ryan McCabe 2008-05-19 12:21:51 EDT
New Package CVS Request
=======================
Package Name: ricci
Short Description: Cluster and systems management agent
Owners: rmccabe
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: Yes
Comment 9 Kevin Fenzi 2008-05-19 14:51:56 EDT
Humm... why do you HUP/kill dbus in post/postun?
Thats very much a no no. dbus can't be killed without messing up the system and
requiring a reboot. 
Comment 10 Ryan McCabe 2008-05-19 16:19:57 EDT
SIGHUP doesn't kill dbus. 

from the man page:

       SIGHUP will cause the D-Bus daemon to PARTIALLY reload  its  configura-
       tion file and to flush its user/group information caches. Some configu-
       ration changes would require kicking all apps off the bus; so they will
       only  take effect if you restart the daemon. Policy changes should take
       effect with SIGHUP.


ricci installs new dbus (and oddjob) policy, so it needs dbus to reread config
files. the reload action in the dbus init script does nothing, and restarting
just does a stop and start, which is undesirable.
Comment 11 Kevin Fenzi 2008-05-19 22:27:59 EDT
ah, quite right. I guess I misread that... sorry about that. ;( 

cvs done.
Comment 12 Kevin Fenzi 2009-01-24 23:18:42 EST
This package has been in for ages now, closing this review.
Comment 13 Andreas Thienemann 2009-01-28 18:28:32 EST
Not to rain on the parade here, but this bug was just mentioned in a discussion about package quality.
This package review is basically a joke.

Just posting rpmlint results is not enough: The review guidelines clearly define several items of which only one is mentioned in the review.
A short look at the spec file makes me wonder if anything else then a cursory rpmlint and mock run was attempted.

While it might be debateable if the .spec file is actually legible but the missing source url is a clear blocker:

"MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the  Source URL Guidelines for how to deal with this."

What about the duplicate Summary line?
What about duplicate Requires? What about unneeded Requires?
Why is the main %description tag talking about Conga and a second %description tag with a name override of "ricci" (identical to the %name tag) talking about the real program?

What happened to the %files section? Why is the directory %{_docdir}/ricci-%{version}/ included as regular files instead of being marked as docs? Why isn't the %doc macro used?

After taking a closer look at the .spec file I have to revise my initial verdict: This is not only a glaring example of a major failure of our review process, the entire .spec file is made out of fail.
Comment 14 Robert Scheck 2009-01-28 18:34:49 EST
Re-opening because of fedora-review-.
Comment 15 Kevin Kofler 2009-01-28 19:12:04 EST
Isn't stuff in %{_docdir} automatically marked %doc by RPM? The other complaints do sound valid though.
Comment 16 Mamoru TASAKA 2009-01-29 04:03:38 EST
(In reply to comment #15)
> Isn't stuff in %{_docdir} automatically marked %doc by RPM? 

This is correct:
http://www.redhat.com/archives/fedora-maintainers/2005-May/msg00118.html
Currently there are more directories under which all files/directories
are automatically marked as %doc

Note that I have not checked the spec file of this package at all.
Comment 17 Chris Feist 2009-06-18 14:11:19 EDT
Pulling myself of the assigned list, it appears as though this can be closed as this package is in fc11.
Comment 18 Kevin Fenzi 2009-06-18 14:27:52 EDT
Chris: Can you address the items in comment #13? We should not close this while there are still outstanding issues to address. It doesn't matter if the package is "already in", it needs to meet guidelines...
Comment 19 Chris Feist 2009-06-18 14:32:54 EDT
I'm re-assigning this to rmccabe to fix the issues in comment #13.  I believe he does all the package builds and can fix these issues.
Comment 20 Ryan McCabe 2009-06-22 10:37:46 EDT
Fixed a while ago.
Comment 21 Kevin Fenzi 2009-06-22 12:00:25 EDT
ok, I guess we should ask Andreas to recheck things? 

Andreas?
Comment 22 Chris Feist 2012-02-15 18:18:46 EST
Closing since ricci has been removed as of F17.

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