Refactor SmartRedis build system (#341)

Refactor the SmartRedis build system as planned in the design document: . add support for SR_BUILD and SR_LINK Make variables . add missing make dependencies . eliminate use of scripts for building the SmartRedis libraries . eliminate unnecessary deletion of SmartRedis builds, allowing repeat builds to be much faster . update examples, test cases, CI/CD to work with new build system . update Intel compiler in CI/CD to move away from deprecated compiler that will EOL in the next few months . make Python and Fortran optional parts of the build, not built by default [ committed by @billschereriii ] [ reviewed by @ashao @MattToast @mellis13 ]
CrayLabs · Jun 2, 2023 · 94f7188 · 94f7188
1 parent 8138be1
commit 94f7188
Show file tree

Hide file tree

Showing 52 changed files with 1,111 additions and 666 deletions.
diff --git a/.github/workflows/run_post_merge_tests.yml b/.github/workflows/run_post_merge_tests.yml
@@ -170,5 +170,5 @@ jobs:
           export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/install/lib &&
           export SR_LOG_FILE=smartredis_examples_log.txt &&
           export SR_LOG_LEVEL=INFO &&
-          make test-examples
+          make test-examples SR_FORTRAN=ON SR_PYTHON=ON
 
diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml
@@ -108,8 +108,8 @@ jobs:
           sudo apt-get install -y intel-oneapi-compiler-fortran intel-oneapi-compiler-dpcpp-cpp-and-cpp-classic
           source /opt/intel/oneapi/setvars.sh &&
           printenv >> $GITHUB_ENV &&
-          echo "CC=icc" >> $GITHUB_ENV &&
-          echo "CXX=icpc" >> $GITHUB_ENV &&
+          echo "CC=icx" >> $GITHUB_ENV &&
+          echo "CXX=icpx" >> $GITHUB_ENV &&
           echo "FC=ifort" >> $GITHUB_ENV
 
       # Install additional dependencies
@@ -120,29 +120,25 @@ jobs:
       - name: Build SmartRedis python and install
         run: python -m pip install -e .[dev,xarray]
 
-      - name: Build and install dependencies
-        run: |
-            mkdir -p ./third-party &&
-            cd ./third-party &&
-            bash ../build-scripts/build-lcov.sh &&
-            bash ../build-scripts/build-catch.sh
+      - name: Build and install test dependencies
+        run: make lcov && make catch2
 
       - name: Install docker, redis-server, and RedisAI
         run: |
           sudo apt-get -y update && sudo apt-get -y install curl gnupg lsb-release software-properties-common ca-certificates && \
           # Add latest redis to apt sources
-          echo "deb [signed-by=/usr/share/keyrings/redis-archive-keyring.gpg] https://packages.redis.io/deb $(lsb_release -cs) main" | sudo tee /etc/apt/sources.list.d/redis.list && \
-          curl -fsSL https://packages.redis.io/gpg | gpg --dearmor -o /usr/share/keyrings/redis-archive-keyring.gpg && \
+          sudo echo "deb [signed-by=/usr/share/keyrings/redis-archive-keyring.gpg] https://packages.redis.io/deb $(lsb_release -cs) main" | sudo tee /etc/apt/sources.list.d/redis.list && \
+          sudo curl -fsSL https://packages.redis.io/gpg | gpg --dearmor -o /usr/share/keyrings/redis-archive-keyring.gpg && \
           # Add latest docker to apt sources
           sudo mkdir -m 0755 -p /etc/apt/keyrings &&
-          curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo gpg --dearmor -o /etc/apt/keyrings/docker.gpg &&
+          sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo gpg --dearmor -o /etc/apt/keyrings/docker.gpg &&
           echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu \
                 $(lsb_release -cs) stable" | sudo tee /etc/apt/sources.list.d/docker.list > /dev/null &&
           # Install and update
           sudo apt-get -y update && \
           sudo apt-get -y install iputils-ping docker-ce docker-ce-cli containerd.io redis-tools=6:6.2.5-1rl1~focal1 redis-server=6:6.2.5-1rl1~focal1 &&
           CONTAINER_NAME="redisai_$RANDOM" &&
-          docker create --name $CONTAINER_NAME --rm redislabs/redisai:${{ matrix.rai_v }}-cpu-bionic && \
+          sudo docker create --name $CONTAINER_NAME --rm redislabs/redisai:${{ matrix.rai_v }}-cpu-bionic && \
           sudo mkdir -p /usr/lib/redis/modules/ &&
           sudo docker cp $CONTAINER_NAME:/usr/lib/redis/modules/redisai.so /usr/lib/redis/modules &&
           sudo docker cp $CONTAINER_NAME:/usr/lib/redis/modules/backends/ /usr/lib/redis/modules/ &&
@@ -162,7 +158,7 @@ jobs:
           popd &&
           export SSDB=$SSDB_SINGLE SMARTREDIS_TEST_CLUSTER=False &&
           utils/check_redis.sh $PORT &&
-          make test-verbose-with-coverage COV_FLAGS="--cov=./src/python/module/smartredis/ --cov-report=xml --cov-append"
+          make test-verbose-with-coverage COV_FLAGS="--cov=./src/python/module/smartredis/ --cov-report=xml --cov-append" SR_FORTRAN=ON SR_PYTHON=ON
 
       - name: Clustered DB tests
         run: |
@@ -181,7 +177,7 @@ jobs:
           redis-cli --cluster create $(echo $SSDB_CLUSTERED | tr "," " ") --cluster-yes &&
           export SSDB=$SSDB_CLUSTERED SMARTREDIS_TEST_CLUSTER=True &&
           python -m pytest --cov=./src/python/module/smartredis/ --cov-report=xml --cov-append \
-                 --ignore ./tests/docker -vv -s ./tests
+                 --ignore ./tests/docker -vv -s ./tests --build Coverage
 
       - name: UDS DB tests
         run: |
@@ -198,11 +194,11 @@ jobs:
           utils/check_redis.sh $SOCKET
           export SSDB=$SSDB_UDS SMARTREDIS_TEST_CLUSTER=False &&
           python -m pytest --cov=./src/python/module/smartredis/ --cov-report=xml --cov-append \
-                 --ignore ./tests/docker -vv -s ./tests
+                 --ignore ./tests/docker -vv -s ./tests --build Coverage
 
       # Process and upload code coverage (Python was collected during pytest)
       - name: Collect coverage from C/C++/Fortran testers
-        run: third-party/lcov/install/usr/local/bin/lcov -c -d build/CMakeFiles -o coverage.info
+        run: third-party/lcov/install/usr/local/bin/lcov -c -d build/Coverage/CMakeFiles -o coverage.info
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -24,47 +24,59 @@
 # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-project(SmartRedis)
+# Enable setting version in the project statement
+if (POLICY CMP0048)
+    cmake_policy(SET CMP0048 NEW)
+endif (POLICY CMP0048)
 
+# Project definition for the SmartRedis project
 cmake_minimum_required(VERSION 3.13)
+project(SmartRedis VERSION "0.4.0")
 
-option(BUILD_PYTHON  "Build the python module" ON)
-option(BUILD_FORTRAN "Build the fortran client library" OFF)
-option(COVERAGE "Build the fortran client library" OFF)
+# Configure options for the SmartRedis project
+option(SR_PYTHON  "Build the python module" OFF)
+option(SR_FORTRAN "Build the fortran client library" OFF)
+option(SR_PEDANTIC "Build with pickiest compiler settings" OFF)
 
-set(CMAKE_BUILD_TYPE RELEASE)
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_INSTALL_PREFIX ${CMAKE_SOURCE_DIR}/install)
 set(CMAKE_CXX_VISIBILITY_PRESET default)
 set(THREADS_PREFER_PTHREAD_FLAG ON)
+set(CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR})
+include(smartredis_defs)
 
-if (BUILD_FORTRAN)
+# If we want to use Fortran, we have to tell CMake to use it
+if (SR_FORTRAN)
     enable_language(Fortran)
 endif()
 
-if (WERROR)
+# For now, we only support Pedantic on the main library build.
+# If/when we fine-tune the examples and test cases, move this block
+# to smartredis_defs.cmake
+if (SR_PEDANTIC)
     if((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (CMAKE_C_COMPILER_ID STREQUAL "GNU"))
         add_compile_options(-Wall -Werror)
     else()
-        message(WARNING "WERROR was specified, but the CMAKE compiler is not GCC")
+        message(WARNING "SR_PEDANTIC was specified, but the CMAKE compiler is not GCC")
     endif()
-endif()
-
-if (COVERAGE)
-    if((CMAKE_CXX_COMPILER_ID STREQUAL "GNU") AND (CMAKE_C_COMPILER_ID STREQUAL "GNU"))
-        add_compile_options(--coverage)
-        add_link_options(--coverage)
-    else()
-        message(WARNING "COVERAGE was specified, but the CMAKE compiler is not GCC")
+    if(CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
+        set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -Wno-maybe-uninitialized")
     endif()
 endif()
 
-find_library(REDISPP redis++ PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH REQUIRED)
-find_library(HIREDIS hiredis PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH REQUIRED)
+# Bring in third-party libaries needed for the SmartRedis library
+find_library(REDISPP redis++
+    PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH
+    REQUIRED STATIC
+)
+find_library(HIREDIS hiredis
+    PATHS ${CMAKE_SOURCE_DIR}/install/lib NO_DEFAULT_PATH
+    REQUIRED STATIC
+)
 find_package(Threads REQUIRED)
-
 set(EXT_CLIENT_LIBRARIES ${REDISPP} ${HIREDIS})
 
+# Define source code that goes into the SmartRedis library
 set(CLIENT_SRC
     src/c/c_client.cpp
     src/c/c_configoptions.cpp
@@ -73,45 +85,47 @@ set(CLIENT_SRC
     src/c/c_logcontext.cpp
     src/c/c_logger.cpp
     src/cpp/address.cpp
+    src/cpp/addressallcommand.cpp
+    src/cpp/addressanycommand.cpp
+    src/cpp/addressatcommand.cpp
     src/cpp/client.cpp
-    src/cpp/dataset.cpp
+    src/cpp/clusterinfocommand.cpp
     src/cpp/command.cpp
-    src/cpp/keyedcommand.cpp
-    src/cpp/nonkeyedcommand.cpp
-    src/cpp/multikeycommand.cpp
-    src/cpp/singlekeycommand.cpp
+    src/cpp/commandlist.cpp
+    src/cpp/commandreply.cpp
     src/cpp/compoundcommand.cpp
-    src/cpp/addressatcommand.cpp
-    src/cpp/addressanycommand.cpp
-    src/cpp/addressallcommand.cpp
-    src/cpp/clusterinfocommand.cpp
+    src/cpp/configoptions.cpp
+    src/cpp/dataset.cpp
     src/cpp/dbinfocommand.cpp
+    src/cpp/dbnode.cpp
     src/cpp/gettensorcommand.cpp
-    src/cpp/commandlist.cpp
+    src/cpp/keyedcommand.cpp
+    src/cpp/logger.cpp
     src/cpp/metadata.cpp
-    src/cpp/tensorbase.cpp
-    src/cpp/tensorpack.cpp
-    src/cpp/dbnode.cpp
-    src/cpp/commandreply.cpp
-    src/cpp/redisserver.cpp
-    src/cpp/rediscluster.cpp
-    src/cpp/redis.cpp
     src/cpp/metadatafield.cpp
-    src/cpp/stringfield.cpp
+    src/cpp/multikeycommand.cpp
+    src/cpp/nonkeyedcommand.cpp
     src/cpp/pipelinereply.cpp
+    src/cpp/redis.cpp
+    src/cpp/rediscluster.cpp
+    src/cpp/redisserver.cpp
+    src/cpp/singlekeycommand.cpp
+    src/cpp/srobject.cpp
+    src/cpp/stringfield.cpp
+    src/cpp/tensorbase.cpp
+    src/cpp/tensorpack.cpp
     src/cpp/threadpool.cpp
     src/cpp/utility.cpp
-    src/cpp/logger.cpp
-    src/cpp/srobject.cpp
-    src/cpp/configoptions.cpp
 )
 
+# Define include directories for header files
 include_directories(SYSTEM
     include
     install/include
 )
 
-if (BUILD_FORTRAN)
+# Build the Fortran library
+if (SR_FORTRAN)
     set(FORTRAN_SRC
         src/fortran/errors.F90
         src/fortran/client.F90
@@ -125,20 +139,31 @@ if (BUILD_FORTRAN)
     # Note the following has to be before ANY add_library command)
     set(CMAKE_Fortran_MODULE_DIRECTORY "${CMAKE_INSTALL_PREFIX}/include")
     # Fortran library
-    add_library(smartredis-fortran SHARED ${FORTRAN_SRC})
-    set_target_properties(smartredis-fortran PROPERTIES SUFFIX ".so")
+    add_library(smartredis-fortran ${SMARTREDIS_LINK_MODE} ${FORTRAN_SRC})
+    set_target_properties(smartredis-fortran PROPERTIES
+        SUFFIX ${SMARTREDIS_LINK_LIBRARY_SUFFIX}
+    )
+	set_target_properties(smartredis-fortran PROPERTIES
+		OUTPUT_NAME ${SMARTREDIS_FORTRAN_LIB}
+	)
     target_link_libraries(smartredis-fortran PUBLIC smartredis ${EXT_CLIENT_LIBRARIES})
     # Install dynamic library and headers
     install(TARGETS smartredis-fortran
     	LIBRARY DESTINATION lib)
 endif()
 
 
-# Build dynamic library
-add_library(smartredis SHARED ${CLIENT_SRC})
-set_target_properties(smartredis PROPERTIES SUFFIX ".so")
+# Build the main SmartRedis library
+add_library(smartredis ${SMARTREDIS_LINK_MODE} ${CLIENT_SRC})
+set_target_properties(smartredis PROPERTIES
+    SUFFIX ${SMARTREDIS_LINK_LIBRARY_SUFFIX}
+)
+set_target_properties(smartredis PROPERTIES
+    OUTPUT_NAME ${SMARTREDIS_LIB}
+)
 target_link_libraries(smartredis PUBLIC ${EXT_CLIENT_LIBRARIES} PRIVATE Threads::Threads)
 
+# Install SmartRedis header files
 install(DIRECTORY "${CMAKE_SOURCE_DIR}/include/"
         DESTINATION "include"
         FILES_MATCHING
@@ -149,13 +174,12 @@ install(DIRECTORY "${CMAKE_SOURCE_DIR}/include/"
 install(TARGETS smartredis
      	LIBRARY DESTINATION lib)
 
-if(BUILD_PYTHON)
+# Build the Python library for SmartRedis
+if(SR_PYTHON)
 	message("-- Python client build enabled")
 	add_subdirectory(${CMAKE_SOURCE_DIR}/third-party/pybind
                      ${CMAKE_SOURCE_DIR}/third-party/pybind/build)
 
-    add_library(smartredis_static STATIC ${CLIENT_SRC})
-
 	pybind11_add_module(smartredisPy
 	                    src/python/src/pyclient.cpp
                         src/python/src/pyconfigoptions.cpp
@@ -166,8 +190,8 @@ if(BUILD_PYTHON)
                         src/python/bindings/bind.cpp)
 
 	target_link_libraries(smartredisPy PUBLIC ${EXT_CLIENT_LIBRARIES})
-	install(TARGETS smartredisPy
-	        LIBRARY DESTINATION lib)
+	install(TARGETS smartredisPy LIBRARY DESTINATION lib)
+    install(TARGETS smartredisPy LIBRARY DESTINATION ../src/python/module/smartredis)
 else()
 	message("-- Skipping Python client build")
 endif()
diff --git a/MANIFEST.in b/MANIFEST.in
@@ -2,6 +2,7 @@ include requirements.txt
 include requirements-dev.txt
 include Makefile
 include CMakeLists.txt
+include smartredis_defs.cmake
 graft src
 graft build-scripts
 graft include