Skip to content

Commit

Permalink
Fix #6 - Make the compiler flags configurable
Browse files Browse the repository at this point in the history
Signed-off-by: Joe Hu <[email protected]>
  • Loading branch information
Joe Hu committed Dec 9, 2024
1 parent 00c7407 commit b7d9bf0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
# 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(
Expand All @@ -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
Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tst/integration/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit b7d9bf0

Please sign in to comment.