Spec URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp.spec SRPM URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-1.fc10.src.rpm Description: Performance Co-Pilot (PCP) provides a framework and services to support system-level performance monitoring and management. It presents a unifying abstraction for all of the performance data in a system, and many tools for interrogating, retrieving and processing that data. PCP is a feature-rich, mature, extensible, cross-platform toolkit supporting both live and retrospective analysis. The distributed PCP architecture makes it especially useful for those seeking centralized monitoring of distributed processing. PCP is an ideal "drop-in" replacement for sysstat/sar for those engaged in supporting enterprise level production computing environments. For more details, documentation and white papers, see the PCP home page at http://oss.sgi.com/projects/pcp The associated GUI package (pcp-gui) exists in a separate tree and will be submitted in a separate review request.
Setting the "seeking a sponsor" tags.
Mark, a few random questions & comments, more may follow. :) What is the reason for the Provides: on the libraries? Generally RPM sorts out library provides on its own. If there's something special here it could use a comment. What's the reason for all the %attr specifications in the file list? Does "make install" not put the proper permissions on them? Assuming "rpmdiff" shows permissions differences, there would appear to be no difference and I'd just lose every explicit %attr set. also: %pre exit 0 the above does nothing and should be removed. We probably should just not package the static libs unless there's a real reason for it - is there? If not, could just rm -f them in %install. ./configure can/should be replaced with %configure, although this seems to break the build. :( I'd have to look and see what the difference is. Lots of explicit paths can (and I think should?) be referred to w/ macros: /usr/lib* -> %{_libdir} /usr/share/man/man3 -> %{_mandir}/man3 (etc) /usr/bin -> %{_bindir} /usr/include -> %{_includedir} In a lot of cases, it may be a lot simpler to use wildcards rather than listing each file & subdir individually. For example: %dir /foo/bar/baz /foo/bar/baz/* But if 2 subpackages share files in a subdir they'd need to be explicitly listed I guess, and only one should own the dir of course. I guess it's your preference but wow, that is a long %files list ;) When I build I get some odd messages: cpio: pcp-3.0.0/src/dbpmda/src/<stdout>: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/dbpmda/src/gram.tab.c: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmie/src/grammar.tab.c: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlc/<stdout>: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlc/gram.tab.c: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlogextract/<stdout>: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlogextract/gram.tab.c: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlogger/<stdout>: Cannot stat: No such file or directory cpio: pcp-3.0.0/src/pmlogger/gram.tab.c: Cannot stat: No such file or directory I think these are generated files, and the debuginfo collection step isn't able to find them. This may mean that the debuginfo rpms are incomplete? You might be able to simplify the README installation etc (DOCFILES) by dropping their install from the makefiles, and just doing: %files %doc README COPYING FOO BAR I'm not sure if hand-recreating what %doc does has any pitfalls or not.
I think a lot of the DIST_ROOT DIST_MANIFEST stuff can be simplified. Also is there any real reason to be using /usr/bin/gmake instead of just calling "make?" %clean -[ ! -z "$DIST_ROOT" ] && rm -rf $DIST_ROOT rm -Rf $RPM_BUILD_ROOT %build -/usr/bin/gmake default_pcp +make default_pcp %install rm -Rf $RPM_BUILD_ROOT -DIST_ROOT=$RPM_BUILD_ROOT -DIST_MANIFEST=`pwd`/install.manifest -export DIST_ROOT DIST_MANIFEST -rm -f $DIST_MANIFEST -/usr/bin/gmake install_pcp +make DIST_ROOT=$RPM_BUILD_ROOT install_pcp
Following lengthy review discussions with Eric Sandeen (thanks Eric) and several PCP community developers, and after working thru errors and warnings reported by 'rpmlint', the changes listed below have been committed to the 'dev' branch in my tree at git://oss.sgi.com/markgw/pcp/pcp.git For review purposes, the Fedora specific spec and SRPM for PCP have been copied up to : Spec URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp.spec SRPM URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-1.fc10.src.rpm Most of the changes made so far are to the Fedora RPM spec itself - these need to be reconciled, where possible, with the community RPM spec. The remaining changes are mostly to silence rpmlint: * pcp-libs.x86_64: E: library-without-ldconfig-postin To fix, need to add a "%post libs" scriptlet to call to ldconfig * 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. * 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. * 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. * 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 * pcp.x86_64: W: log-files-without-logrotate /var/log/pcp PCP has it's own log management system, see pmlogger_daily(1) * 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. * pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmproxy * pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmie * pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pcp rpmlint suggests we need to set up a lock file in /var/lock/subsys. Changes commited so far to git://oss.sgi.com/markgw/pcp/pcp.git (most recent changes are listed first). Please clone my git tree if you want to see the patches : commit 5dc76a384254b3a0b1a72ff1be5faec042effa73 Author: Mark Goodwin <mgoodwin> Date: Fri Jul 31 15:44:40 2009 +1000 RPM spec specifically for the Fedora Project. Signed-off-by: Mark Goodwin <mgoodwin> commit 1274605472853400f4297439c433a05829735a64 Author: Mark Goodwin <mgoodwin> Date: Fri Jul 31 15:40:58 2009 +1000 tweak configure to move PCP_BINADM_DIR out of /usr/share, into /usr/lib. Arch dependent binaries should not be installed below /usr/share. Signed-off-by: Mark Goodwin <mgoodwin> commit 17700e2bdaa0060178c3d025edb92280f36451f6 Author: Mark Goodwin <mgoodwin> Date: Fri Jul 31 11:47:39 2009 +1000 delete unneeded explicit script interpreter to keep rpmlint happy Signed-off-by: Mark Goodwin <mgoodwin> commit b971772c281a5aff6408981e7086cbfa4bcee6ba Author: Mark Goodwin <mgoodwin> Date: Fri Jul 31 11:33:20 2009 +1000 pmpost does not really need to be setuid Signed-off-by: Mark Goodwin <mgoodwin> commit 0a4cd4b8bfe44265bdccfb93db5e19c28c796f79 Author: Mark Goodwin <mgoodwin> Date: Fri Jul 31 11:11:48 2009 +1000 Nuke migrate_pcp_var_dir, no longer needed Signed-off-by: Mark Goodwin <mgoodwin> commit bf98168e8a7cea7697497f4972dd835fa10e29b6 Author: Mark Goodwin <mgoodwin> Date: Thu Jul 30 18:25:12 2009 +1000 default chkconfig off for all PCP services Signed-off-by: Mark Goodwin <mgoodwin> commit 3baf8e7a6491c9533a1b0ff00c6d711a32677536 Author: Mark Goodwin <mgoodwin> Date: Mon Jul 27 09:32:57 2009 +1000 take -fstack-protector-all back out again since it's only supported with newer compilers. Probably should add a configure test for it at some stage Signed-off-by: Mark Goodwin <mgoodwin> commit 0c1b4b2f4bd9b0759964fb056a1469b2dd052ce6 Author: Mark Goodwin <mgoodwin> Date: Wed Jul 22 17:02:40 2009 +1000 Tweak RPM dependency rules for 2.x -> 3.x upgrades and massage the PCP RPM spec to reduce rpmlint noise. Also added -fstack-protector-all to Linux PCFLAGS. On branch 3.0.0: modified: build/rpm/pcp.spec.in modified: src/include/builddefs.in Signed-off-by: Mark Goodwin <mgoodwin> commit ff6cfff7845a832ab3b0affe19c6ffa33293bc93 Author: Mark Goodwin <mgoodwin@fletch.(none)> Date: Fri Jul 10 14:36:29 2009 +1000 move pcp.conf into libs and check for pcp-devel in PMDAs that ship a Makefile commit 728c61eb0aa0ebe9bf3b4096696adfc5b14abf39 Author: Mark Goodwin <mgoodwin@fletch.(none)> Date: Thu Jul 9 17:33:12 2009 +1000 split RPM packaging into pcp, pcp-libs and pcp-devel, bump version to 3.0.0
Ok a few more comments, sorry for being iterative. Naming the spec "pcp_fedora.spec" threw me off, not sure if it's kosher or not. Keeping a fedora spec upstream probably just makes it harder to keep things in sync; fedora cvs should be pretty capable of keeping track of the fedora pcp specfile. --- %if %{have_ibdev} %define ib_prereqs libibmad libibumad libibcommon %define ib_build_prereqs %{ib_prereqs} libibmad-devel libibumad-devel libibcommon-devel %endif %ifarch ia64 Requires: libunwind %endif I think the ib_prereqs can be completely dropped; they're all libraries, and if configured to use it (?) rpm will work that out. So I'd just drop %{_ib_prereqs} altogether. Same goes for the libunwind stuff on ia64 most likely? Or is it explicitly needed for some reason? If so I'd add a comment. --- BuildRequires: gcc-c++ libstdc++-devel procps autoconf bison flex ncurses-devel %{?ib_build_prereqs} as per: https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 There's no need for gcc-c++ or libstdc++-devel (as gcc-c++ requires this) --- However, doing a clean build fails due to missing BuildReqs: checking if ExtUtils::MakeMaker is installed... no FATAL ERROR: Perl ExtUtils::MakeMaker module missing. so add: BuildRequires: perl-ExtUtils-MakeMaker ---
(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
Created attachment 355832 [details] modified specfile This is what I'd propose, assuming it works, and assuming I'm not being too draconian based on misunderstandings of how things actually work :) If stuff needs to go back in, with justification, I'll let you talk that over w/ the final reviewer. Thanks for your patience and understanding ;) -Eric
(In reply to comment #5 and comment #6) > Ok a few more comments, sorry for being iterative. thanks for the review/update Eric, much appreciated, sorry I didn't get back to this yesterday .. those pesky customers again :) > > Naming the spec "pcp_fedora.spec" threw me off, not sure if it's kosher or not. I just checked it into (my) pcp git tree, mainly for safe-keeping, but also for easier future reconcilliation with the upstream spec, which is now quite different. I think it would also be reasonable to check-in the SLES spec as well, since it's different too. > Keeping a fedora spec upstream probably just makes it harder to keep things in > sync; fedora cvs should be pretty capable of keeping track of the fedora pcp > specfile. yeah, once it's reconciled and checked into cvs, then maybe it can be removed from the tree (or that commit can be revoked). > %if %{have_ibdev} > %define ib_prereqs libibmad libibumad libibcommon > %define ib_build_prereqs %{ib_prereqs} libibmad-devel libibumad-devel > libibcommon-devel > %endif > > %ifarch ia64 > Requires: libunwind > %endif > > I think the ib_prereqs can be completely dropped; they're all libraries, and if > configured to use it (?) rpm will work that out. So I'd just drop > %{_ib_prereqs} altogether. yes they're all libraries, but no they can't be dropped. When Max wrote the infiniband PMDA, he needed to change the IB libs. Those IB changes are upstream now, but not (yet) available in any standard RH or FC build AFAIK. So the build and run-time prereqs will need versioning added .. which I'll add once I figure out what it is :) For now, %{have_ibdev} should stay and be false. FWIW, having IB monitoring is important for most HPC users, so we really do want to revisit this. > Same goes for the libunwind stuff on ia64 most likely? Or is it explicitly > needed for some reason? If so I'd add a comment. I'd add a comment if I could remember the context for needing the prereq. Another one lost in the depths of antiquity, probably an SGI-only thing? Maybe we can drop it, run qa and see what breaks on ia64 > BuildRequires: gcc-c++ libstdc++-devel procps autoconf bison flex ncurses-devel > %{?ib_build_prereqs} > > as per: https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 > > There's no need for gcc-c++ or libstdc++-devel (as gcc-c++ requires this) yep ok thanks > However, doing a clean build fails due to missing BuildReqs: > > checking if ExtUtils::MakeMaker is installed... no > FATAL ERROR: Perl ExtUtils::MakeMaker module missing. > > so add: > > BuildRequires: perl-ExtUtils-MakeMaker ok thanks, added. > 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? yes that's correct. the error is basically fatal for a daemon PMDA and is a known way for a daemon PMDA to terminate. (and the exit() is not in the code-path for a DSO PMDA). > > * 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. A better longer term plan would be separate packages for PMDAs, which could also have -devel variants too. > > * 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 :) yep ok, added the following : # Note: there are some headers (e.g. domain.h) and in a few cases some # C source files, that match the following pattern. rpmlint complains # about this, but note: these are not devel files, but rather they are # (slightly obscure) PMDA config files. %{_localstatedir}/lib/pcp/pmdas/*/* > > * 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 :) yep, so we'll ignore that one :) Note that rpmlint also installs %dir /etc/bash_completion.d :) > > 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. ok I've marked it %config, can't hurt > > * 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? the special homegrown tool is integral to the whole PCP logging infrastructure. I can't change that without major architectural changes ... which I'm not willing to undertake at this point! > > * 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? they're there for safety, I think because we weren't sure what umask is prevailing > > 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? well that stuff was put there many moons ago because the default action on an upgrade were NOT what (SGI) customers wanted - they complained. I'll remove them for Fedora, for now, but they might come back oneday. > 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? /etc/pcp.conf sources /etc/pcp.conf, which may or may not get edited after install. That breaks the consistency of the whole rpm macro / /etc/pcp.conf variable sync-up. I don't know what to do about that. For now, yes let's got for the simple case and remove the crud. > > 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. ok, let's go simple and standard, for now. The crud might come back oneday though ... > > Also on the root.saved stuff - I can't find anything in scripts or source that > creates this. I see root.prev though. I'll dig further for this one .. antiquity again. I think it has something to do with the XFS NULL files bug, and the need to preserve a safe copy of a critically important PCP config file. New spec copied up to same place on OSS in a few mins (and checked into my git tree too). I'll attach the new version to this BZ too. Cheers and thanks -- Mark
Eric, didn't quite get to finish this off this arvo - still want to add %docs and also resolve the rc script issues with /var/run/*.pid and rpmlint. I have both figured out, but haven't finished building & QA'ing .. tomorrow. Cheers -- Mark
Oh, I'll take this for review, though I can't sponsor.
Hi Eric, I've just posted v3.0.0-2 : Spec URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp.spec Tar URL : ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-2.src.tar.gz SRPM URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-2.fc10.src.rpm Merged with pcp-2.9.0-1 and pushed up to: git://oss.sgi.com/markgw/pcp/pcp.git dev Changes since 3.0.0-1 are listed in the git logs, in summary : - merge with pcp-2.9.0-1 dev - bump to 3.0.0-2, update changelog and delete unncessary Vendor string - changes to the Fedora RPM spec in response to review feedback: - Incorporate a large number of suggestions and cleanups from Eric Sandeen - Greatly simplified the %post and %postun scriptlets - remove ancient crud - Added ldconfig scriptlets for -libs - Don't explicitly require IB libs, since they're libs and RPM figures it out - No need to explicitly BuildRequire gcc-c++ libstdc++-devel - Add BuildRequires on perl-ExtUtils-MakeMaker - Remove explicit ia64 Requires: libunwind - Have pcp-libs require the base package since Fedora insists - Add and clarify some comments - Create %{_localstatedir}/run/pcp and ship it (so it'll be removed) - Use %doc for CHANGELOG COPYING INSTALL README VERSION.pcp pcp.lsm - git ignore generated gram*.tab.c - preserve generated gram.tab.c because debuginfo needs it - fix minor typo (Redhat -> Red Hat) in comment in Fedora spec - dont install trace demo binaries since src is installed anyway - tweak configure to move PCP_BINADM_DIR out of /usr/share, into /usr/lib. - Arch dependent binaries should not be installed below /usr/share. - delete unneeded explicit script interpreter to keep rpmlint happy - pmpost does not really need to be setuid - Nuke migrate_pcp_var_dir, no longer needed - default chkconfig off for all PCP services Still have the (benign) rpmlint errors: pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmproxy pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmie pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pcp Cheers -- Mark Goodwin
Ok, here goes the last informal review and I'll hand off to Jarod. Koji rawhide scratch build here (successful): http://koji.fedoraproject.org/koji/taskinfo?taskID=1587488 rpmlint: # rpmlint /tmp/pcp-3.0.0-2.fc10.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. # rpmlint RPMS/x86_64/pcp*3.0.0-2.fc12.x86_64.rpm pcp.x86_64: E: obsolete-on-name pcp.x86_64: W: conffile-without-noreplace-flag /etc/bash_completion.d/pcp pcp.x86_64: W: conffile-without-noreplace-flag /etc/pcp.env pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/trivial/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mounts/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/trace/app2.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/weblog/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/simple/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/pmclient/pmclient.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mmv/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/trivial/trivial.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/cisco/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mailq/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/linux/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/trace/stub.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/sample/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/txmon/txrecord.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/jstat/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/shping/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/summary/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/process/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/trace/app1.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/simple/simple.c pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/procmemstat/procmemstat.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/sendmail/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/txmon/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/trace/app3.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/trace/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /usr/share/pcp/demos/trace/pmtrace.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/txmon/txmon.c pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/lmsensors/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/apache/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/roomtemp/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/lustrecomm/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/txmon/txmon.h pcp.x86_64: W: log-files-without-logrotate /var/log/pcp pcp.x86_64: W: dangerous-command-in-%preun rm pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmproxy pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmie pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pcp pcp-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp_gui.so.1 libpcp_gui.so.2 pcp-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp.so.2 libpcp.so.3 pcp-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp_pmda.so.2 libpcp_pmda.so.3 pcp-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libpcp_pmda.so.3 exit.5 pcp-libs.x86_64: W: no-documentation 4 packages and 0 specfiles checked; 4 errors, 41 warnings. * MUST: rpmlint must be run on every package. See above. * MUST: The package must be named according to the Package Naming Guidelines. OK * MUST: The spec file name must match the base package %{name}. OK * MUST: The package must meet the Packaging Guidelines. NEEDSWORK? - 4 errors still above. subsys-not-used should be easy to fix up, A package should not obsolete itself, as it can cause weird errors in tools. # Prior to v3, the PCP package implicitly "provides" -libs and -devel. # Strictly, pcp-libs should obsolete the v2.x PCP package, but since # pcp requires pcp-libs, pcp can just obsolete itself. This is thus # redundant dependency, but included for clarity. Obsoletes: pcp < 3.0 I'll let Jarod be the final arbiter on this. * MUST: The package must be licensed with a Fedora approved license. OK * MUST: The License field in the package spec file must match the actual license. NEEDSWORK? From COPYING: All the libraries in the Performance Co-Pilot (PCP) open source release are licensed under Version 2.1 of the GNU Lesser General Public License. All other components in the PCP open source release are licensed under Version 2 of the GNU General Public License. but the specfile says: License: GPL+ and LGPLv2+ All .c and .h files do say "or any later version" so ideally COPYING should be fixed to reflect this. * MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. OK * MUST: The spec file must be written in American English. OK * MUST: The spec file for the package MUST be legible. OK (I might rather see a little more consistency between wildcards & explicit files, but not a big deal) * MUST: The sources used to build the package must match the upstream source. OK * MUST: The package MUST successfully compile and build into binary rpms OK * MUST: All build dependencies must be listed in BuildRequires OK * MUST: The spec file MUST handle locales properly. OK * MUST: Every binary RPM package (or subpackage) which stores shared library files must call ldconfig in %post and %postun. OK * MUST: A package must own all directories that it creates. OK * MUST: A Fedora package must not list a file more than once in the spec file's %files listings. OK * MUST: Permissions on files must be set properly. OK * MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). OK * MUST: Each package must consistently use macros. OK * MUST: The package must contain code, or permissable content. OK * MUST: Large documentation files must go in a -doc subpackage. OK * MUST: If a package includes something as %doc, it must not affect the runtime of the application. OK * MUST: Header files must be in a -devel package. OK Note: "OK" based on pmda .h files "not being header files but rather used for configuration" * MUST: Static libraries must be in a -static package. N/A * MUST: Packages containing pkgconfig(.pc) files N/A * MUST: If a package contains library files with a suffix then library files that end in .so must go in a -devel package. OK * MUST: Devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} NEEDSWORK: Requires: pcp-libs = %{version} For whatever reason I guess we must require pcp, not pcp-libs. * MUST: Packages must NOT contain any .la libtool archives OK * MUST: Packages containing GUI applications must include a %{name}.desktop file N/A (but keep in mind for the pcp gui?) * MUST: Packages must not own files or directories already owned by other packages. OK * MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) OK * MUST: All filenames in rpm packages must be valid UTF-8. OK SHOULD Items: * SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. N/A (license text is there) * SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. N/A (not available and I've never seen it!) * SHOULD: The reviewer should test that the package builds in mock. OK * SHOULD: The package should compile and build into binary rpms on all supported architectures. OK * SHOULD: The reviewer should test that the package functions as described. OK (I started pcp anyway) ;) * SHOULD: If scriptlets are used, those scriptlets must be sane. OK (now!) * SHOULD: Subpackages other than devel should require the base package using a fully versioned dependency. NO, but it seems ok * SHOULD: The placement of pkgconfig(.pc) files N/A * SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. OK Just a few things left here, which I'll leave to Jarod's discretion. -Eric
Thanks Eric, once again. I'll work thru the list and have another update ready on Monday - particularly the license, which will need to be updated in the spec, based on the COPYING file .. ooops :)
Eric Sandeen wrote: > * MUST: The package must meet the Packaging Guidelines. > NEEDSWORK? - 4 errors still above. subsys-not-used should be easy to fix up, I looked at this and I'd rather not change it. It turned out not to be that easy to fix. The three PCP services (pcp, pmie and pmproxy) all manage their own var/run/pcp/pid files. This pre-dates the standard functionality for managing pid files and is also multi-platform and also rock solid stable. > * MUST: The License field in the package spec file must match the actual > license. > > NEEDSWORK? > From COPYING: > All the libraries in the Performance Co-Pilot (PCP) open source > release are licensed under Version 2.1 of the GNU Lesser General > Public License. > > All other components in the PCP open source release are licensed > under Version 2 of the GNU General Public License. > > but the specfile says: > License: GPL+ and LGPLv2+ The previous version tried to specify the license for the base package and the two subpackages, which is wrong because -libs has a different license. So I've now split this so each package specifies it's own license. Changed the spec so the base package and -devel specify "GPLv2+" (assuming "GPLv2+" is the best match for "GPLv2.1", as specified in COPYING since there is no explicit option for "GPLv2.1"), and -libs is "LGPLv2" (exactly matching what's in COPYING). > * MUST: Header files must be in a -devel package. OK > Note: "OK" based on pmda .h files "not being header files but rather used for > configuration" yes that's correct, as already discussed. > * MUST: Devel packages must require the base package using a fully > versioned dependency: Requires: %{name} = %{version}-%{release} > > NEEDSWORK: Requires: pcp-libs = %{version} > > For whatever reason I guess we must require pcp, not pcp-libs. Nathan and I discussed this and we concluded the only True Dependencies are: pcp-devel requires pcp-libs pcp requires pcp-libs Neither pcp-devel nor pcp-libs actually requires pcp. There is a good reason we don't want pcp-devel to require pcp - basically it has to do with the pcp daemon on the build machine getting killed when pcp in the chroot gets uninstalled, i.e. we want to be able to build packages (such as pcp-gui) that BuildRequires pcp-devel *without* pcp installed (just pcp-devel and pcp-libs should be installed). > * 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? 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 :) Other changes in this round: - reconciled the open source spec with the current Fedora spec. - merged with latest 2.9 dev tree - bumped to 3.0.0-3 and pushed updated spec, .tgz and srpm files to oss. - pushed my tree to git://oss.sgi.com/markgw/pcp/pcp.git dev Cheers -- Mark
(In reply to comment #14) > Eric Sandeen wrote: > > * MUST: The package must meet the Packaging Guidelines. > > NEEDSWORK? - 4 errors still above. subsys-not-used should be easy to fix up, > > I looked at this and I'd rather not change it. > > It turned out not to be that easy to fix. The three PCP services > (pcp, pmie and pmproxy) all manage their own var/run/pcp/pid files. > This pre-dates the standard functionality for managing pid files and > is also multi-platform and also rock solid stable. I think we can live with that. > > * MUST: The License field in the package spec file must match the actual > > license. > > > > NEEDSWORK? > > From COPYING: > > All the libraries in the Performance Co-Pilot (PCP) open source > > release are licensed under Version 2.1 of the GNU Lesser General > > Public License. > > > > All other components in the PCP open source release are licensed > > under Version 2 of the GNU General Public License. > > > > but the specfile says: > > License: GPL+ and LGPLv2+ > > The previous version tried to specify the license for the base package > and the two subpackages, which is wrong because -libs has a different > license. So I've now split this so each package specifies it's own license. > Changed the spec so the base package and -devel specify "GPLv2+" > (assuming "GPLv2+" is the best match for "GPLv2.1", as specified in COPYING > since there is no explicit option for "GPLv2.1"), and -libs is "LGPLv2" > (exactly matching what's in COPYING). "GPLv2+" is only for "GPLv2.anything or later", if it doesn't say "or later", then simply "GPLv2" is the tag you want. > > * MUST: Devel packages must require the base package using a fully > > versioned dependency: Requires: %{name} = %{version}-%{release} > > > > NEEDSWORK: Requires: pcp-libs = %{version} > > > > For whatever reason I guess we must require pcp, not pcp-libs. > > Nathan and I discussed this and we concluded the only True Dependencies are: > pcp-devel requires pcp-libs > pcp requires pcp-libs > > Neither pcp-devel nor pcp-libs actually requires pcp. There is a good > reason we don't want pcp-devel to require pcp - basically it has to do with > the pcp daemon on the build machine getting killed when pcp in the chroot > gets uninstalled, i.e. we want to be able to build packages (such as pcp-gui) > that BuildRequires pcp-devel *without* pcp installed (just pcp-devel and > pcp-libs should be installed). 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. > > * 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). > 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
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 go into somewhere like /usr/share/doc/pcp/{sample,demo}/, not into /usr/share/pcp or /var/lib/pcp. 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. If the file continues to exist, an rpm -e will remove it, no need for the rm in the %postun scriptlet. 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.
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> 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> 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
(In reply to comment #17) > 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. Just checking in to see where we're at on this. I think I was waiting to hear back on this part before doing another (hopefully the last) pass over things.
(In reply to comment #18) > (In reply to comment #17) > > 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. > > Just checking in to see where we're at on this. I think I was waiting to hear > back on this part before doing another (hopefully the last) pass over things. Hi Jarod, thanks for checking back - sorry I've been completely snowed under with other stuff. I've moved the C source to a better place and have just a few things left to clean up, so I'll make a concerted effort on that front :) We also had a discussion on freenode.net #pcp about what to do with the Perl binding for PCP. Perhaps you could provide some guidance here for the packaging perspective - the perl binding is part of the main src tree (below src/cpan) but the packaging is currently driven by MakeMaker (with a manual step to build the pcp perl RPMs from the resulting spec). Ideally, we'd like the perl binding to be a sub-package, e.g. "pcp-perl-this-and-that", with the build and packaging all driven by the main spec. I have some simple changes to the spec that implements this but it probably violates the Fedora Perl packaging guidelines - anything perl'ish should be named "perl-something". So the only way to resolve that would be to run MakeMaker in %build or %install and to generate additional perl-pcp-something RPMs that way ... which seems hacky. Any advice? maybe we should split the PCP perl binding into a separate src tree, with appropriate BuildRequires dependencies? Cheers -- Mark
> We also had a discussion on freenode.net #pcp about what to do with the Perl > binding for PCP. Perhaps you could provide some guidance here for the packaging > perspective - the perl binding is part of the main src tree (below src/cpan) > but the packaging is currently driven by MakeMaker (with a manual step to build > the pcp perl RPMs from the resulting spec). Ideally, we'd like the perl binding > to be a sub-package, e.g. "pcp-perl-this-and-that", with the build and > packaging all driven by the main spec. I have some simple changes to the spec > that implements this but it probably violates the Fedora Perl packaging > guidelines - anything perl'ish should be named "perl-something". Yes, they should definitely be built from the main spec and the same source rpm, since they're all in the same source tarball. If naming is the only issue here, then simply use '%package -n perl-pcp-something'. The -n says "don't prepend the main package name to this". Do teh same for %description, %files, etc.
Hi Jarod, v3.0.0-5 is ready for review : Spec URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp.spec SRPM URL: ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-5.fc10.src.rpm Src.tgz : ftp://oss.sgi.com/projects/pcp/download/v3/pcp-3.0.0-5.src.tar.gz (spec and src.tgz are identical to what's in the SRPM). The pcp-perl packaging (thanks for the comments) is not included - I'd rather avoid any further delay and instead add the perl packaging later once we're in Fedora - if this is a bad plan, please let me know :) The demo agents that ship src have been moved into the devel package. They're required for the PCP QA suite to run, but so is the rest of the devel package, so no problem there. I've also merged up with the recent 2.9.1 community release, which is stable and fully QA'd. [mgoodwin@fletch rpmbuild]$ rpmlint SRPMS/pcp-3.0.0-5.fc10.src.rpm RPMS/x86_64/{pcp-3.0.0-5.fc10.x86_64.rpm,pcp-libs-3.0.0-5.fc10.x86_64.rpm,pcp-libs-devel-3.0.0-5.fc10.x86_64.rpm} pcp.x86_64: W: conffile-without-noreplace-flag /etc/bash_completion.d/pcp pcp.x86_64: W: conffile-without-noreplace-flag /etc/pcp.env pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mounts/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/weblog/domain.h pcp.x86_64: W: hidden-file-or-dir /var/lib/pcp/pmns/.NeedRebuild pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mmv/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/cisco/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/mailq/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/linux/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/jstat/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/shping/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/summary/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/process/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/sendmail/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/trace/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/lmsensors/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/apache/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/roomtemp/domain.h pcp.x86_64: W: devel-file-in-non-devel-package /var/lib/pcp/pmdas/lustrecomm/domain.h pcp.x86_64: W: log-files-without-logrotate /var/log/pcp pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmproxy pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pmie pcp.x86_64: E: subsys-not-used /etc/rc.d/init.d/pcp pcp-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libpcp_pmda.so.3 exit.5 pcp-libs.x86_64: W: no-documentation pcp-libs-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp_gui.so.1 libpcp_gui.so.2 pcp-libs-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp.so.2 libpcp.so.3 pcp-libs-devel.x86_64: W: dangling-relative-symlink /usr/lib64/libpcp_pmda.so.2 libpcp_pmda.so.3 4 packages and 0 specfiles checked; 3 errors, 25 warnings. Many thanks -- Mark Goodwin
Finally getting back to looking at this again, sorry for the delay. Formal review check list from https://fedoraproject.org/wiki/Packaging:ReviewGuidelines: MUST items- * rpmlint output: still rather noisy, but has all been explained acceptably * package naming: ok * spec file name: ok * package meets guidelines: almost... Just noticed the use of %makeinstall, which I don't think we've touched on yet... Its use is HIGHLY frowned upon... * license: ok * license in spec matches: ok * license doc included: ok * spec in english: ok * spec legible: ok * sources match upstream: ok $ md5sum pcp-3.0.0-5.src.tar.gz* de5b6be8fcd08985756a01fc276774d7 pcp-3.0.0-5.src.tar.gz de5b6be8fcd08985756a01fc276774d7 pcp-3.0.0-5.src.tar.gz.1 * compiles and packages correctly: ok (F12/x86_64 smoke-tested) * ExcludeArch where necessary: n/a * ExcludeArch for build failure filed and noted: n/a * sane BuildRequires: ok * locales handled properly: n/a * ldconfig called where appropriate: ok * no bundled copies of system libs: ok * relocatable justification: n/a * owns created directories: ok * files not listed 2x: ok * permissions: ok * consistent macros: ok * permissible code: ok * documentation: ok * header files in -devel: ok (with exceptions as noted earlier in review) * static libs: n/a * pkgconfig bits: n/a * versioned libs properly split up: ok * devel packages requires its base package: ok * no libtool archives: ok * gui .desktop file: n/a * no owning others files: ok * %install starts w/rm -rf BR: ok * %files all UTF-8: ok SHOULD items- * source needs license text: n/a * spec translations, if available: n/a * package builds in mock: ok * builds on all supported arches: untested * package functions as expected: presumed * sane scriptlets: ok * fully version requires for sub-packages: ok and/or n/a * pkgconfig stuff: n/a * file-based deps: n/a Just the %makeinstall bit that needs fixing up, and I think we're all set. https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
Thanks Jarod, we've moved along a little bit and now have the perl-PCP packages being built from the same spec. I've nuked %makeinstall and further tweaked things for rpmlint - we (the PCP community developers) are just going thru final QA now and expect to be all done by the end of this week. So I'll UPDATE here again with the new spec and tarball details when that occurs. Thanks again -- Mark
Hi Jarod, final spec and src tarball are now ready for final review: The src tarball is in it's permanent place (matching the spec) : ftp://oss.sgi.com/projects/pcp/download/pcp-3.0.0-9.src.tar.gz I'll attach the spec to this BZ in the next update. Thanks -- Mark Goodwin
Created attachment 364404 [details] pcp.spec for final Fedora review, as in the pcp-3.0.0-9 community release pcp.spec for final Fedora review. This is the Fedora version of the specfile that was used to build the RPMS for the pcp-3.0.0-9 community release. The URL to the matching src tarball is specified as Source0 in this specfile.
Created attachment 367782 [details] updated pcp spec for Fedora, matching pcp-3.0.1-2 community release Minor update to the Fedora spec after the pcp-3.0.1-2 community release. Note there is one new rpmlint 'E': fter doing some upgrade testing, we changed /etc/pcp.conf to be tagged as %config rather than %config(noreplace). Jarod, I'd appreciate an ack on this if you have time :) Thanks -- Mark
Jarod, can I take comment #22 as final ack? Thanks -- Mark
Apologies for the delays, I've been a bit, um... tied up... with new happenings here in the office. Yeah, with the tweaks requested in comment #22 done, I'm going to say you're all set. Thanks much for your patience working though this Package APPROVED. iirc, we still need to get you hooked up packager group sponsorship, which I think needs to happen prior to filing a cvs request to add the package...
Hm, what's next? An awful lot of effort on this bug to not yet have it built ;)
(In reply to comment #29) > Hm, what's next? An awful lot of effort on this bug to not yet have it built > ;) Hi Eric, yes activity has resumed - Bill Cohen is talking with Andrew Overholt in the tools group as a possible sponsor. Alternatively, we could get you promoted to become sponsor-capable - you maintain enough important packages in Fedora that maybe this would be a good idea regardless (?). Cheers -- Mark
Fine by me, I am not terribly well-versed in the ins & outs of fedora policy TBH...
This is the first I've heard of this :) However, I'm not really well-versed enough in the packaging guidelines for non-Java stuff so I can't be a sponsor, sorry.
Bah, sorry, I was unclear. I can take care of sponsorship, but iirc, you have to get a fedora account set up and apply for membership in the packager group before I can do that.
Many thanks Jarod. Returning to this task again now. I already have a Fedora FAS account, though it's not entirely clear to me on how to get added to the packager group. The docs at http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Get_Sponsored says "Your sponsor can add you to the packager group. You should receive email confirmation of your sponsorship." In any case, removing FE-NEEDSPONSOR now we have a sponsor. Next update will include a New Package CVS Request. Once processed, that might let me set the fedora‑cvs flag (which may be how I get into the packager group?). Cheers -- Mark
New Package CVS Request ======================= Package Name: pcp Short Description: System-level performance monitoring and performance management toolkit Owners: mgoodwin Branches: F-13 EL-4 EL-5 EL-6 InitialCC: jarod
Typically, you request inclusion in the group 'packager' within the Fedora Account System (https://admin.fedoraproject.org/accounts/), and then someone agrees to sponsor you, at which point you're approved for membership in that group. I went ahead and applied for you and sponsored you in one pass just now.
Thanks again Jarod. PCP version 3.3.0 is just about done now and the community are asking for Fedora and RHEL builds. So setting the fedora‑cvs flag for the branches specified in comment #35. -- Mark
InitialCC takes a Fedora account, not an email address, and the account "jarod" seems to belong to someone else so I've simply done the request without any InitialCC. Please add yourself to the package in pkgdb as appropriate. CVS done (by process-cvs-requests.py).
pcp-3.3.3-1.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/pcp-3.3.3-1.el5
pcp-3.3.3-1.el4 has been submitted as an update for Fedora EPEL 4. http://admin.fedoraproject.org/updates/pcp-3.3.3-1.el4
pcp-3.3.3-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/pcp-3.3.3-1.fc13
Mark, FWIW, you could build this for EPEL6 now as well -Eric
pcp-3.3.3-1.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update pcp'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pcp-3.3.3-1.fc13
pcp-3.3.3-1.el4 has been pushed to the Fedora EPEL 4 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update pcp'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pcp-3.3.3-1.el4
pcp-3.3.3-1.el5 has been pushed to the Fedora EPEL 5 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update pcp'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/pcp-3.3.3-1.el5
pcp-3.3.3-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.3.3-1.el4 has been pushed to the Fedora EPEL 4 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.3.3-1.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.5.11-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/pcp-3.5.11-2.el6
pcp-3.5.11-2.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/pcp-3.5.11-2.el5
pcp-3.5.11-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/pcp-3.5.11-2.fc16
pcp-3.5.11-2.el4 has been submitted as an update for Fedora EPEL 4. https://admin.fedoraproject.org/updates/pcp-3.5.11-2.el4
pcp-3.5.11-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/pcp-3.5.11-2.fc15
pcp-3.6.3-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/pcp-3.6.3-1.fc16
pcp-3.6.3-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/pcp-3.6.3-1.el6
pcp-3.6.3-1.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/pcp-3.6.3-1.fc15
pcp-3.6.3-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/pcp-3.6.3-1.el5
pcp-3.6.3-1.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.6.3-1.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.6.3-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.6.3-1.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.