Skip to content

Conversation

@cinterloper
Copy link

@cinterloper cinterloper commented Sep 12, 2025

Built packages with links to JFrog are available in the Actions tab above

Integration tests are here.

Test cases download the current build from JFrog and install it in a fresh container, then execute it with the --help argument

You need a JFrog username and token to execute the test cases, an example is in the README.sh

# Conflicts:
#	.github/docker/entrypoint.sh
#	.github/docker/install_deps.sh
#	.github/docker/tests.bats
#	.github/workflows/build-artifacts.yml
@cinterloper
Copy link
Author

will refactor based on asconfig with common code from https://github.com/aerospike/server-packaging-common/

@cinterloper cinterloper marked this pull request as draft October 29, 2025 18:22
Copy link
Collaborator

@dwelch-spike dwelch-spike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that the build options in build_package.sh and install_deps.sh don't match the QE build system. Also I think it would be best to pin dependency versions at the top of files for consistency and ease of maintenance.

Comment on lines +13 to +17
if [ "$ENV_DISTRO" = "amzn2023" ] || [ "$ENV_DISTRO" = "el8" ] || [ "$ENV_DISTRO" = "el9" ]; then
make EVENT_LIB=libuv AWS_SDK_STATIC_PATH=/usr/local/lib JANSSON_STATIC_PATH=/usr/local/lib/
else
make EVENT_LIB=libuv ZSTD_STATIC_PATH=/usr/lib/$ARCH-linux-gnu AWS_SDK_STATIC_PATH=/usr/local/lib CURL_STATIC_PATH=/usr/local/lib OPENSSL_STATIC_PATH=/usr/lib/$ARCH-linux-gnu AWS_SDK_STATIC_PATH=/usr/local/lib JANSSON_STATIC_PATH=/usr/lib/$ARCH-linux-gnu
fi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. The same libs should be linked the same way (static vs dynamic) across all distros. For example OPENSSL_STATIC_PATH should be used in all cases. The tools package does this on the qe docker images by specifying env vars in https://github.com/citrusleaf/aerospike-tools/blob/21bdadd056a3b681bc2c2e43df4d70f119bd1cc2/.build.yml#L29 . Check the qe docker images to see how they do it but keep in mind that some environment variables are already set by that build.yml. https://github.com/citrusleaf/qe-docker/tree/master/build/aerospike-tools

distro:
- el8
- el9
#- el10
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is el10 disabled?

Comment on lines +13 to +16
FPM_DEPS_AMAZON="ruby rpmdevtools make git python3 python3-pip rsync"
FPM_DEPS_REDHAT_8="python3 python3-pip rsync"
FPM_DEPS_REDHAT_9="ruby rpmdevtools make git python3 python3-pip rsync zlib zlib-devel"
FPM_DEPS_REDHAT_10="ruby rpmdevtools make git python3 python3-pip rsync zlib zlib-devel"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checking that fpm needs Python

git checkout v1.43.0
sh autogen.sh
./configure
make -j8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is -j8 needed with the alias at the top of the file?


git clone https://github.com/curl/curl.git
cd curl
git checkout curl-8_14_1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other steps use curl-7_81_0, we should standardize on whatever the current build of the tools uses which, I think, is https://github.com/citrusleaf/qe-docker/blob/cd479406f0f1792b3539b5ff032b95afd368f169/build/aerospike-tools/ubuntu-24.04.Dockerfile#L14

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a good case for defining dep version numbers like this at the top of the file similar to the AWS_SDK_VERSION var.

Comment on lines 414 to 458
# install libunistring
cd /opt
curl -LO https://ftp.gnu.org/gnu/libunistring/libunistring-1.2.tar.xz
tar xf libunistring-1.2.tar.xz
cd libunistring-1.2
./configure --prefix=/usr --libdir=/usr/lib64 --with-pkgconfigdir=/usr/lib64/pkgconfig
make -j"$(nproc)"
make install
ldconfig
cd /opt
rm -rf libunistring-1.2 libunistring-1.2.tar.xz

# sanity
cat >/usr/lib64/pkgconfig/libunistring.pc <<'EOF'
prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib64
includedir=${prefix}/include

Name: libunistring
Description: Unicode string library
Version: 1.2
Libs: -L${libdir} -lunistring
Cflags: -I${includedir}
EOF
export PKG_CONFIG_PATH=/usr/lib64/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig
pkg-config --modversion libunistring
pkg-config --cflags --libs libunistring

cd /opt
wget https://mirrors.ocf.berkeley.edu/gnu/gettext/gettext-0.21.tar.gz
tar -zxvf gettext-0.21.tar.gz
cd gettext-0.21
autoconf
./configure
make -j8
make install

cd /opt
git clone https://github.com/rockdaboot/libpsl.git
cd libpsl
git checkout 0.21.5
./autogen.sh
./configure
make -j8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see these used in https://github.com/citrusleaf/qe-docker/blob/master/build/aerospike-tools/rhel-10.Dockerfile so double checking that they are needed.

Comment on lines 469 to 475
cd /opt
git clone https://github.com/akheron/jansson.git
cd jansson
autoreconf -i
./configure
make -j8
make install
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pin a jansson version.

Comment on lines 495 to 503
cd /opt
git clone https://github.com/aws/s2n-tls.git
cd s2n-tls

cmake . -Bbuild \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX=./s2n-tls-install
cmake --build build
make install
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeat comment: I don't see this used in the qe build system so double checking that it is needed.

make install

cd /opt
git clone https://github.com/aws/aws-sdk-cpp.git
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

future improvement: we can cache the aws sdk build so this doesn't need to be done on every run. See

Comment on lines 525 to 549
cd /opt
wget https://mirrors.ocf.berkeley.edu/gnu/gettext/gettext-0.21.tar.gz
tar -zxvf gettext-0.21.tar.gz
cd gettext-0.21
autoconf
./configure
make -j8
make install

cd /opt
git clone https://github.com/rockdaboot/libpsl.git
cd libpsl
git checkout 0.21.5
./autogen.sh
./configure
make -j8
make install

cd /opt
git clone https://https.git.savannah.gnu.org/git/readline.git
cd readline
git checkout readline-8.3
./configure
make -j8
make install
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants