Skip to content

Commit

Permalink
Build and CI fixes.
Browse files Browse the repository at this point in the history
* Use __attribute__((unused)) for the cluster asyn attach static
  functions.  This lets us continue to build with -Werror
* Fix the tests target to include ssl objects if needed.
* Tweak CI invocation of test.sh
* Install valkey-server for tests.
* Darwin doesn't have clock_nanosleep
* Add a sanity check to test.sh
  • Loading branch information
michael-grunder committed Jun 24, 2024
1 parent a0fcb70 commit ce9f77b
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 24 deletions.
45 changes: 32 additions & 13 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ jobs:

- name: Install dependencies
run: |
curl -fsSL https://packages.redis.io/gpg | sudo gpg --dearmor -o /usr/share/keyrings/redis-archive-keyring.gpg
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 apt-get update
sudo apt-get install -y redis-server valgrind libevent-dev
sudo apt-get install -y valgrind libevent-dev cmake libssl-dev
# TODO: Test against Redis and KeyDB in addition to Valkey
- name: Install Valkey
run: |
git clone --depth 1 --branch 7.2.5 https://github.com/valkey-io/valkey.git
cd valkey && sudo BUILD_TLS=yes make install
- name: Build using cmake
env:
Expand All @@ -26,10 +30,11 @@ jobs:
run: USE_SSL=1 TEST_ASYNC=1 make

- name: Run tests
working-directory: tests
env:
SKIPS_AS_FAILS: 1
TEST_SSL: 1
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

# - name: Run tests under valgrind
# env:
Expand All @@ -48,7 +53,12 @@ jobs:
run: |
yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm
yum -y --enablerepo=remi install redis
yum -y install gcc gcc-c++ make openssl openssl-devel cmake3 valgrind libevent-devel
yum -y install git gcc gcc-c++ make openssl openssl-devel cmake3 valgrind libevent-devel
- name: Install Valkey
run: |
git clone --depth 1 --branch 7.2.5 https://github.com/valkey-io/valkey.git
cd valkey && BUILD_TLS=yes make install
- name: Build using cmake
env:
Expand All @@ -61,17 +71,19 @@ jobs:
run: USE_SSL=1 TEST_ASYNC=1 make

- name: Run tests
working-directory: tests
env:
SKIPS_AS_FAILS: 1
TEST_SSL: 1
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

- name: Run tests under valgrind
working-directory: tests
env:
SKIPS_AS_FAILS: 1
TEST_SSL: 1
TEST_PREFIX: valgrind --error-exitcode=99 --track-origins=yes --leak-check=full
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

centos8:
name: RockyLinux 8
Expand All @@ -88,6 +100,11 @@ jobs:
dnf -y group install "Development Tools"
dnf -y install openssl-devel cmake valgrind libevent-devel
- name: Install Valkey
run: |
git clone --depth 1 --branch 7.2.5 https://github.com/valkey-io/valkey.git
cd valkey && BUILD_TLS=yes make install
- name: Build using cmake
env:
EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON -DENABLE_ASYNC_TESTS:BOOL=ON
Expand All @@ -99,17 +116,19 @@ jobs:
run: USE_SSL=1 TEST_ASYNC=1 make

- name: Run tests
working-directory: tests
env:
SKIPS_AS_FAILS: 1
TEST_SSL: 1
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

- name: Run tests under valgrind
working-directory: tests
env:
SKIPS_AS_FAILS: 1
TEST_SSL: 1
TEST_PREFIX: valgrind --error-exitcode=99 --track-origins=yes --leak-check=full
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

freebsd:
runs-on: ubuntu-latest
Expand All @@ -133,16 +152,16 @@ jobs:

- name: Install dependencies
run: |
brew install openssl [email protected]
brew link [email protected] --force
brew install openssl valkey
- name: Build hiredis
run: USE_SSL=1 make

- name: Run tests
working-directory: tests
env:
TEST_SSL: 1
run: $GITHUB_WORKSPACE/test.sh
run: ./test.sh

windows:
name: Windows
Expand All @@ -163,7 +182,7 @@ jobs:
- name: Run tests
run: |
./build/hiredis-test.exe
./build/test.exe
- name: Install Cygwin Action
uses: cygwin/cygwin-install-action@v2
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ jobs:
- uses: actions/checkout@v3
- name: Run CMake (shared lib)
run: cmake -Wno-dev CMakeLists.txt
- name: Build hiredis (shared lib)
- name: Build shared library
run: MSBuild hiredis.vcxproj /p:Configuration=Debug
- name: Run CMake (static lib)
run: cmake -Wno-dev CMakeLists.txt -DBUILD_SHARED_LIBS=OFF
- name: Build hiredis (static lib)
- name: Build static library
run: MSBuild hiredis.vcxproj /p:Configuration=Debug
- name: Build hiredis-test
- name: Build test.exe
run: MSBuild hiredis-test.vcxproj /p:Configuration=Debug
# use memurai, redis compatible server, since it is easy to install. Can't
# install official redis containers on the windows runner
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ ifeq ($(uname_S),Darwin)
DYLIB_PLUGIN=-Wl,-undefined -Wl,dynamic_lookup
endif

all: dynamic static pkgconfig
all: dynamic static pkgconfig tests

$(DYLIBNAME): $(OBJS) | $(LIB_DIR)
$(DYLIB_MAKE_CMD) -o $(DYLIBNAME) $(OBJS) $(REAL_LDFLAGS)
Expand All @@ -187,7 +187,7 @@ $(OBJ_DIR)/%.o: $(TEST_DIR)/%.c | $(OBJ_DIR)
$(CC) -std=c99 $(REAL_CFLAGS) -I$(INCLUDE_DIR) -MMD -MP -c $< -o $@

$(TEST_DIR)/%: $(OBJ_DIR)/%.o $(STLIBNAME)
$(CC) -o $@ $< $(STLIBNAME) $(LDFLAGS) $(SSL_LDLAGS) $(TEST_LDFLAGS)
$(CC) -o $@ $< $(STLIBNAME) $(SSL_STLIB) $(LDFLAGS) $(SSL_LDLAGS) $(TEST_LDFLAGS)

$(OBJ_DIR):
mkdir -p $(OBJ_DIR)
Expand Down Expand Up @@ -219,7 +219,7 @@ examples: $(STLIBNAME)

clean:
rm -rf $(OBJ_DIR) $(LIB_DIR) $(TEST_BINS) *.gcda *.gcno *.gcov
rm -rf examples/example-*
make -C examples clean

INSTALL?= cp -pPR

Expand Down
1 change: 1 addition & 0 deletions include/valkey/adapters/ae.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ static int valkeyAeAttach_link(valkeyAsyncContext *ac, void *base) {
return valkeyAeAttach((aeEventLoop *)base, ac);
}

VALKEY_UNUSED
static int valkeyClusterAeAttach(aeEventLoop *loop,
valkeyClusterAsyncContext *acc) {
if (acc == NULL || loop == NULL) {
Expand Down
1 change: 1 addition & 0 deletions include/valkey/adapters/glib.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ static int valkeyGlibAttach_link(valkeyAsyncContext *ac, void *adapter) {
return VALKEY_ERR;
}

VALKEY_UNUSED
static int valkeyClusterGlibAttach(valkeyClusterAsyncContext *acc,
valkeyClusterGlibAdapter *adapter) {
if (acc == NULL || adapter == NULL) {
Expand Down
1 change: 1 addition & 0 deletions include/valkey/adapters/libev.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ static int valkeyLibevAttach_link(valkeyAsyncContext *ac, void *loop) {
return valkeyLibevAttach((struct ev_loop *)loop, ac);
}

VALKEY_UNUSED
static int valkeyClusterLibevAttach(valkeyClusterAsyncContext *acc,
struct ev_loop *loop) {
if (loop == NULL || acc == NULL) {
Expand Down
2 changes: 2 additions & 0 deletions include/valkey/adapters/libevent.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,12 @@ static int valkeyLibeventAttach(valkeyAsyncContext *ac, struct event_base *base)
return VALKEY_OK;
}

VALKEY_UNUSED
static int valkeyLibeventAttach_link(valkeyAsyncContext *ac, void *base) {
return valkeyLibeventAttach(ac, (struct event_base *)base);
}

VALKEY_UNUSED
static int valkeyClusterLibeventAttach(valkeyClusterAsyncContext *acc,
struct event_base *base) {
if (acc == NULL || base == NULL) {
Expand Down
1 change: 1 addition & 0 deletions include/valkey/adapters/libuv.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ static int valkeyLibuvAttach_link(valkeyAsyncContext *ac, void *loop) {
return valkeyLibuvAttach(ac, (uv_loop_t *)loop);
}

VALKEY_UNUSED
static int valkeyClusterLibuvAttach(valkeyClusterAsyncContext *acc,
uv_loop_t *loop) {
if (acc == NULL || loop == NULL) {
Expand Down
7 changes: 7 additions & 0 deletions include/valkey/async.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
extern "C" {
#endif

/* For the async cluster attach functions. */
#if defined(__GNUC__) || defined(__clang__)
#define VALKEY_UNUSED __attribute__((unused))
#else
#define VALKEY_UNUSED
#endif

struct valkeyAsyncContext; /* need forward declaration of valkeyAsyncContext */
struct dict; /* dictionary header is included in async.c */

Expand Down
6 changes: 4 additions & 2 deletions src/valkeycluster.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <valkey/alloc.h>

#include "alloc.h"
#include "adlist.h"
#include "command.h"
#include "dict.h"
Expand Down Expand Up @@ -3353,6 +3353,7 @@ int valkeyClusterAppendCommandArgv(valkeyClusterContext *cc, int argc,
return ret;
}

VALKEY_UNUSED
static int valkeyClusterSendAll(valkeyClusterContext *cc) {
dictEntry *de;
valkeyClusterNode *node;
Expand Down Expand Up @@ -3388,6 +3389,7 @@ static int valkeyClusterSendAll(valkeyClusterContext *cc) {
return VALKEY_OK;
}

VALKEY_UNUSED
static int valkeyClusterClearAll(valkeyClusterContext *cc) {
dictEntry *de;
valkeyClusterNode *node;
Expand Down Expand Up @@ -3532,7 +3534,7 @@ int valkeyClusterGetReply(valkeyClusterContext *cc, void **reply) {
}

/**
* Resets cluster state after pipeline.
* Resets cluster state after pipeline.
* Resets Valkey node connections if pipeline commands were not called beforehand.
*/
void valkeyClusterReset(valkeyClusterContext *cc) {
Expand Down
2 changes: 1 addition & 1 deletion tests/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static void millisleep(int ms) {
Sleep(ms);
#else
struct timespec ts = { ms / 1000, (ms % 1000) * 1000000 };
clock_nanosleep(CLOCK_REALTIME, 0, &ts, NULL);
nanosleep(&ts, NULL);
#endif
}

Expand Down
17 changes: 15 additions & 2 deletions tests/test.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
#!/bin/sh -ue
#
check_executable() {
if ! command -v "$1" >/dev/null 2>&1; then
echo "Error: $1 is not found or not executable."
exit 1
fi
}

VALKEY_SERVER=${VALKEY_SERVER:-valkey-server}
VALKEY_PORT=${VALKEY_PORT:-56379}
Expand All @@ -10,6 +17,8 @@ SSL_TEST_ARGS=
SKIPS_ARG=${SKIPS_ARG:-}
VALKEY_DOCKER=${VALKEY_DOCKER:-}

check_executable "$VALKEY_SERVER"

# Enable debug command for redis-server >= 7.0.0 or any version of valkey-server.
VALKEY_MAJOR_VERSION="$("$VALKEY_SERVER" --version|awk -F'[^0-9]+' '{ print $2 }')"
if [ "$VALKEY_MAJOR_VERSION" -gt "6" ]; then
Expand Down Expand Up @@ -104,9 +113,13 @@ else
fi
# Wait until we detect the unix socket
echo waiting for server
while [ ! -S "${SOCK_FILE}" ]; do sleep 1; done
while [ ! -S "${SOCK_FILE}" ]; do
2>&1 echo "Waiting for server..."
ps aux|grep valkey-server
sleep 1;
done

# Treat skips as failures if directed
[ "$SKIPS_AS_FAILS" = 1 ] && SKIPS_ARG="${SKIPS_ARG} --skips-as-fails"

${TEST_PREFIX:-} ./libvalkey-test -h 127.0.0.1 -p ${VALKEY_PORT} -s ${SOCK_FILE} ${SSL_TEST_ARGS} ${SKIPS_ARG}
${TEST_PREFIX:-} ./test -h 127.0.0.1 -p ${VALKEY_PORT} -s ${SOCK_FILE} ${SSL_TEST_ARGS} ${SKIPS_ARG}

0 comments on commit ce9f77b

Please sign in to comment.