-
Notifications
You must be signed in to change notification settings - Fork 56
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
Modernize the build setup #107
Conversation
hey @lemire
Do you know what might be the cause of this? I can take a look later before merging if you don't have time. |
The proposed build setup will compile to static libraries:
If one must have dynamic libraries then this should not be OFF, but then you need to make sure that the linker finds the location of the dynamic library. |
I have pushed an update which should generate a shared library as well. |
@aviggiano I pushed another update. |
Thanks, I'll try to merge this ASAP |
@aviggiano I am sure we can improve the PR. Get in touch with me if I can help further. There is obviously a lot of interest for this project and I am available to help. |
@aviggiano @lemire thanks all of you,this project is very very useful ! |
@lemire I've managed to fix the tests, the library name needed to change. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 0c7bb92..c27107e 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -11,6 +11,11 @@ jobs:
uses: actions/checkout@v2
- name: Configure
run: ./configure.sh
+ - name: Check configuration
+ run: |
+ ./dist/redis-server --version
+ ./dist/redis-cli --version
+ ls ./build/libredis-roaring-shared.so
test:
name: Test project
runs-on: ubuntu-latest
diff --git a/Dockerfile b/Dockerfile
index 5224480..ab59880 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -22,11 +22,11 @@ RUN set -ex; \
cmake ..; \
make -j; \
cd ..; \
- cp build/libredis-roaring.so /usr/local/lib/; \
+ cp build/libredis-roaring-shared.so /usr/local/lib/; \
\
apt-get purge -y --auto-remove $BUILD_DEPS
ADD http://download.redis.io/redis-stable/redis.conf /usr/local/etc/redis/redis.conf
-RUN sed -i '1i loadmodule /usr/local/lib/libredis-roaring.so' /usr/local/etc/redis/redis.conf; \
+RUN sed -i '1i loadmodule /usr/local/lib/libredis-roaring-shared.so' /usr/local/etc/redis/redis.conf; \
chmod 644 /usr/local/etc/redis/redis.conf; \
sed -i 's/^bind 127.0.0.1/#bind 127.0.0.1/g' /usr/local/etc/redis/redis.conf; \
sed -i 's/^protected-mode yes/protected-mode no/g' /usr/local/etc/redis/redis.conf
diff --git a/deps/CRoaring b/deps/CRoaring
index c501a95..fd6f6ff 160000
--- a/deps/CRoaring
+++ b/deps/CRoaring
@@ -1 +1 @@
-Subproject commit c501a95206dec8187aa63a053b1f1a8acb943f7c
+Subproject commit fd6f6ff423bf1f9d240e23e9d2a900885f809c2c
diff --git a/deps/hiredis b/deps/hiredis
index 60e5075..3d8709d 160000
--- a/deps/hiredis
+++ b/deps/hiredis
@@ -1 +1 @@
-Subproject commit 60e5075d4ac77424809f855ba3e398df7aacefe8
+Subproject commit 3d8709d19d7fa67d203a33c969e69f0f1a4eab02
diff --git a/deps/redis b/deps/redis
index d2c8a4b..58f79e2 160000
--- a/deps/redis
+++ b/deps/redis
@@ -1 +1 @@
-Subproject commit d2c8a4b91e8c0e6aefd1f5bc0bf582cddbe046b7
+Subproject commit 58f79e2ff48bb20e48d5eb60ff6a87c9afae5fe9
diff --git a/helper.sh b/helper.sh
index 0aafd5f..014d2c9 100755
--- a/helper.sh
+++ b/helper.sh
@@ -31,7 +31,7 @@ function start_redis()
shift
done
- local REDIS_COMMAND="./deps/redis/src/redis-server --loadmodule ./build/libredis-roaring.so"
+ local REDIS_COMMAND="./deps/redis/src/redis-server --loadmodule ./build/libredis-roaring-shared.so"
local VALGRIND_COMMAND="valgrind --leak-check=yes --show-leak-kinds=definite,indirect --suppressions=./deps/redis/src/valgrind.sup --error-exitcode=1 --log-file=$LOG_FILE"
local AOF_OPTION="--appendonly $USE_AOF"
if [ "$USE_VALGRIND" == "no" ]; then
|
@aviggiano I have updated the PR with something that I hope is even better than your patch. |
Merging |
This should improve the build setup.