Skip to content
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

Allow to use platform hiredis libs on build. #90

Open
tuxmaster5000 opened this issue Oct 17, 2019 · 3 comments
Open

Allow to use platform hiredis libs on build. #90

tuxmaster5000 opened this issue Oct 17, 2019 · 3 comments

Comments

@tuxmaster5000
Copy link

At the build stage, the internal hiredis lib are used.
But on Linux the hiredis C lib are available, so in many cases it will be better to use the system one.

@michael-grunder
Copy link
Contributor

michael-grunder commented Oct 17, 2019

By vendoring hiredis it's possible to work against a specific version, right down to a single commit.

I don't know if there are other reasons hiredis is vendored, specific to ruby, but that's my guess.

Edit: Python, not ruby. There are too many hiredis wrappers!

@tuxmaster5000
Copy link
Author

I think this be controlled by an build setting like "use_system_hiredis=false" by default. So it will not break anything.

@ifduyue
Copy link
Collaborator

ifduyue commented Oct 22, 2019

diff --git a/setup.py b/setup.py
index bc8a95c..c28dd94 100755
--- a/setup.py
+++ b/setup.py
@@ -11,9 +11,8 @@ def version():
   return module.__version__
 
 ext = Extension("hiredis.hiredis",
-  sources=sorted(glob.glob("src/*.c") +
-                 ["vendor/hiredis/%s.c" % src for src in ("read", "sds")]),
-  include_dirs=["vendor"])
+  sources=sorted(glob.glob("src/*.c")),
+  libraries=['hiredis'])
 
 setup(
   name="hiredis",

It's easy, until you have to deal with API breaking changes.

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

No branches or pull requests

3 participants