[Bug 513896] Review Request: pcp - performance monitoring and collection service

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=513896





--- Comment #6 from Eric Sandeen <esandeen at redhat.com>  2009-07-31 12:34:14 EDT ---
(In reply to comment #4)

Ok and some comments on some of the warnings & errors

> * pcp-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libpcp_pmda.so.3
> Nothing can really be done about this - the exit() context is well
> known and understood.

By the pcp folks I guess?  It's expected that the library exits, not the
application?

> * pcp.x86_64: E: arch-dependent-file-in-usr-share
> Several demo binaries are being installed in /usr/share/pcp/demo.
> I can move these elsewhere, if requested. These binaries are not
> used by the PCP core functionality.

Maybe don't build but just ship the c files?   As a hack, if you don't want to
change the upstream build, you could rm them post-%install.

FWIW I've seen other such things for example in db4-devel:

/usr/share/doc/db4-devel-4.7.25/examples_c/bench_001.c

> * pcp.x86_64: W: devel-file-in-non-devel-package
> Several headers and some 'C' files in PCP are actually configuration
> files. Discussed with the PCP community and they agree to leave these
> as-is.

Maybe a comment in the %files section would help the next reviewer :)

> * pcp.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/pcp
> Well, it actually isn't a config file - we want it unconditionally
> updated after an upgrade. Same for /etc/pcp.env

hah, well:

# rpmlint rpmlint
rpmlint.noarch: W: non-conffile-in-etc /etc/bash_completion.d/rpmlint

so I suppose it'll be hard to argue too much :)

But I think if you mark it as %config, it will still be unconditionally updated
on an upgrade, but if it's edited, the edited one will go to .rpmsave.

> * pcp.x86_64: W: log-files-without-logrotate /var/log/pcp
> PCP has it's own log management system, see pmlogger_daily(1)

Just to be a pain; is that necessary?  If logrotate is available, can it not be
used so as not to surprise the admin?  Or is there a requirement for a special
homegrown tool?

> * pcp.x86_64: W: dangerous-command-in-%post chmod
> * pcp.x86_64: W: dangerous-command-in-%preun rm
> This could be avoided with %ghost directive, if needed.

just out of curiosity are the chmod's actually needed?

Looking at the scriptlets, I have to say that the .rpmsave & .rpmnew
manipulation kinda bugs me.  Those are supposed to be left for the admin to
deal with.  If it really has to be there can you add a comment?

Also just thinking aloud; the scripts source /etc/pcp.env but that's not
expected  to change, right; would it make any more sense to drop that and just
use the same rpm path macros as you installed to?

IOW could you just do:

%preun
if [ "$1" -eq 0 ]
then
    #
    # Stop daemons before erasing the package
    #
    /sbin/service pcp stop >/dev/null 2>&1
    /sbin/service pmie stop >/dev/null 2>&1
    /sbin/service pmproxy stop >/dev/null 2>&1

    /sbin/chkconfig --del pcp >/dev/null 2>&1
    /sbin/chkconfig --del pmie >/dev/null 2>&1
    /sbin/chkconfig --del pmproxy >/dev/null 2>&1

    rm -f %{_datadir}/pcp/lib/.NeedRebuild
fi
exit 0

and similar for the creation of the .NeedRebuild stuff.  It just seems simpler
to me, but just a suggestion.

Also on the root.saved stuff - I can't find anything in scripts or source that
creates this.  I see root.prev though.

Thanks,
-Eric

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.