-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
enhance(scripts/lint-packages.sh): optimize handling of version arrays #27095
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.
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
Merged
TomJo2000
merged 4 commits into
termux:master
from
TomJo2000:linter-optimize-array-version-handling
Nov 11, 2025
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
02808ac
fix(scripts/lint-packages.sh): fix heredoc_regex for on-device use wi…
TomJo2000 42a73a5
enhance(scripts/lint-packages.sh): optimize handling of version arrays
TomJo2000 159772e
fix(scripts/lint-packages): attempt not to trip over merge commits
TomJo2000 07754ad
enhance(scripts/lint-packages): make the linter more talkative when s…
TomJo2000 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
|
|
||
| set -e -u | ||
|
|
||
| start_time="$(date +%10s.%3N)" | ||
|
|
||
| TERMUX_SCRIPTDIR=$(realpath "$(dirname "$0")/../") | ||
| . "$TERMUX_SCRIPTDIR/scripts/properties.sh" | ||
|
|
||
|
|
@@ -73,7 +75,7 @@ check_indentation() { | |
| local pkg_script="$1" | ||
| local line='' heredoc_terminator='' in_array=0 i=0 | ||
| local -a issues=('' '') bad_lines=('FAILED') | ||
| local heredoc_regex="[^\(/%#]<{2}-?\s*(['\"]?(\w*(\\\.)?)*['\"]?)" | ||
| local heredoc_regex="[^\(/%#]<{2}-?[[:space:]]*(['\"]?([[:alnum:]_]*(\\\.)?)*['\"]?)" | ||
| # We don't wanna hit version constraints "(<< x.y.z)" with this, so don't match "(<<". | ||
| # We also wouldn't wanna hit parameter expansions "${var/<<}", ${var%<<}, ${var#<<} | ||
|
|
||
|
|
@@ -94,7 +96,12 @@ check_indentation() { | |
| (( ${#heredoc_terminator} )) && continue | ||
| fi | ||
|
|
||
| # check for mixed indentation | ||
| # Check for mixed indentation. | ||
| # We do this after the heredoc checks because space indentation | ||
| # is significant for languages like Haskell or Nim. | ||
| # Those probably shouldn't get inlined as heredocs, | ||
| # but the Haskell `cabal.project.local` overrides currently are. | ||
| # So let's not break builds for that. | ||
| [[ "$line" =~ ^($'\t'+ +| +$'\t'+) ]] && { | ||
| issues[0]='Mixed indentation' | ||
| bad_lines[i]="${pkg_script}:${i}:$line" | ||
|
|
@@ -121,12 +128,19 @@ check_indentation() { | |
| return 0 | ||
| } | ||
|
|
||
| # We'll need the 'origin/master' as a base commit when running the version check. | ||
| # So try fetching it now if it doesn't exist. | ||
| if ! base_commit="HEAD~$(git rev-list --count FETCH_HEAD..)"; then | ||
| git fetch https://github.com/termux/termux-packages.git | ||
| base_commit="HEAD~$(git rev-list --count FETCH_HEAD..)" | ||
| fi | ||
| { | ||
| # We'll need the termux/termux-packages master@HEAD commit as a base commit when running the version check. | ||
| # So try fetching it now. | ||
| git fetch https://github.com/termux/termux-packages.git || { | ||
| echo "ERROR: Unable to fetch 'https://github.com/termux/termux-packages.git'" | ||
| echo "Falling back to HEAD~1" | ||
| } | ||
| base_commit="HEAD~$(git rev-list --count FETCH_HEAD.. -- || printf 1)" | ||
| } 2> /dev/null | ||
|
|
||
| # Also figure out if we have a `%ci:no-build` trailer in the commit range, | ||
| # we may skip some checks later if yes. | ||
| no_build="$(git log --fixed-strings --grep '%ci:no-build' --pretty=format:%H "$base_commit")" | ||
|
|
||
| check_version() { | ||
| local package_dir="${1%/*}" | ||
|
|
@@ -140,12 +154,11 @@ check_version() { | |
| } >&2 | ||
|
|
||
| # If TERMUX_PKG_VERSION is an array that changes the formatting. | ||
| local version i=-1 error=0 is_array="${TERMUX_PKG_VERSION@a}" | ||
| local version i=0 error=0 is_array="${TERMUX_PKG_VERSION@a}" | ||
| printf '%s' "${is_array:+$'ARRAY\n'}" | ||
|
|
||
| for version in "${TERMUX_PKG_VERSION[@]}"; do | ||
| printf '%s' "${is_array:+$'\t'}" | ||
| (( i++ )) | ||
|
|
||
| # Is this version valid? | ||
| dpkg --validate-version "${version}" &> /dev/null || { | ||
|
|
@@ -154,38 +167,49 @@ check_version() { | |
| continue | ||
| } | ||
|
|
||
| # Was the package modified in this branch? | ||
| git diff --exit-code "${base_commit}" -- "${package_dir}" &> /dev/null && { | ||
| printf '%s\n' "PASS - ${version} (not modified in this branch)" | ||
| continue | ||
| } | ||
|
|
||
| local version_new version_old | ||
| version_new="${version}-${TERMUX_PKG_REVISION:-0}" | ||
| version_old=$( | ||
| unset TERMUX_PKG_VERSION TERMUX_PKG_REVISION | ||
| # shellcheck source=/dev/null | ||
| . <(git -P show "${base_commit}:${package_dir}/build.sh" 2> /dev/null) | ||
| if [[ -n "$is_array" ]]; then | ||
| echo "${TERMUX_PKG_VERSION[$i]:-0}-${TERMUX_PKG_REVISION:-0}" | ||
| else | ||
| echo "${TERMUX_PKG_VERSION:-0}-${TERMUX_PKG_REVISION:-0}" | ||
| fi | ||
| # ${TERMUX_PKG_VERSION[0]} also works fine for non-array versions. | ||
| # Since those resolve in 1 iteration, no higher index is ever attempted to be called. | ||
| echo "${TERMUX_PKG_VERSION[$i]:-0}-${TERMUX_PKG_REVISION:-0}" | ||
| ) | ||
|
|
||
| # Is ${version_old} valid? | ||
| local version_old_is_bad="" | ||
| dpkg --validate-version "${version_old}" &> /dev/null || version_old_is_bad="0~invalid" | ||
|
|
||
| # The rest of the checks aren't useful past the first index when $TERMUX_PKG_VERSION is an array | ||
| # since that is the index that determines the actual version. | ||
| if (( i++ > 0 )); then | ||
| echo "PASS - ${version_old%-0}${version_old_is_bad:+" (INVALID)"} -> ${version_new%-0}" | ||
| continue | ||
| fi | ||
|
|
||
| # Was the package modified in this branch? | ||
| git diff --no-merges --exit-code "${base_commit}" -- "${package_dir}" &> /dev/null && { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing a note here as a reminder that unfortunately, However, it probably doesn't break anything either, so it's fine to leave like this. |
||
| printf '%s\n' "PASS - ${version_new%-0} (not modified in this branch)" | ||
| return 0 | ||
| } | ||
|
|
||
| [[ -n "$no_build" ]] && { | ||
| echo "SKIP - ${version_new%-0} ('%ci:no-build' trailer detected on commit ${no_build::7})" | ||
| return 0 | ||
| } | ||
|
|
||
| # If ${version_new} isn't greater than "$version_old" that's an issue. | ||
| # If ${version_old} isn't valid this check is a no-op. | ||
| if dpkg --compare-versions "$version_new" le "${version_old_is_bad:-$version_old}"; then | ||
| printf '%s\n' \ | ||
| "FAILED" \ | ||
| "FAILED ${version_old_is_bad:-$version_old} -> ${version_new}" \ | ||
| "" \ | ||
| "Version of '$package_name' has not been incremented." \ | ||
| "Either 'TERMUX_PKG_VERSION' or 'TERMUX_PKG_REVISION'" \ | ||
| "need to be modified in the build.sh when changing a package build." | ||
| "need to be modified in the build.sh when changing a package build." \ | ||
| "You can use ./scripts/bin/revbump '$package_name' to do this automatically." | ||
|
|
||
| # If the version decreased throw in a suggestion for how to downgrade packages | ||
| dpkg --compare-versions "$version_new" lt "$version_old" && \ | ||
|
|
@@ -217,9 +241,7 @@ check_version() { | |
| continue | ||
| # If that check passed the TERMUX_PKG_VERSION must have changed, | ||
| # in which case TERMUX_PKG_REVISION should be reset to 0. | ||
| # This check isn't useful past the first index when $TERMUX_PKG_VERSION is an array | ||
| # since the main version of such a package may remain unchanged when another is changed. | ||
| elif [[ "${version_new%-*}" != "${version_old%-*}" && "$new_revision" != "0" && "$i" == 0 ]]; then | ||
| elif [[ "${version_new%-*}" != "${version_old%-*}" && "$new_revision" != "0" ]]; then | ||
| (( error++ )) # Not reset | ||
| printf '%s\n' \ | ||
| "FAILED - $version_old -> $version_new" \ | ||
|
|
@@ -685,6 +707,18 @@ linter_main() { | |
| return | ||
| } | ||
|
|
||
| time_elapsed() { | ||
| local start="$1" end="$(date +%10s.%3N)" | ||
| local elapsed="$(( ${end/.} - ${start/.} ))" | ||
| echo "[INFO]: Finished linting build scripts ($(date -d "@$end" --utc '+%Y-%m-%dT%H:%M:%SZ' 2>&1))" | ||
| printf '[INFO]: Time elapsed: %s\n' \ | ||
| "$(sed 's/0m //;s/0s //' <<< "$(( elapsed % 3600000 / 60000 ))m$(( elapsed % 60000 / 1000 ))s$(( elapsed % 1000 ))ms")" | ||
| } | ||
|
|
||
| echo "[INFO]: Starting build script linter ($(date -d "@$start_time" --utc '+%Y-%m-%dT%H:%M:%SZ' 2>&1))" | ||
| echo "[INFO]: $base_commit ($(git rev-parse "$base_commit"))" | ||
| trap 'time_elapsed "$start_time"' EXIT | ||
|
|
||
| package_counter=0 | ||
| if (( $# )); then | ||
| linter_main "$@" | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added support for the
%ci:no-buildcommit trailer to the version check.Looks like this in the output:
TERMUX_PKG_VERSION: SKIP - 3.14.1.1-1 ('%ci:no-build' trailer detected on commit 42942d0)We can also consider a
%ci:no-linttrailer, which would probably warrant shuffling this up to the very top of the script.