Bug 525909

Summary: Review Request: sysprof - A system-wide Linux profiler
Product: [Fedora] Fedora Reporter: Gianluca Sforna <giallu>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, pahan, sandmann, sandmann, susi.lehtola, walovaton
Target Milestone: ---Flags: susi.lehtola: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-09-28 22:53:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Gianluca Sforna 2009-09-26 23:36:50 UTC
Spec URL: http://giallu.fedorapeople.org/sysprof.spec
SRPM URL: http://giallu.fedorapeople.org/sysprof-1.1.2-1.fc12.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 Gianluca Sforna 2009-09-26 23:39:54 UTC
This package was in Fedora but moved to RPMFusion when kmods were banned. Now it does not need anymore a kernel module so I'd like to move it back to Fedora.

Old review:
https://bugzilla.redhat.com/show_bug.cgi?id=191743

Comment 2 Susi Lehtola 2009-09-27 09:04:17 UTC
rpmlint output:
sysprof.x86_64: W: non-conffile-in-etc /etc/udev/rules.d/60-sysprof.rules
sysprof.x86_64: W: file-not-utf8 /usr/share/doc/sysprof-1.1.2/README
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

- Convert README to UTF8 with the time stamp preserving version in
http://fedoraproject.org/wiki/Packaging_tricks#Convert_encoding_to_UTF-8

**

MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
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 be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A

MUST: A package must own all directories that it creates or require the package that owns the directory. OK
- Instead of 
 %dir %{_datadir}/sysprof
 %{_datadir}/sysprof/sysprof.glade
I'd use just
 %{_datadir}/sysprof/
KISS

MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK
- Add AUTHORS.

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A

MUST: Desktop files are installed properly. NEEDSWORK
- Drop the vendor tag from desktop-file-install. Or if you want to build for EPEL, just use --vendor="".

MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

**

Fix the issues before import to CVS, the package has been

APPROVED

Comment 3 Gianluca Sforna 2009-09-27 14:20:13 UTC
Thanks Jussi, really appreciated! I'm doing a mock build to double check I didn't break anything with the last changes before importing in CVS.

Comment 4 Gianluca Sforna 2009-09-27 14:28:39 UTC
Soeren, can you help me refine the description of the package since it does not depend anymore on the kmod?

additionally, are we still limited to the x86 archs?

Comment 5 Søren Sandmann Pedersen 2009-09-27 14:45:30 UTC
Something like this maybe:

    Sysprof is a sampling CPU profiler for Linux that collects accurate,
    high-precision data and provides efficient access to the sampled
    calltrees.

I think we are probably still limited to the x86 architectures for now. In principle, it should work as well as it does on x86-64, but since I'm not really happy with the state of userspace stacktracing on x86-64, I'd like to avoid claiming support for further architectures.

Comment 6 Susi Lehtola 2009-09-27 15:52:46 UTC
Hmm, on x86-64 I get

$ sysprof
Time to populate 468.337158
perf_counter_open: Function not implemented

when I click on 'Start'. I get the same thing using sudo. What's the problem?

Comment 7 Susi Lehtola 2009-09-27 16:06:25 UTC
By the way, I'd appreciate if you could review bug #524888, which is a rather simple Java package (ant specfile template) which I need in order to update jmol to the new release series.

Comment 8 Søren Sandmann Pedersen 2009-09-27 16:58:36 UTC
Jussi, is that on a 2.6.31 kernel?

Comment 9 Susi Lehtola 2009-09-27 17:11:41 UTC
No, on 2.6.30.5-43.fc11.x86_64

Comment 10 Søren Sandmann Pedersen 2009-09-27 17:17:32 UTC
It requires 2.6.31 to work, so there should be a dependency on that I guess.

Comment 11 Gianluca Sforna 2009-09-27 21:17:36 UTC
I didn't put an explicit dependency on 2.6.31 because I planned to import this just in rawhide, but looks like it's probably a good idea to add it...

Comment 12 Gianluca Sforna 2009-11-04 07:45:30 UTC
Package Change Request
======================
Package Name: sysprof
InitialCC: sandmann.dk

Upstream would like to CCed on sysprof bugs using this address

Comment 13 Kevin Fenzi 2009-11-06 20:47:37 UTC
We cannot add arbitrary initialcc addresses. ;( 

If they sign up for a fas account we can add them then. 

Feel free to reset the flag when/if they get a fedora account.

Comment 14 Søren Sandmann Pedersen 2009-11-06 21:49:55 UTC
Well, sandmann.dk is the same person as sandmann. Is it possible to add more than one email address to an fas account?

I'm not really interested in getting more bug spam to my redhat account.

Comment 15 Søren Sandmann Pedersen 2009-11-06 21:50:42 UTC
Also, sandmann.dk does have a Red Hat bugzilla account, just not an fas account.

Comment 16 William Lovaton 2009-11-08 18:05:49 UTC
(In reply to comment #8)
> Jussi, is that on a 2.6.31 kernel?  

Soren, I am running Fedora 12 Rawhide with the latest updates running a 2.6.31 kernel and I get the following when I press the play button in sysprof as root:

[root@localhost ~]# sysprof
Time to populate 161.459000
perf_counter_open: No such device

The kernel I am using is: 2.6.31.5-122.fc12.i686.PAE, maybe my CPU does not support perf counters, how can I check for that?  My CPU is an Intel Core Duo T2400  @ 1.83GHz

Thanks for your help.

Comment 17 Søren Sandmann Pedersen 2009-11-09 20:02:47 UTC
Which version of the sysprof package is that?

Also, if you can open a separate bug (and cc sandmann.dk), that would be appreciated. Please attach /proc/cpuinfo. 

Thanks.

Comment 18 Gianluca Sforna 2013-02-12 18:04:03 UTC
Package Change Request
======================
Package Name: sysprof
New Branches: el6
Owners: giallu lkundrak

Comment 19 Gwyn Ciesla 2013-02-12 18:21:48 UTC
Git done (by process-git-requests).