Bug 459281

The actual SRPM URL is:
http://developer.osdl.org/dev/openais/downloads/review/corosync-0.91-1.src.rpm

Any particular reason this requires a fixed uid?

Why call %{_initrddir}/corosync directly instead of using /sbin/service corosync?

Also note that if you're targeting F10 and later, you can use _initddir.  I'm not sure anyone knows where that extra 'r' came from.

What is a ".lcrso" file, and why do they need to be under %{_libexecdir}?  Have you investigated how this works with multilib?

You shouldn't need to explicitly mark anything under %{_docdir} as %doc.  In fact, you don't usually install doc files in %doc, but just list them as %doc in the %files list and they'll be copied there.  I.e. you can just do
  %doc LICENSE SECURITY

Are you sure the libraries should be in the -devel package?  The unversioned .so files should go there, definitely, but the actual libraries should generally be in the main package.  Otherwise, in order to run something built against them you have to install the -devel package, which is rarely correct. 

Why is corosync-objctl installed in the main package while its manpage is in the -devel package?  I didn't read the other manpages, but if you're sure that they're of use only to developers then they're fine in the -devel package.

You could probably make your %files lists very much smaller if you didn't list each file separately.  The manpages, though, I guess you can't do much about.
You could have %files lists like these, but maybe you just prefer to list everything out:

%files
%defattr(-,root,root,-)
%doc LICENSE SECURITY
%{_sbindir}/corosync*
%config(noreplace) /etc/corosync.conf
%config(noreplace) /etc/ld.so.conf.d/corosync-*.conf
%{_initrddir}/corosync
%{_libexecdir}/lcrso
%dir %{_libdir}/corosync/
%{_libdir}/corosync/*.so.*
%{_mandir}/man8/corosync-objctl.8*
%{_mandir}/man8/corosync_overview.8*
%{_mandir}/man8/logsys_overview.8*
%{_mandir}/man5/corosync.conf.5*

%files devel
%defattr(-,root,root,-)
%doc README.devmap
%{_includedir}/corosync
%{_libdir}/corosync/*.so
%{_mandir}/man3/*
%{_mandir}/man8/cpg_overview.8*
%{_mandir}/man8/evs_overview.8*
%{_mandir}/man8/confdb_overview.8*

Jason,

Thanks for your review.

To ansewr your questions:
1) the user id is needed for IPC security.  That user id was previously part of openais and has been moved into this package instead.  IPC authenticates all users using a UID equal to ais (or root of course).

2) i will change the initrddir issue you mentioned.  This spec file was taken from the original openais spec file from fedora 6 ages ago.

3) A lcrso file is a plugin with a specific binary format.  It will only be loaded by the corosync binary for which the architecture is built.  Therefore multilib is not an issue, since we wont have a 64 bit corosync and 32 bit plugins.  The plugins are required to match the binary bit size of the corosync binary.

4) Thanks for the tips on doc I will fix that.

5) The corosync main package doesn't use any library in the -devel package.  The devel package shared objects are libraries for use by third party developers writing software.  Just so we are clear, here is an example, and you can correct me if I misunderstand the packaging guidelines.

User wants to write an application using the cpg service.  They install corosync and corosync-devel.  corosync-devel provides the API header files and library objects for usage by the application's cpg utilization.

At no time does anything in the main corosync package reference any shared object in the devel package.  The -devel libraries are only for third party developers.

6) man page in wrong location - thanks for the catch.

7) I really like to list everything out so that rpmbuild catches maintainer (my) errors in packaging by telling me about a missing file or unpackaged files.  If this will be an issue for review I can trim the list however.

---

I will make a new RPM with the following changes:
a) fix the initrdir
b) fix the service start
c) move the corosync-ctl man page to the proper package

If you feel more actions are required please let me know.

And thank you for your time reviewing the package.

Regards
-steve

New package available for review (corosync-0.91-2):

http://developer.osdl.org/dev/openais/downloads/review/corosync-0.91-1.src.rpm
http://developer.osdl.org/dev/openais/downloads/review/corosync.spec

The changelog is:
* Wed Aug 20 2008 Steven Dake <sdake> - 0.91-2
- use /sbin/service instead of calling init script directly.
- put corosync-objctl man page in the main package.
- change all initrddir to initddir for fedora 10 guidelines.

Thank you for taking the time to review this package.

Regards
-steve

New package available for review (corosync-0.91-2):

http://developer.osdl.org/dev/openais/downloads/review/corosync-0.91-2.src.rpm
http://developer.osdl.org/dev/openais/downloads/review/corosync.spec

The changelog is:
* Wed Aug 20 2008 Steven Dake <sdake> - 0.91-2
- use /sbin/service instead of calling init script directly.
- put corosync-objctl man page in the main package.
- change all initrddir to initddir for fedora 10 guidelines.

Thank you for taking the time to review this package.

Regards
-steve

Hi Tom,

i am not an official sponsor, but the spec file looks good to me.

corosync is now a major (Build)Require for cluster. Is there any way we can speed this up?

Thanks
Fabio

Based on Fabio's feedback:

New package available for review (corosync-0.91-3):

http://developer.osdl.org/dev/openais/downloads/review/corosync-0.91-3.src.rpm
http://developer.osdl.org/dev/openais/downloads/review/corosync.spec

* Sun Aug 24 2008 Steven Dake <sdake> - 0.91-3
- move logsys_overview.8.* to devel package.
- move shared libs to main package.

I've verified that Jason's requests have made it into the 0.91-3 src rpm.  And the .spec file looks good to me with no rpmlint output.  I'm marking this packages as approved.

New Package CVS Request
=======================
Package Name: corosync
Short Description: The Corosync Cluster Engine
Owners: sdake
Branches: F-10 Rawhide
InitialCC: sdake

(In reply to comment #9)
> New Package CVS Request
> =======================
> Package Name: corosync
> Short Description: The Corosync Cluster Engine
> Owners: sdake
> Branches: F-10 Rawhide
there is no F-10 Rawhide branch, the rawhide branch is called "devel" do you want me to go ahead with this?

> InitialCC: sdake

yes please.  I apologize for the confusion.  I didn't relize the trunk was called "devel".

Thanks
-steve

cvs is done
Please check if everything is ok.

Hi Steve,

if builds fine in koji, close this bug as NEXTRELEASE.

merged into rawhide.