Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 191743 - Review Request: sysprof - a sampling CPU profiler
Summary: Review Request: sysprof - a sampling CPU profiler
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 191745
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-15 15:32 UTC by Gianluca Sforna
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-10-08 08:51:56 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Gianluca Sforna 2006-05-15 15:32:38 UTC
Spec URL: http://giallu.interfree.it/fedora/sysprof.spec
SRPM URL: http://giallu.interfree.it/fedora/sysprof-1.0.2-1.src.rpm

Description:
Sysprof is a sampling CPU profiler for Linux that uses a kernel module
to profile the entire system, not just a single application.
Sysprof handles shared libraries and applications do not need to be
recompiled. In fact they don't even have to be restarted.

Comment 1 Parag AN(पराग) 2006-06-02 04:43:53 UTC
Some Comments:-
rpmling gives error as
E: sysprof configure-without-libdir-spec

Comment 2 Gianluca Sforna 2006-06-02 10:36:48 UTC
I am leaving for a couple of weeks, so I can not look into this until I come back.

thanks for your patience...


Comment 3 Gianluca Sforna 2006-06-22 08:47:40 UTC
I fixed the rpmlint error by using the %configure macro and also updated the
package to version 1.0.3

Spec URL: http://giallu.interfree.it/fedora/sysprof.spec
SRPM URL: http://giallu.interfree.it/fedora/sysprof-1.0.3-1.src.rpm

Comment 4 Dan Horák 2006-06-26 19:42:59 UTC
The package should own the directory %{_datadir}/sysprof and this line will be
sufficient in the files section. Then there is also no need to specify each file
explicitly.

Comment 5 Dan Horák 2006-06-26 20:39:45 UTC
%{_datadir}/%{name} will be even better, similar package the binary as
%{_bindir}/%{name}

Comment 6 Gianluca Sforna 2006-06-28 08:08:34 UTC
Thanks. I added a %dir %{_datadir}/sysprof line to the spec, but I prefer to
maintain the explicit list of files, since there are really few of them.

I am waiting a bit for new comments/requests before updating the .spec and
src.rpm files.

Comment 7 Gianluca Sforna 2006-07-07 21:58:29 UTC
Forgot to mention this is my first package submission so I am seeking for a sponsor

Comment 8 Stewart Adam 2006-09-18 21:28:57 UTC
I'm not a reviewer, but I can help:
This is shown by rpmlin on the SRPMS...

E: sysprof configure-without-libdir-spec
A configure script is run without specifying the libdir. configure
options must be augmented with something like --libdir=%{_libdir}.

Comment 9 Gianluca Sforna 2006-09-29 23:12:44 UTC
New package addressing comment #1 (and comment #8...)

http://giallu.homelinux.org/fedora/sysprof.spec
http://giallu.homelinux.org/fedora/sysprof-1.0.3-2.src.rpm

Please note I have been sponsored, so now it only need an official review from
another regular contributor to have this piece of software in Fedora.


Comment 10 Parag AN(पराग) 2006-09-30 06:23:42 UTC
I suggest you to change SPEC name to sysprof-kmod-common.spec and resubmit
package. This is according to kernel module packaging guidelines. Thoug its
given that userlan package should 
Provides: %{name}- kmod-common = %{version}
it will be good idea to have similar name given to SPEC also.


Comment 11 Parag AN(पराग) 2006-09-30 06:38:13 UTC
Anyway i tried with latest uploaded SRPM link. Mock build is failed with error
checking for cplus_demangle_opname in -liberty... no
configure: error: libiberty is required to compile sysprof
error: Bad exit status from /var/tmp/rpm-tmp.63043 (%build)


Comment 12 Ville Skyttä 2006-09-30 10:58:43 UTC
(In reply to comment #10)
> I suggest you to change SPEC name to sysprof-kmod-common.spec and resubmit
> package. This is according to kernel module packaging guidelines.

Nope, see
http://fedoraproject.org/wiki/Packaging/KernelModules#head-164fc2703b23579d258b39d675a92643669507e0
In this case such a rename would sound plain wrong to me.

Provides: sysprof-kmod-common is lacking a version, though.

Comment 13 Parag AN(पराग) 2006-10-01 08:59:52 UTC
so i can give any name to userlan package SPEC file with following MUST right?
      - MUST: The package needs to require the belonging kernel-module with
something like 'Requires: %{name}- kmod = %{version}'

      - MUST: The package needs to provide %{name}-kmod-common with something
like 'Provides: %{name}- kmod-common = %{version}' or the name of the package
must be %{name}-kmod-common

Comment 14 Ville Skyttä 2006-10-01 16:15:40 UTC
(In reply to comment #13)
> so i can give any name to userlan package SPEC file

Within the usual package naming guidelines, yes.  (I assume you're talking 
about the userland package name, not only the specfile filename.)

>       - MUST: The package needs to require the belonging kernel-module with
> something like 'Requires: %{name}- kmod = %{version}'

">=" is better than "=" here - the kmod guidelines are in need of some 
updates.

Comment 15 Kevin Fenzi 2006-10-01 21:12:43 UTC
Parag: Seems you are reviewing this package, so I am changing the blocker to FE-
REVIEW. If thats not the case you can change it back to FE-NEW and reassign to 
nobody

Comment 16 Gianluca Sforna 2006-10-01 22:29:22 UTC
New package: this one builds correctly in mock (-devel)

http://giallu.homelinux.org/fedora/sysprof.spec
http://giallu.homelinux.org/fedora/sysprof-1.0.3-3.src.rpm

Ville: I am not sure why the Require should read ">=": I assumed kmod- and
kmod-common should always be updated in sync, while the ">=" seems to imply I
could update only the kmod- retaining the older kmod-common. Am I missing something?

Anyway, if you like I could also update the wiki page.

Comment 17 Ville Skyttä 2006-10-02 15:46:06 UTC
The exact details escape me at the moment, but while = would be closer to the 
intent, >= here IIRC helps in some upgrade scenarios.  It's also possible that 
comment 14 was a brainfart - thl, do you remember better?  Anyway, upgrading 
only the kmod package is prevented by the specfile emitted by kmodtool; it 
produces a >= dependency to the corresponding kmod-common in the kmod package.

Comment 18 Thorsten Leemhuis 2006-10-02 17:28:58 UTC
(In reply to comment #17)
> The exact details escape me at the moment, but while = would be closer to the 
> intent, >= here IIRC helps in some upgrade scenarios.  It's also possible that 
> comment 14 was a brainfart - thl, do you remember better?

/me scratches his head and tries to remember but fails for now, too

Anyway: I agree ">= here IIRC helps in some upgrade scenarios" and that should
be used.

Comment 19 Parag AN(पराग) 2006-10-03 09:36:52 UTC
Review:
+ package builds in mock (development i386).
+ source files match upstream.
8949fe32a073b84cb2abb7f9d608f755  sysprof-1.0.3.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.  License text included in package.
+ BuildRequires are proper.
+ %clean is present.
+ package installs properly
+ rpmlint is silent.
+ Provides: sysprof-kmod-common = 1.0.3
+ Requires: kmod-sysprof >= 1.0.3 
libatk-1.0.so.0 
libc.so.6 
libc.so.6(GLIBC_2.0) 
libc.so.6(GLIBC_2.1) 
libc.so.6(GLIBC_2.2.3) 
libc.so.6(GLIBC_2.3.4) 
libc.so.6(GLIBC_2.4) 
libcairo.so.2 
libdl.so.2 
libdl.so.2(GLIBC_2.0) 
libdl.so.2(GLIBC_2.1) 
libgdk-x11-2.0.so.0 
libgdk_pixbuf-2.0.so.0 
libglade-2.0.so.0 
libglib-2.0.so.0 
libgmodule-2.0.so.0 
libgobject-2.0.so.0 
libgthread-2.0.so.0 
libgtk-x11-2.0.so.0
libm.so.6 
libpango-1.0.so.0
libpangocairo-1.0.so.0
libpangoft2-1.0.so.0
libpthread.so.0 
libpthread.so.0(GLIBC_2.0) 
libxml2.so.2 
libz.so.1 
rtld(GNU_HASH)

+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets present.
+ documentation is small, so no -docs subpackage is necessary.
+ Not a GUI app

APPROVED.

Comment 20 Parag AN(पराग) 2006-10-03 09:43:00 UTC
Don't forget to close this NEXTRELASE when you have imported and built it.

Comment 21 Gianluca Sforna 2006-10-04 11:12:45 UTC
(In reply to comment #19)
> + Not a GUI app

???
it is actually a GNOME program... and this let me realize I did not add a
.desktop file (sorry, this was my first packaging attempt).

I will import a fixed spec. Thanks a lot

Comment 22 Parag AN(पराग) 2006-10-04 11:35:49 UTC
(In reply to comment #21)
> (In reply to comment #19)
> > + Not a GUI app
> 
> ???
> it is actually a GNOME program... and this let me realize I did not add a
> .desktop file (sorry, this was my first packaging attempt).
> 

Yup By mistake i wrote that. in fact that was from my template for review which
i just copied but not erased. I in fact check thet GUI APP already. That was
because i could not found any .desktop file.

> I will import a fixed spec. Thanks a lot

Yes .desktop file is not available in this package. Add it and then import your
package.


Comment 23 Gianluca Sforna 2006-10-08 08:51:56 UTC
Package imported and built


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