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.
2026-08-05 00:15:32 +00:00 · 2024-02-02 20:15:22 +01:00 · 2024-02-02 20:15:22 +01:00 · 883b9fc4c8
commit 883b9fc4c8
parent 97bbcee3ef
4 changed files with 250 additions and 12 deletions
--- a/linux/.gitignore
+++ b/linux/.gitignore
@ -6,7 +6,7 @@ ltmain.sh
 __pycache__
 *.egg-info
 build-*
-deb-*
+./deb-*
 builddebs
 keyman-config/make_deb
 tmp/
--- 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
--- 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
--- a/linux/scripts/test/deb-packaging.tests.sh
+++ 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/<testname>.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