[ClusterLabs Developers] If anybody develops against libpe_status.so: skipped soname bump (Was: Pacemaker 2.0.2 final release now available)

[Ken Gaillot]

On Fri, 2019-06-14 at 23:57 +0200, Jan Pokorný wrote:
> On 14/06/19 14:56 -0500, Ken Gaillot wrote:
> > On Fri, 2019-06-14 at 20:13 +0200, Jan Pokorný wrote:
> > > > On Thu, 2019-06-06 at 10:12 -0500, Ken Gaillot wrote:
> > > > 
> > > > Source code for the Pacemaker 2.0.2 and 1.1.21 releases is now
> > > > available:
> > > > 
> > > > 
> > 
> > 
https://github.com/ClusterLabs/pacemaker/releases/tag/Pacemaker-2.0.2
> > > > 
> > > > 
> > 
> > 
https://github.com/ClusterLabs/pacemaker/releases/tag/Pacemaker-1.1.21
> > > 
> > > In retrospect (I know, everybody is a general once the battle is
> > > over), called out for with some automated tests in Fedora, there
> > > were
> > > some slight discrepancies -- depending on whether any external
> > > clients
> > > of particular "wannabe internal" libraries of pacemaker
> > > accompanied
> > > with "wannabe internal" headers, none of which are marked so
> > > expressly
> > > (and in case of headers, are usually shipped in dev packages
> > > anyway).
> > 
> > All public API is documented:
> > 
> >   https://clusterlabs.org/pacemaker/doxygen/
> > 
> > Anything not documented there is private API.
> 
> It's rather simplistic (and hypocritical) view not being part of any
> written "contract", isn't it? :-)
> 
> > remote.h should be in noinst_HEADERS, thanks for catching that. It
> > would also be a good idea to put "_internal" in all internal
> > headers' names to be absolutely clear; most of them already have
> > it.
> 
> Yes, that was that surprising moment here.
> 
> > > For the piece of mind, I am detailing the respective library that
> > > would likely have been eligible for an explicit soname bump and
> > > why.
> > > If you feel affected, please speak up so we have a clear
> > > incentive to
> > > publish a "hotfix" for downstreams and direct consumers,
> > > otherwise
> > > at least I don't feel compelled to anything immediate beyond this
> > > FYI,
> > > and we shall rather do it in 2.0.3 even if not otherwise
> > > justified
> > > with an inter-release delta, so there isn't a tiniest glitch
> > > possible
> > > when 2.0.2 is skipped on the upgrade path (which is generally not
> > > recommended but would be understandable if you happen to rely on
> > > those very libpe_status.so ABI details).
> > > 
> > > The mentioned ABI changes are:
> > > 
> > > * libpe_status.so.28.0.2 (2.0.1: soname 28.0.1)
> > >   - include/crm/pengine/remote.h: function renames, symbolic
> > > notation:
> > >     { -> pe__}{is_baremetal_remote_node -> is_remote_node,
> > >                is_container_remote_node -> is_guest_node,
> > >                is_remote_node -> is_guest_or_remote_node,
> > >                is_rsc_baremetal_remote_node ->
> > > resource_is_remote_conn,
> > >                rsc_contains_remote_node ->
> > > resource_contains_guest_node}
> > > 
> > > (all other ABI breaking changes appear self-contained for not
> > > being related to anything exposed through what could be
> > > considered
> > > a public header/API -- not to be confused with ABI)
> > 
> > Since those functions are internal API, we don't need a soname
> > bump.
> > Distributing the header was a mistake and should not be considered
> > making it public API. The only functions in there that had doxygen
> > blocks were marked internal, so that helps.
> > 
> > As an aside, the entire libpe_status was undocumented until 2.0.1,
> > but that was an oversight (a missing \file line).
> 
> In FOSS, (un)documentation aspect doesn't play that much of a role...
> 
> > In practice there were some projects that used it, and we did bump
> > the soname for most functions. Now however it's documented
> > properly,
> > so the line should be clear.
> 
> Not at all, see above.
> 
> Traces of the pre-existing mess have some momentum.
> 
> Anyway, good to know the root cause, question is how to deal with
> the still real fallout.

What's the fallout? An internal function that no external application
uses changed, which doesn't require a soname bump.

I'll handle it by renaming the header and moving it to noinst.

> 
> > > Note that there's at least a single publicly known consumer of
> > > libpe_status.so, but luckily, sbd only uses some unaffected pe_*
> > > functions.  Said after-the-fact bump of said library would
> > > require
> > > it to be rebuilt as well (and all the SW that'd be in the same
> > > boat), so even less appealing to do that now, but note that
> > > such rebuild will be needed with said planned bump for 2.0.3.
> > > 
> > > But perhaps, some other changes as announced in [1] will be
> > > faster
> > > than that -- to that account, I'd note that perhaps applying
> > > single source -> multiple binary copies of code scheme is not all
> > > that bad and we could move some of shared internal only code into
> > > static libraries subsequently used to feed the links from the
> > > actual daemons/tools code objects -- or the private libraries
> > > shall at least be factually privatized/unshared, i.e., put into
> > > a private, non-standard location (this is what, e.g., systemd
> > > uses)
> > > where only "accustomed" executables can find them.
> > > 
> > > [1]
> > > https://lists.clusterlabs.org/pipermail/developers/2019-February/001358.html
> 
> _______________________________________________
> Manage your subscription:
> https://lists.clusterlabs.org/mailman/listinfo/developers
> 
> ClusterLabs home: https://www.clusterlabs.org/
-- 
Ken Gaillot <kgaillot at redhat.com>