Spec URL: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec SRPM URL: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-2.fc12.src.rpm Description: Reusable cluster components - part of the new cluster stack A collection of common tools that are useful for writing cluster managers such as Pacemaker. Provides a local resource manager that understands the OCF and LSB standards, and an interface to common STONITH devices. Background: I've recently been hired by RedHat in order to work on Pacemaker (which requires this new package). We'd like to include it in F12 so that we can offer it as a tech preview in RHEL6. This package is built from the pre-release snapshots that will become 1.0.0. Submission of the Pacemaker package will follow shortly. Please note, this is my first package so I am looking for a sponsor. rpmlint output: [1] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package. [2] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package. [3] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package. [4] libcluster-glue1.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. [5] libcluster-glue-devel.x86_64: W: no-dependency-on libcluster-glue/libcluster-glue-libs/liblibcluster-glue [6] libcluster-glue-devel.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. 5 packages and 1 specfiles checked; 3 errors, 3 warnings. Items 1-3 appear to be an rpmlint bug, the directories exist to record core files. The choice of 0700 was intentional and is intended to restrict access to the core file contents to the users after which the directories are named. I believe items 4 and 6 are erroneous as the relevant files are included in the cluster-glue package. I believe item 5 is a bug in rpmlint as the subpackage is libcluster-glue1 and the -devel package does indeed have a dependancy on it.
I noticed some other review requests include koji builds, so here is the one for cluster-glue: https://koji.fedoraproject.org/koji/taskinfo?taskID=1485160
New spec + source rpm uploaded: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-3.fc12.src.rpm In particular, Source0 is now a reference to the package's Mercurial repo. Conducted a self-review using http://fedoraproject.org/wiki/Packaging/ReviewGuidelines and it is my assessment that the package meets all the requirements.
I believe the following needs to change, but I am not an official reviewer. 1) All RPMs created need a LICENSE file added to their respective %doc sections. This will get rid of the annoying rpmlint warning about docs, and also follow the proper guidelines for packaging. 2) libcluster-glue should be called cluster-glue-lib, or cluster-gluelib, or cluster-glue-libs, but not 100% positive on what the exact policy is regarding naming of library packages. A more experienced packager may know or you could ask on the fedora developer ml or in the fedora developer channel on irc. 3) I assume the main cluster-glue package depends on libcluster-glue, but there is no requires for that. (this is the 5 rpmlint bug). I don't believe rpm automatically puts dependencies in subpackages. You have to do that yourself. 4) libcluster-glue-devel should be called cluster-gluelib-devel or whatever you decided from step #2 above. The rest looks good and the 0700 directory permissions make sense.
Thanks for taking the time to look over the package. Regarding the review flag, is the follow from http://fedoraproject.org/wiki/Package_Review_Process incorrect? [Quote] Wait for someone to review your package! At this point in the process, the fedora-review flag is blank, meaning that no reviewer is assigned. A reviewer takes on the task of reviewing your package. They will set the fedora-review flag to ? [/Quote] 1) I saw this under the SHOULD section and decided to consider it optional. But I think you're right, I may as well add it and make rpmlint happy. 3) Np. I'll specify it manually, but as binaries from cluster-glue depend on libs from libcluster-glue I had assumed that there would be an automatic dependancy created. Is that not the case? Also, the warning mentioned is from libcluster-glue-devel.x86_64, so I don't believe it will go away after this change. 2,4) I thought I remembered seeing that both lib%{name} and %{name}-lib were allowed, but I can't find anything one way or the other. If anyone knows the official policy here I'd be happy to comply.
Quick update, while searching for the shared library naming policy, I came across this: [Quote from https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires] We generally rely on rpmbuild to automatically add dependencies on library SONAMEs. Modern package management tools are capable of resolving such dependencies to determine the required packages. [/Quote] Which seems to recommend against item 3 from the review.
I probably incorrectly set it; I should clear it then.
I hear varying things sometimes - in this case, that setting it to '?' sends a message to the reviewers prompting them to do a review.
Another update: - Update the tarball from upstream to version 75cab275433e - Include an AUTHORS and license file in each package - Change the library package name to cluster-glue-libs to be more Fedora compliant SPEC: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-4.fc12.src.rpm Current rpmlint output follows: [beekhof@rawhide glue]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm cluster-glue.spec cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700 5 packages and 1 specfiles checked; 3 errors, 0 warnings.
ok. Finally found some time to look at this a bit. ;) So, my understanding is that this is a newerversion/fork of heartbeat thats stripped down and only has the parts pacemaker needs/wants in it. Is that correct or am I misunderstanding? :) First, Fedora really would prefer to avoid Conflicts if at all possible. (See https://fedoraproject.org/wiki/Packaging/Conflicts). So, is there some way we can avoid that here? - Can this package just rename the dirs/files that conflict with 'cluster-glue' or something and that way they could both be installed and pacemaker could have a compile time option to use whichever one. (Or autodetect it based on the files found, etc). - Is using this better than using heartbeat for pacemaker? Or are they the same? If the same, perhaps we hold off on adding this to fedora for now, let pacemaker use heartbeat and then down the road when enough folks have moved to pacemaker submit this and obsolete heartbeat? - Perhaps pacemaker could fold in the needed parts of this and just use them internally and avoid conflict with heartbeat that way? Thoughts?
(In reply to comment #9) > ok. Finally found some time to look at this a bit. ;) Much appreciated > So, my understanding is that this is a newerversion/fork of heartbeat thats > stripped down and only has the parts pacemaker needs/wants in it. Is that > correct or am I misunderstanding? :) We're in the process of splitting the rest of the codebase. Its essentially done but just needs the sort of packaging cleanup that we're doing here before it will be made available for general consumption. I've also been on vacation a bit which slowed things down. The idea is to split the code-base into pieces based on their usage and update frequency. So essentially: heartbeat 2.1 + features + bugfixes = cluster-glue + cluster-agents + heartbeat 3.0 + pacemaker 1.0 or more recently (and relevantly for those not using the crm): heartbeat 2.99 + bugfixes = cluster-glue + cluster-agents + heartbeat 3.0 This was mentioned on the mailing list late last month: http://www.gossamer-threads.com/lists/linuxha/dev/56290 > > First, Fedora really would prefer to avoid Conflicts if at all possible. > (See https://fedoraproject.org/wiki/Packaging/Conflicts). > So, is there some way we can avoid that here? The Conflicts was only intended to be temporary. I didn't want to force a synchronized update to the heartbeat package just to get Pacemaker (and therefor cluster-glue) in. The intention from upstream is that heartbeat 3.0 (which would be the messaging and membership layer as well as the v1 resource manager) would build against cluster-glue. If/when the cluster-glue package gets approved, I would then submit an update to the Fedora's heartbeat package and we could then remove the Conflicts directive. On the other hand, if Fedora wanted to preserve heartbeat in its current pre-split state, I figured the Conflicts would (safely) allow that. > > - Can this package just rename the dirs/files that conflict with 'cluster-glue' > or something and that way they could both be installed and pacemaker could have > a compile time option to use whichever one. (Or autodetect it based on the > files found, etc). In theory this could be possible, but ideally it wouldn't be necessary since we'd have both Pacemaker and an updated Heartbeat both using cluster-glue as a common base. > > - Is using this better than using heartbeat for pacemaker? Or are they the > same? Its a subset, essentially: clplumbing + lrmd + stonith > If the same, perhaps we hold off on adding this to fedora for now, let > pacemaker use heartbeat and then down the road when enough folks have moved to > pacemaker submit this and obsolete heartbeat? I don't think that's possible, since heartbeat 2.1 conflicts massively with Pacemaker. I suspect there'd be all sorts of compiler problems, not least of all due to multiple copies of "crm" header files. > - Perhaps pacemaker could fold in the needed parts of this and just use them > internally and avoid conflict with heartbeat that way? That was considered undesirable. If we were intending to fork the code, then putting it in Pacemaker would make more sense. But that would be a last possible resort. Does that help?
ok, that makes sense. So, we need to get this reviewed and in, then update heartbeat and also review/import pacemaker. Conflicts are nasty in that basically we have to tell the user "hey, we can't do what you want". :( I think we can possibly avoid the conflict by importing this around the same time we update heartbeat to use it. I don't know if there is a big group that wants to keep heartbeat 2.1.x in fedora, but time does march on, and fedora should be moving with it. ;) Anyhow, I guess I will look at doing a review here soon... ;)
Hi guys, I did run through a review of cluster-glue. rpmlint output: [fabbione@daitarn-fedora rpmbuild]$ rpmlint SPECS/cluster-glue.spec SRPMS/cluster-glue-0.9-4.fc12.src.rpm RPMS/x86_64/cluster-glue-* cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700 5 packages and 1 specfiles checked; 3 errors, 0 warnings. For the record: SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [22] the -devel package Requires: %{name} = %{version}-%{release} If this is done on purpose it should be documented in the spec file otherwise dropped. SHOULD: The reviewer should test that the package builds in mock. [29] For some reasons mock is busted in rawhide and I am unable to perform this check. The package builds fine in an installed rawhide machine. The package passes all the checklist for ReviewGuidelines. Kevin: I hope this can help speed up the process. Fabio
(In reply to comment #12) > SHOULD: Usually, subpackages other than devel should require the base package > using a fully versioned dependency. [22] > > the -devel package Requires: %{name} = %{version}-%{release} > > If this is done on purpose it should be documented in the spec file otherwise > dropped. I'll drop it. > SHOULD: The reviewer should test that the package builds in mock. [29] > > For some reasons mock is busted in rawhide and I am unable to perform this > check. The package builds fine in an installed rawhide machine. Strange, I'm running rawhide from last week and mock seems to work fine.
New version: - Use linux-ha.org for Source0 - Remove Requires: $name from -devel as its implied - Instead of 'daemon', use the same user and group from the Heartbeat package and create it if necessary* SPEC:http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-5.fc12.src.rpm * Originally I had been of the notion that we should use daemon instead of hacluster/haclient. However for the sake of consistency, its probably better not to change the user/group. Looking at the Heartbeat spec, it does not create a user with a fixed id. Is that intentional? There is a comment in the changelog indicating that it used to, with uid=24 it seems, but that functionality seems to have been dropped somewhere along the line. Anyway, I've preserved the Heartbeat's %pre semantics when adding it to cluster-glue. Btw. I can no longer confirm that it builds with mock as the big rebuild seems to cause: ERROR with rpm_check_debug vs depsolve: rpmlib(PayloadIsXz) <= 5.2-1 is needed by glibc-devel-2.10.90-7.1.x86_64
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPLv2+ and LGPLv2+) OK - License field in spec matches See below - License file included in package OK - Spec in American English OK - Spec is legible. See below - Sources match upstream md5sum: a9aba6ae59030a148dd95bfea163852c 75cab275433e.tar.gz ecc791dec7788293ad3d7f22b1d80cf5 75cab275433e.tar.gz.orig OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Headers/static libs in -devel subpackage. OK - Spec has needed ldconfig in post and postun OK - .so files in -devel subpackage. OK - -devel package Requires: %{name} = %{version}-%{release} OK - .la files are removed. OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. OK - Package obey's FHS standard (except for 2 exceptions) See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have subpackages require base package with fully versioned depend. OK - Should have dist tag OK - Should package latest version OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin Issues: 1. What is the 'uid' define for? 2. You should use %global instead of %define. 3. Is this a pre-release for version 1.0? Or a post release of 0.9? or something else? The release might need adjustment. See: https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release I suspect it should be something like: 1.0.0-0.0.75cab275433e ? 4. The %{configure} macro should pass all of these I think, so no need to do so: --prefix=%{_prefix} \ --localstatedir=%{_var} \ --libdir=%{_libdir} 5. Your source doesn't match the upstream Source url. Perhaps you are using a checkout? It needs to match exactly. 6. rpmlint says: cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700 cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700 5 packages and 0 specfiles checked; 3 errors, 0 warnings. I think those can be ignored.
(In reply to comment #15) > a9aba6ae59030a148dd95bfea163852c 75cab275433e.tar.gz > ecc791dec7788293ad3d7f22b1d80cf5 75cab275433e.tar.gz.orig > Issues: > > 1. What is the 'uid' define for? Copied from heartbeat.spec, will remove. > 2. You should use %global instead of %define. can do > 3. Is this a pre-release for version 1.0? Or a post release of 0.9? or > something else? pre-release of 1.0 I'll change the version and add the hg revision as part of the alphatag. > The release might need adjustment. See: > https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release > I suspect it should be something like: 1.0.0-0.0.75cab275433e ? > > 4. The %{configure} macro should pass all of these I think, so no need to do > so: > --prefix=%{_prefix} \ > --localstatedir=%{_var} \ > --libdir=%{_libdir} Will remove and confirm. > 5. Your source doesn't match the upstream Source url. Perhaps you are using a > checkout? > It needs to match exactly. Mercurial embeds the repo in the tarball (in the .hg_archival.txt file). [beekhof@rawhide Pacemaker-1-0-c9120a53a6ae]$ cat .hg_archival.txt repo: 8448b17e67437947c48c639c6faa4371c4a14b3b node: c9120a53a6ae63119f96456cafb082d3c9f8a3d2 I forgot about this and only confirmed that the new url worked, not that md5sum's matched. Consistent use of hg.linux-ha.org for all tarballs will avoid this problem. New SRPM/SPEC to follow
Update: - Use %global instead of %define - Remove unused rpm variable - Remove redundant configure options - Change version to 1.0.0 pre-release and include Mercurial tag in version - md5sums now match SPEC: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-1.0-6.75cab275433e.hg.fc12.src.rpm I followed the svn pattern for the alphatag, however since letters appear in Mercurial tags, I added an additional decimal point. Unfortunately, presumably due to the rebuild, I can't build anything at the moment. For this reason, I retained --localstatedir=%{_var} as it wasn't 100% obvious that redundant.
(In reply to comment #17) > For this reason, I retained --localstatedir=%{_var} as it wasn't 100% obvious > that redundant. I got things building (and installing) again and was able to confirm that the above does not seem to be passed by default and is therefore required.
This looks pretty close, but the release is not quite right. You want a 0. leading there. So, 1.0-0.6.75cab275433e.hg This is so that when 1.0 comes out, it can be 1.0-1.. and update cleanly from the 1.0-0.N.tag Can you spin up an update with that and I will check the other items and confirm... ;)
New SRPM that includes a leading zero when alphatag is defined http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-1.0-0.7.75cab275433e.hg.fc12.src.rpm SPEC is in the same location as normal
ok, everything looks good with that version from what I can see. Sources checkout ok. a9aba6ae59030a148dd95bfea163852c ./75cab275433e.tar.gz a9aba6ae59030a148dd95bfea163852c ./75cab275433e.tar.gz.orig All items from above seem to be solved, so this package is APPROVED. I will go ahead and sponsor you now. Shall we wait to import this until we make changes to heartbeat? I will try and look at the pacemaker review soon too... if you could change the version there based on this that would be great.
Kevin, Thanks for the review. I appreciate it.
(In reply to comment #21) > All items from above seem to be solved, so this package is APPROVED. > I will go ahead and sponsor you now. Awesome! :-) > Shall we wait to import this until we make changes to heartbeat? I think it would make reviewing pacemaker and verifying the heartbeat changes easier if it was already imported (since it would simplify mock builds and make koji --scratch possible). So I'd vote for not waiting. > I will try and look at the pacemaker review soon too... if you could change the > version there based on this that would be great. Already done :-)
Requesting the creation of a CVS module as per http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Sorry, didnt read far enough down the page. Missing information follows New Package CVS Request ======================= Package Name: cluster-glue Short Description: Reusable cluster components Owners: andrew Branches: F-11 F-12 InitialCC: llh fdinitto
Owners and InitialCC need to be FAS account names, not email addresses. Also, we cannot yet make F-12 branches. Please submit a corrected CVS request and re-set the fedora-cvs flag and we'll take care of things.
New Package CVS Request ======================= Package Name: cluster-glue Short Description: Reusable cluster components Owners: andrew Branches: F-11 InitialCC: lon fabbione
not enough coffee: New Package CVS Request ======================= Package Name: cluster-glue Short Description: Reusable cluster components Owners: beekhof Branches: InitialCC: lon fabbione (I also confirm that we don't need F-11 branch)
CVS done.
Built and tagged: http://koji.fedoraproject.org/koji/buildinfo?buildID=124411 Closing. Thanks everyone!