Re: codec changes

[Rafael Schloming <rh...@alum.mit.edu>]

We seem to have reached consensus here, but I haven't seen any commits on
this. We should probably fix this before 0.10 so we don't end up putting
out a new API and then deprecating it in the next release. Is anyone
actually working on this?

--Rafael

On Wed, Apr 15, 2015 at 2:29 PM, Andrew Stitcher <as...@redhat.com>
wrote:

> On Wed, 2015-04-15 at 07:13 -0400, Rafael Schloming wrote:
> > On Tue, Apr 14, 2015 at 1:27 PM, Alan Conway <ac...@redhat.com> wrote:
> >
> > > That works for me, now how do we manage the transition? I don't think
> we
> > > can afford to inflict "yum update proton; all proton apps crash" on our
> > > users. That means we cannot change the behavior of existing function
> > > names. I don't much like adding encode_foo2 for every encode_foo but I
> > > don't see what else to do.
> > >
> >
> > We could mark the old ones as deprecated and add the new ones as
> > pn_xxx_encode2 and provide a feature macro that #defines pn_xxx_encode to
> > pn_xxx_encode2. Then after a sufficient transition period we can get rid
> of
> > the old versions.
>
> Along these lines it is also possible to keep ABI (binary library
> compatibility) by using versioned library symbols as well. There is a
> level of linker magic needed, but at least on Unix it's well understood,
> if a little arcane.
>
> Another perfectly reasonable approach would be to create a new name for
> the new API and deprecate the old name.
>
> So for example deprecate pn_data_t in favour of pn_value_t (or whatever
> better but new name). Where pn_value_t has all the old functionality of
> pn_data_t and indeed may be the same internal structure initially, but
> with a different interface.
>
> Incidentally C++ does make this easier because it allows function
> overloading.
>
> Andrew
>
>
>

Re: codec changes

[Rafael Schloming <rh...@alum.mit.edu>]

On Mon, May 11, 2015 at 3:49 PM, Alan Conway <ac...@redhat.com> wrote:

> On Thu, 2015-05-07 at 15:53 -0400, Rafael Schloming wrote:
> > I believe where we ended up was standardizing on a single snprintf-style
> > signature for all encode functions, i.e.:
> >
> >     // always returns encoded size, never overwrites limit, pass in
> NULL, 0
> > to compute size in advance
> >     // returns < 0 if there was an actual error of some sort, e.g. xxx_t
> > cannot be validly encoded for some reason
> >     ssize_t pn_xxx_encode(xxx_t, char *buf, size_t limit);
> >
> > And transitioning to this with a feature macro.
>
> I'm good with sprintf-style.
>
> I'm not sure what you mean by a feature macro. Does that addresses
> binary compatibility or is just a source-level switch? If we break
> binary compatibility we need to make sure we bump the library version
> appropriately to avoid breaking existing apps. If we set the switch to
> default to the new behavior we should make sure the compiler barfs on
> old code and it's fairly obvious what to do about it.
>

I've never actually defined feature test macros before, just used them, so
I'm just making this up as we go along. What I imagine could work though is
to define pn_xxx_encode2 in the .c file to provide the new behavior. By
default it would not be visible from the header files unless the
appropriate feature macro was defined upon inclusion, in which case we
would alias pn_xxx_encode to pn_xxx_encode2, e.g.:

--------------
#include <codec.h>
#include <message.h>
...
int err = pn_message_encode(msg, buf, &in_out_size); // we could make this
spew a deprecated warning by default
--------------

--------------
#define PN_STANDARD_ENCODE_SIGNATURE
#include <codec.h>
#include <message.h>
...
ssize_t encoded_size = pn_message_encode(msg, buf, bufsize);
--------------

This would allow people to incrementally upgrade and would not break binary
compatibility. We could of course at some later point break binary
compatibility if we want to and clean up the encode2 symbols, especially if
we have other reasons to change ABI.

One thing I'm not sure of is the granularity of the feature test, e.g.
should the macro be specific to the change (PN_STANDARD_ENCODE_SIGNATURE)
or should it be related to the version somehow, e.g. (PN_0_10_SOURCE) or
something like that. I guess both could be an option too, e.g.
PN_0_10_SOURCE could be an alias for all the new features in 0.10,
presumably just this and possibly the sasl stuff if there is a way to make
that work.

--Rafael

Re: codec changes

[Alan Conway <ac...@redhat.com>]

On Thu, 2015-05-07 at 15:53 -0400, Rafael Schloming wrote:
> I believe where we ended up was standardizing on a single snprintf-style
> signature for all encode functions, i.e.:
> 
>     // always returns encoded size, never overwrites limit, pass in NULL, 0
> to compute size in advance
>     // returns < 0 if there was an actual error of some sort, e.g. xxx_t
> cannot be validly encoded for some reason
>     ssize_t pn_xxx_encode(xxx_t, char *buf, size_t limit);
> 
> And transitioning to this with a feature macro.

I'm good with sprintf-style.

I'm not sure what you mean by a feature macro. Does that addresses
binary compatibility or is just a source-level switch? If we break
binary compatibility we need to make sure we bump the library version
appropriately to avoid breaking existing apps. If we set the switch to
default to the new behavior we should make sure the compiler barfs on
old code and it's fairly obvious what to do about it.

Cheers,
Alan.

> 
> --Rafael
> 
> On Thu, May 7, 2015 at 3:28 PM, Andrew Stitcher <as...@redhat.com>
> wrote:
> 
> > On Wed, 2015-05-06 at 14:03 -0400, Rafael Schloming wrote:
> > > We seem to have reached consensus here, but I haven't seen any commits on
> > > this. We should probably fix this before 0.10 so we don't end up putting
> > > out a new API and then deprecating it in the next release. Is anyone
> > > actually working on this?
> >
> > Could you articulate the consensus then.
> >
> > Asserting "we have reached consensus" without explicitly stating what
> > you think the consensus to be is remarkably likely to be proven wrong by
> > subsequent events!
> >
> > Andrew
> >
> >
> >

Re: codec changes

[Rafael Schloming <rh...@alum.mit.edu>]

I believe where we ended up was standardizing on a single snprintf-style
signature for all encode functions, i.e.:

    // always returns encoded size, never overwrites limit, pass in NULL, 0
to compute size in advance
    // returns < 0 if there was an actual error of some sort, e.g. xxx_t
cannot be validly encoded for some reason
    ssize_t pn_xxx_encode(xxx_t, char *buf, size_t limit);

And transitioning to this with a feature macro.

--Rafael

On Thu, May 7, 2015 at 3:28 PM, Andrew Stitcher <as...@redhat.com>
wrote:

> On Wed, 2015-05-06 at 14:03 -0400, Rafael Schloming wrote:
> > We seem to have reached consensus here, but I haven't seen any commits on
> > this. We should probably fix this before 0.10 so we don't end up putting
> > out a new API and then deprecating it in the next release. Is anyone
> > actually working on this?
>
> Could you articulate the consensus then.
>
> Asserting "we have reached consensus" without explicitly stating what
> you think the consensus to be is remarkably likely to be proven wrong by
> subsequent events!
>
> Andrew
>
>
>

Re: codec changes

[Andrew Stitcher <as...@redhat.com>]

On Wed, 2015-05-06 at 14:03 -0400, Rafael Schloming wrote:
> We seem to have reached consensus here, but I haven't seen any commits on
> this. We should probably fix this before 0.10 so we don't end up putting
> out a new API and then deprecating it in the next release. Is anyone
> actually working on this?

Could you articulate the consensus then.

Asserting "we have reached consensus" without explicitly stating what
you think the consensus to be is remarkably likely to be proven wrong by
subsequent events!

Andrew