Re: ZooKeeper 3.1 and C API/ABI
Hi -- Btw, the version is in the config.h file, generated by autotools, as VERSION. We don't break that out as individual parameters but we can if there is interest. That's useful, I'd missed that. Thanks; that should work for me for now. Sorry ... on second glance, I'll have to retract that. The problem here is that config.h doesn't get installed by make install, it's just used by the autoconf stuff. So there's no simple way that I'm aware of to check the C API version at compile time. For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0, or making sure there's at least a way of determining the API version using C macros. For example, I'd want to be able to do something like: #if ZOO_MAJOR_VERSION = 3 ZOO_MINOR_VERSION = 1 zoo_set(..., stat); #else zoo_set(...); #endif Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality would be moved to a zoo_stat_set() or zoo_set_exec() function, and zoo_set() would keep its existing definition but become just a wrapper that invoked the new function with a NULL stat argument. That would be the APR way, I think, of handling this situation. With the next major version the new function with the extra argument could be renamed back to zoo_set(). It's slightly ugly, I know, if you're thinking of this as a bug which needs to be fixed urgently. If you're not concerned about backward API compatibility, at a minimum I'd request externally visible macros in zookeeper.h for 3.1.0: #define ZOO_MAJOR_VERSION 3 #define ZOO_MINOR_VERSION 1 #define ZOO_PATCH_VERSION 0 Thanks, Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: ZooKeeper 3.1 and C API/ABI
Chris, that's unfortunate re the version number (config.h), but I think I see why that is -- config.h should only really be visible in the implementation, not exposed through the includes. I've created a JIRA for this: https://issues.apache.org/jira/browse/ZOOKEEPER-293 We'll hold 3.1 for this JIRA, I'll create a new release candidate when the patch is ready. (hopefully today) Ben, Mahadev please be available to review/fasttrack this JIRA. Patrick Chris Darroch wrote: Hi -- Btw, the version is in the config.h file, generated by autotools, as VERSION. We don't break that out as individual parameters but we can if there is interest. That's useful, I'd missed that. Thanks; that should work for me for now. Sorry ... on second glance, I'll have to retract that. The problem here is that config.h doesn't get installed by make install, it's just used by the autoconf stuff. So there's no simple way that I'm aware of to check the C API version at compile time. For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0, or making sure there's at least a way of determining the API version using C macros. For example, I'd want to be able to do something like: #if ZOO_MAJOR_VERSION = 3 ZOO_MINOR_VERSION = 1 zoo_set(..., stat); #else zoo_set(...); #endif Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality would be moved to a zoo_stat_set() or zoo_set_exec() function, and zoo_set() would keep its existing definition but become just a wrapper that invoked the new function with a NULL stat argument. That would be the APR way, I think, of handling this situation. With the next major version the new function with the extra argument could be renamed back to zoo_set(). It's slightly ugly, I know, if you're thinking of this as a bug which needs to be fixed urgently. If you're not concerned about backward API compatibility, at a minimum I'd request externally visible macros in zookeeper.h for 3.1.0: #define ZOO_MAJOR_VERSION 3 #define ZOO_MINOR_VERSION 1 #define ZOO_PATCH_VERSION 0 Thanks, Chris.
Re: ZooKeeper 3.1 and C API/ABI
Chris, please take a look at the patch on 293 asap and let us know if you have any issues -- I will be spinning a new release once mahadev/ben review and commit the change. Patrick ps. I noticed you had some additional suggestions for the c code in JIRA, thanks. FYI we do accept contributions from anyone. ;-) Patrick Hunt wrote: Chris, that's unfortunate re the version number (config.h), but I think I see why that is -- config.h should only really be visible in the implementation, not exposed through the includes. I've created a JIRA for this: https://issues.apache.org/jira/browse/ZOOKEEPER-293 We'll hold 3.1 for this JIRA, I'll create a new release candidate when the patch is ready. (hopefully today) Ben, Mahadev please be available to review/fasttrack this JIRA. Patrick Chris Darroch wrote: Hi -- Btw, the version is in the config.h file, generated by autotools, as VERSION. We don't break that out as individual parameters but we can if there is interest. That's useful, I'd missed that. Thanks; that should work for me for now. Sorry ... on second glance, I'll have to retract that. The problem here is that config.h doesn't get installed by make install, it's just used by the autoconf stuff. So there's no simple way that I'm aware of to check the C API version at compile time. For 3.1.0, I'd suggest either reverting ZOOKEEPER-255 until 4.0.0, or making sure there's at least a way of determining the API version using C macros. For example, I'd want to be able to do something like: #if ZOO_MAJOR_VERSION = 3 ZOO_MINOR_VERSION = 1 zoo_set(..., stat); #else zoo_set(...); #endif Ideally, as I mentioned, until 4.0.0 the zoo_set() functionality would be moved to a zoo_stat_set() or zoo_set_exec() function, and zoo_set() would keep its existing definition but become just a wrapper that invoked the new function with a NULL stat argument. That would be the APR way, I think, of handling this situation. With the next major version the new function with the extra argument could be renamed back to zoo_set(). It's slightly ugly, I know, if you're thinking of this as a bug which needs to be fixed urgently. If you're not concerned about backward API compatibility, at a minimum I'd request externally visible macros in zookeeper.h for 3.1.0: #define ZOO_MAJOR_VERSION 3 #define ZOO_MINOR_VERSION 1 #define ZOO_PATCH_VERSION 0 Thanks, Chris.
Re: ZooKeeper 3.1 and C API/ABI
Patrick Hunt wrote: Chris, please take a look at the patch on 293 asap and let us know if you have any issues -- I will be spinning a new release once mahadev/ben review and commit the change. That looks great -- covers a bunch of things, thanks! ps. I noticed you had some additional suggestions for the c code in JIRA, thanks. FYI we do accept contributions from anyone. ;-) Yes, I've submitted a couple of patches (see ZOOKEEPER-262) and written an httpd module for ZooKeeper, and hope to write another one when the slotmem API is baked. The small-object cache one is available from my Apache page: http://people.apache.org/~chrisd/projects/shared_map/ But it's all a question of spare time. :-) I hope to have another more generally useful package available shortly; I'll put it up on my Apache page when its ready for general testing and use. Working on that has been what turned up some of the issues I submitted (e.g., deallocating memory per ZOOKEEPER-294). Cheers, Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: ZooKeeper 3.1 and C API/ABI
Benjamin Reed wrote: you are correct we usually increment the version number on an API breakage. in the olden days if you called a function with less parameters than expected, a null would get passed. if this still happens we are ABI compatible. (i haven't tried it though...) Yeah, I wondered about that; it's not something I'd want to assume worked on every platform, I think. Patrick Hunt wrote: Btw, the version is in the config.h file, generated by autotools, as VERSION. We don't break that out as individual parameters but we can if there is interest. That's useful, I'd missed that. Thanks; that should work for me for now. To get this worked out and working will will take some time. If it's ok with you, and if there is community interest in doing this, why don't we address these process changes in 3.2? Please enter a JIRA to document this for 3.2 and we'll work something out. We'll also do a better job of documenting exactly what the rules are related to non-bw compat api changes and version numbering. I'll think about doing that, sure. FWIW, there are also the libtool conventions for library file naming to consider. Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
