Bug 191743 - Review Request: sysprof - a sampling CPU profiler
Review Request: sysprof - a sampling CPU profiler
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On: 191745
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-15 11:32 EDT by Gianluca Sforna
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-08 04:51:56 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Gianluca Sforna 2006-05-15 11:32:38 EDT
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 00:43:53 EDT
Some Comments:-
rpmling gives error as
E: sysprof configure-without-libdir-spec
Comment 2 Gianluca Sforna 2006-06-02 06:36:48 EDT
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 04:47:40 EDT
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 15:42:59 EDT
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 16:39:45 EDT
%{_datadir}/%{name} will be even better, similar package the binary as
%{_bindir}/%{name}
Comment 6 Gianluca Sforna 2006-06-28 04:08:34 EDT
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 17:58:29 EDT
Forgot to mention this is my first package submission so I am seeking for a sponsor
Comment 8 Stewart Adam 2006-09-18 17:28:57 EDT
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 19:12:44 EDT
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 02:23:42 EDT
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 02:38:13 EDT
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 06:58:43 EDT
(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 04:59:52 EDT
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 12:15:40 EDT
(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 17:12:43 EDT
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@fedorproject.org
Comment 16 Gianluca Sforna 2006-10-01 18:29:22 EDT
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 11:46:06 EDT
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 13:28:58 EDT
(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 05:36:52 EDT
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 05:43:00 EDT
Don't forget to close this NEXTRELASE when you have imported and built it.
Comment 21 Gianluca Sforna 2006-10-04 07:12:45 EDT
(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 07:35:49 EDT
(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 04:51:56 EDT
Package imported and built

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