[MIR] cluster-glue

Bug #527142 reported by Ante Karamatić
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cluster-glue (Ubuntu)
Fix Released
Undecided
Unassigned
heartbeat (Ubuntu)
Invalid
Undecided
Unassigned

Bug Description

Binary package hint: cluster-glue

1. Availability: all

2. Rationale: The package helps meet https://blueprints.launchpad.net/ubuntu/+spec/packageselection-server-n-cluster-stack blueprint goal. Needed binary packages are cluster-glue and cluster-glue-dev

3. Security: No CVEs

4. QA: Package is on Debian and Ubuntu has latest release. Upstream is very active (http://hg.linux-ha.org/glue/). 1 bugs reported in Debian, 1 bug report in Ubuntu.

5. UI standards: none

6. Dependencies: most in main. MIRs for Universe packages:

https://bugs.edge.launchpad.net/ubuntu/+source/libnet/+bug/515973
https://bugs.edge.launchpad.net/ubuntu/+source/openhpi/+bug/515976
https://bugs.edge.launchpad.net/ubuntu/+source/libesmtp/+bug/515996

7. Standards: Lintian warnings:

W: cluster-glue: binary-without-manpage usr/sbin/lrmadmin
W: cluster-glue: binary-without-manpage usr/sbin/sbd

Package is packaged with debhelper and has no patching system. Source format is 3.0.

8. Maintenance: easy

9. Background information: this package is one of dependencies for new cluster stack in Ubuntu. This package, cluster-agents and heartbeat together form what was known as heartbeat 2.99.

Revision history for this message
Ante Karamatić (ivoks) wrote :

Subscribing ubuntu-release was a mistake, sorry.

Martin Pitt (pitti)
Changed in cluster-glue (Ubuntu):
assignee: nobody → Michael Terry (mterry)
Revision history for this message
Michael Terry (mterry) wrote :

- The lintian warnings about missing man pages don't excite me, but isn't a dealbreaker.

- The packaging is a little old school (doesn't use cdbs or dh7) but is not terribly arcane. The .install file manually lists every file to be installed (instead of just directories) which makes me worry about missing new files when upstream adds them.

- I don't understand why this package is a hodgepodge of libraries. Each library should be split into its own binary package. For example, at least libplumbgpl2 (-dev), libpils2 (-dev), libstonith1 (-dev), liblrm2 (-dev), and libplumb2 (-dev) as well as non-library packages for the daemons and executables (like ha_logd). This does strike me as a dealbreaker.

- debian/copyright should have the GPL and LPGL header text verbatim (the "This program is free software..." bit). Just the reference to common-licenses is not enough. It should also mention which versions of the GPL apply. Also, I'm 70% sure that while using BSD code in GPL programs is legitimate, you actually have to relicense the BSD as GPL. So those files should have GPL boilerplate as well. I realize this is not a packaging bug but an upstream one. But debian/copyright needn't mention BSD, since no binary or library is apparently (to me) being released with BSD license.

- There is no debian/watch file.

- There are some tests in at least lrm/tests. Can those be made to run during package build to catch any errors?

- There are some minor issues in the use of sprintf (instead of snprintf or g_strdup_printf (which is used in one file), even in files with comments at the top about how much better snprintf is -- see lib/clplumbing/cl_netstring.c) and malloc (which is weird since there is an included cl_malloc, a special wrapper for it). While I'm not a security expert, these usages don't strike me as bad enough to hold up the package though since this is pretty special-case software.

- The HA team seems on top of this package, which is great.

So all in all, I don't think I can approve this. The biggest issue is that the libraries aren't split out into their own, versioned packages. If that and the debian/copyright file is fixed, I would approve. The rest of the issues would definitely be nice to see addressed (or passed upstream) too though.

Changed in cluster-glue (Ubuntu):
assignee: Michael Terry (mterry) → nobody
status: New → Incomplete
Revision history for this message
Ante Karamatić (ivoks) wrote :

So, this patch solves some of the issues reported by Michael:

1) man pages: i'll talk with upstream on solving this
2) packaging now done with cdbs
3) headers and libraries from cluster-glue(-dev) are now in separate packages
4) I've added copyright notice and license headers; I need to check with upstream if that's copyright notice is actually correct. As for BSD: there are no BSD licensed files in the source, all is GPL or LGPL
5) upstream uses tagging in mercurial, so I'm not sure how to handle that with debian/watch file
6) Those tests can be used only with additional packages installed (not part of this source); i guess they are leftover from heartbeat split. I'll check with upstream.
7) will talk with upstream

Michael, if these changes seem ok, I'll upload them and fix other packages depending on libraries from this source.

Changed in cluster-glue (Ubuntu):
status: Incomplete → New
Revision history for this message
Ante Karamatić (ivoks) wrote :

Ah, btw, remaining lintian warnings:

W: cluster-glue: binary-without-manpage usr/sbin/lrmadmin
W: cluster-glue: binary-without-manpage usr/sbin/sbd

Revision history for this message
Ante Karamatić (ivoks) wrote :

I've deleted the patch cause we'll do library split in debian first and then merge that into ubuntu.

Changed in cluster-glue (Ubuntu):
status: New → Incomplete
Revision history for this message
Alexander Sack (asac) wrote :

please let us know once that happened, get the package synched and set the status back to New to get MIR team take a look at it again.

Changed in cluster-glue (Ubuntu):
assignee: nobody → Andres Rodriguez (andreserl)
description: updated
Changed in cluster-glue (Ubuntu):
assignee: Andres Rodriguez (andreserl) → nobody
status: Incomplete → New
description: updated
Kees Cook (kees)
Changed in cluster-glue (Ubuntu):
status: New → Incomplete
Revision history for this message
Ante Karamatić (ivoks) wrote :

All deps are now in main except for these two:

https://bugs.edge.launchpad.net/ubuntu/+source/heartbeat/+bug/527182
https://bugs.edge.launchpad.net/ubuntu/+source/openhpi/+bug/515976

which would be pulled automaticaly (IIUC)

Ante Karamatić (ivoks)
Changed in cluster-glue (Ubuntu):
status: Incomplete → New
Revision history for this message
Alexander Sack (asac) wrote :

mterry, could you give your final blessing here?

Changed in cluster-glue (Ubuntu):
assignee: nobody → Michael Terry (mterry)
Revision history for this message
Michael Terry (mterry) wrote :

My original big complaint was that the package was a collection of libraries, rather than being multiple individual library packages. Ante, you indicated that was going to happen in Debian and we'd sync, but I don't see that in the packaging...

Changed in cluster-glue (Ubuntu):
status: New → Incomplete
Revision history for this message
Ante Karamatić (ivoks) wrote : Re: [Bug 527142] Re: [MIR] cluster-glue

On 24.08.2010 19:48, Michael Terry wrote:

> My original big complaint was that the package was a collection of
> libraries, rather than being multiple individual library packages.
> Ante, you indicated that was going to happen in Debian and we'd sync,
> but I don't see that in the packaging...

Right. That split didn't happen and Debian (and even upstream, IIRC)
devs didn't like the idea. Even though I agree that there should be a
separate libraries, but as far as I know there are no plans to make them
in near future.

FWIW, both RedHat/Fedora and SUSE also keep all the libs bundled in a
single package.

Revision history for this message
Michael Terry (mterry) wrote :

So, I re-examined the source and none of the issues I originally listed have been addressed.

The problem with bundled libraries is that if the SONAME changes on just one library, you have to change all packages that depend on any one of them. Is upstream guaranteeing that all SONAMEs will change in lock-step?

Such bundling is not recommended by Policy [1] or the Debian library packaging guide [2]. Seems odd for the Debian packagers to refuse to split. Is that discussion in a bug or mailing list somewhere?

It's possible that bundled libraries aren't really as big a maintenance pain as I think they are. I will ping some other people for insight.

[1] http://www.debian.org/doc/debian-policy/ch-sharedlibs.html
[2] http://www.netfort.gr.jp/~dancer/column/libpkg-guide/libpkg-guide.html#id291475

Revision history for this message
Michael Terry (mterry) wrote :

Talked to pitti about it, and he confirmed that bundled libs are really a "disaster waiting to happen whenever the ABI changes". So that remains a blocker for MIR.

Revision history for this message
Ante Karamatić (ivoks) wrote : Re: [Ubuntu-ha] [Bug 527142] Re: [MIR] cluster-glue

On 25.08.2010 17:23, Michael Terry wrote:

> Talked to pitti about it, and he confirmed that bundled libs are really
> a "disaster waiting to happen whenever the ABI changes". So that
> remains a blocker for MIR.

I've had a talk with upstream, Debian devs and people interested in
seeing this in Ubuntu. Problems mentioned are real and existing, that's
not questionable. But the thing is that cluster-glue will never change
and is a package that will eventually be replaced by other libraries and
binary packages. It's just one part of the ongoing effort to standardize
on cluster stack for Linux distributions.

If we split that single library into multiple packages, we will create a
diff with Debian not only in cluster-glue, but also in cluster-agents
and pacemaker. Having that diff isn't worth it since Debian won't do it
and package will go away before next LTS.

We need it in main cause it's a build-dep for pacemaker which, as it is
in universe, blocks building redhat-cluster-suite, which is in main.

If we keep it in universe, we'll have to drop couple of features that
we've been planing for quite some time; OCFS2 and GFS2 support for
pacemaker. Alternative is to not have a cluster stack at all in Ubuntu
main. That fact would hardly qualify Ubuntu server as an alternative to
some other distributions or operating systems.

Bottom line; packaging isn't perfect, but no other projects except
pacemaker (and cluster-agents) should build-depend on it and before
12.04 it will cease to exist anyway. Debian (and all other
distributions) is fine with current state of the package, why should we
be an exception?

Thank you for you time

Revision history for this message
Loïc Minier (lool) wrote :

We sometimes bundle libraries when they are guaranteed to change SONAME at the same time and upstream makes good uses of SONAMES, libglib2.0-0 is one example. This has to be decided on a case-by-case basis. Don't look too closely at the library packaging guide, it's relatively old and never was a consensual document.

After taking a look at the source, it seems like the libs are entirely different in this case and aren't particularly expected to share a SONAME. I was a bit surprized that the ChangeLog doens't mention any SONAME change, either this has been perfectly designed or this didn't change for a long time or the SONAME change was forgotten.

Some of the libs look like convenience libs, but I'm not familiar enough to judge.

Given that only 4 packages build-dep on libcluster-glue-dev, I'm tempted to think that this is a case of an upstream providing all the clients for the libs, and releasing updates in lockstep. Perhaps this is an indication that some tighter versioning should be enforced, either by matter of symbol files (which are really missing here) or via "dh_makeshlibs -V". The latter seems easy to add and likely to prevent accidents, it's pain at a larger scale though.

Revision history for this message
Michael Terry (mterry) wrote :

Ante, how about dh_makeshlibs -V as a way to reduce risk?

Revision history for this message
Ante Karamatić (ivoks) wrote :

Sure, I could make all packages hard depens on exact version of cluster-glue.

Revision history for this message
Loïc Minier (lool) wrote :

Note that dh_makeshlibs -V will only add depends on >= the upstream version the packages are built against.

If the rdeps are NOT rebuilt, then they will stay with the Depends/shlibs they got at the time of their build. So there's still potential for screwup if the lib break ABI moving forward and you don't upload the rdeps subsequently. But it's still less risky than when allowing any combinations because we don't review ABI changes properly.

Revision history for this message
Ante Karamatić (ivoks) wrote :

On 08.09.2010 14:07, Loïc Minier wrote:

> Note that dh_makeshlibs -V will only add depends on >= the upstream
> version the packages are built against.

But with -V 'libcluster-glue (=x.y.z)' rdep would require exact version
of libcluster and one couldn't update libcluster-glue without updating
rdep (and vice versa).

Revision history for this message
Matthias Klose (doko) wrote :

seeding heartbeat in supported-misc-servers for maverick; this seed changed should be reverted once cluster-glue is in main in natty.

Revision history for this message
Matthias Klose (doko) wrote :

silly me, had to demote heartbeat, libnet and openhpi.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

Michael,

Cluster-glue has been split into their own binary packages as requested.

Could you please re-evaluate this MIR?

Thank you!

description: updated
Revision history for this message
Andres Rodriguez (andreserl) wrote :

Also, could you please promote heartbeat, libnet and openhpi again given that they were demoted because cluster-glue wasn't in man?
Thank you.

Revision history for this message
Michael Terry (mterry) wrote :

Shouldn't there be .symbols files for the libraries or -V arguments to dh_makeshlibs?

Revision history for this message
Andres Rodriguez (andreserl) wrote :

Hi Michael,

I believe that's missing. So that would be just to change the following in binary common, correct?:

dh_makeshlibs
to
dh_makeshlibs -V

Should I go ahead and do it, and upload a new Ubuntu revision?

Revision history for this message
Michael Terry (mterry) wrote :

OK, with the new -V change and the split, approved. Thanks Ante for driving this.

Changed in cluster-glue (Ubuntu):
assignee: Michael Terry (mterry) → nobody
status: Incomplete → Fix Committed
Michael Terry (mterry)
Changed in heartbeat (Ubuntu):
status: New → Invalid
Revision history for this message
Matthias Klose (doko) wrote :

promoted

Changed in cluster-glue (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.