rabbitmq-c signature changed in amqp_exchange_declare

[Elias Mårtenson]

As I was looking through the commit log of rabbitmq-c I noticed commit id 9626dd5cd5f78894f1416a1afd2d624ddd4904ae which adds two new parameters to the function amqp_exchange_declare(). This is a compatibility-breaking change, and I was wondering how this will be handled once this version is released?

This is particularly nasty for the Common Lisp API which is built on this, since still change will not even cause compile errors (the Lisp runtime directly loads shared object and calls the function based on its own function declaration which is now incompatible with the new version of rabbitmq-c) but instead cause a crash (if we're lucky).

Wouldn't it make more sense to introduce a new call, similar to what was done for amqp_error_string2() in order to preserve compatibility?

Regards,

Elias

[Michael Klishin]

  On 26 February 2015 at 08:47:16, Elias Mårtenson (lok...@gmail.com) wrote:
> As I was looking through the commit log of rabbitmq-c I noticed

> commit id 9626dd5cd5f78894f1416a1afd2d624ddd4904ae(https://github.com/alanxz/rabbitmq-c/commit/9626dd5cd5f78894f1416a1afd2d624ddd4904ae)

> which adds two new parameters to the function amqp_exchange_declare().
> This is a compatibility-breaking change, and I was wondering
> how this will be handled once this version is released?
>
> This is particularly nasty for the Common Lisp API which is built
> on this, since still change will not even cause compile errors
> (the Lisp runtime directly loads shared object and calls the
> function based on its own function declaration which is now incompatible
> with the new version of rabbitmq-c) but instead cause a crash
> (if we're lucky).
>
> Wouldn't it make more sense to introduce a new call, similar to
> what was done for amqp_error_string2() in order to preserve
> compatibility?

While I understand the desire to keep APIs backwards compatible (we generally do with
our clients), that won't always be the case, and having N functions that do the same
thing and exist for backwards compatibility reasons will be quite confusing.

On top of that, it *increases* the burden of compatibility maintenance in the long run
because now instead of maintaining 1 function that takes all the possible arguments for
a method, you have to maintain N.

So no, please don't introduce amqp_exchange_declare2 and so on.
This is not a major change, so please update and re-compile the code that depends on it
and lets move on to more interesting things.
--
MK

Staff Software Engineer, Pivotal/RabbitMQ

[Elias Mårtenson]

On 26 February 2015 at 16:04, Michael Klishin <mkli...@pivotal.io> wrote:

While I understand the desire to keep APIs backwards compatible (we generally do with
our clients), that won't always be the case, and having N functions that do the same
thing and exist for backwards compatibility reasons will be quite confusing.

On top of that, it *increases* the burden of compatibility maintenance in the long run
because now instead of maintaining 1 function that takes all the possible arguments for
a method, you have to maintain N.

So no, please don't introduce amqp_exchange_declare2 and so on.
This is not a major change, so please update and re-compile the code that depends on it
and lets move on to more interesting things.

While it's true that cl-rabbit (Common Lisp API) makes things more complicated since it's not actually using the prototype declaration of this function, this is a problem even for applications written in pure C.

Let's assume that you have application X, in binary form, on your system. Then you do apt-get upgrade which updates rabbitmq-c to the new, incompatible, version. All of a sudden application X starts to crash because it now tried to call the same function which now accepts different arguments.

What's a user to do? In this case, he or she would have to download the source code and recompile it, which will fail because the prototype doesn't match anymore. So not only is a recompilation needed, the user would have to understand how the API has changed and to modify the code to conform to the new calling convention.

C libraries generally never do this, and the way this is generally dealt with is to increase the major version number of the shared object file (i.e. libfoo.so.1 becomes libfoo.so.2) so that multiple versions of the libraries can be installed at the same time. This is of course a possible solution in this case, but since this seems to be the only API change that I can see, updating the major version for the library because of it seems to be a bit much.

Finally, if my arguments are not accepted (which is fine, mine is just one voice of many), there needs to be a way to detect the library version at runtime so that I can add code to dynamically detect what version is used in order to call this function with the correct arguments (again, cl-rabbit is not statically compiled with librabbit-mq but rather loads the library dynamically).

Regards,

Elias

[Michael Klishin]

On 26 February 2015 at 11:21:27, Elias Mårtenson (lok...@gmail.com) wrote:
> C libraries generally never do this, and the way this is generally dealt with is to increase the major version
> number of the shared object file

Even for a single function signature change? I think breaking the API of 1 function is not unreasonable
for a minor release. I understand that this is more painful in native code than, say, on the JVM or .NET.

> Finally, if my arguments are not accepted (which is fine, mine
> is just one voice of many), there needs to be a way to detect the
> library version at runtime so that I can add code to dynamically
> detect what version is used in order to call this function with
> the correct arguments (again, cl-rabbit is not statically compiled
> with librabbit-mq but rather loads the library dynamically).

Ultimately Alan makes decisions about librabbitmq-c, I just cast my (often uninformed)
opinions on this list :)

For pure C projects, I suspect that static compilation can be an answer. Just so that we understand,
what about Common Lisp? Is there a way to statically compile a binary with most of code in CL and
some C libraries? 

[Elias Mårtenson]

On 26 February 2015 at 16:28, Michael Klishin <mkli...@pivotal.io> wrote:

On 26 February 2015 at 11:21:27, Elias Mårtenson (lok...@gmail.com) wrote:
> C libraries generally never do this, and the way this is generally dealt with is to increase the major version
> number of the shared object file

Even for a single function signature change? I think breaking the API of 1 function is not unreasonable
for a minor release. I understand that this is more painful in native code than, say, on the JVM or .NET.

Yes, for a single function. Once you compile a C program, the signature information is erased and you won't even get an error message when the function is called. It will just randomly crash (if we're lucky, in a worse case scenario, you'll end up with incorrect behaviour).

In other words, not only do you need to recompile every single program that uses the library. You receive no indication that will will need to do so, other than random crashes.

If the change had instead removed a function (or renamed it), you'd get a linkage error when you attempt to run the program. Remember, in C a function is uniquely identified by only its name. This is in contrast to most other languages (including C++) where the function arguments are part of the signature. I.e. in Erlang a function is identified by not only its name but also the number of arguments. Adding an argument changes the function signature and the problems that can be seen in C just doesn't happen.

So to reiterate, from the point of view of the application that uses the library, there is absolutely no way it can detect that the required function arglist have changed.

For pure C projects, I suspect that static compilation can be an answer. Just so that we understand,

It would, but most distributions doesn't ship with static versions of the libraries. I certainly don't have a static librabbitmq on my Arch Linux system.

 

what about Common Lisp? Is there a way to statically compile a binary with most of code in CL and
some C libraries? 

No. There is not, and the same would apply for most languages that can link with C.

Regards,

Elias

[Michael Klishin]

  On 26 February 2015 at 11:42:03, Elias Mårtenson (lok...@gmail.com) wrote:
> In other words, not only do you need to recompile every single
> program that uses the library.

This makes sense.

Why don't we just bump the version, has librabbitmq done this much in the past?

[Elias Mårtenson]

On 26 February 2015 at 16:44, Michael Klishin <mkli...@pivotal.io> wrote:

  On 26 February 2015 at 11:42:03, Elias Mårtenson (lok...@gmail.com) wrote:
> In other words, not only do you need to recompile every single
> program that uses the library.

This makes sense.

Why don't we just bump the version, has librabbitmq done this much in the past?

I don't think so. I have the latest release that was installed by the Arch Linux package manager, and it's library version 1.2.0. There may have been a 0.x version in the past, but I haven't followed RabbitMQ for long enough to know that.

Regards,

Elias

[Alan Antonuk]

On Wed, Feb 25, 2015 at 9:47 PM, Elias Mårtenson <lok...@gmail.com> wrote:

As I was looking through the commit log of rabbitmq-c I noticed commit id 9626dd5cd5f78894f1416a1afd2d624ddd4904ae which adds two new parameters to the function amqp_exchange_declare(). This is a compatibility-breaking change, and I was wondering how this will be handled once this version is released?

Yes, this is an ABI-breaking change. As such the SONAME for the library was incremented. This should prevent binaries that link against an older SONAME from loading the new version of the library. A note was also made in the release notes. FYI: this was released as a part of v0.6.0.

 

This is particularly nasty for the Common Lisp API which is built on this, since still change will not even cause compile errors (the Lisp runtime directly loads shared object and calls the function based on its own function declaration which is now incompatible with the new version of rabbitmq-c) but instead cause a crash (if we're lucky).

If your code doesn't already: I would make sure that it validates the SONAME of whatever its loading. I do make every effort to make the SONAME reflect non-backwards compatible ABI changes. There are methods to get the version of the library at runtime: amqp_version and amqp_version_number, though those values give you the API version and not the ABI version.

Wouldn't it make more sense to introduce a new call, similar to what was done for amqp_error_string2() in order to preserve compatibility?

The short answer is: I don't believe rabbitmq-c is mature enough to warrant maintaining perfect ABI backward compatibility at the cost of adding API complexity at this point. (amqp_error_string2 is one example of a mistake that was made). 

The longer more philosophical answer: when I first started working on rabbitmq-c I took the position that rabbitmq-c was 'mature' and I put in quite an effort into maintaining the API/ABI backwards compatibility. Looking back on it, that effort was a bit pre-mature: today I do not consider rabbitmq-c a complete or mature library. Its missing large chunks of functionality that other RabbitMQ/AMQP clients have, such as proper support for asynchronous AMQP events. As a result of an attempt to maintain this ABI compatibility: the public API has grown with nearly-duplicated APIs such as amqp_login_with_properties, amqp_simple_wait_frame_noblock, amqp_error_string2. This increases the complexity to the public API increasing the maintenance burden, but more importantly makes the library harder to use: even with documentation as a consumer of the library it becomes more difficult to determine how to use the library 'correctly'. 

I hope that one day I can push rabbitmq-c to a level of API maturity and feature completeness that I'm confident new features can be added without breaking the ABI. At this point I will give it the v1.0.0 label in semantic versioning parlance, and maintaining ABI backward compatibility will become a priority once again.

That said: I do understand that there are users of rabbitmq-c, and as such I'll try make it obvious when breaking changes are made to the library by continuing to increment the SONAME as necessary and clearly documenting what has changed.

HTH

-Alan

[Elias Mårtenson]

On 27 February 2015 at 10:57, Alan Antonuk <alan.a...@gmail.com> wrote:

Yes, this is an ABI-breaking change. As such the SONAME for the library was incremented. This should prevent binaries that link against an older SONAME from loading the new version of the library. A note was also made in the release notes. FYI: this was released as a part of v0.6.0.

Thank you for this clarification. I haven't actually compiled the latest version yet, but I can assume it's librabbitmq.so.2 now, yes?

 

If your code doesn't already: I would make sure that it validates the SONAME of whatever its loading. I do make every effort to make the SONAME reflect non-backwards compatible ABI changes. There are methods to get the version of the library at runtime: amqp_version and amqp_version_number, though those values give you the API version and not the ABI version.

Right now, I simply load librabbitmq.so, which clearly isn't ideal. I will change this to explicitly load librabbitmq.so.1 instead to at least avoid random crashes.

 

I need to change the code to check for library version so that I can do the right thing regardless of what version the user has installed.

Do you collect a complete list of breaking changes so that I can keep cl-rabbit up to date? Especially for function argument list changes, this is more important for CL than for traditional C code, since I'm not compiling against the prototypes in the .h file, but I rather have to define the argument lists like this.

Wouldn't it make more sense to introduce a new call, similar to what was done for amqp_error_string2() in order to preserve compatibility?

The short answer is: I don't believe rabbitmq-c is mature enough to warrant maintaining perfect ABI backward compatibility at the cost of adding API complexity at this point. (amqp_error_string2 is one example of a mistake that was made). 

Fair enough. However, this begs the question as to what the future will look like. Do you foresee lots of breaking changes before you hit 1.0?

 

This increases the complexity to the public API increasing the maintenance burden, but more importantly makes the library harder to use: even with documentation as a consumer of the library it becomes more difficult to determine how to use the library 'correctly'. 

Another alternative would be to add things in a backward-compatible manner until you hit 1.0, at which point all backwards compatibility is dropped at once with 1.0 being a completely cleaned-up API.

 

I hope that one day I can push rabbitmq-c to a level of API maturity and feature completeness that I'm confident new features can be added without breaking the ABI. At this point I will give it the v1.0.0 label in semantic versioning parlance, and maintaining ABI backward compatibility will become a priority once again.

I'm certainly OK with bending backwards a bit extra to maintain cl-rabbit compatibility until 1.0, but if that is very far off in time, or if there is lots of incremental breakage, that may be simply too much and I would have to lock cl-rabbit to a specific version of rabbitmq-c, which would be unfortunate but probably better than not having a stable product.

 

That said: I do understand that there are users of rabbitmq-c, and as such I'll try make it obvious when breaking changes are made to the library by continuing to increment the SONAME as necessary and clearly documenting what has changed.

Yes, at least this is workable.

Speaking of adoption, the biggest user of cl-rabbit right now is the project I'm working on myself (which is in limited production right now). Based on the feedback I have received there are are at least a couple of other people who have been trying it out. Whether it's actively being used in production anywhere else, I cannot say, but given the fact that the project is new, I'd say probably not.

Finally, I have to extend my thanks to you for building this product in the first place. Without it, there would be no reasonable way to use RabbitMQ from CL, short of reimplementing the entire protocol. This solution has proven to be incredibly stable for us so far, having had exactly zero issues with messaging in our application since migrating from zeromq to RabbitMQ.

Regards,

Elias

[Alan Antonuk]

On Thu, Feb 26, 2015 at 10:36 PM, Elias Mårtenson <lok...@gmail.com> wrote:

On 27 February 2015 at 10:57, Alan Antonuk <alan.a...@gmail.com> wrote:

Yes, this is an ABI-breaking change. As such the SONAME for the library was incremented. This should prevent binaries that link against an older SONAME from loading the new version of the library. A note was also made in the release notes. FYI: this was released as a part of v0.6.0.

Thank you for this clarification. I haven't actually compiled the latest version yet, but I can assume it's librabbitmq.so.2 now, yes?

librabbitmq.so.4. 

 

If your code doesn't already: I would make sure that it validates the SONAME of whatever its loading. I do make every effort to make the SONAME reflect non-backwards compatible ABI changes. There are methods to get the version of the library at runtime: amqp_version and amqp_version_number, though those values give you the API version and not the ABI version.

Right now, I simply load librabbitmq.so, which clearly isn't ideal. I will change this to explicitly load librabbitmq.so.1 instead to at least avoid random crashes.

 

I need to change the code to check for library version so that I can do the right thing regardless of what version the user has installed.

Do you collect a complete list of breaking changes so that I can keep cl-rabbit up to date? Especially for function argument list changes, this is more important for CL than for traditional C code, since I'm not compiling against the prototypes in the .h file, but I rather have to define the argument lists like this.

I attempt to do this when writing out the release notes, that would be the place to look when evaluating new rabbitmq-c releases.

Wouldn't it make more sense to introduce a new call, similar to what was done for amqp_error_string2() in order to preserve compatibility?

The short answer is: I don't believe rabbitmq-c is mature enough to warrant maintaining perfect ABI backward compatibility at the cost of adding API complexity at this point. (amqp_error_string2 is one example of a mistake that was made). 

Fair enough. However, this begs the question as to what the future will look like. Do you foresee lots of breaking changes before you hit 1.0?

Hard to say. Some features like proper async support will likely break a good number of things, other things like better memory management will likely be more transparent.

 

This increases the complexity to the public API increasing the maintenance burden, but more importantly makes the library harder to use: even with documentation as a consumer of the library it becomes more difficult to determine how to use the library 'correctly'. 

Another alternative would be to add things in a backward-compatible manner until you hit 1.0, at which point all backwards compatibility is dropped at once with 1.0 being a completely cleaned-up API.

 

I hope that one day I can push rabbitmq-c to a level of API maturity and feature completeness that I'm confident new features can be added without breaking the ABI. At this point I will give it the v1.0.0 label in semantic versioning parlance, and maintaining ABI backward compatibility will become a priority once again.

I'm certainly OK with bending backwards a bit extra to maintain cl-rabbit compatibility until 1.0, but if that is very far off in time, or if there is lots of incremental breakage, that may be simply too much and I would have to lock cl-rabbit to a specific version of rabbitmq-c, which would be unfortunate but probably better than not having a stable product.

 

That said: I do understand that there are users of rabbitmq-c, and as such I'll try make it obvious when breaking changes are made to the library by continuing to increment the SONAME as necessary and clearly documenting what has changed.

Yes, at least this is workable.

Speaking of adoption, the biggest user of cl-rabbit right now is the project I'm working on myself (which is in limited production right now). Based on the feedback I have received there are are at least a couple of other people who have been trying it out. Whether it's actively being used in production anywhere else, I cannot say, but given the fact that the project is new, I'd say probably not.

Finally, I have to extend my thanks to you for building this product in the first place. Without it, there would be no reasonable way to use RabbitMQ from CL, short of reimplementing the entire protocol. This solution has proven to be incredibly stable for us so far, having had exactly zero issues with messaging in our application since migrating from zeromq to RabbitMQ.

You are welcome. 

Regards,

Elias