Fix #6 - Make the compiler flags configurable (#23)

Signed-off-by: Joe Hu <jowhuw@amazon.com>
Co-authored-by: Joe Hu <jowhuw@amazon.com>

.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

@@ -32,6 +32,13 @@ if(NOT VALKEY_VERSION)
 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
@@ -118,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")
-
-# RapidJSON SIMD optimization
-if("${ARCHITECTURE}" STREQUAL "x86_64")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_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.")
-endif()
-
 # Additional flags for all architectures
 set(ADDITIONAL_FLAGS "-fPIC")
 
-# Compiler warning flags
+# RapidJSON SIMD optimization
-set(C_WARNING "-Wall -Werror -Wextra")
+if("${ARCHITECTURE}" STREQUAL "x86_64")
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${ADDITIONAL_FLAGS} ${C_WARNING}")
+    set(ADDITIONAL_FLAGS "${ADDITIONAL_FLAGS} -march=nehalem")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ADDITIONAL_FLAGS} ${C_WARNING}")
+elseif("${ARCHITECTURE}" STREQUAL "aarch64")
+    set(ADDITIONAL_FLAGS "${ADDITIONAL_FLAGS} -march=armv8-a")
+endif()
+
+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(
@@ -159,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

@@ -18,6 +18,11 @@ The default valkey version is "unstable". To override it, do:
 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
@@ -34,6 +39,14 @@ cmake .. -DVALKEY_VERSION=8.0.0
 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

@@ -23,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.
@@ -42,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

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