Bug 210007 - Review Request: libtune - standard API to access the kernel tunables
Summary: Review Request: libtune - standard API to access the kernel tunables
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Konrad Rzeszutek Wilk
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW 184824 240441
TreeView+ depends on / blocked
 
Reported: 2006-10-09 14:44 UTC by Nadia Derbey
Modified: 2008-04-04 04:19 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-03-10 14:28:16 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Nadia Derbey 2006-10-09 14:44:27 UTC
Spec URL: <http://libtune.sourceforge.net/specfiles/libtune.spec>
SRPM URL: <http://prdownloads.sourceforge.net/libtune/libtune-0.10-1.src.rpm?download>
Description:
libtune (Tunables library) is an API that helps accessing the kernel tunables and system information in a "standard way".
The actual pseudo-files that contain the tunables values are hidden to the developer, making any binary built on top of this API completely portable across kernel releases or across distro families.

The libtune library provides the following features:
1) get the value of a tunable, either stored as a part of a pseudo-file or as a complete pseudo-file
2) set the value of a writable tunable
3) locate a tunable (in terms of the associated pseudo-file)
4) get a help string for a tunable

libtune is different from sysctl in that:
1) there is no need to know where the underlying pseudo-file is located: the tunables are manipulated via key words rather than their actual file location.
2) it is possible to get or set a value that is a part of pseudo-file (eg it is possible to get or set semmns value that is part of /proc/sys/kernel/sem).

Today, there is a limited set of supported kernel releases, distros, as well as a limited set of supported tunables.

Support for new kernel releases, distros, families or tunables can easily be achieved thanks to a set of scripts that are delivered with the libtune API.

Comment 1 Josh Boyer 2006-10-15 20:24:53 UTC
A few comments:

1) The package fails to build on rawhide due to some unpackaged files:

Checking for unpackaged file(s): /usr/lib/rpm/check-files
/var/tmp/libtune-0.10-1-root-jwboyer
error: Installed (but unpackaged) file(s) found:
   /var/lib/tundb/tundb_distro
   /var/lib/tundb/tundb_kernel


RPM build errors:
    Installed (but unpackaged) file(s) found:
   /var/lib/tundb/tundb_distro
   /var/lib/tundb/tundb_kernel

2) While a generic spec file is probably a great idea for the upstream project,
having tunables-data and tunables-distro packages for kernels that aren't part
of the current Fedora distro (SLES, RHEL, older FC) doesn't make much sense from
and Extras viewpoint.

Is it possible to generate tunables-data and tunables-distro packages at RPM
build time?  If so, that would allow a very generic spec file that would work
for any kernel or distro the RPMs were built on.

3) If the tunables-data and tunables-distro packages are very dependent upon a
specific version of a kernel, they should actually Requires: that particular
kernel version.  This might quickly become cumbersome for the development tree,
as new kernels are released on almost a daily basis.

4) The location of the of the libtune_* scripts does not comply with the FHS. 
/var/lib/ is to be used for data and state information.  The scripts should
probably go into /usr/share/libtune/bin/ or /usr/share/tundb/bin/.  The same
applies to the doc directory.

Comment 2 Nadia Derbey 2006-10-16 12:15:37 UTC
1st of all, thanks for taking some time to review this package!

Issue #1:
tundb_kernel and tundb_distro are symbolic links to the kernel / distro
databases that have actually been RPM installed. During the %build and %install
phases all the possible databases are built, and we still don't know which one
of them will actually be downloaded. That's why I postponed the symlink in the
post install. I'll look for a solution to this issue.

Issue #2:
Sure, it would be possible to generate the appropriate packages at RPM build
time. But you should have to build the package on a victim that has exactly the
same kernel release / distro as the target one. While with the current build
process, we use cross-compiling and we are completely independent of the system
we are building on.
But on the other hand, I like your proposal, since it could be a solution to
issue # 1!

Issue #3:
Do not agree: it's true that a dependency exists. But you should be allowed to
install tunables databases that do not necessarily match you current kernel /
distro. This is because you should be able to boot the same machine using
different kernel/distro releases:
if you are using, say an FC4 and want to reboot your machine in FC5, if you have
the databases for both releases, the reboot will be "transparent" from the
tunables databases point.
That's why I didn't use the Requires:
But if you think I'm wrong, only using the 1st 3 indices of the kernel release
should be enough (those are not changing too fast, are they?)

Issue #4:
To be fixed.
I must admit that I was not convinced that this was the best place for these
files. The only reason to put them there where to have all the files centralized
somewhere!

I'll update the bug as soon as I'm done with the fixes.



Comment 3 Konrad Rzeszutek 2006-10-18 20:56:39 UTC
1).
You are not using the Release tag everywhere.

For example you do use it for building a tree, but you are not including it
in the source. 

Source:         %{name}-%{version}.tar.gz
BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Are you never going to increment the -release in the source code? If so, then
it is OK to leave as is.

2). 

%description -n tunables-data-SLES9-2.6.5
This package supplies the tunables database for SuSE Linux Enterprise Server
9's 2.6.5
....
%description -n .. FC2/FC3/etc

This package is for RHEL5/FC5,6. There is no need for these descriptions.
Remove them and leave the one for FC5,6.
3). 
The spec file does not use %{optflags}, so nothing is compiled with the typical
flags a Fedora build would use. For example - -j64 or optimizations.

One way to do this is via export CFLAGS, as such:
export CFLAGS="%optflags}"
make %{?_smp_mpflags}

4).
%{_libdir}/libtune.la
%{_libdir}/libtune.so.0
%{_libdir}/libtune.so.0.0.0

you can glob them using the *

5).

%post devel
/sbin/chkconfig --add chtunedb
/sbin/ldconfig

You should have 'ldconfig' before running chkconfig.

6). 
%post
/sbin/ldconfig -n %{_libdir}

The -n %{_libdir} is not needed.

7).
%preun devel
/sbin/chkconfig --del chtunedb

you probably should also check if the service is running (or just asssume it is
) before deleting it.

/sbin/service chtunedb stop > /dev/null 2>&1
/sbin/chkconfig --del chtunedb

Would do nicely.


8).
you are using 'ldconfig' and 'chkconfig' programs. You should specify in the
Requiere sections:


Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig
... and so on.

9)
Run 'rpmlint' on your SRPM. Fix all of the errors.

Comment 4 Michael Schwendt 2006-10-19 11:54:10 UTC
> %post
> /sbin/ldconfig -n %{_libdir}
> 
> The -n %{_libdir} is not needed.

More than that: it is _wrong_ because it doesn't update the cache.


Comment 5 Nadia Derbey 2006-10-19 14:27:55 UTC
(In reply to comment #3)
> 1).
> You are not using the Release tag everywhere.

It's true that I used to "play" with the version numbers, but reading the
"naming guidelines" I'm realizing that using the release tag may be sometimes
more appropriate!

to be fixed


> 
> 2). 
> 
> %description -n tunables-data-SLES9-2.6.5
> This package supplies the tunables database for SuSE Linux Enterprise Server
> 9's 2.6.5
> ....
> %description -n .. FC2/FC3/etc
> 
> This package is for RHEL5/FC5,6. There is no need for these descriptions.
> Remove them and leave the one for FC5,6.

The only thing I regret in removing the older releases is that having FC3 and
FC4 used to show how the library can be useful, with "exec-shield-randomize"
moving to "randomize_va_space" from FC3 to FC4 :-(

To be fixed

> 3). %{optflags} / %{?_smp_mflags}

To be fixed

> 
> 4).
> %{_libdir}/libtune.la
> %{_libdir}/libtune.so.0
> %{_libdir}/libtune.so.0.0.0
> 
> you can glob them using the *
> 

Doing this I would get libtune.a and libtune.so too, while these 2 files have
to go in the -devel package. Am I wrong?

> 5).
> 
> %post devel
> /sbin/chkconfig --add chtunedb
> /sbin/ldconfig
> 
> You should have 'ldconfig' before running chkconfig.

The service to be added (chtundb) doesn't use the library.
So I think that using that ordering or the one you propose is equivalent
(or may be I missed something?)

> 
> 6). ldconfig -n

To be fixed

> 
> 7).
> %preun devel
> /sbin/chkconfig --del chtunedb
> 
> you probably should also check if the service is running (or just asssume it is
> ) before deleting it.
> 
> /sbin/service chtunedb stop > /dev/null 2>&1
> /sbin/chkconfig --del chtunedb
> 
> Would do nicely.
> 

Actually, chtundb is run once at boot time: it just checks that the tunables
databases are coherent with the running kernel, then it leaves. That's why I 
think there is no need to stop it before removing it. But if you think it
is better to do it the "standard way", I can fix this anyway.

> 
> 8). Missing Requires:

To be fixed

> 
> 9)
> Run 'rpmlint' on your SRPM. Fix all of the errors.

I used and old version of rpmlint that was silent...
Sorry!

To be fixed



Comment 6 Konrad Rzeszutek 2006-10-19 16:07:38 UTC
> The only thing I regret in removing the older releases is that having FC3 and
> FC4 used to show how the library can be useful, with "exec-shield-randomize"
> moving to "randomize_va_space" from FC3 to FC4 :-(

<grin> 

> > 4).
> > %{_libdir}/libtune.la
> > %{_libdir}/libtune.so.0
> > %{_libdir}/libtune.so.0.0.0
> > 
> > you can glob them using the *
> > 
> 
> Doing this I would get libtune.a and libtune.so too, while these 2 files have
> to go in the -devel package. Am I wrong?
> 

Before I address your question, I just realized that the version of the libtune
libariries is 0.0.0. Should it be 0.10.1 ?

The right globbing would be:
%{_libdir}/*.so.*

while for the devel:

%{_libdir}/libtune.so
%{_libdir}/libtune.a

(which you have already) would work fine.


> > 5).
> > 
> > %post devel
> > /sbin/chkconfig --add chtunedb
> > /sbin/ldconfig
> > 
> > You should have 'ldconfig' before running chkconfig.
> 
> The service to be added (chtundb) doesn't use the library.
> So I think that using that ordering or the one you propose is equivalent
> (or may be I missed something?)
> 
If it does not use the library, you are clear.

> > 7).
> > %preun devel
> > /sbin/chkconfig --del chtunedb
> > 
> > you probably should also check if the service is running (or just asssume it is
> > ) before deleting it.
> > 
> > /sbin/service chtunedb stop > /dev/null 2>&1
> > /sbin/chkconfig --del chtunedb
> > 
> > Would do nicely.
> > 
> 
> Actually, chtundb is run once at boot time: it just checks that the tunables
> databases are coherent with the running kernel, then it leaves. That's why I 
> think there is no need to stop it before removing it. But if you think it
> is better to do it the "standard way", I can fix this anyway.

That is OK then. You can put a comment in the spec file saying that it is not
needed since it is _not_ a daemon.

> > 9)
> > Run 'rpmlint' on your SRPM. Fix all of the errors.
> 
> I used and old version of rpmlint that was silent...
> Sorry!
> 
> To be fixed
> 
> 


9). correction:

rpmlint -v libtune-0.10-1.src.rpm

I meant fixing all warnings and any errors if they are there ( I don't see
errors, but there were a bit of warnings).

Comment 7 Nadia Derbey 2006-10-20 10:14:32 UTC
(In reply to comment #6)

> > > 4).
> > > %{_libdir}/libtune.la
> > > %{_libdir}/libtune.so.0
> > > %{_libdir}/libtune.so.0.0.0
> > > 
> > > you can glob them using the *
> > > 
> > 
> > Doing this I would get libtune.a and libtune.so too, while these 2 files have
> > to go in the -devel package. Am I wrong?
> > 
> 
> Before I address your question, I just realized that the version of the libtune
> libariries is 0.0.0. Should it be 0.10.1 ?
> 

I thought library version number shouldn't necessarily be the same as the
project one? (see http://sources.redhat.com/autobook/autobook/autobook_91.html)

I'd rather say it should be 0.10.0



Comment 8 Ralf Corsepius 2006-10-20 11:28:41 UTC
(In reply to comment #7)
> (In reply to comment #6)

> I thought library version number shouldn't necessarily be the same as the
> project one? (see http://sources.redhat.com/autobook/autobook/autobook_91.html)
Right. A library's version is not related to a package's version.

> I'd rather say it should be 0.10.0
Chosing libraries versions (more precisely SONAMES) should be upstream's task,
not yours.

Comment 9 Nadia Derbey 2006-10-20 13:17:20 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> 
> > I thought library version number shouldn't necessarily be the same as the
> > project one? (see http://sources.redhat.com/autobook/autobook/autobook_91.html)
> Right. A library's version is not related to a package's version.
> 
> > I'd rather say it should be 0.10.0
> Chosing libraries versions (more precisely SONAMES) should be upstream's task,
> not yours.


I am the one who's taking care of the library development ;-)

Comment 10 Nadia Derbey 2006-10-23 14:22:49 UTC
Uploaded the following new files:

http://libtune.sourceforge.net/specfiles/FE2/libtune.spec
http://prdownloads.sourceforge.net/libtune/libtune-0.10-2.src.rpm?download

All the issues that have been pointed out should now be fixed, except the following:
Comment #3 - issue #8 (from Konrad):
ldconfig is automatically added as a Requires: by rpmbuild, so I didn't add it
as a Requires(post): / Requires(postun):

Comment 11 Konrad Rzeszutek 2006-10-23 14:32:23 UTC
> > I'd rather say it should be 0.10.0
> Chosing libraries versions (more precisely SONAMES) should be upstream's task,
> not yours.

Sure. Any number, but 0.0.0, looks good.



Comment 12 Konrad Rzeszutek 2006-11-01 14:54:59 UTC
1).
Licenses. The spec files says GPL v2.1, the tests,scripts, and templates usE GPL
v2.0 and the lib uses LGPG.

There are no GPL v2.1, there is LGPG v2.1, which is what I think you meant for
the spec file, but it states: "
  This library is free software; you can redistribute it and/or
#    modify it under the terms of the GNU General Public
#    License as published by the Free Software Foundation; either
#    version 2.1 of the License, or (at your option) any later version.
"

The field: License has only LGPL, but this has GPL as well (in -devel) package.

2). rpmlint:

[konrad@dyn448114 i386]$ rpmlint -v libtune-devel-0.10-2.i386.rpm
I: libtune-devel checking
W: libtune-devel service-default-enabled /etc/rc.d/init.d/chtunedb
W: libtune-devel incoherent-init-script-name chtunedb

I would consider the two above to be harmless and intentional.

3). It fails compilation on 64-bit (this FC5-x86_64)

make: Leaving directory `/builddir/build/BUILD/libtune-0.10-2/templates'
+ db/libtuncleandb
/var/tmp/libtune-0.10-2-root-brewbuilder//usr/share/libtune/tundb_kernel
/var/tmp/libtune-0.10-2-root-brewbuilder//usr/share/libtune/tundb_distro
+ /usr/lib/rpm/find-debuginfo.sh /builddir/build/BUILD/libtune-0.10-2
0 blocks
find: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib/debug: No such file or
directory
+ /usr/lib/rpm/redhat/brp-compress
+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
+ /usr/lib/rpm/brp-python-bytecompile
Processing files: libtune-0.10-2
error: File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.la
error: File not found by glob:
/var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.so.*
Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.80952
+ umask 022
+ cd /builddir/build/BUILD
+ cd libtune-0.10-2
+ DOCDIR=/var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-0.10
+ export DOCDIR
+ rm -rf /var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-0.10
+ /bin/mkdir -p /var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-0.10
+ cp -pr COPYING lib/LGPL ChangeLog
/var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-0.10
+ exit 0
Processing files: libtune-devel-0.10-2
error: File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.a
error: File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.so
Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.80952
+ umask 022
+ cd /builddir/build/BUILD
+ cd libtune-0.10-2
+ DOCDIR=/var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-devel-0.10
+ export DOCDIR
+ rm -rf /var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-devel-0.10
+ /bin/mkdir -p
/var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-devel-0.10
+ cp -pr templates/GPL
/var/tmp/libtune-0.10-2-root-brewbuilder/usr/share/doc/libtune-devel-0.10
+ exit 0
Processing files: tunables-data-2.6.16-0.10-2
Provides: tunables-generic-data = 0.10
Requires(interp): /bin/sh /bin/sh
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(VersionedDependencies) <= 3.0.3-1
Requires(post): /bin/sh
Requires(preun): /bin/sh
Requires: /bin/sh
Processing files: tunables-data-2.6.18-0.10-2
Provides: tunables-generic-data = 0.10
Requires(interp): /bin/sh /bin/sh
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(VersionedDependencies) <= 3.0.3-1
Requires(post): /bin/sh
Requires(preun): /bin/sh
Requires: /bin/sh
Processing files: tunables-data-FC5-2.6.16-0.10-2
Provides: tunables-distro-data = 0.10
Requires(interp): /bin/sh /bin/sh
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(VersionedDependencies) <= 3.0.3-1
Requires(post): /bin/sh
Requires(preun): /bin/sh
Requires: tunables-data-2.6.16 = 0.10
Processing files: tunables-data-FC6-2.6.18-0.10-2
Provides: tunables-distro-data = 0.10
Requires(interp): /bin/sh /bin/sh
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(VersionedDependencies) <= 3.0.3-1
Requires(post): /bin/sh
Requires(preun): /bin/sh
Requires: tunables-data-2.6.18 = 0.10
Processing files: libtune-debuginfo-0.10-2


RPM build errors:
    File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.la
    File not found by glob:
/var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.so.*
    File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.a
    File not found: /var/tmp/libtune-0.10-2-root-brewbuilder/usr/lib64/libtune.so



Comment 13 Nadia Derbey 2006-11-09 14:10:20 UTC
Konrad, sorry for the delay but I have been out of my office for almost 2 weeks!

(In reply to comment #12)
> 1).
> Licenses. The spec files says GPL v2.1, the tests,scripts, and templates usE GPL
> v2.0 and the lib uses LGPG.
> 

Fixed the License: field in the spec file + many source files where the license
text was completely wrong (it was a big mess and don't know where I got that
text from :-( ).

> 2). rpmlint:
> 
> [konrad@dyn448114 i386]$ rpmlint -v libtune-devel-0.10-2.i386.rpm
> I: libtune-devel checking
> W: libtune-devel service-default-enabled /etc/rc.d/init.d/chtunedb
> W: libtune-devel incoherent-init-script-name chtunedb
> 
> I would consider the two above to be harmless and intentional.

Right: 
W1: the service has to be enabled by default
W2: I thought that calling that binary libtune or libtuned was not meaningful enough

> 
> 3). It fails compilation on 64-bit (this FC5-x86_64)
> 

Fixed

New links are:
Spec URL: http://libtune.sourceforge.net/specfiles/FE3/libtune.spec
SRPM URL: http://prdownloads.sourceforge.net/libtune/libtune-0.10-3.src.rpm?download

Comment 14 Konrad Rzeszutek 2006-11-20 20:01:41 UTC
* I think the need to have in the spec file the build against different kernels
and releases is not needed.

The reason for this is that when I build the package for FC6, I end up with:
[konrad@dhcp83-148 task_394874]$ ls
libtune-0.10-3.i386.rpm                tunables-data-2.6.16-0.10-3.ppc.rpm
libtune-0.10-3.ia64.rpm                tunables-data-2.6.16-0.10-3.s390.rpm
libtune-0.10-3.ppc64.rpm               tunables-data-2.6.16-0.10-3.s390x.rpm
libtune-0.10-3.ppc.rpm                 tunables-data-2.6.16-0.10-3.x86_64.rpm
libtune-0.10-3.s390.rpm                tunables-data-2.6.18-0.10-3.i386.rpm
libtune-0.10-3.s390x.rpm               tunables-data-2.6.18-0.10-3.ia64.rpm
libtune-0.10-3.src.rpm                 tunables-data-2.6.18-0.10-3.ppc64.rpm
libtune-0.10-3.x86_64.rpm              tunables-data-2.6.18-0.10-3.ppc.rpm
libtune-debuginfo-0.10-3.i386.rpm      tunables-data-2.6.18-0.10-3.s390.rpm
libtune-debuginfo-0.10-3.ia64.rpm      tunables-data-2.6.18-0.10-3.s390x.rpm
libtune-debuginfo-0.10-3.ppc64.rpm     tunables-data-2.6.18-0.10-3.x86_64.rpm
libtune-debuginfo-0.10-3.ppc.rpm       tunables-data-FC5-2.6.16-0.10-3.i386.rpm
libtune-debuginfo-0.10-3.s390.rpm      tunables-data-FC5-2.6.16-0.10-3.ia64.rpm
libtune-debuginfo-0.10-3.s390x.rpm     tunables-data-FC5-2.6.16-0.10-3.ppc64.rpm
libtune-debuginfo-0.10-3.x86_64.rpm    tunables-data-FC5-2.6.16-0.10-3.ppc.rpm
libtune-devel-0.10-3.i386.rpm          tunables-data-FC5-2.6.16-0.10-3.s390.rpm
libtune-devel-0.10-3.ia64.rpm          tunables-data-FC5-2.6.16-0.10-3.s390x.rpm
libtune-devel-0.10-3.ppc64.rpm         tunables-data-FC5-2.6.16-0.10-3.x86_64.rpm
libtune-devel-0.10-3.ppc.rpm           tunables-data-FC6-2.6.18-0.10-3.i386.rpm
libtune-devel-0.10-3.s390.rpm          tunables-data-FC6-2.6.18-0.10-3.ia64.rpm
libtune-devel-0.10-3.s390x.rpm         tunables-data-FC6-2.6.18-0.10-3.ppc64.rpm
libtune-devel-0.10-3.x86_64.rpm        tunables-data-FC6-2.6.18-0.10-3.ppc.rpm
logs                                   tunables-data-FC6-2.6.18-0.10-3.s390.rpm
tunables-data-2.6.16-0.10-3.i386.rpm   tunables-data-FC6-2.6.18-0.10-3.s390x.rpm
tunables-data-2.6.16-0.10-3.ia64.rpm   tunables-data-FC6-2.6.18-0.10-3.x86_64.rpm
tunables-data-2.6.16-0.10-3.ppc64.rpm


It is not neccessary to build all those extra tunables-data for different
kernels and releases when I am building just against FC6. The end result should
be just one "tunables-data-<something>.fc6.<platform>" for each platform.
Otherwise the CD with FC6 ends up with RPMs for FC5, which is clearly not needed.

I think it would be better to remove the big loop, use the %dist as part of the
devel package. The kernel number component should be automaticly extracted -
this should be possible if you make the BuildReq depend on kernel-devel. Perhaps
it is not even neccesary to put the kernel version in the name of the RPM, but
just have it as a dependency? 

That would also remove lots hard-coded 2.6.16/2.6.18/FC5/FC6 strings used in the
names.

* The Release tag in the spec file. It should have "%{dist}" post-fix.

Comment 15 Nadia Derbey 2006-11-21 14:44:57 UTC
(In reply to comment #14)
> * I think the need to have in the spec file the build against different kernels
> and releases is not needed.
> 

I agree with you that this makes quite a lot of rpm's when building for the
various platforms.

But if we keep only the FC6 specific part, we will loose the benefits of the
library: it is supposed to ensure portability across kernel releases / distros
for what concerns the kernel tunables. To ensure that portability, a 'tunable
database' is needed for each supported kernel / distro. Delivering only the FC6
specific part will prevent a client with both the FC5 and FC6 installed on its
machines to use the library.

One thing that can be done is to deliver all the supported kernel databases in a
single sub-package, and all the supported distro databases in a single sub-package.

But again, this can be a problem, since somebody downloading the kernel database
sub-package may potentially have unneeded data installed on its machine.
Actually that's why we have decided to split each sub-package: in order to let a
user choose what to download depending on what is installed on its machine.





Comment 16 Konrad Rzeszutek 2006-11-21 18:19:06 UTC
I think I was not specific enough. The thought is that it would be generic
enough that when build against a specific release - it would create an RPM
against that one.

If I built under FC5, it would build a FC5-tunable package for the kernel that
is in FC5. and not a FC6 one.

If I built it under FC6, it would build a FC-6-tunable package for the kernel
that is in FC6, and not a FC5 one.

and so on.

This would have to be generic enough so that when FC7 comes, there is no need to
alter the SPEC file. Or if it has to be done - just the minum if possible.


In regards to your comment about "portability across distros" is a moot point.
This RPM/SPEC is specific for Fedora Core and RHEL. This spec file will not be
used by Novell/SuSE. Novell will have require their own .spec file, with
different fields.


I am not sure what you mean by "somebody downloading the kernel database". When
the RPM is installed, does it not include all the neccesary data? 

Comment 17 Nadia Derbey 2006-11-23 16:53:12 UTC
(In reply to comment #16)
> I think I was not specific enough. The thought is that it would be generic
> enough that when build against a specific release - it would create an RPM
> against that one.
> 
> If I built under FC5, it would build a FC5-tunable package for the kernel that
> is in FC5. and not a FC6 one.
> 
> If I built it under FC6, it would build a FC-6-tunable package for the kernel
> that is in FC6, and not a FC5 one.
> 
> and so on.
> 
> This would have to be generic enough so that when FC7 comes, there is no need to
> alter the SPEC file. Or if it has to be done - just the minum if possible.
> 

OK, I'm conviced

> 
> In regards to your comment about "portability across distros" is a moot point.
> This RPM/SPEC is specific for Fedora Core and RHEL. This spec file will not be
> used by Novell/SuSE. Novell will have require their own .spec file, with
> different fields.
> 

Yeah! I know, that's what I have been fighting with these last days!
What I meant here was not portability for the spec file, but for a binary linked
with the libtune.

> 
> I am not sure what you mean by "somebody downloading the kernel database". When
> the RPM is installed, does it not include all the neccesary data? 

We have 1 rpm for the kernel data part and 1 rpm for the more distro-specific part.

Ok I'll change everything and update the bug when I'm ready.

Comment 18 Nadia Derbey 2006-11-28 10:34:10 UTC
Konrad,

I have changed my spec file to make it build the rpm against the kernel / distro
installed on the machine building the RPM.

*but* I have 1 question: I need the kernel tunables database rpm to be rebuilt
for each kernel "supported" with FC6. I need it to be something ala kernel-devel
pacakge: for FC5, kernel-devel packages are available starting from 2.6.15, up
to 2.6.18 kernel releases. I guess for FC6 there will be some kernel-devel
packages delivered for 2.6.19, etc.

This is exactly the same thing I need for my rpm. What should I do in my spec
file (or may be elsewhere?) to specify that?

Hope my question is clear enough ;-)

Comment 19 Konrad Rzeszutek 2006-11-28 16:56:13 UTC
Nadia,

Your question is clear enough, the answer is hard :-)

Dave Jones, do you have an idea of how to do this?


Comment 20 Nadia Derbey 2006-12-04 13:47:00 UTC
While waiting for Dave's answer, here are the new links after your last review
(sorry for not reacting fast these days: I'm often out my office):

SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-11-1/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.11-1.src.rpm



Comment 21 Nadia Derbey 2007-01-18 13:36:30 UTC
ping?

Comment 22 Nadia Derbey 2007-01-25 15:28:35 UTC
Konrad,

Looking at packages such as iproute or kernel-devel, I saw that they hardcode
the kernel release number into the spec file.

So the solution I implemented is a mix between these packages and something more
"automatic":
I %define'd a variable in the specfile (base_kernel). This variable is presently
set by extracting the installed kernel release. 
But using base_kernel makes it possible to hard-code its value as needed, and
build against that hard-coded kernel release.

BTW, added support for FC7 / 2.6.19 in the upstream library.

New links:

SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-12-1/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.12-1.src.rpm

Regards,
Nadia

Comment 23 Konrad Rzeszutek 2007-05-21 23:13:04 UTC
I've completly missed this BZ. Let me take a look at those RPMs.

Comment 24 Konrad Rzeszutek 2007-06-11 19:52:21 UTC
I get this when I try to build the package using mock with fc6 as a target.
error: Bad package specification: %package -n tunables-base-data-no package
provides kernel


Is this suppose to be only target fc7?

Comment 25 Konrad Rzeszutek 2007-06-11 20:01:25 UTC
It seems that the mock system doesn't install the kernel package. Instead it
installs the kernel headers instead.

bash-3.1# rpm -qa | grep ker
kernel-headers-2.6.18-1.2798.fc6

which means that the macro doesn't extra the version correctly:

bash-3.1# rpm -q --queryformat='%{VERSION}' --whatprovides kernel
no package provides kernel


Comment 26 Nadia Derbey 2007-06-14 08:41:18 UTC
Konrad,

Happy to see you came back reviewing my package!

Fixed the specfile to take the kernel-headers version.
I should definitely install mock on my machine!
It is strange you don't even get kernel-devel when you query for packages
containing "kernel".

Also Added 2.6.21 as a supported kernel release.

New links:

SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-12-2/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.12-2.fc7.src.rpm

Regards,
Nadia

Comment 27 Adel Gadllah 2007-07-20 07:05:14 UTC
1) you don't need to buildrequire gcc.
2) don't package .la files.
3) static libs should be packaged into a seperate package (-static)


Comment 28 Jean-Pierre Dion 2007-07-20 13:35:49 UTC
Adel,

Thank you for your comments.
Nadia is not here at the moment and she
will be back mid August, so I will try
do what I can or wait for her to come back ;-)


Comment 29 Nadia Derbey 2007-08-28 13:51:57 UTC
Adel,

Fix the issues, but sourceforge (it is the palce I upload my specfile to) is
unreacheable via ssh.
Will ping you as soon as the spec ans srpm url's are updated.

Comment 30 Nadia Derbey 2007-08-28 14:04:27 UTC
oops, I meant "fixed the issues" !

Comment 31 Nadia Derbey 2007-08-30 13:40:41 UTC
I finally found another location for my files, since sourceforge is still
unreachable via ssh:

SPEC URL: http://www.bullopensource.org/libtune/libtune.spec
SRPM URL: http://www.bullopensource.org/libtune/libtune-0.12-3.fc7.src.rpm



Comment 32 Adel Gadllah 2007-08-30 14:00:52 UTC
Ok, some other issues with the spec file:

>%define base_kernel %(rpm -q --queryformat='%{VERSION}' --whatprovides
>kernel-headers)
>%define family FC
>%define frelease %(rpm -q --queryformat='%{VERSION}' fedora-release)
>%define distro_string %{family}%{frelease}
seems like the only one used from them is the "base_kernel" drop the other ones
and use uname -r instead of rpm to get the kernel version; this also means you
have to require the extact kernel version it is build against or isn't this needed? 
----------------
>License:        GPL/LGPL
the license guidlines changed please see:
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
and
http://fedoraproject.org/wiki/Licensing
-----------------
Remove the dot from the end of the description
-----------------
use %{dist} for distro_string
-----------------


Comment 33 Nadia Derbey 2007-08-31 13:16:14 UTC
1) variables:
The %{distro_string} is used too.
Since it is built from the %{family} and %{release} variables, we can say they
are all used.

2) base_kernel:
uname -r would give me something like "2.6.21-1.3194.fc7", while I only need the
first 3 fields of it: "2.6.21"

3) kernel required:
Don't need to require the exact kernel, since several releases are supported,
starting from 2.6.9. Actually, I want to avoid changing my spec file each time
the kernel changes. That's why I'm extracting the running kernel release.

4) Licensing:
Fixed - But rpmlint should be fixed accordingly: it is now telling me my License
string is invalid. May be there was another version released after license
guidelines has changed?

5) dot
Whish one are you talking about?
Couldn't find anything about dots in 
http://fedoraproject.org/wiki/Packaging/Guidelines#head-ef67b32cfe3903b0aaab1b3c920940769007da6a

6) distro_string:
I actually only changed it in the tunables-distro-data description.
The problem is that anyewhere else, the "FC7" string is used instead of f7. So I
would have to change things upstream.
But if it is really a problem, please tell me and I'll do the changes, since I
am the upstream.

SPEC URL: http://www.bullopensource.org/libtune/libtune-0.12-4/libtune.spec
SRPM URL:
http://www.bullopensource.org/libtune/libtune-0.12-4/libtune-0.12-4.fc7.src.rpm

Comment 34 Adel Gadllah 2007-08-31 13:54:37 UTC
(In reply to comment #33)
> 1) variables:
> The %{distro_string} is used too.
> Since it is built from the %{family} and %{release} variables, we can say they
> are all used.

ok

> 2) base_kernel:
> uname -r would give me something like "2.6.21-1.3194.fc7", while I only need the
> first 3 fields of it: "2.6.21"

ok, but you don't need the --whatprovides 
rpm -q --queryformat='%{VERSION}' kernel-headers should be enough
also buildrequire kernel-headers for this


> 3) kernel required:
> Don't need to require the exact kernel, since several releases are supported,
> starting from 2.6.9. Actually, I want to avoid changing my spec file each time
> the kernel changes. That's why I'm extracting the running kernel release.

ok

> 4) Licensing:
> Fixed - But rpmlint should be fixed accordingly: it is now telling me my License
> string is invalid. May be there was another version released after license
> guidelines has changed?

yes rpmlint got updated
=> rpmlint-0.80-3.fc7 should know about the new license tags.

> 5) dot
> Whish one are you talking about?
> Couldn't find anything about dots in 
>
>http://fedoraproject.org/wiki/Packaging/Guidelines#head-ef67b32cfe3903b0aaab1b3c920940769007da6a

sry confused the summary with the description this is ok.

 
> 6) distro_string:
> I actually only changed it in the tunables-distro-data description.
> The problem is that anyewhere else, the "FC7" string is used instead of f7. So I
> would have to change things upstream.
> But if it is really a problem, please tell me and I'll do the changes, since I
> am the upstream.

%dist should be fc7 (fedora collection 7) so you should be fine there or did I
miss anything? btw this is only a cosmetic change.

> SPEC URL: http://www.bullopensource.org/libtune/libtune-0.12-4/libtune.spec
> SRPM URL:
> http://www.bullopensource.org/libtune/libtune-0.12-4/libtune-0.12-4.fc7.src.rpm
-----------

The package fails to build here:
   SYMLINK include/base -> include/basedbs/base-2.6.22.4
Version 2.6.22.4 is not supported
is it because of the .4 ? 
if yes use 
rpm -q --queryformat='%{VERSION}' kernel-headers | cut -c 0-6
to get the kernel version.
I tested this and it complains about kernel 2.6.22 not supported.



Comment 35 Nadia Derbey 2007-09-03 06:15:59 UTC
> > 2) base_kernel:
> > uname -r would give me something like "2.6.21-1.3194.fc7", while I only need the
> > first 3 fields of it: "2.6.21"
> 
> ok, but you don't need the --whatprovides 
> rpm -q --queryformat='%{VERSION}' kernel-headers should be enough
> also buildrequire kernel-headers for this

OK

> > 6) distro_string:
> > I actually only changed it in the tunables-distro-data description.
> > The problem is that anyewhere else, the "FC7" string is used instead of f7. So I
> > would have to change things upstream.
> > But if it is really a problem, please tell me and I'll do the changes, since I
> > am the upstream.
> 
> %dist should be fc7 (fedora collection 7) so you should be fine there or did I
> miss anything? btw this is only a cosmetic change.

OK

> The package fails to build here:
>   SYMLINK include/base -> include/basedbs/base-2.6.22.4

This is because 2.6.22 is not supported. Will add support for it and ping when
everything is fixed.

Comment 36 Nadia Derbey 2007-09-04 07:36:50 UTC
Here are the new links:

SPEC URL: http://www.bullopensource.org/libtune/libtune-0.13-1/libtune.spec
SRPM URL:
http://www.bullopensource.org/libtune/libtune-0.13-1/libtune-0.13-1.fc7.src.rpm

Coming back to the remark you did about the distro_string variable in comment #32:
> use %{dist} for distro_string

Actually, I used fc%{fedora} instead, since %dist} begins with a dot.

Comment 37 Nadia Derbey 2007-09-18 14:54:58 UTC
ping?

Comment 38 Konrad Rzeszutek 2007-09-24 13:57:23 UTC
I gave it a spin on mock and I got this:

make: Entering directory `/builddir/build/BUILD/libtune-0.13/db'
   SYMLINK include/base -> include/basedbs/base-2.6.21
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
../scripts/get_current_distro.sh: line 67: lsb_release: command not found
   SYMLINK include/distro -> include/distrodbs/distro-UNSUPPORTED
Your distro is not supported
make: *** [include/distro] Error 1
make: Leaving directory `/builddir/build/BUILD/libtune-0.13/db'
error: Bad exit status from /var/tmp/rpm-tmp.4327 (%install)

Seems that is is missing as pre-requisite the lsb package too?

Comment 40 Konrad Rzeszutek 2007-10-01 19:49:38 UTC
I get this now when doing a mock build:

make: Entering directory `/builddir/build/BUILD/libtune-0.13/db'
   SYMLINK include/base -> include/basedbs/base-2.6.21
   SYMLINK include/distro -> include/distrodbs/distro-fc6.93
Combination fc/6.93 is not supported
make: *** [include/distro] Error 1
make: Leaving directory `/builddir/build/BUILD/libtune-0.13/db'
error: Bad exit status from /var/tmp/rpm-tmp.61288 (%install)




Comment 41 Nadia Derbey 2007-10-02 05:33:49 UTC
what is your Fedora release number (lsb_release -r)?
Is it 6.93?

Comment 42 Konrad Rzeszutek 2007-10-02 15:37:51 UTC
Yup:
+ lsb_release -r
Release:	6.93

Comment 43 Konrad Rzeszutek 2007-10-02 15:45:23 UTC
And I do a fresh install of FC7, the command shows:
[root@hp-bl460c-01 SPECS]# cat /etc/redhat-release
Fedora release 7 (Moonshine)
[root@hp-bl460c-01 SPECS]# lsb_release -r
Release:        7

So the build system is in some way borked. Let me re-init the whole mock system
and make sure I am not doing anything wrong.

Comment 44 Konrad Rzeszutek 2007-10-02 21:27:29 UTC
It was a borked. Now 'mock' builds on my machine. yeeey!


Comment 45 Konrad Rzeszutek 2007-10-03 14:41:12 UTC
I used the Fedora build system and:

Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=181452
Watching tasks (this may be safely interrupted)...
181452 build (dist-fc7, libtune-0.13-2.fc7.src.rpm): free
181452 build (dist-fc7, libtune-0.13-2.fc7.src.rpm): free -> open
(xenbuilder4.fedora.phx.redhat.com)
  181455 buildArch (libtune-0.13-2.fc7.src.rpm, i386): free
  181453 buildArch (libtune-0.13-2.fc7.src.rpm, ppc): free
  181454 buildArch (libtune-0.13-2.fc7.src.rpm, x86_64): free
  181456 buildArch (libtune-0.13-2.fc7.src.rpm, ppc64): open
(ppc2.fedora.redhat.com)
  181453 buildArch (libtune-0.13-2.fc7.src.rpm, ppc): free -> open
(ppc3.fedora.redhat.com)
  181454 buildArch (libtune-0.13-2.fc7.src.rpm, x86_64): free -> open
(xenbuilder4.fedora.phx.redhat.com)
  181455 buildArch (libtune-0.13-2.fc7.src.rpm, i386): free -> open
(xenbuilder2.fedora.redhat.com)
  181455 buildArch (libtune-0.13-2.fc7.src.rpm, i386): open
(xenbuilder2.fedora.redhat.com) -> closed
  0 free  4 open  1 done  0 failed
  181456 buildArch (libtune-0.13-2.fc7.src.rpm, ppc64): open
(ppc2.fedora.redhat.com) -> closed
  0 free  3 open  2 done  0 failed
  181453 buildArch (libtune-0.13-2.fc7.src.rpm, ppc): open
(ppc3.fedora.redhat.com) -> closed
  0 free  2 open  3 done  0 failed
  181454 buildArch (libtune-0.13-2.fc7.src.rpm, x86_64): open
(xenbuilder4.fedora.phx.redhat.com) -> closed
  0 free  1 open  4 done  0 failed
181452 build (dist-fc7, libtune-0.13-2.fc7.src.rpm): open
(xenbuilder4.fedora.phx.redhat.com) -> closed
  0 free  0 open  5 done  0 failed

181452 build (dist-fc7, libtune-0.13-2.fc7.src.rpm) completed successfully


Comment 46 Konrad Rzeszutek 2007-10-03 14:47:21 UTC
I tried FC8 but that didn't work that well:
http://koji.fedoraproject.org/koji/taskinfo?taskID=181477

FYI: FC8 is GA-ing on 8 November 2007, so I think making it build should also be
addressed.

Comment 47 Jean-Pierre Dion 2007-10-03 16:19:16 UTC
Hi Konrad,

Nadia will have a look at this ASAP to try make it fit for FC8 GA.


jean-pierre


Comment 48 Nadia Derbey 2007-10-05 07:03:42 UTC
Fixed - added support for fc8.

SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-13-3/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.13-3.fc7.src.rpm

Alos Matt suggested that when a kernel release or distro is not supported we
should link against a "default" tunables DB: will be working on that soon.

Comment 49 Konrad Rzeszutek 2007-10-08 20:47:55 UTC
fc8 build: http://koji.fedoraproject.org/koji/taskinfo?taskID=187351

shows this error:
make: Entering directory `/builddir/build/BUILD/libtune-0.13/db'
   SYMLINK include/base -> include/basedbs/base-2.6.23
   SYMLINK include/distro -> include/distrodbs/distro-fc7.92
Combination fc/7.92 is not supported
make: *** [include/distro] Error 1
make: Leaving directory `/builddir/build/BUILD/libtune-0.13/db'
error: Bad exit status from /var/tmp/rpm-tmp.14837 (%install)



Comment 50 Nadia Derbey 2007-10-11 09:21:09 UTC
The way the package builds currently is that it takes the kernel release and
distro release from what is installed on the machine we are building on:
1) kernel release comes from the kernel-headers installed package version and is
passed to the make command as the KVERSION to build against.
I think this part is OK since we have the right info in the mock environment.
2) distro release is computed in the Makefile (if not passed to the Makefile)
and is the result of an "lsb_release -r".
I think the problem comes from this 2nd part: we should also provide the distro
release to the make command in the spec file.
Now, can you please confirm what I say after:
the %{?dist} tag is THE way we have to know what distro release target we are
building for.
Or is there a more secure way to know the target we are building for?

As soon as I get your answer, I'll send you the fix: it should only be a small
change in the spec file.

Thanks,
Nadia

Comment 51 Nadia Derbey 2007-10-25 13:56:53 UTC
Now the package is built taking the dist tag to get the distro release.
+ changes upstream to switch to a default tunables DB if ever the kernel release
/ distro is not supported.
This second change covers the issue of fedora releases that look like: fc7.92

SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-13-4/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.13-4.fc7.src.rpm

Comment 52 Nadia Derbey 2007-11-15 09:01:19 UTC
New changes upstream:
- introduced a default tunables DB for fc, one for SuSE and a default "other" DB.
- Added support for fc9.
- bug fix in db/Makefile.
- new libtune interfaces for tun_set() and tun_get().

Also changed the source files' owner to root to avoid the koji warning:
####
warning: user derbeyn does not exist - using root
warning: group cooplin does not exist - using root
####

New links:
SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-14-1/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.14-1.fc8.src.rpm

Comment 53 Nadia Derbey 2007-11-16 07:26:04 UTC
Still new changes in the specfile (non-i386 arches coulld not build because of a
missing dependency detected for mkinitrd).

Matt, thanks for your help!

New links:
SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-14-2/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.14-2.fc8.src.rpm

Comment 54 Konrad Rzeszutek 2007-12-21 21:36:54 UTC
Per http://fedoraproject.org/wiki/Packaging/ReviewGuidelines:

1).
"MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL. Reviewers should use md5sum for this task."

The SRPM has:
098e787f377632793ec25dc8f208176b  libtune-0.14.tar.bz2

While the version from the URL is:
666cc8c413f9f783c2bec5bd37d33cb0  libtune-0.14-1.tar.bz2


Please release upstream the newer version (0.14-2) and as well in the .spec file
reference the %{name}-%{version}-%{release}.tar.bz2

2). MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release} 

Currently the .spec file has:
Requires: %{name} = %{version}

That should be:
Requires: %{name} = %{version}-%{release}


Please fix those two issues.
I've built the package in koji under F8, and F7.

Comment 55 Konrad Rzeszutek 2007-12-21 21:39:53 UTC
There is also an inconsistent usage of spaces.

At the beginning of it shows:

Name:           libtune
Version:        0.14
Release:        2%{?dist}
Summary:        Library tha

while later
ackage -n tunables-distro-data
Summary:  Distro-dependent tunables database
Group:    System Environment/Libraries
License:        GPLv2


Can you make it use the same exact spacing, please?

Comment 56 Konrad Rzeszutek 2007-12-21 21:48:30 UTC
Please also add this for the main package and -devel package
Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig

As you are using ldconfig in the post and postun section. 

Furthermore for the -devel, also add:
Requires(post): /sbin/chkconfig
Requires(postun): /sbin/chkconfig
Requires(post): /usr/lib/lsb/install_initd
Requires(postun): /usr/lib/lsb/remove_initd

As you are using /sbin/chkconfig and /usr/lib/lsb/install_initd in post, and 
postun sections. 


Comment 57 Konrad Rzeszutek 2007-12-21 21:51:30 UTC
You also need to update your %post, %preun and %postun to take advantage of a
user updating the package.

Look for details in http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

Comment 58 Konrad Rzeszutek 2007-12-21 21:52:05 UTC
(In reply to comment #57)
> You also need to update your %post, %preun and %postun to take advantage of a
> user updating the package.
> 
> Look for details in http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

And here is an example of review in which it was used:
https://bugzilla.redhat.com/show_bug.cgi?id=323441

Comment 59 Konrad Rzeszutek 2007-12-21 21:52:52 UTC
FYI: It builds fine under F9, F8, and F7.

Comment 60 Jean-Pierre Dion 2007-12-22 16:05:05 UTC
Thank you for the review, Konrad.
Nadia will fix all that up when she is back early January.

Comment 61 Nadia Derbey 2008-01-08 11:46:17 UTC
Konrad,

Sorry for the late answer!

(In reply to comment #54)
> Per http://fedoraproject.org/wiki/Packaging/ReviewGuidelines:
> 
> 1).
<snip>
> Please release upstream the newer version (0.14-2) and as well in the .spec file
> reference the %{name}-%{version}-%{release}.tar.bz2

Fixed.
Actually, didn't use the release tag in the source name: only the release number
is referenced (%{release} contains the dist tag and I'd like the source.tar.bz2
to be independant from the distro name). 

> 
> 2). MUST: In the vast majority of cases, devel packages must require the base
> package using a fully versioned dependency: Requires: %{name} =
> %{version}-%{release} 
> 
> Currently the .spec file has:
> Requires: %{name} = %{version}
> 
> That should be:
> Requires: %{name} = %{version}-%{release}
> 

Fixed.

(In reply to comment #55)
> There is also an inconsistent usage of spaces.

Fixed.

(In reply to comment #56)
> Please also add this for the main package and -devel package
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> 
> As you are using ldconfig in the post and postun section. 

Only added this Requires: to the post section for the devel package: in the
other cases, rpmbuild generates it automatically (checked through the rpmbuild
traces), so I thought it was not necessary.

> 
> Furthermore for the -devel, also add:
> Requires(post): /sbin/chkconfig
> Requires(postun): /sbin/chkconfig
> Requires(post): /usr/lib/lsb/install_initd
> Requires(postun): /usr/lib/lsb/remove_initd
> 
> As you are using /sbin/chkconfig and /usr/lib/lsb/install_initd in post, and 
> postun sections.

Well, only added the chkconfig Requires: install/remove_initd is called only if
present on the system we are installing on/deinstalling from, i.e. if the system
is LSB compliant. If it is not, chkconfig is called. So chkconfig is THE one we
need.

(In reply to comment #57)
> You also need to update your %post, %preun and %postun to take advantage of a
> user updating the package.

Good catch: actually, I had never tested the upgrade part of it and it was not
correct!
Fixed.

New links:
SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-14-3/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.14-3.fc8.src.rpm

Comment 62 Konrad Rzeszutek 2008-01-11 20:34:31 UTC
1). English 

Summary:          Library that Standardizes the Access to Kernel Tunables

Why not say:
Summary:          Library that standardizes the access to Kernel tunables

?

2).  In http://fedoraproject.org/wiki/Packaging/Guidelines, section titled:
"Using %{buildroot} and %{optflags} vs $RPM_BUILD_ROOT and $RPM_OPT_FLAGS"
it mentions that you should use RPM_OPT_FLAGS instead of "optflags".



Comment 64 Konrad Rzeszutek 2008-01-20 03:31:18 UTC
MUST fix:
The license field in libtune-devel and tunables-base-data-x.y.z is incorrect. 
It says 'LGPLv2' while the files that are in it are under GPL license.
It wouldn't hurt to also bundle the GPL license file with each of those 
packages as well.

I think that is it. I can't find anyting else that might need attention.

Comment 65 Nadia Derbey 2008-01-21 09:44:23 UTC
1) tunables-base-data-XXX: the license tag already specifies GPLv2

2) Fixed the license tag for the devel package

3) the GPL license was already delivered for the devel package

4) Added the GPL license delivery for the tunables-base-data-XXX and
tunables-distro-data-XXX packages.

New links:
SPEC URL: http://libtune.sourceforge.net/specfiles/FE/FE-14-5/libtune.spec
SRPM URL: http://libtune.sourceforge.net/srpms/FE/libtune-0.14-5.fc8.src.rpm

Comment 66 Konrad Rzeszutek 2008-01-30 23:15:35 UTC
Hmm, right now I am able to do this:

oot@dl380 i386]# rpm -hiv tunables-distro-data-0.14-4.fc8.i386.rpm
tunables-distro-data-0.14-5.fc8.i386.rpm 
Preparing...                ########################################### [100%]
   1:tunables-distro-data   ########################################### [ 50%]
   2:tunables-distro-data   ########################################### [100%]
root@dl380 i386]# rpm -qa | grep tunables-distro-data
tunables-distro-data-0.14-4.fc8
tunables-distro-data-0.14-5.fc8

I would think it would have complained about overwriting but I didn't see anything. 

Comment 67 Konrad Rzeszutek 2008-01-30 23:19:11 UTC
Installing and older applications doesn't work (good):
[root@dl380 i386]# rpm -hiv tunables-distro-data-0.14-4.fc8.i386.rpm 
Preparing...                ########################################### [100%]
        package tunables-distro-data-0.14-5.fc8 (which is newer than
tunables-distro-data-0.14-4.fc8) is already installed

But I can just install a newer version on top of the old one:

[root@dl380 i386]# rpm -hiv tunables-distro-data-0.14-4.fc8.i386.rpm 
Preparing...                ########################################### [100%]
   1:tunables-distro-data   ########################################### [100%]
rpm -hiv [root@dl380 i386]# rpm -hiv tunables-distro-data-0.14-5.fc8.i386.rpm 
Preparing...                ########################################### [100%]
   1:tunables-distro-data   ########################################### [100%]


Comment 68 Nadia Derbey 2008-01-31 15:51:36 UTC
You're right: I've tried that with the tunables-base-data package and got the
same result.
I'm not an rpm expert, but I'm wondering whether this wouldn't be because
version .5 contains exactly the same files as version .4 + a brand new file. So
rpm doens't doesn't see that as a conflict?

I'll try to test something on my side: make version .5 with a file that has the
same name as in .4, but with a different content.

Comment 69 Nadia Derbey 2008-02-01 09:58:27 UTC
Ok, this is exactly what I suspected: tried to change the README file delivered
by tunables-base-data, and here's the result I got:

<snip>
D: opening  db index       /var/lib/rpm/Name create mode=0x42
D:  read h#     913 Header SHA1 digest: OK
(4c878e96685b21513e679f21dc15cc800d901239)
D:  read h#     917 Header SHA1 digest: OK
(16ffc11b812ff117879d896cc00e7e5ac100d998)
D: computing 5 file fingerprints
Preparing...                D: computing file dispositions
D: opening  db index       /var/lib/rpm/Basenames create mode=0x42
########################################### [100%]
        package tunables-base-data-2.6.21-0.14-5.fc8 is already installed
        file /usr/share/libtune/doc/README from install of
tunables-base-data-2.6.21-0.14-5.fc8 conflicts with file from package
tunables-base-data-2.6.21-0.14-4.fc8
        file /usr/share/libtune/doc/README from install of
tunables-base-data-2.6.21-0.14-5.fc8 conflicts with file from package
tunables-base-data-2.6.21-0.14-5.fc8
D: closed   db index       /var/lib/rpm/Basenames
D: closed   db index       /var/lib/rpm/Name
D: closed   db index       /var/lib/rpm/Packages
D: closed   db environment /var/lib/rpm/Packages
D: May free Score board((nil))
[root@akt i386]# echo $?
1

So sounds like it is normal to be able to install a newer version over an older
one without conflicts problems, given that the common files have the same contents.

Comment 70 Nadia Derbey 2008-02-01 10:01:28 UTC
Oops, Konrad, so sorry I gave you the wrong traces in last comment. So, please
forget them. Here are the good ones:

<snip>
D: running pre-transaction scripts
D: computing 5 file fingerprints
Preparing...                D: computing file dispositions
D: opening  db index       /var/lib/rpm/Basenames create mode=0x42
########################################### [100%]
        file /usr/share/libtune/doc/README from install of
tunables-base-data-2.6.21-0.14-5.fc8 conflicts with file from package
tunables-base-data-2.6.21-0.14-4.fc8
D: closed   db index       /var/lib/rpm/Basenames
D: closed   db index       /var/lib/rpm/Name
D: closed   db index       /var/lib/rpm/Packages
D: closed   db environment /var/lib/rpm/Packages
D: May free Score board((nil))
[root@akt i386]# echo $?
1

Comment 71 Konrad Rzeszutek 2008-02-04 23:10:33 UTC
ok MUST items with explanations:

 - rpmlint: ok
$rpmlint -v libtune-0.14-5.fc8.i386.rpm
libtune.i386: I: checking
$rpmlint -v libtune-devel-0.14-5.fc8.i386.rpm
libtune-devel.i386: I: checking
libtune-devel.i386: W: service-default-enabled /etc/rc.d/init.d/chtunedb
libtune-devel.i386: W: service-default-enabled /etc/rc.d/init.d/chtunedb
libtune-devel.i386: W: incoherent-init-script-name chtunedb
$rpmlint -v tunables-base-data-2.6.23-0.14-5.fc8.i386.rpm
tunables-base-data-2.6.23.i386: I: checking
$rpmlint -v tunables-distro-data-0.14-5.fc8.i386.rpm
tunables-distro-data.i386: I: checking

The warnings for libtune-devel can be ignored. The 'chtunedb' script is
the mechanism for the devel package to "harvest" kernel parameters (so
if a new kernel is added they are automaticly regenerated).

ok MUST items:
 - package name: ok.
 - spec file name match base package: ok.
 - package meet packaging guidelines: ok
 - package must contin approved license: ok
 - license field equal the actual license: ok
 - spec file written in American English: ok
 - spec file legible: ok
 - source files in rpmbuild match upstream: ok
 - package compile and build binary RPM: ok (built on f-7, f-8, f-9 under
scratch koji build)
 - build dependencies listed in BuildRequires: ok
 - binary RPM which stores shared library must call ldconfig in %post and
%postun: ok
 - a package must own all the directories it creates: ok
 - a package must not contain duplicates in %files: ok
 - permissions must be properly: ok
 - must have %clean section: ok
 - must use consistently use macros: ok
 - package must contain code or permissiable content: ok
 - if a package includes %doc it must not reflect the runtime of the application: ok
 - header files must be in -devel package: ok
 - if package contains library files with a suffix, then one ending with .so
must be in -devel: ok
 - fully versioned depedency: ok
 - not have any .la archives :ok
 - package must not own files or directories already owned by other packages: ok
 - At the beginning of %install, the rm -Rf .. : ok
 - all filenames in the package must be valid UTF-8: ok

N/A MUST items:
 - spec file handle locales using %find_lang: There are no translation usages on
this package, hence N/A.
 - if package is relocatable ....: Package is not relocatable, hence N/A.
 - large doc should go in -doc subpackage .. : no big doc, hence N/A.
 - static libraries must be in .. : not using static libraries, hence N/A.
 - packages containing pkgconfig(..), not using pkgconfig, hence N/A.
 - package containing GUI .. : no GUI, hence N/A.


ok SHOULD items with explanations.
 - usually subpackages other than devel should require the base package using a
fully versioned  dependency. This package 'libtune' requires the
'tunabled-base-distro-%{version}' package instead of using the
'tunable-base-distro-%{version}-${release}'. This is done b/c the
tunables-base-distro package can be respun many times during a release (for new
kernels that might change their /proc or  /sysfs layout) and there is absolutely
no reason to re-release the libtune package.

ok SHOULD items:
 - if the source package does not include license text... : it does have it: ok
 - test package in koji: ok
 - compile and build binary packages on all architecture: ok
 - test the package functions as described: ok
 - if scriptlets are used... : ok.

N/A SHOULD items:
 - The placement of pkgconfig ... : N/A
 - If the package has file dependency on .. : N/A
 - The description and summary sections in the package should contain
translations for supported Non-English language: not done. This package does not
contain any localized information, hence no translation effort was involved and
this is N/A.

PACKAGE REVIEWED. ACCEPT.


Comment 72 Konrad Rzeszutek 2008-02-05 17:01:29 UTC
Yikes, so I need to retract the 'ACCEPT'/APPROVED part since I am not a sponser
(per
http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b:
First reviews for new packagers must be done by registered sponsors. Informal
reviews can be done by anyone interested.").

Next stage is actually for a sponsor to do the review.



Comment 73 Bill Nottingham 2008-02-13 22:49:19 UTC
So.

This code is a *really bad idea*.

Apps should not be twiddling with tunables in /proc or /sys - they are system
wide attributes. You can have multiple apps - what if they want different things?
This library doesn't actually prevent you from situations where the kernel
changes without patching and rebuild, so you don't gain anything from an app
standpoint. It's never the sort of thing which will be in Fedora base, or RHEL,
and therefore any app that would want to use it would need ot have code to
handle it not being there anyway. So, what's the point?





Comment 74 Nadia Derbey 2008-02-14 16:01:26 UTC
(In reply to comment #73)
> So.
> 
> This code is a *really bad idea*.
> 
> Apps should not be twiddling with tunables in /proc or /sys - they are system
> wide attributes. 

Agreed, but many applications need some tunables to be set higher than the
default values. So, today, there must be administrators doing an "echo xxx >
/proc/kernel/sys/yyy", either by hand or via a script.
This is only what the libtune is intended to do.

> You can have multiple apps - what if they want different things?

This is a related problem which libtune was not made to solve. Applications made
to solve this problem in different ways could use libtune. System administrators
could then choose between a variety of potentially competing applications or
manual tuning.

Then, if a single such application emerges libtune would be a good place to add
that without worrying about changing every app that uses the tunable. Though
this does assume changing most or all existing apps to use the library initially.

> This library doesn't actually prevent you from situations where the kernel
> changes without patching and rebuild, so you don't gain anything from an app
> standpoint.

Yes, in that case the libtune package must be patched and rebuilt. If the
tunable change is simple enough the advantage is the apps that depend on that
tunable shouldn't need to be patched and rebuilt themselves -- libtune insulates
them against most tunable interface changes.

> It's never the sort of thing which will be in Fedora base, or RHEL,
> and therefore any app that would want to use it would need ot have code to
> handle it not being there anyway. So, what's the point?
> 

 Yes, if it got into only one then there'd be little point. However isn't
it safer to have a migration path? There'd be a period where an app
would use both libtune and its own code. Then, once libtune is in both
Fedora and RHEL, the application-specific code could be dropped.
Alternatively, if libtune were thrown out then use of libtune could be
reverted without breaking things. Without this kind of migration you
almost have a chicken-and-the-egg paradox.

Comment 75 Bill Nottingham 2008-02-14 17:58:32 UTC
(In reply to comment #74)

> > Apps should not be twiddling with tunables in /proc or /sys - they are system
> > wide attributes. 
> 
> Agreed, but many applications need some tunables to be set higher than the
> default values. So, today, there must be administrators doing an "echo xxx >
> /proc/kernel/sys/yyy", either by hand or via a script.
> This is only what the libtune is intended to do.

It is an administrator issue - they are the only ones who can arbitrate between
the needs of different apps.
> > It's never the sort of thing which will be in Fedora base, or RHEL,
> > and therefore any app that would want to use it would need ot have code to
> > handle it not being there anyway. So, what's the point?
> > 
> 
>  Yes, if it got into only one then there'd be little point. However isn't
> it safer to have a migration path? There'd be a period where an app
> would use both libtune and its own code. Then, once libtune is in both
> Fedora and RHEL,

Re-read above please. It's *never* going to be in the Fedora base distribution
or RHEL, as the upstream kernel and development community has rejected it
as a way of doing things.


Comment 76 Josh Boyer 2008-02-14 18:46:19 UTC
(In reply to comment #75)
> >  Yes, if it got into only one then there'd be little point. However isn't
> > it safer to have a migration path? There'd be a period where an app
> > would use both libtune and its own code. Then, once libtune is in both
> > Fedora and RHEL,
> 
> Re-read above please. It's *never* going to be in the Fedora base distribution
> or RHEL, as the upstream kernel and development community has rejected it
> as a way of doing things.

By "Fedora base distribution" I believe Bill means it will not be included in a
default install or on the release .isos.  The package would reside only in the
yum repositories and would require uses to explicitly install it.



Comment 77 Bill Nottingham 2008-02-14 19:28:31 UTC
Also, from the description of how this is architected, you're basically
requiring all apps to:

1) run as root
2) have SELinux allow them to access all of /proc, /sys, etc.

That is awful for security reasons.

Comment 78 Josh Boyer 2008-02-15 22:46:54 UTC
From a packaging standpoint, Konrad did a great review.  I'd be willing to
sponsor Nadia for this.  Consider it APPROVED.

Nadia, please apply for sponsorship and I'll take care of that.

Word of warning, rawhide has massive kernel churn.  This package seems like it
might need frequent updating because of that, so please be prepared.

Comment 79 Josh Boyer 2008-03-10 13:08:02 UTC
(In reply to comment #78)
> From a packaging standpoint, Konrad did a great review.  I'd be willing to
> sponsor Nadia for this.  Consider it APPROVED.
> 
> Nadia, please apply for sponsorship and I'll take care of that.

Nadia, is there still interest in having this in Fedora?  If so, please apply
for sponsorship in the accounts system and we'll proceed.  If not, please close
out the bug as WONTFIX.



Comment 80 Nadia Derbey 2008-03-10 13:16:52 UTC
Konrad, sorry for the delay, but I had plenty of other things to do!

No there is nomore interest since it won't be in the base distribution.
I'll close the bug.

Thanks!


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