From 13caa2e07494b180e68367af2160b85de5607f50 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Wed, 9 Oct 2024 16:04:03 +0200 Subject: [PATCH] refactor(linux): move API verification functions to separate file This will make it easier to unit test individual functions. --- linux/scripts/deb-packaging.sh | 260 +--------------------- linux/scripts/test/deb-packaging.tests.sh | 14 +- linux/scripts/verify_api.inc.sh | 258 +++++++++++++++++++++ 3 files changed, 268 insertions(+), 264 deletions(-) create mode 100644 linux/scripts/verify_api.inc.sh diff --git a/linux/scripts/deb-packaging.sh b/linux/scripts/deb-packaging.sh index 4f9be7d329d..923e5955c62 100755 --- a/linux/scripts/deb-packaging.sh +++ b/linux/scripts/deb-packaging.sh @@ -12,6 +12,8 @@ THIS_SCRIPT="$(readlink -f "${BASH_SOURCE[0]}")" . "${THIS_SCRIPT%/*}/../../resources/build/build-utils.sh" ## END STANDARD BUILD SCRIPT INCLUDE +. "${KEYMAN_ROOT}/linux/scripts/verify_api.inc.sh" + builder_describe \ "Helper for building Debian packages." \ "dependencies Install dependencies as found in debian/control" \ @@ -61,262 +63,6 @@ source_action() { mv builddebs/* "${OUTPUT_PATH:-..}" } -output_log() { - echo "$1" >&2 - builder_echo "$1" -} - -output_ok() { - echo ":heavy_check_mark: $1" >&2 - builder_echo green "OK: $1" -} - -output_warning() { - echo ":warning: $1" >&2 - builder_echo warning "WARNING: $1" -} - -output_error() { - echo ":x: $1" >&2 - builder_echo error "ERROR: $1" -} - -check_api_not_changed() { - if [[ -z "${BIN_PKG:-}" ]]; then - output_warning "Skipping check for API change because binary Debian package not specified" - return - fi - # Checks that the API did not change compared to what's documented in the .symbols file - tmpDir=$(mktemp -d) - # shellcheck disable=SC2064 - trap "rm -rf \"${tmpDir}\"" ERR - dpkg -x "${BIN_PKG}" "${tmpDir}" - mkdir -p debian/tmp/DEBIAN - dpkg-gensymbols -v"${VERSION}" -p"${PKG_NAME}" -e"${tmpDir}"/usr/lib/x86_64-linux-gnu/"${LIB_NAME}".so* -c4 - output_ok "${LIB_NAME} API didn't change" - cd "${REPO_ROOT}/linux" - rm -rf "${tmpDir}" - trap ERR -} - -# -# Compare the SHA of the base and head commits for changes to the .symbols file -# -is_symbols_file_changed() { - local CHANGED_REF CHANGED_BASE - CHANGED_REF=$(git rev-parse "${GIT_SHA}":"linux/debian/${PKG_NAME}.symbols") - CHANGED_BASE=$(git rev-parse "${GIT_BASE}":"linux/debian/${PKG_NAME}.symbols") - if [[ "${CHANGED_REF}" == "${CHANGED_BASE}" ]]; then - return 1 - fi - return 0 -} - -get_changes() { - local WHAT_CHANGED - WHAT_CHANGED=$(git diff -I "^${LIB_NAME}.so" "$1".."$2" | diffstat -m -t | grep "${PKG_NAME}.symbols" ) - - IFS=',' read -r -a CHANGES <<< "${WHAT_CHANGED:-0,0,0}" -} - -check_updated_version_number() { - # Checks that the package version number got updated in the .symbols file if it got changed - # shellcheck disable=SC2310 - if is_symbols_file_changed; then - # .symbols file changed, now check if the package version got updated as well - # Note: We don't check that ALL changes in that file have an updated package version - - # we hope this gets flagged in code review. - # Note: This version number check may not match the actual released version, if the branch - # is out of date when it is merged to the release branch (master/beta/stable-x.y). If this - # is considered important, then make sure the branch is up to date, and wait for test - # builds to complete, before merging. - get_changes "${GIT_BASE}" "${GIT_SHA}" - INSERTED="${CHANGES[0]}" - DELETED="${CHANGES[1]}" - MODIFIED="${CHANGES[2]}" - - if (( DELETED > 0 )) && (( MODIFIED == 0 )) && (( INSERTED == 0)); then - # If only lines got removed we basically skip this test. A later check will - # test that the API version got updated. - output_ok "${PKG_NAME}.symbols file did change but only removed lines" - elif ! git log -p -1 -- "debian/${PKG_NAME}.symbols" | grep -q "${VERSION}"; then - output_error "${PKG_NAME}.symbols file got changed without changing the package version number of the symbol" - EXIT_CODE=1 - else - output_ok "${PKG_NAME}.symbols file got updated with package version number" - fi - else - output_ok "${PKG_NAME}.symbols file didn't change" - fi -} - -get_api_version_in_symbols_file() { - # Retrieve symbols file at commit $1 and extract "1" from - # "libkeymancore.so.1 libkeymancore1 #MINVER#" - local firstline tmpfile - local sha="$1" - - tmpfile=$(mktemp) - if ! git cat-file blob "${sha}:linux/debian/${PKG_NAME}.symbols" > "${tmpfile}" 2>/dev/null; then - rm "${tmpfile}" - echo "-1" - return - fi - - firstline="$(head -1 "${tmpfile}")" - firstline="${firstline#"${LIB_NAME}".so.}" - firstline="${firstline%% *}" - - rm "${tmpfile}" - echo "${firstline}" -} - -get_api_version_from_core() { - # Retrieve CORE_API_VERSION.md from commit $1 and extract major version - # number ("1") from "1.0.0" - local api_version tmpfile - local sha="$1" - tmpfile=$(mktemp) - - if ! git cat-file blob "${sha}:core/CORE_API_VERSION.md" > "${tmpfile}" 2>/dev/null; then - rm "${tmpfile}" - echo "-1" - return - fi - - api_version=$(cat "${tmpfile}") - api_version=${api_version%%.*} - - rm "${tmpfile}" - echo "${api_version}" -} - -# Check if the API version got updated -# Returns: -# 0 - if the API version got updated -# 1 - the .symbols file got changed but the API version didn't get updated -# 2 - if we're in the alpha tier and the API version got updated since -# the last stable version -# NOTE: it is up to the caller to check if this is a major version -# change that requires an API version update. -# Check if the API version got updated -# Returns: -# 0 - if the API version got updated -# 1 - the .symbols file got changed but the API version didn't get updated -# 2 - if we're in the alpha tier and the API version got updated since -# the last stable version -# NOTE: it is up to the caller to check if this is a major version -# change that requires an API version update. -is_api_version_updated() { - local OLD_API_VERSION NEW_API_VERSION TIER - OLD_API_VERSION=$(get_api_version_in_symbols_file "${GIT_BASE}") - NEW_API_VERSION=$(get_api_version_in_symbols_file "${GIT_SHA}") - if (( NEW_API_VERSION > OLD_API_VERSION )); then - echo "0" - return - fi - - # API version didn't change. However, that might be ok if we're in alpha - # and a major change happened previously. - TIER=$(cat "${REPO_ROOT}/TIER.md") - case ${TIER} in - alpha) - local STABLE_VERSION STABLE_API_VERSION STABLE_BRANCH - STABLE_VERSION=$((${VERSION%%.*} - 1)) - STABLE_BRANCH="origin/stable-${STABLE_VERSION}.0" - STABLE_API_VERSION=$(get_api_version_in_symbols_file "${STABLE_BRANCH}") - if (( STABLE_API_VERSION == -1 )); then - # .symbols file doesn't exist in stable branch, so let's check CORE_API_VERSION.md. That - # doesn't exist in 16.0 but appeared in 17.0. - STABLE_API_VERSION=$(get_api_version_from_core "${STABLE_BRANCH}") - if (( STABLE_API_VERSION == -1 )); then - # CORE_API_VERSION.md doesn't exist either - if (( NEW_API_VERSION > 0 )); then - # .symbols and CORE_API_VERSION.md file don't exist in stable branch; however, we - # incremented the version number compared to 16.0, so that's ok - echo "2" - return - fi - fi - fi - if (( NEW_API_VERSION > STABLE_API_VERSION )); then - echo "2" - return - fi ;; - *) - ;; - esac - - echo "1" -} - -check_for_major_api_changes() { - # Checks that API version number gets updated if API changes - local WHAT_CHANGED CHANGES INSERTED DELETED MODIFIED UPDATED - - # shellcheck disable=2310 - if ! is_symbols_file_changed; then - output_ok "No major API change" - return - fi - - get_changes "${GIT_BASE}" "${GIT_SHA}" - INSERTED="${CHANGES[0]}" - DELETED="${CHANGES[1]}" - MODIFIED="${CHANGES[2]}" - - if (( DELETED > 0 )) || (( MODIFIED > 0 )); then - builder_echo "Major API change: ${DELETED} lines deleted and ${MODIFIED} lines modified" - UPDATED=$(is_api_version_updated) - if [[ ${UPDATED} == 1 ]]; then - output_error "Major API change without updating API version number in ${PKG_NAME}.symbols file" - EXIT_CODE=2 - elif [[ ${UPDATED} == 2 ]]; then - output_ok "API version number got previously updated in ${PKG_NAME}.symbols file after major API change; no change within alpha necessary" - else - output_ok "API version number got updated in ${PKG_NAME}.symbols file after major API change" - fi - elif (( INSERTED > 0 )); then - output_ok "Minor API change: ${INSERTED} lines added" - # We currently don't check version number for minor API changes - else - output_ok "No major API change" - fi -} - -check_for_api_version_consistency() { - # Checks that the (major) API version number in the .symbols file and - # in CORE_API_VERSION.md are the same - local symbols_version api_version - symbols_version=$(get_api_version_in_symbols_file "HEAD") - api_version=$(get_api_version_from_core "HEAD") - - if (( symbols_version == api_version )); then - output_ok "API version in .symbols file and in CORE_API_VERSION.md is the same" - else - output_error "API version in .symbols file and in CORE_API_VERSION.md is different" - EXIT_CODE=3 - fi -} - -verify_action() { - local SONAME - SONAME=$(get_api_version_from_core "HEAD") - LIB_NAME=libkeymancore - PKG_NAME="${LIB_NAME}${SONAME}" - if [[ ! -f debian/${PKG_NAME}.symbols ]]; then - output_error "Missing ${PKG_NAME}.symbols file" - exit 0 - fi - - EXIT_CODE=0 - check_api_not_changed - check_updated_version_number - check_for_major_api_changes - check_for_api_version_consistency - exit "${EXIT_CODE}" -} - builder_run_action dependencies dependencies_action builder_run_action source source_action -builder_run_action verify verify_action +builder_run_action verify verify_api_action diff --git a/linux/scripts/test/deb-packaging.tests.sh b/linux/scripts/test/deb-packaging.tests.sh index 04ff42b7115..134421e0a43 100755 --- a/linux/scripts/test/deb-packaging.tests.sh +++ b/linux/scripts/test/deb-packaging.tests.sh @@ -224,10 +224,10 @@ test_check_updated_version_number__LineRemoved_InAlpha_FileMissingInStable_ApiVe git checkout master # simulate a commit that renamed the .symbols file and updated the API version git mv linux/debian/libkeymancore1.symbols linux/debian/libfoo2.symbols - sed -i 's/libkeymancore/libfoo/' linux/scripts/deb-packaging.sh + sed -i 's/libkeymancore/libfoo/' linux/scripts/verify_api.inc.sh # shellcheck disable=2016 # single quotes are intentional here - sed -i 's/${SONAME}/2/' linux/scripts/deb-packaging.sh - git add linux/scripts/deb-packaging.sh + sed -i 's/${SONAME}/2/' linux/scripts/verify_api.inc.sh + git add linux/scripts/verify_api.inc.sh echo "2.0.0" > core/CORE_API_VERSION.md git add core/CORE_API_VERSION.md sed -i 's/libkeymancore1/libfoo2/' linux/debian/libfoo2.symbols @@ -251,8 +251,8 @@ test_check_updated_version_number__LineRemoved_InAlpha_FileMissingInStable_ApiVe git checkout master # simulate a commit that renamed the .symbols file git mv linux/debian/libkeymancore1.symbols linux/debian/libfoo1.symbols - sed -i 's/libkeymancore/libfoo/' linux/scripts/deb-packaging.sh - git add linux/scripts/deb-packaging.sh + sed -i 's/libkeymancore/libfoo/' linux/scripts/verify_api.inc.sh + git add linux/scripts/verify_api.inc.sh sed -i 's/libkeymancore/libfoo/' linux/debian/libfoo1.symbols git add linux/debian/libfoo1.symbols git commit -m "renamed library" @@ -344,8 +344,8 @@ test_check_updated_version_number__LineRemoved_InBeta_FileMissingInStable_ApiVer git checkout -b beta # simulate a commit that renamed the .symbols file git mv linux/debian/libkeymancore1.symbols linux/debian/libfoo1.symbols - sed -i 's/libkeymancore/libfoo/' linux/scripts/deb-packaging.sh - git add linux/scripts/deb-packaging.sh + sed -i 's/libkeymancore/libfoo/' linux/scripts/verify_api.inc.sh + git add linux/scripts/verify_api.inc.sh sed -i 's/libkeymancore/libfoo/' linux/debian/libfoo1.symbols git add linux/debian/libfoo1.symbols git commit -m "renamed library" diff --git a/linux/scripts/verify_api.inc.sh b/linux/scripts/verify_api.inc.sh new file mode 100644 index 00000000000..63072bc4f7c --- /dev/null +++ b/linux/scripts/verify_api.inc.sh @@ -0,0 +1,258 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2154 # (variables are set in build-utils.sh) + +output_log() { + echo "$1" >&2 + builder_echo "$1" +} + +output_ok() { + echo ":heavy_check_mark: $1" >&2 + builder_echo green "OK: $1" +} + +output_warning() { + echo ":warning: $1" >&2 + builder_echo warning "WARNING: $1" +} + +output_error() { + echo ":x: $1" >&2 + builder_echo error "ERROR: $1" +} + +check_api_not_changed() { + if [[ -z "${BIN_PKG:-}" ]]; then + output_warning "Skipping check for API change because binary Debian package not specified" + return + fi + # Checks that the API did not change compared to what's documented in the .symbols file + tmpDir=$(mktemp -d) + # shellcheck disable=SC2064 + trap "rm -rf \"${tmpDir}\"" ERR + dpkg -x "${BIN_PKG}" "${tmpDir}" + mkdir -p debian/tmp/DEBIAN + dpkg-gensymbols -v"${VERSION}" -p"${PKG_NAME}" -e"${tmpDir}"/usr/lib/x86_64-linux-gnu/"${LIB_NAME}".so* -c4 + output_ok "${LIB_NAME} API didn't change" + cd "${REPO_ROOT}/linux" + rm -rf "${tmpDir}" + trap ERR +} + +# +# Compare the SHA of the base and head commits for changes to the .symbols file +# +is_symbols_file_changed() { + local CHANGED_REF CHANGED_BASE + CHANGED_REF=$(git rev-parse "${GIT_SHA}":"linux/debian/${PKG_NAME}.symbols") + CHANGED_BASE=$(git rev-parse "${GIT_BASE}":"linux/debian/${PKG_NAME}.symbols") + if [[ "${CHANGED_REF}" == "${CHANGED_BASE}" ]]; then + return 1 + fi + return 0 +} + +get_changes() { + local WHAT_CHANGED + WHAT_CHANGED=$(git diff -I "^${LIB_NAME}.so" "$1".."$2" | diffstat -m -t | grep "${PKG_NAME}.symbols" ) + + IFS=',' read -r -a CHANGES <<< "${WHAT_CHANGED:-0,0,0}" +} + +check_updated_version_number() { + # Checks that the package version number got updated in the .symbols file if it got changed + # shellcheck disable=SC2310 + if is_symbols_file_changed; then + # .symbols file changed, now check if the package version got updated as well + # Note: We don't check that ALL changes in that file have an updated package version - + # we hope this gets flagged in code review. + # Note: This version number check may not match the actual released version, if the branch + # is out of date when it is merged to the release branch (master/beta/stable-x.y). If this + # is considered important, then make sure the branch is up to date, and wait for test + # builds to complete, before merging. + get_changes "${GIT_BASE}" "${GIT_SHA}" + INSERTED="${CHANGES[0]}" + DELETED="${CHANGES[1]}" + MODIFIED="${CHANGES[2]}" + + if (( DELETED > 0 )) && (( MODIFIED == 0 )) && (( INSERTED == 0)); then + # If only lines got removed we basically skip this test. A later check will + # test that the API version got updated. + output_ok "${PKG_NAME}.symbols file did change but only removed lines" + elif ! git log -p -1 -- "debian/${PKG_NAME}.symbols" | grep -q "${VERSION}"; then + output_error "${PKG_NAME}.symbols file got changed without changing the package version number of the symbol" + EXIT_CODE=1 + else + output_ok "${PKG_NAME}.symbols file got updated with package version number" + fi + else + output_ok "${PKG_NAME}.symbols file didn't change" + fi +} + +get_api_version_in_symbols_file() { + # Retrieve symbols file at commit $1 and extract "1" from + # "libkeymancore.so.1 libkeymancore1 #MINVER#" + local firstline tmpfile + local sha="$1" + + tmpfile=$(mktemp) + if ! git cat-file blob "${sha}:linux/debian/${PKG_NAME}.symbols" > "${tmpfile}" 2>/dev/null; then + rm "${tmpfile}" + echo "-1" + return + fi + + firstline="$(head -1 "${tmpfile}")" + firstline="${firstline#"${LIB_NAME}".so.}" + firstline="${firstline%% *}" + + rm "${tmpfile}" + echo "${firstline}" +} + +get_api_version_from_core() { + # Retrieve CORE_API_VERSION.md from commit $1 and extract major version + # number ("1") from "1.0.0" + local api_version tmpfile + local sha="$1" + tmpfile=$(mktemp) + + if ! git cat-file blob "${sha}:core/CORE_API_VERSION.md" > "${tmpfile}" 2>/dev/null; then + rm "${tmpfile}" + echo "-1" + return + fi + + api_version=$(cat "${tmpfile}") + api_version=${api_version%%.*} + + rm "${tmpfile}" + echo "${api_version}" +} + +# Check if the API version got updated +# Returns: +# 0 - if the API version got updated +# 1 - the .symbols file got changed but the API version didn't get updated +# 2 - if we're in the alpha tier and the API version got updated since +# the last stable version +# NOTE: it is up to the caller to check if this is a major version +# change that requires an API version update. +# Check if the API version got updated +# Returns: +# 0 - if the API version got updated +# 1 - the .symbols file got changed but the API version didn't get updated +# 2 - if we're in the alpha tier and the API version got updated since +# the last stable version +# NOTE: it is up to the caller to check if this is a major version +# change that requires an API version update. +is_api_version_updated() { + local OLD_API_VERSION NEW_API_VERSION TIER + OLD_API_VERSION=$(get_api_version_in_symbols_file "${GIT_BASE}") + NEW_API_VERSION=$(get_api_version_in_symbols_file "${GIT_SHA}") + if (( NEW_API_VERSION > OLD_API_VERSION )); then + echo "0" + return + fi + + # API version didn't change. However, that might be ok if we're in alpha + # and a major change happened previously. + TIER=$(cat "${REPO_ROOT}/TIER.md") + case ${TIER} in + alpha) + local STABLE_VERSION STABLE_API_VERSION STABLE_BRANCH + STABLE_VERSION=$((${VERSION%%.*} - 1)) + STABLE_BRANCH="origin/stable-${STABLE_VERSION}.0" + STABLE_API_VERSION=$(get_api_version_in_symbols_file "${STABLE_BRANCH}") + if (( STABLE_API_VERSION == -1 )); then + # .symbols file doesn't exist in stable branch, so let's check CORE_API_VERSION.md. That + # doesn't exist in 16.0 but appeared in 17.0. + STABLE_API_VERSION=$(get_api_version_from_core "${STABLE_BRANCH}") + if (( STABLE_API_VERSION == -1 )); then + # CORE_API_VERSION.md doesn't exist either + if (( NEW_API_VERSION > 0 )); then + # .symbols and CORE_API_VERSION.md file don't exist in stable branch; however, we + # incremented the version number compared to 16.0, so that's ok + echo "2" + return + fi + fi + fi + if (( NEW_API_VERSION > STABLE_API_VERSION )); then + echo "2" + return + fi ;; + *) + ;; + esac + + echo "1" +} + +check_for_major_api_changes() { + # Checks that API version number gets updated if API changes + local WHAT_CHANGED CHANGES INSERTED DELETED MODIFIED UPDATED + + # shellcheck disable=2310 + if ! is_symbols_file_changed; then + output_ok "No major API change" + return + fi + + get_changes "${GIT_BASE}" "${GIT_SHA}" + INSERTED="${CHANGES[0]}" + DELETED="${CHANGES[1]}" + MODIFIED="${CHANGES[2]}" + + if (( DELETED > 0 )) || (( MODIFIED > 0 )); then + builder_echo "Major API change: ${DELETED} lines deleted and ${MODIFIED} lines modified" + UPDATED=$(is_api_version_updated) + if [[ ${UPDATED} == 1 ]]; then + output_error "Major API change without updating API version number in ${PKG_NAME}.symbols file" + EXIT_CODE=2 + elif [[ ${UPDATED} == 2 ]]; then + output_ok "API version number got previously updated in ${PKG_NAME}.symbols file after major API change; no change within alpha necessary" + else + output_ok "API version number got updated in ${PKG_NAME}.symbols file after major API change" + fi + elif (( INSERTED > 0 )); then + output_ok "Minor API change: ${INSERTED} lines added" + # We currently don't check version number for minor API changes + else + output_ok "No major API change" + fi +} + +check_for_api_version_consistency() { + # Checks that the (major) API version number in the .symbols file and + # in CORE_API_VERSION.md are the same + local symbols_version api_version + symbols_version=$(get_api_version_in_symbols_file "HEAD") + api_version=$(get_api_version_from_core "HEAD") + + if (( symbols_version == api_version )); then + output_ok "API version in .symbols file and in CORE_API_VERSION.md is the same" + else + output_error "API version in .symbols file and in CORE_API_VERSION.md is different" + EXIT_CODE=3 + fi +} + +verify_api_action() { + local SONAME + SONAME=$(get_api_version_from_core "HEAD") + LIB_NAME=libkeymancore + PKG_NAME="${LIB_NAME}${SONAME}" + if [[ ! -f debian/${PKG_NAME}.symbols ]]; then + output_error "Missing ${PKG_NAME}.symbols file" + exit 0 + fi + + EXIT_CODE=0 + check_api_not_changed + check_updated_version_number + check_for_major_api_changes + check_for_api_version_consistency + exit "${EXIT_CODE}" +}