Fix valkey-io#6 - Make the compiler flags configurable

.github/workflows/ci.yml

@@ -11,7 +11,7 @@ jobs:
         server_version: ['unstable', '8.0.0']
     steps:
     - uses: actions/checkout@v4
-    - name: Set the server verison for python integeration tests
+    - name: Set the server version for python integration tests
       run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV
     - name: Set up Python
       uses: actions/setup-python@v3

CMakeLists.txt

@@ -10,12 +10,10 @@ EXECUTE_PROCESS(
     OUTPUT_VARIABLE ARCHITECTURE
 )
 
-if("${ARCHITECTURE}" STREQUAL "x86_64")
-    message("Building JSON for x86_64")
-elseif("${ARCHITECTURE}" STREQUAL "aarch64")
-    message("Building JSON for aarch64")
+if("${ARCHITECTURE}" STREQUAL "x86_64" OR "${ARCHITECTURE}" STREQUAL "aarch64")
+    message("Building valkeyJSON for ${ARCHITECTURE}")
 else()
-    message(FATAL_ERROR "Unsupported architecture: ${ARCHITECTURE}. JSON is only supported on x86_64 and aarch64.")
+    message(FATAL_ERROR "Unsupported architecture: ${ARCHITECTURE}. valkeyJSON is only supported on x86_64 and aarch64.")
 endif()
 
 # Project definition
@@ -28,6 +26,19 @@ set(JSON_MODULE_LIB json)
 set(VALKEY_DOWNLOAD_DIR "${CMAKE_BINARY_DIR}/_deps/valkey-src")
 set(VALKEY_BIN_DIR "${CMAKE_BINARY_DIR}/_deps/valkey-src/src/valkey/src")
 
+# Valkey version
+if(NOT VALKEY_VERSION)
+    set(VALKEY_VERSION unstable)
+endif()
+message("Valkey version: ${VALKEY_VERSION}")
+
+# Compiler flags that can be overridden in command line
+if(NOT CFLAGS)
+    # Include debug symbols and set optimize level
+    set(CFLAGS "-g -O3 -fno-omit-frame-pointer -Wall -Werror -Wextra")
+endif()
+message("CFLAGS: ${CFLAGS}")
+
 # Download and build Valkey
 ExternalProject_Add(
     valkey
@@ -114,26 +125,20 @@ set(CMAKE_C_STANDARD_REQUIRED True)
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED True)
 
-# Always include debug symbols and optimize the code
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3 -g -fno-omit-frame-pointer")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3 -g -fno-omit-frame-pointer")
+# Additional flags for all architectures
+set(ADDITIONAL_FLAGS "-fPIC")
 
 # RapidJSON SIMD optimization
 if("${ARCHITECTURE}" STREQUAL "x86_64")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=nehalem")
+    set(ADDITIONAL_FLAGS "${ADDITIONAL_FLAGS} -march=nehalem")
 elseif("${ARCHITECTURE}" STREQUAL "aarch64")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=armv8-a")
-else()
-    message(FATAL_ERROR "Unsupported architecture: ${ARCHITECTURE}. JSON is only supported on x86_64 and aarch64.")
+    set(ADDITIONAL_FLAGS "${ADDITIONAL_FLAGS} -march=armv8-a")
 endif()
 
-# Additional flags for all architectures
-set(ADDITIONAL_FLAGS "-fPIC")
-
-# Compiler warning flags
-set(C_WARNING "-Wall -Werror -Wextra")
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${ADDITIONAL_FLAGS} ${C_WARNING}")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ADDITIONAL_FLAGS} ${C_WARNING}")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CFLAGS} ${ADDITIONAL_FLAGS}")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CFLAGS} ${ADDITIONAL_FLAGS}")
+message("CMAKE_C_FLAGS: ${CMAKE_C_FLAGS}")
+message("CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}")
 
 # Fetch RapidJSON
 FetchContent_Declare(
@@ -155,7 +160,7 @@ add_subdirectory(src)
 add_subdirectory(tst)
 
 add_custom_target(test
-    COMMENT "Run JSON integration tests."
+    COMMENT "Run valkeyJSON integration tests"
     USES_TERMINAL
     COMMAND rm -rf ${CMAKE_BINARY_DIR}/tst/integration
     COMMAND mkdir -p ${CMAKE_BINARY_DIR}/tst/integration

README.md

@@ -13,10 +13,15 @@ export SERVER_VERSION=unstable
 ./build.sh
 
 # Builds the valkey-server (8.0.0) for integration testing.
-export SERVER_VERSION=8.0.0
+SERVER_VERSION=8.0.0
 ./build.sh
 ```
 
+Custom compiler flags can be passed to the build script via environment variable CFLAGS. For example:
+```text
+CFLAGS="-O0 -Wno-unused-function" ./build.sh
+```
+
 #### To build just the module
 ```text
 mdkir build
@@ -25,6 +30,14 @@ cmake .. -DVALKEY_VERSION=unstable
 make
 ```
 
+Custom compiler flags can be passed to cmake via variable CFLAGS. For example:
+```text
+mdkir build
+cd build
+cmake .. -DCFLAGS="-O0 -Wno-unused-function"
+make
+```
+
 #### To run all unit tests:
 ```text
 cd build

build.sh

@@ -9,16 +9,11 @@ SCRIPT_DIR=$(pwd)
 echo "Script Directory: $SCRIPT_DIR"
 
 # Ensure SERVER_VERSION environment variable is set
-if [ -z "$SERVER_VERSION" ]; then
+if [ -z "${SERVER_VERSION}" ]; then
     echo "WARNING: SERVER_VERSION environment variable is not set. Defaulting to unstable."
     export SERVER_VERSION="unstable"
 fi
 
-if [ "$SERVER_VERSION" != "unstable" ] && [ "$SERVER_VERSION" != "8.0.0" ] ; then
-  echo "ERROR: Unsupported version - $SERVER_VERSION"
-  exit 1
-fi
-
 # Variables
 BUILD_DIR="$SCRIPT_DIR/build"
 
@@ -28,7 +23,11 @@ if [ ! -d "$BUILD_DIR" ]; then
     mkdir $BUILD_DIR
 fi
 cd $BUILD_DIR
-cmake .. -DVALKEY_VERSION=$SERVER_VERSION
+if [ -z "${CFLAGS}" ]; then
+  cmake .. -DVALKEY_VERSION=${SERVER_VERSION}
+else
+  cmake .. -DVALKEY_VERSION=${SERVER_VERSION} -DCFLAGS=${CFLAGS}
+fi
 make
 
 # Running the Valkey JSON unit tests.
@@ -47,7 +46,6 @@ if command -v pip > /dev/null 2>&1; then
 elif command -v pip3 > /dev/null 2>&1; then
     echo "Using pip3 to install packages..."
     pip3 install -r "$SCRIPT_DIR/$REQUIREMENTS_FILE"
-
 else
     echo "Error: Neither pip nor pip3 is available. Please install Python package installer."
     exit 1

tst/integration/run.sh

@@ -9,7 +9,7 @@ pkill -9 -f Valgrind || true
 pkill -9 -f "valkey-benchmark" || true
 
 # If environment variable SERVER_VERSION is not set, default to "unstable"
-if [ -z "$SERVER_VERSION" ]; then
+if [ -z "${SERVER_VERSION}" ]; then
     echo "WARNING: SERVER_VERSION environment variable is not set. Defaulting to \"unstable\"."
     export SERVER_VERSION="unstable"
 fi
@@ -19,13 +19,13 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 cd "${DIR}"
 
 export MODULE_PATH=$2/build/src/libjson.so 
-echo "Running integration tests against Valkey version $SERVER_VERSION"
+echo "Running integration tests against Valkey version ${SERVER_VERSION}"
 
 if [[ ! -z "${TEST_PATTERN}" ]] ; then
     export TEST_PATTERN="-k ${TEST_PATTERN}"
 fi
 
-BINARY_PATH=".build/binaries/$SERVER_VERSION/valkey-server"
+BINARY_PATH=".build/binaries/${SERVER_VERSION}/valkey-server"
 
 if [[ ! -f "${BINARY_PATH}" ]] ; then
     echo "${BINARY_PATH} missing"