Provide a typedef for memcached_server_instance_st in deprecated_types.h

Bug #1190240 reported by Michael Fladischer
30
This bug affects 5 people
Affects Status Importance Assigned to Milestone
libmemcached
Fix Committed
Medium
Brian Aker

Bug Description

1.0.9 broke the API by removing memcached_server_instance_st. Could you add a typdef to libmemcached-1.0/deprecated_types.h that reflects that change, so people can use IFDEFs on LIBMEMCACHED_VERSION_HEX to make their code compatible with libmemcached before and after the API break?

libmemcached-1.0/deprecated_types.h:
typedef const memcached_instance_st *memcached_server_instance_st;

Related branches

Revision history for this message
Faidon Liambotis (paravoid) wrote :

[ This came from me, via https://github.com/php-memcached-dev/php-memcached/pull/80 ]

To clarify, adding this to deprecated_types would *avoid* people using #ifdefs for their code and just use the pre-1.0.9 API on their code with no modifications. That, or deprecated types should probably go altogether as it's a bit pointless to provide compatibility with 0.37 or whatever for parts of the API but break others with 1.0.9.

Revision history for this message
Marc Abramowitz (msabramo) wrote :

I also noticed this because pylibmc fails to build with newer versions of libmemcached -- see https://github.com/lericson/pylibmc/issues/133

Perhaps because of this issue, I noticed that Ubuntu hasn't packaged libmemcached-dev >= 1.0.8-1:

http://packages.ubuntu.com/search?keywords=libmemcached-dev

It would be nice if libmemcached fixed this problem so that other projects can use the new versions easily without hacking in #ifdefs.

Revision history for this message
Marc Abramowitz (msabramo) wrote :
Download full text (4.0 KiB)

Actually, it looks like XXX was present in 1.0.9. It seems to have disappeared in 1.0.17.

```
marca@marca-mac:~/src$ cat look_for_memcached_server_instance_st.sh
#!/bin/sh

for ver in 1.0.7 1.0.8 1.0.9 1.0.10 1.0.11 1.0.12 1.0.13 1.0.14 1.0.15 1.0.16 1.0.17 1.0.18; do
  echo "=================== $ver ================="
  wget -q https://launchpad.net/libmemcached/1.0/$ver/+download/libmemcached-$ver.tar.gz
  tar xf libmemcached-$ver.tar.gz
  grep -l -r 'memcached_server_instance_st' libmemcached-$ver/libmemcached-1.0
done

marca@marca-mac:~/src$ ./look_for_memcached_server_instance_st.sh
=================== 1.0.7 =================
libmemcached-1.0.7/libmemcached-1.0/callbacks.h
libmemcached-1.0.7/libmemcached-1.0/error.h
libmemcached-1.0.7/libmemcached-1.0/memcached.h
libmemcached-1.0.7/libmemcached-1.0/memcached.hpp
libmemcached-1.0.7/libmemcached-1.0/server.h
libmemcached-1.0.7/libmemcached-1.0/types.h
=================== 1.0.8 =================
libmemcached-1.0.8/libmemcached-1.0/callbacks.h
libmemcached-1.0.8/libmemcached-1.0/error.h
libmemcached-1.0.8/libmemcached-1.0/memcached.h
libmemcached-1.0.8/libmemcached-1.0/memcached.hpp
libmemcached-1.0.8/libmemcached-1.0/server.h
libmemcached-1.0.8/libmemcached-1.0/types.h
=================== 1.0.9 =================
libmemcached-1.0.9/libmemcached-1.0/callbacks.h
libmemcached-1.0.9/libmemcached-1.0/error.h
libmemcached-1.0.9/libmemcached-1.0/memcached.h
libmemcached-1.0.9/libmemcached-1.0/memcached.hpp
libmemcached-1.0.9/libmemcached-1.0/server.h
libmemcached-1.0.9/libmemcached-1.0/types.h
=================== 1.0.10 =================
libmemcached-1.0.10/libmemcached-1.0/callbacks.h
libmemcached-1.0.10/libmemcached-1.0/error.h
libmemcached-1.0.10/libmemcached-1.0/memcached.h
libmemcached-1.0.10/libmemcached-1.0/memcached.hpp
libmemcached-1.0.10/libmemcached-1.0/server.h
libmemcached-1.0.10/libmemcached-1.0/types.h
=================== 1.0.11 =================
libmemcached-1.0.11/libmemcached-1.0/callbacks.h
libmemcached-1.0.11/libmemcached-1.0/error.h
libmemcached-1.0.11/libmemcached-1.0/memcached.h
libmemcached-1.0.11/libmemcached-1.0/memcached.hpp
libmemcached-1.0.11/libmemcached-1.0/server.h
libmemcached-1.0.11/libmemcached-1.0/types.h
=================== 1.0.12 =================
libmemcached-1.0.12/libmemcached-1.0/callbacks.h
libmemcached-1.0.12/libmemcached-1.0/error.h
libmemcached-1.0.12/libmemcached-1.0/memcached.h
libmemcached-1.0.12/libmemcached-1.0/memcached.hpp
libmemcached-1.0.12/libmemcached-1.0/server.h
libmemcached-1.0.12/libmemcached-1.0/types.h
=================== 1.0.13 =================
libmemcached-1.0.13/libmemcached-1.0/callbacks.h
libmemcached-1.0.13/libmemcached-1.0/error.h
libmemcached-1.0.13/libmemcached-1.0/memcached.h
libmemcached-1.0.13/libmemcached-1.0/memcached.hpp
libmemcached-1.0.13/libmemcached-1.0/server.h
libmemcached-1.0.13/libmemcached-1.0/types.h
=================== 1.0.14 =================
libmemcached-1.0.14/libmemcached-1.0/callbacks.h
libmemcached-1.0.14/libmemcached-1.0/error.h
libmemcached-1.0.14/libmemcached-1.0/memcached.h
libmemcached-1.0.14/libmemcached-1.0/memcached.hpp
libmemcached-1.0.14/libmemcached-1.0/server.h
libmemcached-1.0.14/libmemcach...

Read more...

Revision history for this message
Marc Abramowitz (msabramo) wrote :
Revision history for this message
Marc Abramowitz (msabramo) wrote :

memcached_server_instance_st seems to have disappeared in 1.0.17

As a result, code like this:

```
static memcached_return
_PylibMC_AddServerCallback(memcached_st *mc,
                           memcached_server_instance_st instance,
                           void *user) {
...
    desc = PyString_FromFormat("%s:%d (%u)",
            memcached_server_name(instance), memcached_server_port(instance),
            (unsigned int)context->index);
...
}
```

fails to compile with:

```
src/_pylibmcmodule.c:1848: error: expected declaration specifiers or '...' before 'memcached_server_instance_st'
src/_pylibmcmodule.c: In function '_PylibMC_AddServerCallback':
src/_pylibmcmodule.c:1896: error: 'instance' undeclared (first use in this function)
```

Question: Was removing memcached_server_instance_st intentional? Should we be using some other API now?

Revision history for this message
Marc Abramowitz (msabramo) wrote :

Here are the declarations for memcached_server_name and memcached_server_port from http://bazaar.launchpad.net/~tangent-trunk/libmemcached/1.0/view/head:/libmemcached-1.0/server.h#L103 (these declarations also appear in libmemcached-1.0.17):

```C
LIBMEMCACHED_API
const char *memcached_server_name(const memcached_instance_st * self);

LIBMEMCACHED_API
in_port_t memcached_server_port(const memcached_instance_st * self);
```

Note that they take a memcached_instance_st * as an argument.

So I am thinking that you want us to now use memcached_instance_st * in place of memcached_server_instance_st?

I would still contend that it would be nice to maintain the typedef for backward compatibility, at least until there is a major version bump. Changing the API during a patch level version bump is surprising and makes it difficult for folks that build software that uses libmemcached. For example, it looks like pylibmc will have to have this bit of #ifdef ugliness:

```C
static memcached_return
_PylibMC_AddServerCallback(memcached_st *mc,
#if LIBMEMCACHED_VERSION_HEX >= 0x01000017
                           memcached_instance_st instance,
#elif LIBMEMCACHED_VERSION_HEX >= 0x00039000
                           memcached_server_instance_st instance,
#else
                           memcached_server_st *server,
#endif
                           void *user) {
...
}
```

Brian Aker (brianaker)
Changed in libmemcached:
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Brian Aker (brianaker)
milestone: none → 1.0.18
status: In Progress → Fix Committed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.