Skip to content

Build with memcached >= 1.0.9 #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

paravoid
Copy link

A commit to make php-memcached build against libmemcached >= 1.0.9 (tested with 1.0.17). Unfortunately libmemcached broke API on a minor release, so we have to do #ifdef trickery. You might be better off asking libmemcached to provide some backwards compatibility, but here's the patch anyway.

Along with that, a couple of minor semi-unrelated commits.

paravoid added 3 commits June 12, 2013 05:06
memcached_server_list_append_with_weight() returns a
memcached_server_list_st, not a memcached_server_st *. They're currently
equivalent typedefs, but as experience has shown this doesn't mean
they'll be in the future.
Commit 5450f45 ported this to use the newer
memcached_server_instance_st instead of the old and incorrect
memcached_server_st. During this, since there's no accessor function to
"weight" anymore (the internal structure is now a C++ class),
getServerByKey()'s output was changed to return a dummy value of "0" for
weight.

Remove this dummy value altogether, as people relying on getting a
weight should not get silent breakages.

(this probably deserves a changelog entry)
memcached_server_instance_st is no more and surprisingly there's no
deprecated typedef to help with that. Add a bunch of #ifdefs and adapt
to the new API.
@paravoid
Copy link
Author

FWIW, a simple

typedef const memcached_instance_st *memcached_server_instance_st;

In libmemcached's deprecated_types.h would probably do the trick (untested). If that gets accepted, you could then #ifdef >= 1.0.9 && < 1.0.18 and add it yourself… If it isn't accepted, then my original patch stands: it's cleaner/easier to unifdef when you decide to drop compatibility for older versions.

In any case, note that the current code is buggy, because of 5450f45: in getServerByKey the code should read memcached_server_instance_st server_instance not memcached_server_instance_st *server_instance (it's a pointer already).

@paravoid
Copy link
Author

Debian's libmemcahed maintainer & myself raised this with upstream libmemcached: https://bugs.launchpad.net/libmemcached/+bug/1190240

@igalic
Copy link

igalic commented Sep 14, 2013

I like how Mark is the only one commenting on that bug :\

@mkoppanen
Copy link
Member

The latest PR merged drops support for pre 1.0.9, is that actually needed?

@mkoppanen
Copy link
Member

Should be fixed now

@mkoppanen mkoppanen closed this Oct 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants