Bug 191745 - Review Request: sysprof-kmod - kernel module for the sysprof profiler
Review Request: sysprof-kmod - kernel module for the sysprof 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:
Blocks: FE-ACCEPT 191743 FE-KMOD-APPROVED
  Show dependency treegraph
 
Reported: 2006-05-15 11:37 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:53:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Gianluca Sforna 2006-05-15 11:37:24 EDT
Spec URL: http://giallu.interfree.it/fedora/sysprof-kmod.spec
SRPM URL: http://giallu.interfree.it/fedora/sysprof-kmod-1.0.2-1.2.6.16_1.2111_FC5.src.rpm

Description:
This is the kernel module needed for sysprof operations
Comment 1 Gianluca Sforna 2006-05-16 04:34:00 EDT
I updated the spec to remove some diffs from the standard template:

Spec URL: http://giallu.interfree.it/fedora/sysprof-kmod.spec
SRPM URL:
http://giallu.interfree.it/fedora/sysprof-kmod-1.0.2-2.2.6.16_1.2111_FC5.src.rpm
Comment 2 Parag AN(पराग) 2006-06-02 00:48:16 EDT
Some comments:-
rpmlint sysprof-kmod-1.0.2-2.2.6.16_1.2111_FC5.src.rpm gives output as
W: sysprof-kmod summary-not-capitalized sysprof kernel module
W: sysprof-kmod strange-permission kmodtool 0666
E: sysprof-kmod configure-without-libdir-spec
Comment 3 Gianluca Sforna 2006-06-02 06:38:00 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 4 Gianluca Sforna 2006-06-22 04:46:17 EDT
Here I am again. I updated to 1.0.3 and silenced the first warning.

Permissions on kmodtool seems to be the same as other kmod packages, so I think
I will leave it alone.

The error is a false positive, since I completely skip configure (see comment in
the spec) for the kmod build.

Spec URL: http://giallu.interfree.it/fedora/sysprof-kmod.spec
SRPM URL:
http://giallu.interfree.it/fedora/sysprof-kmod-1.0.3-1.2.6.16_1.2133_FC5.src.rpm
Comment 5 Jason Tibbitts 2006-07-20 15:09:41 EDT
According to http://fedoraproject.org/wiki/Packaging/KernelModules you will need
to provide certain information to the Extras steering committee, in particular:

A publishable explanation from the author(s) why the module is not merged with
the mainline kernel yet and when it's planned to get merged.

Could you please provide this information so that the committee can consider
this module?
Comment 6 Gianluca Sforna 2006-07-20 17:06:55 EDT
(In reply to comment #5)
> According to http://fedoraproject.org/wiki/Packaging/KernelModules you will need
> to provide certain information to the Extras steering committee, in particular:
> 
> A publishable explanation from the author(s) why the module is not merged with
> the mainline kernel yet and when it's planned to get merged.
> 
> Could you please provide this information so that the committee can consider
> this module?

OK, I asked the author to provide that info. 
My opinion is that this particular package does not really fall into the picture
covered by the guidelines: this is not a driver, it's a (small) module which is
exposing CPU sampling data for the GUI to show.
Of course, it would be easier if I had only to package the GUI...
Comment 7 Thorsten Leemhuis 2006-07-21 02:05:01 EDT
(In reply to comment #6)
> My opinion is that this particular package does not really fall into the picture
> covered by the guidelines: this is not a driver, it's a (small) module which is
> exposing CPU sampling data for the GUI to show.

The main reason for this extras-work is that we want to tell everyone: "Get your
shit into the kernel soon, that the proper place for it. kmod are only a interim
solution" (sorry, sounds a bit harsh, but I think these words describe the
situation well).

In the begining we even considered rules like "kmods are only allowed for 18
months in Extras -- that should be enough time to get stuff upstream (e.g. into
the kernel)"
Comment 8 Gianluca Sforna 2006-07-29 19:09:37 EDT
This is the response I got from the author which is, BTW, a Red Hat employee
(though sysprof is not a Red Hat project):

<snippet from original email>
The reason I didn't reply is that the issue with the sysprof kernel
module is a little tricky. Basically, the kernel developers I have
talked to are saying that sysprof overlaps with the oprofile module,
so I haven't attempted getting it upstream.

One possibility is that I port sysprof to use the oprofile module, but
there are some issues with that as well.
<end snippet>

Moreover, I was told it already exists a proof of concept of the latter solution
(using the oprofile module), so chances are the kmod could go away in a
reasonable timeframe.
Comment 9 Thorsten Leemhuis 2006-08-03 14:53:25 EDT
kmod was allowed by FESCo (review pending)
Comment 10 Parag AN(पराग) 2006-09-30 01:18:19 EDT
Will Do Full Review today
Comment 11 Parag AN(पराग) 2006-09-30 02:10:57 EDT
I think kernels above 2.6.16 do not require any smp kernels so either SPEC need
to  have some kversion checking or remove smp build lines.
Here is Review for your package which i built for 2.6.18 which do not have smp
support so i remove those lines form SPEC

Package Built successfully for i386 FC6 development
Rpmlint on SRPM
W: sysprof-kmod strange-permission kmodtool 0666
A file that you listed to include in your package has strange
permissions. Usually, a file should have 0644 permissions.
=> This is well known rpmlint Warning on kmod packages. Forget it.

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

I am not able to found any ./configure call in SPEC. Why this Error is coming 
Gianluca, any idea?


Rpmlint on RPM
W: kmod-sysprof summary-not-capitalized sysprof kernel module(s)
Summary doesn't begin with a capital letter.
=> This is well known rpmlint Warning on kmod packages. Forget it.

W: kmod-sysprof unstripped-binary-or-object
/lib/modules/2.6.18-1.2699.fc6/extra/sysprof/sysprof-module.ko
=> This is well known rpmlint Warning on kmod packages. Forget it.

W: kmod-sysprof no-documentation
The package contains no documentation (README, doc, etc).
You have to include documentation files.
=>Ignore this


Otherwise package looks ok.
Comment 12 Parag AN(पराग) 2006-09-30 02:17:51 EDT
I am not able to find sysprof-kmod-common package. When i tried to install
sysprof-kmod, i got following output.

error: Failed dependencies: 
        sysprof-kmod-common = 1.0.3 is needed by
kmod-sysprof-1.0.3-1.2.6.18_1.2699.fc6.i686


In this case i suggest you to modify kmodtool to remove dependency of
sysprof-kmod-common for sysprof-kmod and also ask same on Fedora-extras mailing
list.
Comment 13 Gianluca Sforna 2006-09-30 18:15:39 EDT
(In reply to comment #11)
> I think kernels above 2.6.16 do not require any smp kernels so either SPEC need
> to  have some kversion checking or remove smp build lines.

Though they are not required, we are still building them at least until
2.6.17-1.2187, which I am happily running at home. 
I hope there will be an announcement when they become history...


> E: sysprof-kmod configure-without-libdir-spec
> A configure script is run without specifying the libdir. configure
> options must be augmented with something like --libdir=%{_libdir}.
> 
> I am not able to found any ./configure call in SPEC. Why this Error is coming 
> Gianluca, any idea?

it seems rpmlint is not ignoring the comment line which says:

# ./configure breaks in mock builds (missing BRs), so we create here

and spit that warning... I think I can easily fix it (or maybe just submit a bug
report against rpmlint :P )


(In reply to comment #12)
> I am not able to find sysprof-kmod-common package. When i tried to install
> sysprof-kmod, i got following output.
> 

sysprof-kmod-common is provided by sysprof-common

Comment 14 Kevin Fenzi 2006-10-01 17:10:59 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 15 Parag AN(पराग) 2006-10-03 02:05:54 EDT
any update on libdir rpmlint warning?
Once you update the new package i will approve this package.
Comment 16 Gianluca Sforna 2006-10-03 03:10:19 EDT
(In reply to comment #15)
> any update on libdir rpmlint warning?
> Once you update the new package i will approve this package.

As I said, in comment #13, the warning is a false positive, so it does not seem
to be a showstopper, is it?
Anyway, if you like a cleaner rpmlint output, I think you can approve the
package with the provision I import it with the warning fixed ( which I will
happily do... ).
Comment 17 Parag AN(पराग) 2006-10-03 03:19:16 EDT
OK.
I did review in comment #11 and with rpmlint warning which is false one (see
comment #16), I am going to approve this package.

APPROVED.
Comment 18 Parag AN(पराग) 2006-10-03 03:25:11 EDT
Don't forget to close this NEXTRELASE when you have imported and built it.
Comment 19 Thorsten Leemhuis 2006-10-03 07:31:56 EDT
(In reply to comment #17)
> APPROVED.

thx for review. Get's also an APPROVED from me as (kmod packages need a second
approval from someone that's faimilar with kernel module packaging -- see
http://www.fedoraproject.org/wiki/Packaging/KernelModules )

Small note: The header that make sure that modules for all shipped kernels get
builds needs some adjustments for both devel (xen and smp stuff) and FC5 (no
i586 smp). Please do that after importing. If you need some hints take a look at
the em8300-kmod package (or ask on IRC for help)
Comment 20 Gianluca Sforna 2006-10-08 04:53:09 EDT
Thanks Thorsten, I used that as a template and the build was (mostly) flawless.

Closing

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