Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make install_shared_and_static test more robust #2179

Merged
merged 4 commits into from
Feb 13, 2025
Merged

Make install_shared_and_static test more robust #2179

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8b883a
more robust
smittals2 Feb 7, 2025
66d3110
move --fresh to the front
smittals2 Feb 8, 2025
62fe5e5
update print statements
smittals2 Feb 11, 2025
706fc6f
changed print
smittals2 Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions tests/ci/run_install_shared_and_static.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set -exo pipefail

source tests/ci/common_posix_setup.sh

export CMAKE_BUILD_PARALLEL_LEVEL=${NUM_CPU_THREADS}
export CMAKE_BUILD_PARALLEL_LEVEL=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Great find, were most of the random CI failures due to race conditions with the CMake building or the invalid file states? Seems like the latter's more of the cause of our issues, but I could be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure what the CI failures were because of but my hypothesis is the main culprit is this parallelism. I wasn't able to reproduce the issues.

The other file state stuff and using --fresh is to be more explicit about our expectations of state during the execution.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think --fresh and the file state issues you've discovered seem more like the culprit. CMake parallelism's generally relatively harmless (ninja uses multiple threads by default and make -j(threads) is often used to speed things up).
But nothing to get too fixated over here, this build test is fairly small and fast already.


# Set up environment.

Expand All @@ -29,6 +29,7 @@ ROOT=$(realpath ${AWS_LC_DIR}/..)
SCRATCH_DIR=${ROOT}/SCRATCH_AWSLC_INSTALL_SHARED_AND_STATIC
mkdir -p ${SCRATCH_DIR}
rm -rf "${SCRATCH_DIR:?}"/*
sync

function fail() {
echo "test failure: $1"
Expand All @@ -41,7 +42,9 @@ function install_aws_lc() {

local BUILD_DIR=${SCRATCH_DIR}/build
rm -rf "${BUILD_DIR:?}"
${CMAKE_COMMAND} -H${AWS_LC_DIR} -B${BUILD_DIR} -GNinja -DCMAKE_INSTALL_PREFIX=${INSTALL_DIR} -DBUILD_TESTING=OFF -D${BUILD_SHARED_LIBS}
sync

${CMAKE_COMMAND} --fresh -H${AWS_LC_DIR} -B${BUILD_DIR} -GNinja -DCMAKE_INSTALL_PREFIX=${INSTALL_DIR} -DBUILD_TESTING=OFF -D${BUILD_SHARED_LIBS}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does --fresh behave any differently than removing the entire build folder on line 44?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's about having an explicit instruction to cmake to prevent any old cached stuff from being used. Removing the build folder likely takes care of this, but it doesn't hurt to be explicit here. Especially to safeguard against any future changes that might change this expectation

${CMAKE_COMMAND} --build ${BUILD_DIR} --target install
}

Expand All @@ -61,6 +64,7 @@ install_aws_lc install-both BUILD_SHARED_LIBS=ON
MYAPP_SRC_DIR=${SCRATCH_DIR}/myapp-src
rm -rf "${MYAPP_SRC_DIR:?}"
mkdir -p ${MYAPP_SRC_DIR}
sync

cat <<EOF > ${MYAPP_SRC_DIR}/mylib.c
#include <openssl/ssl.h>
Expand Down Expand Up @@ -92,10 +96,16 @@ build_myapp() {
local AWS_LC_INSTALL_DIR=$2 # which install dir should be used
local EXPECT_USE_LIB_TYPE=$3 # (".so" or ".a") which types of libssl and libcrypto are expected to be used

echo "Build Parameters:"
echo "BUILD_SHARED_LIBS: ${BUILD_SHARED_LIBS}"
echo "AWS_LC_INSTALL_DIR: ${AWS_LC_INSTALL_DIR}"
echo "EXPECT_USE_LIB_TYPE: ${EXPECT_USE_LIB_TYPE}"

local BUILD_DIR=${SCRATCH_DIR}/build
rm -rf "${BUILD_DIR:?}"
sync

cmake -H${MYAPP_SRC_DIR} -B${BUILD_DIR} -GNinja -D${BUILD_SHARED_LIBS} -DCMAKE_PREFIX_PATH=${SCRATCH_DIR}/${AWS_LC_INSTALL_DIR}
cmake --fresh -H${MYAPP_SRC_DIR} -B${BUILD_DIR} -GNinja -D${BUILD_SHARED_LIBS} -DCMAKE_PREFIX_PATH=${SCRATCH_DIR}/${AWS_LC_INSTALL_DIR}
cmake --build ${BUILD_DIR}
ldd ${BUILD_DIR}/myapp

Expand All @@ -109,7 +119,10 @@ test_lib_use() {
local LIB_NAME=$2 # name of lib that app should be using, without file extension
local EXPECT_USE_LIB_TYPE=$3 # (".so" or ".a") expected type of lib that app should be using

if ldd ${APP} | grep -q ${LIB_NAME}.so; then
local LDD_OUTPUT=$(ldd ${APP})
echo "${LDD_OUTPUT}" | grep "${LIB_NAME}" || echo "No matches found"

if echo "${LDD_OUTPUT}" | grep -q ${LIB_NAME}.so; then
local ACTUAL_USE_LIB_TYPE=.so
else
local ACTUAL_USE_LIB_TYPE=.a
Expand Down Expand Up @@ -137,6 +150,7 @@ build_myapp BUILD_SHARED_LIBS=OFF install-both .a # myapp should use libssl.a/li
# ------------------------------------------------------- #
rm -rf "${MYAPP_SRC_DIR:?}"
mkdir -p ${MYAPP_SRC_DIR}
sync

cat <<EOF > ${MYAPP_SRC_DIR}/static_constructor_test.c
#include <stdint.h>
Expand Down
Loading