redis / hiredis

Minimalistic C client for Redis >= 1.2
BSD 3-Clause "New" or "Revised" License
6.24k stars 1.81k forks source link

No local rpath settings #36

Closed shakthimaan closed 13 years ago

shakthimaan commented 13 years ago

When packaging software, the installed executables will obtain the library paths from the environment. Hence, it is not recommended to use a local rpath. The patch below removes hard-coding of the same, and also creates versioned shared libraries.

diff -up antirez-hiredis-0fbfa45/Makefile.fix antirez-hiredis-0fbfa45/Makefile --- antirez-hiredis-0fbfa45/Makefile.fix 2011-04-19 13:06:43.214439002 +0530 +++ antirez-hiredis-0fbfa45/Makefile 2011-04-19 13:12:52.137439004 +0530 @@ -27,9 +27,12 @@ else ifeq ($(uname_S),Darwin) else CFLAGS?=-std=c99 -pedantic $(OPTIMIZATION) -fPIC -Wall -W -Wwrite-strings $(ARCH) $(PROF) CCLINK?=-lm -pthread

pietern commented 13 years ago

Thanks for sending this as a pull request, much easier to keep track of things :-)

I've removed the rpath flag in 4feec1f252b87b43b5a480c0918297a15ffd8f9e. However, I don't agree with the install target: when you're packaging, you a) know the version and b) know where to install the libraries, so why do this as well in the Makefile? When hiredis is compiled and installed from source, the person using it is aware that upgrades overwrite the existing library...

shakthimaan commented 13 years ago

Thanks for removing the rpath! The reasons for the versionto be included are that:

(1) When a new release is made, just the VERSION is increased, and it reflects in your original sources. (2) We could possibly still have multiple shared libraries installed, and the users can choose which versioned library that they want to use, as against having just libhiredis.so getting overwritten across multiple releases.

You can also refer 3.4 Creating a Shared Library from:

http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html#AEN95

pietern commented 13 years ago

Ok, I'm convinced :-). Added this in 81c00aab3233e9465eef6d6684553f5aa57288b6. Note that I use both the major and minor version in the soname because the API may change between minors (but not between patch versions), in contrary to what I believe the convention is (use only the major version in the soname).