From 883b9fc4c83bbace73492a0f12b0ef7683879d37 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 2 Feb 2024 20:15:22 +0100 Subject: [PATCH] chore(linux): Fix API check if lines got removed in .symbols file If lines got removed in the .symbols file we won't find the current version number in the .symbols file. This change fixes the expectations. Also, if a major API change happens during the Alpha cycle we don't expect another API version increment if one already happened since we released the stable version. This change also adds some unit tests for the new functionality. These can be run manually and won't run during CI builds. Fixes #10453. --- linux/.gitignore | 2 +- linux/scripts/.editorconfig | 2 +- linux/scripts/deb-packaging.sh | 56 ++++-- linux/scripts/test/deb-packaging.tests.sh | 202 ++++++++++++++++++++++ 4 files changed, 250 insertions(+), 12 deletions(-) create mode 100755 linux/scripts/test/deb-packaging.tests.sh diff --git a/linux/.gitignore b/linux/.gitignore index 625bdd136c..f85acc0f35 100644 --- a/linux/.gitignore +++ b/linux/.gitignore @@ -6,7 +6,7 @@ ltmain.sh __pycache__ *.egg-info build-* -deb-* +./deb-* builddebs keyman-config/make_deb tmp/ diff --git a/linux/scripts/.editorconfig b/linux/scripts/.editorconfig index 020438e0c6..52ca11513e 100644 --- a/linux/scripts/.editorconfig +++ b/linux/scripts/.editorconfig @@ -1,4 +1,4 @@ # Editor configuration, see https://editorconfig.org [*] indent_style = space -indent_size = 4 +indent_size = 2 diff --git a/linux/scripts/deb-packaging.sh b/linux/scripts/deb-packaging.sh index d9c9f30d1b..1baf17ff85 100755 --- a/linux/scripts/deb-packaging.sh +++ b/linux/scripts/deb-packaging.sh @@ -103,6 +103,13 @@ is_symbols_file_changed() { return 0 } +get_changes() { + local WHAT_CHANGED + WHAT_CHANGED=$(git diff -I "^${PKG_NAME}.so" "$1".."$2" -- "debian/${PKG_NAME}.symbols" | diffstat -m -t | tail -n +2) + + 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 @@ -114,7 +121,16 @@ check_updated_version_number() { # 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. - if ! git log -p -1 -- "debian/${PKG_NAME}.symbols" | grep -q "${VERSION}"; then + 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 @@ -135,20 +151,40 @@ get_api_version_in_symbols_file() { } is_api_version_updated() { - local NEW_VERSION OLD_VERSION + local NEW_VERSION OLD_VERSION TIER git checkout "${GIT_BASE}" -- "debian/${PKG_NAME}.symbols" OLD_VERSION=$(get_api_version_in_symbols_file) git checkout "${GIT_SHA}" -- "debian/${PKG_NAME}.symbols" NEW_VERSION=$(get_api_version_in_symbols_file) if (( NEW_VERSION > OLD_VERSION )); then - return 0 + echo "0" + return fi - return 1 + + # 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_VERSION=$((${VERSION%%.*} - 1)) + git checkout "stable-${STABLE_VERSION}.0" -- "debian/${PKG_NAME}.symbols" + STABLE_API_VERSION=$(get_api_version_in_symbols_file) + git checkout "${GIT_SHA}" -- "debian/${PKG_NAME}.symbols" + if (( NEW_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 + local WHAT_CHANGED CHANGES INSERTED DELETED MODIFIED UPDATED # shellcheck disable=2310 if ! is_symbols_file_changed; then @@ -156,19 +192,19 @@ check_for_major_api_changes() { return fi - WHAT_CHANGED=$(git diff "${GIT_BASE}".."${GIT_SHA}" -- "debian/${PKG_NAME}.symbols" | diffstat -m -t | tail -1) - - IFS=',' read -r -a CHANGES <<< "${WHAT_CHANGED}" + 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" - # shellcheck disable=2310 - if ! is_api_version_updated; then + 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 diff --git a/linux/scripts/test/deb-packaging.tests.sh b/linux/scripts/test/deb-packaging.tests.sh new file mode 100755 index 0000000000..9fb1349886 --- /dev/null +++ b/linux/scripts/test/deb-packaging.tests.sh @@ -0,0 +1,202 @@ +#!/bin/bash +set -eu + +## START STANDARD BUILD SCRIPT INCLUDE +# adjust relative paths as necessary +THIS_SCRIPT="$(readlink -f "${BASH_SOURCE[0]}")" +. "${THIS_SCRIPT%/*}/../../../resources/build/build-utils.sh" +## END STANDARD BUILD SCRIPT INCLUDE + +mockDebPkgTools() { + echo "#!/bin/bash + " > ${tmpDir}/dpkg + chmod +x ${tmpDir}/dpkg + cp ${tmpDir}/dpkg ${tmpDir}/dpkg-gensymbols + PATH=${tmpDir}:${PATH} +} + +createBase() { + TIER=$1 + workDir=$(mktemp -d) + cd ${workDir} + git init . + mkdir -p linux/debian + echo "libkeymancore.so.1 libkeymancore #MINVER# +* Build-Depends-Package: libkeymancore-dev + + km_core_actions_dispose@Base 17.0.197 + km_core_context_clear@Base 17.0.195 + km_core_context_get@Base 17.0.195 + km_core_context_item_list_size@Base 17.0.195 + km_core_context_items_dispose@Base 17.0.195 +" > linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + + mkdir -p core + echo "1.0.0" > core/CORE_API_VERSION.md + git add core/CORE_API_VERSION.md + + echo "16.0.145" > VERSION.md + git add VERSION.md + + echo "stable" > TIER.md + git add TIER.md + + mkdir -p linux/scripts + cp -r ${REPO_ROOT}/linux/scripts/* linux/scripts + git add linux/scripts + + mkdir -p resources/build + cp -r ${REPO_ROOT}/resources/build/* resources/build + cp ${REPO_ROOT}/resources/builder.inc.sh resources/ + git add resources + + git commit -m "Initial" + + git branch -c stable-16.0 + git tag -m "16.0.145" release-16.0.145 + + echo "${TIER}" > TIER.md + git add TIER.md + + echo "17.0.255" > VERSION.md + git add VERSION.md + git commit -m "Commit for Alpha" + + git checkout -b chore + + BINPKG_NAME=${tmpDir}/libkeymancore_17.0.257-1+noble1_amd64.deb + touch "${BINPKG_NAME}" +} + +setup() { + OLDPATH=${PATH} + tmpDir=$(mktemp -d) + mockDebPkgTools +} + +teardown() { + PATH=${OLDPATH} + rm -rf ${tmpDir} +} + +run_test() { + setup + $1 > /tmp/$1.log 2>&1 && echo -e "${COLOR_GREEN}$1: OK${COLOR_RESET}" || echo -e "${COLOR_RED}$1: FAILED${COLOR_RESET}" + teardown +} + +test_check_updated_version_number_NoChange_OK() { + createBase alpha + + echo "readme" > README.md + git add README.md + git commit -m "Some change on the branch" + + pwd + linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify +} + +test_check_updated_version_number_LineAdded_OK() { + createBase alpha + + sed -i 's/ km_core_actions_dispose@Base 17.0.197/ km_core_actions_dispose@Base 17.0.197\n km_core_added@Base 17.0.255/' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method added" + + pwd + linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify +} + +test_check_updated_version_number_LineAddedWithoutVerUpd_ERROR() { + createBase alpha + + sed -i 's/ km_core_actions_dispose@Base 17.0.197/ km_core_actions_dispose@Base 17.0.197\n km_core_added@Base 17.0.197/' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method added" + + pwd + output=$(linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify || true) + echo "${output[*]}" # for logging + [[ "${output[*]}" == *"ERROR: libkeymancore.symbols file got changed without changing the package version number of the symbol"* ]] +} + +test_check_updated_version_number_LineRemovedWithAPIUpd_OK() { + createBase alpha + echo "2.0.0" > core/CORE_API_VERSION.md + git add core/CORE_API_VERSION.md + sed -i 's/libkeymancore.so.1/libkeymancore.so.2/' linux/debian/libkeymancore.symbols + sed -i '6d' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method removed" + + pwd + linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify +} + +test_check_updated_version_number_LineRemoved_InAlpha_OK() { + createBase alpha + git checkout master + # simulate a commit that already introduced an API version change + echo "2.0.0" > core/CORE_API_VERSION.md + git add core/CORE_API_VERSION.md + sed -i 's/libkeymancore.so.1/libkeymancore.so.2/' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API version change" + git checkout chore + git rebase master + + sed -i '6d' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method removed" + + pwd + linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify +} + +test_check_updated_version_number_LineRemoved_ChangeFromStable_ERROR() { + createBase alpha + + sed -i '6d' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method removed" + + pwd + output=$(linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base master verify || true) + echo "${output[*]}" # for logging + [[ "${output[*]}" == *" ERROR: Major API change without updating API version number in libkeymancore.symbols file"* ]] +} + +test_check_updated_version_number_LineRemoved_ChangeInBeta_ERROR() { + createBase beta + + # simulate a commit that already introduced an API version change in Beta + git checkout -b beta + echo "2.0.0" > core/CORE_API_VERSION.md + git add core/CORE_API_VERSION.md + sed -i 's/libkeymancore.so.1/libkeymancore.so.2/' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API version change" + git checkout chore + git rebase beta + + sed -i '6d' linux/debian/libkeymancore.symbols + git add linux/debian/libkeymancore.symbols + git commit -m "API method removed" + + pwd + output=$(linux/scripts/deb-packaging.sh --bin-pkg "${BINPKG_NAME}" --git-sha "$(git rev-parse HEAD)" --git-base beta verify || true) + echo "${output[*]}" # for logging + [[ "${output[*]}" == *" ERROR: Major API change without updating API version number in libkeymancore.symbols file"* ]] +} + +echo "(test logs are in /tmp/.log)" +run_test test_check_updated_version_number_NoChange_OK +run_test test_check_updated_version_number_LineAdded_OK +run_test test_check_updated_version_number_LineAddedWithoutVerUpd_ERROR +run_test test_check_updated_version_number_LineRemovedWithAPIUpd_OK +run_test test_check_updated_version_number_LineRemoved_InAlpha_OK +run_test test_check_updated_version_number_LineRemoved_ChangeFromStable_ERROR +run_test test_check_updated_version_number_LineRemoved_ChangeInBeta_ERROR + +# TODO: still some test cases missing for the different checks