aviggiano / redis-roaring

Roaring Bitmaps for Redis
MIT License
348 stars 56 forks source link

Modernize the build setup #107

Closed lemire closed 7 months ago

lemire commented 7 months ago

This should improve the build setup.

aviggiano commented 7 months ago

hey @lemire Thank you very much for this update. It seems like the build step is not saving the library at the previous location

12110:M 15 Apr 2024 00:03:31.009 # Module ./build/libredis-roaring.so failed to load: ./build/libredis-roaring.so: cannot open shared object file: No such file or directory 12110:M 15 Apr 2024 00:03:31.010 # Can't load module from ./build/libredis-roaring.so: server aborting

Do you know what might be the cause of this? I can take a look later before merging if you don't have time.

lemire commented 7 months ago

@aviggiano

The proposed build setup will compile to static libraries:

option(BUILD_SHARED_LIBS "use shared libraries" OFF)

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.

lemire commented 7 months ago

I have pushed an update which should generate a shared library as well.

lemire commented 7 months ago

@aviggiano I pushed another update.

aviggiano commented 7 months ago

Thanks, I'll try to merge this ASAP For some reason the test is not finishing, not sure why

lemire commented 7 months ago

@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.

xl-xueling commented 7 months ago

@aviggiano @lemire thanks all of you,this project is very very useful !

aviggiano commented 7 months ago

@lemire I've managed to fix the tests, the library name needed to change. Can you add this to your PR so that we can merge it? I can't edit it.

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
lemire commented 7 months ago

@aviggiano I have updated the PR with something that I hope is even better than your patch.

aviggiano commented 7 months ago

Merging