Skip to content

refactor(ci): enforce required env variables in build scripts #35990

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

Open
esolitos wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor(ci): enforce required env variables in build scripts #35990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions .buildkite/bootstrap-cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"
Copy link
Member

@hakonhall hakonhall Feb 23, 2026

Choose a reason for hiding this comment

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

Verify that $SOURCE_DIR is a directory is a better test? (Such a test would be even better before nounset)


echo "--- 🔧 Configuring CMake build"
# shellcheck disable=1091
source /etc/profile.d/enable-gcc-toolset.sh
Expand Down
3 changes: 3 additions & 0 deletions .buildkite/bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"
: "${VESPA_CPP_TEST_JARS:?Environment variable VESPA_CPP_TEST_JARS must be set (path for C++ test JARs)}"

echo "--- 🔧 Running Vespa bootstrap"
cd "$SOURCE_DIR"
./bootstrap.sh full
Expand Down
2 changes: 2 additions & 0 deletions .buildkite/build-container.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${VESPA_BUILDOS_LABEL:?Environment variable VESPA_BUILDOS_LABEL must be set (build OS label)}"
Copy link
Member

@hakonhall hakonhall Feb 23, 2026

Choose a reason for hiding this comment

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

It's not only verified to be set here, but also by set -o nounset when referenced on line 21 below, and by the fallback on lines 30-32.

This is one of the downsides of nounset - it would be better to reach line 31 to print a proper error message, than an unbound error on line 21.


if ! docker ps &> /dev/null; then
echo "No working docker command found."
exit 1
Expand Down
3 changes: 2 additions & 1 deletion .buildkite/cpp-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ set -o pipefail
if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${NUM_CPU_LIMIT:?Environment variable NUM_CPU_LIMIT must be set (CPU limit)}"
: "${LOG_DIR:?Environment variable LOG_DIR must be set (log directory)}"
echo "--- 🧪 Running C++ tests"
PATH=/opt/vespa-deps/bin:$PATH

Expand Down
3 changes: 3 additions & 0 deletions .buildkite/cpp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ fi

mydir=${0%/*}
shlim=${mydir}/show-limits.sh

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"
: "${NUM_CPP_THREADS:?Environment variable NUM_CPP_THREADS must be set (number of C++ threads)}"
if [ -x "${shlim}" ]; then
"${shlim}" || echo "failed: ${shlim}"
fi
Expand Down
3 changes: 2 additions & 1 deletion .buildkite/go.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ set -o pipefail
if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"
: "${WORKDIR:?Environment variable WORKDIR must be set (working directory)}"
echo "--- 🐹 Building Go components"
cd "$SOURCE_DIR"

Expand Down
3 changes: 2 additions & 1 deletion .buildkite/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ set -o pipefail
if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${NUM_CPU_LIMIT:?Environment variable NUM_CPU_LIMIT must be set (CPU limit)}"
: "${WORKDIR:?Environment variable WORKDIR must be set (working directory)}"
echo "--- 📦 Installing Vespa components"
echo "Running make install with $NUM_CPU_LIMIT parallel jobs..."
make -j "$NUM_CPU_LIMIT" install DESTDIR="$WORKDIR/vespa-install"
Expand Down
4 changes: 4 additions & 0 deletions .buildkite/java.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ fi

mydir=${0%/*}
shlim=${mydir}/show-limits.sh

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"
: "${VESPA_MAVEN_TARGET:?Environment variable VESPA_MAVEN_TARGET must be set (Maven target)}"
: "${NUM_MVN_THREADS:?Environment variable NUM_MVN_THREADS must be set (Maven threads)}"
if [ -x "${shlim}" ]; then
"${shlim}" || echo "failed: ${shlim}"
fi
Expand Down
2 changes: 2 additions & 0 deletions .buildkite/quick-start-guide.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ if [[ -n "${DEBUG:-}" ]]; then
set -o xtrace
fi

: "${SOURCE_DIR:?Environment variable SOURCE_DIR must be set (path to source code)}"

if [[ $VESPA_USE_SANITIZER != null ]]; then
echo "Skipping quick start guide test for sanitizer builds."
exit 0
Expand Down
10 changes: 2 additions & 8 deletions .buildkite/upload-test-results.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,8 @@ else
CPP_TEST_TOKEN=$UNIT_TEST_CPP_ARM64_TOKEN
fi

if [[ -z $JAVA_TEST_TOKEN ]]; then
echo "Missing JAVA_TEST_TOKEN. Exiting."
exit 1
fi
if [[ -z $CPP_TEST_TOKEN ]]; then
echo "Missing CPP_TEST_TOKEN. Exiting."
exit 1
fi
: "${JAVA_TEST_TOKEN:?Environment variable JAVA_TEST_TOKEN must be set (token for uploading Java test results)}"
: "${CPP_TEST_TOKEN:?Environment variable CPP_TEST_TOKEN must be set (token for uploading C++ test results)}"
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The script uses WORKDIR (line 53) and LOG_DIR (line 61) but does not validate these required environment variables. For consistency with the pattern established in this PR and other scripts (e.g., install.sh, cpp-test.sh, go.sh), these variables should be validated at the beginning of the script using the shell parameter expansion pattern.

Suggested change
: "${CPP_TEST_TOKEN:?Environment variable CPP_TEST_TOKEN must be set (token for uploading C++ test results)}"
: "${CPP_TEST_TOKEN:?Environment variable CPP_TEST_TOKEN must be set (token for uploading C++ test results)}"
: "${WORKDIR:?Environment variable WORKDIR must be set (working directory for test results)}"
: "${LOG_DIR:?Environment variable LOG_DIR must be set (directory containing C++ test results)}"

Copilot uses AI. Check for mistakes.

upload_result() {
curl \
Expand Down
Loading