Bug 448491

This is my first package to the Fedora and I need a sponsor

Not a formal review, but the spec file name should match package name/upstream
name, as a start.

The upstream name is libcgroup itself.

That's not what the upstream tarball is named. If it is named libcgroup, then
the package names should match it as well (not cgroup-lib)

OK,I updated the filename.

spec file is now available at
http://nchc.dl.sourceforge.net/sourceforge/libcg/cgroup.spec

The name should be libcgroup, not cgroup.

The point is this: The SRPM name should match the SPEC name and the generated
binary name.

So, if the name is libcgroup, then you should have:

libcgroup-0.1b-1.fc8.src.rpm
libcgroup.spec

and when built,
libcgroup-0.1b-1.fc8.x86_64.rpm

Dhaval has renamed the spec to cgroup.spec and the RPMS built are

cgroup-lib-0.1b-*.rpm and cgroup-lib-devel-0.1b-*rpm

We decided to drop the lib prefix from our package since we in the long run
intend to provide initscripts and potentially a daemon for configuraiton.

Am I missing something?

Library packages are conventionally named "libfoo", with a "libfoo-devel"
package for the headers.  "foo-lib" is not conventional and I can't think of any
reason why you would want to break tradition here.  Your SF.net name is libcg,
your file release is libcg, but the rest of your files are different.  Now is
the time to settle on a single consistent name for all of it, and libcgroup
would be my suggestion.

Other library packages include daemons, scripts, etc in their "libfoo" package.
 Look at libvirt as a prime (and related) example.

I don't intend to start a flame war here, but I did my changes based on the RPMS
I saw in F9

My F9 system shows

bluez-libs-3.30-1.fc9.i386
compat-libstdc++-296-2.96-140.i386
paps-libs-0.6.8-5.fc9.i386
nfs-utils-lib-1.1.1-3.fc9.i386
PackageKit-libs-0.1.12-10.20080505.fc9.i386
kaffeine-libs-0.8.6-4.fc9.i386
lame-libs-3.97-6.lvn8.i386
cdparanoia-libs-alpha9.8-30.i386
kdegraphics-libs-4.0.3-3.fc9.i386
and many more

Having said that, I'll go by what the majority users of cgroups would like the
library to be named as.

Those packages are libraries specific to a parent package, not ones meant for
general development consumption, AFAICT.

However, for comparison:

% rpm -qa | grep '^lib' -c     
235

I've done the renaming of files. Run rpmlint (I had done that before as well) on
the generated RPMS. Please do review the following files. 

Spec file is at

http://downloads.sourceforge.net/libcg/libcgroup.spec?use_mirror=osdn

Source file is at

http://downloads.sourceforge.net/libcg/libcgroup-0.1b-1.fc9.src.rpm?use_mirror=osdn

We would really appreciate early feedback, so that we can fix the problems (if
any) and revert back with a good clean package. Thanks for your patience and help.

Scratch build passed:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=636762

One minor issue:

Please don't use %makeinstall, use make DESTDIR=$RPM_BUILD_ROOT install

See:
http://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

Dan, thanks for the build results

Tom, I've updated the spec file at the same location as mentioned earlier. In
case you see an older version of the spec file (still), sf.net does not update
all its mirrors immediately and hence the older file might still be around as
stale data.

I've replace %makeinstall with make DESTDIR=$RPM_BUILD_ROOT install as suggested.

The file sizes are
-rw-r--r-- 1 balbir balbir 1638 2008-05-30 23:29 libcgroup.spec
-rw-r--r-- 1 balbir balbir 64201 2008-05-30 23:30 libcgroup-0.1b-1.fc9.src.rpm

In general, any time you make a change to the spec file, you need to increment
the release, and add a new entry to the %changelog section. I'll ignore that for
now, but please be sure to do it in the future.

In the spec, you've got:

URL: http://downloads.sourceforge.net/libcg/%{name}-%{version}.tar.bz2
Source0: libcgroup-0.1b.tar.bz2

URL is intended to point to the libcgroup website.
Source0 should be the URL path to download the tarball.

So, you need to change it to:

URL: http://libcg.sourceforge.net/
Source0: http://downloads.sourceforge.net/libcg/%{name}-%{version}.tar.bz2

Please make a new SRPM, and I'll finish this review.

Thanks for your review comments and patience Tom.

I was aware of editing the changelog, but I thought it was for software changes
rather then the spec file itself. Sorry about missing that part. What I've done
this time is updated Source0 and URL as per your suggestion (in fact just copied
them over)

I've updated the release to 2 and updated the changelog as well

The spec file is at

http://downloads.sourceforge.net/libcg/libcgroup.spec?use_mirror=osdn
(it is 1771 bytes in size)

The source RPM is at
http://downloads.sourceforge.net/libcg/libcgroup-0.1b-2.fc9.src.rpm?use_mirror=osdn
(it's 64333 bytes in size)

I hope the new files will get sync'ed across mirrors soon

Hopefully, I've got it right this time :-)

OK, two more issues (sorry I didn't catch them last time).

1. You need to make sure that the devel package Requires the main package
explicitly. Currently, you have:

Requires: libcgroup >= 0.1b

This should be :

Requires: libcgroup = %{version}-%{release}

2. You need to have %post and %postun sections which call ldconfig. This way,
the shared library will get added to the system cache and be immediately
available. Just add these lines below your %clean section

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

I'll go ahead and do the rest of the review, but please make a -3 package and
upload it.

REVIEW
=======
Good:

- rpmlint checks return: Nothing
- package meets naming guidelines
- package meets packaging guidelines
- license (LGPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (5eebc14487c4e7c05808d60b86151d9881828eb3)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR (well, coreutils is questionable, but harmless)
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- no .la files
X post/postun ldconfig missing
X missing proper devel requires of base n-v-r

thanks for the comments. I have uploaded the changed specfile and src RPM

http://downloads.sourceforge.net/libcg/libcgroup.spec?use_mirror=osdn
(1971 bytes)
http://downloads.sourceforge.net/libcg/libcgroup-0.1b-3.fc8.src.rpm?use_mirror=osdn
(64646 bytes)

Looks good, package is approved. Thanks for your patience here, you guys did a
wonderful job of resolving the issues. I will sponsor you, Dhaval.

You need to continue in the process here:

https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account

Thanks! That's great news Tom.

Could you sponsor me as well, I intend to co-maintain this package with Dhaval.

New Package CVS Request
=======================
Package Name: libcgroup
Short Description: A collection of tools and libraries to control and monitor
control groups the associated controllers
Owners: dhavalgiani, balbirsingh
Branches: F-9, devel
InitialCC: spot
Cvsextras Commits: yes

New Package CVS Request
=======================
Package Name: libcgroup
Short Description: A collection of tools and libraries to control and monitor
control groups the associated controllers
Owners: dhavalgiani, balbirsingh
Branches: F-9 devel
InitialCC: spot
Cvsextras Commits: yes

Balbir still does not have cvsextras membership, so removing his name from
owners for now.

New Package CVS Request
=======================
Package Name: libcgroup
Short Description: A collection of tools and libraries to control and monitor
control groups the associated controllers
Owners: dhavalgiani
Branches: F-9 devel
InitialCC:
Cvsextras Commits: yes

cvs done.

libcgroup-0.1c-2.fc9 has been submitted as an update for Fedora 9

libcgroup-0.1c-2.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libcgroup'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-5252

In rawhide, closing this one out.

libcgroup-0.1c-2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.