[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 #17 from Mark Goodwin <mgoodwin at redhat.com>  2009-08-21 01:50:29 EDT ---
Jarod, thanks - I think I've addressed most of your questions, apart
from the C source files, which I'll get to early next week if not sooner.
Comments below.

In Comment #15 from Jarod Wilson <jwilson at redhat.com> wrote:
> "GPLv2+" is only for "GPLv2.anything or later", if it doesn't say "or later",
> then simply "GPLv2" is the tag you want.

Thanks for the clarification, fixed.

> I think it might be cleanest/most obvious to users if the -devel package were
> renamed to pcp-libs-devel, since its really the devel headers for the
> libraries. Then we're even reasonably satisfying the guidelines, I'd argue.

done. consulted the PCP community and they agree, good suggestion.

> >> > > * SHOULD: Subpackages other than devel should require the base package
> >> > > using a fully versioned dependency.  NO, but it seems ok
> > > 
> > > Comment: if -libs and the base package require each other, then what's the
> > > point
> > > of splitting out -libs since they can never be installed separately?
> 
> Welcome to multiarch.  :)  Splitting the libs out allows you to have pcp.x86_64,
> pcp-libs.x86_64 and pcp-libs.i686 all installed at the same time without any
> file conflicts (in theory).

ah ok, that's a good reason then :)

> > > So based on the above, I'm leaving the run-time and build-time dependencies
> > > as they strictly need to be. If the final Fedora reviewer and/or sponsor insist
> > > on additional dependencies, then I'll conform, reluctantly  :) 
> 
> I don't think its necessary on this one, there's a reasonable reason not to.
> I presume this is the srpm I should be looking at now?
> ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-3.fc10.src.rpm

yes and I see you found it. The new version will be pcp-3.0.0-4.fc10 after the
changes in this update.

In Comment #16 from Jarod Wilson <jwilson at redhat.com> wrote:
> Okay, there's still some slightly alarming rpmlint spew... I understand the
> header files need to be in the main package, but what about all these c files?
> Particularly the ones under 'sample' and 'demo' sub-directories. These should

The /usr/share/pcp/demos directory can easily enough be moved to
/usr/share/doc/pcp-*/demos  so I'll do that (haven't in this update
though because I need to investigate the QA fall-out).

> ... go into somewhere like /usr/share/doc/pcp/{sample,demo}/, not into
> /usr/share/pcp or /var/lib/pcp.

I'll investigate simply not installing the source for the 'simple',
'trivial' and 'sample' agents at all. This source was shipped back
in the days when PCP was proprietary; nowdays of course it's all readily
available anyway :)

> The .NeedRebuild touch and rm in the scriptlets is also... Not pretty. Files
> created by an rpm need to be owned by the rpm, even if they're temporary files.
> You could add an empty file to the rpm %files list itself, so the rpm lays it
> down and owns it, and there's technically no harm in the file being deleted
> from the system after this rebuild takes place. 

OK, I've added .NeedRebuild as an install target in src/pmns/GNUmakefile
(since other platforms need it too, so just touching it during %install in
the spec would be sub-optimal), and added an entry in %files for it.

> If the file continues to exist,
> an rpm -e will remove it, no need for the rm in the %postun scriptlet.

ok fine, removed.

> Speaking of the scriptlets... Standard convention is to put them before the
> %files lists, not after. %files is generally the last thing before the
> %changelog.

ok, fixed.

New srpm, spec and *.tgz uploaded to ftp://oss.sgi.com/projects/pcp/download/v3
and pushed the changes up to git://oss.sgi.com/markgw/pcp/pcp.git dev
(see the git log in that tree for commits)

Thanks
-- Mark Goodwin

-- 
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.