Re: [ClusterLabs Developers] If anybody develops against libpe_status.so: skipped soname bump (Was: Pacemaker 2.0.2 final release now available)
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
>
Re: [ClusterLabs Developers] If anybody develops against libpe_status.so: skipped soname bump (Was: Pacemaker 2.0.2 final release now available)
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.
>> 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
--
Jan (Poki)
pgpJu_eOQaMYq.pgp
Description: PGP signature
___
Manage your subscription:
https://lists.clusterlabs.org/mailman/listinfo/developers
ClusterLabs home: https://www.clusterlabs.org/
Re: [ClusterLabs Developers] If anybody develops against libpe_status.so: skipped soname bump (Was: Pacemaker 2.0.2 final release now available)
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. 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.
> 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 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.
> 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
--
Ken Gaillot
___
Manage your subscription:
https://lists.clusterlabs.org/mailman/listinfo/developers
ClusterLabs home: https://www.clusterlabs.org/
Re: [ClusterLabs Developers] If anybody develops against libpe_status.so: skipped soname bump (Was: Pacemaker 2.0.2 final release now available)
On 14/06/19 20:13 +0200, Jan Pokorný wrote:
> 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).
Of course, an alternative applicable right now (also suitable for
those who self-compile ... and use libpe_status.so in their client
code at the same time) and avoiding the soname bump is to add the
original symbols back, e.g. using
__attribute__((alias ("original_name")))
for brevity.
> 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)
--
Jan (Poki)
pgpBiHNh8gfAK.pgp
Description: PGP signature
___
Manage your subscription:
https://lists.clusterlabs.org/mailman/listinfo/developers
ClusterLabs home: https://www.clusterlabs.org/
