From 0b8458fc531e60ade5c2567c2c9ecc3cee2c6f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Desbiens?= Date: Sun, 9 Aug 2026 10:04:40 -0400 Subject: [PATCH] Added port consistency checks and wired them into CI and release preparation Three defects reached the repository through the port trees recently, and each of them is mechanically detectable without a cross compiler. Add scripts/check_ports.sh, which looks for exactly those three, and give CI and the release process the same command a contributor can run locally. The generated Cortex-M ports must be reproducible from ports_arch. Fixes were applied to the generated copies instead of the source for eight months, and the next run of the copy scripts would have reverted them. Preprocessor directives must balance. A fix left the Cortex-M85 IAR tx_port.h with one more #endif than #if, so that header could not compile. No port header may carry a statement outside a function body. A fix left a second, headerless copy of a function body in the Cortex-M4 AC6 tx_port.h, which is issue 569. The check tracks brace depth while skipping preprocessor lines, multi-line macro bodies and comments, and reports assignments, dereferences and control statements that land at file scope. Headers under example_build are excluded, since those trees vendor third party SDK code. A fourth section reports, without failing the run, on port families that have no copy script and so cannot be checked for reproducibility. It currently observes that the Cortex-M0 ac5, ac6 and keil ports lack the barriers their gnu and iar siblings have. ports_arch_check now calls the script rather than inlining a copy and diff, so CI and the command line check the same things by the same definition, and the workflow now triggers on pull requests to dev as well as master. Triggering on master alone is why the drift went unseen. prepare_release.sh runs the checks before it branches or rewrites anything, and stops if they fail, with SKIP_PORT_CHECKS=1 as the escape hatch. Each check was verified by reintroducing the defect it exists to catch and confirming that the script fails, then confirming it passes on a clean tree. Assisted-by: Claude Code (Opus 5) --- .github/workflows/ports_arch_check.yml | 44 ++--- scripts/check_ports.sh | 224 +++++++++++++++++++++++++ scripts/prepare_release.sh | 14 ++ 3 files changed, 262 insertions(+), 20 deletions(-) create mode 100755 scripts/check_ports.sh diff --git a/.github/workflows/ports_arch_check.yml b/.github/workflows/ports_arch_check.yml index 4a99c032d..a2c69aeaf 100644 --- a/.github/workflows/ports_arch_check.yml +++ b/.github/workflows/ports_arch_check.yml @@ -6,7 +6,10 @@ name: ports_arch_check # events but only for the master branch on: pull_request: - branches: [ master ] + # dev is included as well as master. The check only ever ran against master, + # so eight months of port fixes merged into dev without it, and the ports + # drifted from ports_arch unnoticed. + branches: [ master, dev ] paths: - ".github/workflows/ports_arch_check.yml" - 'common/**' @@ -27,21 +30,20 @@ jobs: # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - name: Checkout sources recursively - uses: actions/checkout@v2 - with: - token: ${{ secrets.REPO_SCOPED_TOKEN }} - submodules: true + # No token input: secrets are not available to pull requests from forks, so + # passing one made this job fail at checkout with "Input required and not + # supplied: token" and the check never evaluated anything. The default + # GITHUB_TOKEN is enough to check out a public repository, and the + # repository has no submodules. + - name: Checkout sources + uses: actions/checkout@v4 - # Copy ports arch - - name: Copy ports arch - run: | - scripts/copy_armv7_m.sh && scripts/copy_armv8_m.sh && scripts/copy_module_armv7_m.sh - if [[ -n $(git status --porcelain -uno) ]]; then - echo "Ports for ARM architecture is not updated" - git status - exit 1 - fi + # Check the port trees: the generated ports must be reproducible from + # ports_arch, and no port header may be left unbalanced or carrying code + # outside a function. The same script runs locally, so a contributor sees + # exactly what CI sees. + - name: Check ports + run: scripts/check_ports.sh cortex-a: # Check ports for cortex-a @@ -50,11 +52,13 @@ jobs: # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - name: Checkout sources recursively - uses: actions/checkout@v2 - with: - token: ${{ secrets.REPO_SCOPED_TOKEN }} - submodules: true + # No token input: secrets are not available to pull requests from forks, so + # passing one made this job fail at checkout with "Input required and not + # supplied: token" and the check never evaluated anything. The default + # GITHUB_TOKEN is enough to check out a public repository, and the + # repository has no submodules. + - name: Checkout sources + uses: actions/checkout@v4 # Copy ports arch - name: Copy ports arch diff --git a/scripts/check_ports.sh b/scripts/check_ports.sh new file mode 100755 index 000000000..5add21b79 --- /dev/null +++ b/scripts/check_ports.sh @@ -0,0 +1,224 @@ +#!/bin/bash +############################################################################## +# Copyright (C) 2026 Eclipse ThreadX contributors +# +# This program and the accompanying materials are made available under the +# terms of the MIT License which is available at +# https://opensource.org/licenses/MIT. +# +# AI Disclosure: This file was largely AI-generated by Claude Code (Opus 5). +# The AI-generated portions may be considered public domain (CC0-1.0) +# and not subject to the project's licence. The human contributor has +# reviewed and verified that the code is correct. +# +# SPDX-License-Identifier: MIT and CC0-1.0 +############################################################################## + +# Consistency checks for the port trees. Run it before cutting a release, or +# any time the ports have been touched: +# +# scripts/check_ports.sh +# +# Options: +# --no-regen Skip the reproducibility check, which needs a clean tree. +# --quiet Print only failures and the summary. +# +# Exit status is 0 when every check passes and 1 otherwise, so the same +# command serves CI and the command line. +# +# Each check exists because a real defect reached the repository through it: +# +# 1. Reproducibility. The Cortex-M ports are generated by the copy scripts, +# but fixes were applied to the generated copies instead of the source for +# eight months. The next run of those scripts would have reverted them. +# +# 2. Preprocessor balance. A fix left ports/cortex_m85/iar/inc/tx_port.h with +# one more #endif than #if, so that header could not compile. +# +# 3. Code at file scope. A fix left a second, headerless copy of a function +# body in ports/cortex_m4/ac6/inc/tx_port.h, which placed statements +# outside any function. See issue 569. +# +# The last section reports, without failing, on port families that have no copy +# script and so cannot be checked for reproducibility. +# +# Headers under example_build are skipped: those trees vendor third party SDK +# code, which is not ours to hold to these rules. + +set -u + +cd "$(dirname "$(realpath "$0")")/.." + +regen=1 +quiet=0 +for arg in "$@"; do + case "$arg" in + --no-regen) regen=0 ;; + --quiet) quiet=1 ;; + -h|--help) sed -n '17,30p' "$0"; exit 0 ;; + *) echo "Unknown option: $arg" >&2; exit 2 ;; + esac +done + +failures=0 + +say() { [ "$quiet" -eq 1 ] || echo "$@"; } +fail() { echo " FAIL: $*"; failures=$((failures + 1)); } + +# -------------------------------------------------------------------------- +# 1. The generated ports must be reproducible from ports_arch. +# -------------------------------------------------------------------------- +say "" +say "== Generated ports are reproducible from ports_arch ==" + +if [ "$regen" -eq 0 ]; then + say " skipped (--no-regen)" +elif ! command -v git >/dev/null 2>&1; then + say " skipped (git not available)" +elif [ -n "$(git status --porcelain -uno)" ]; then + fail "the working tree has uncommitted changes; commit or stash them, or pass --no-regen" +else + ./scripts/copy_armv7_m.sh >/dev/null 2>&1 + ./scripts/copy_armv8_m.sh >/dev/null 2>&1 + ./scripts/copy_module_armv7_m.sh >/dev/null 2>&1 + + drift="$(git status --porcelain -uno)" + if [ -n "$drift" ]; then + fail "running the copy scripts changed $(echo "$drift" | wc -l) file(s)." + echo " The ports below were edited directly instead of through ports_arch." + echo " Re-apply the change to ports_arch and re-run the copy scripts." + echo "$drift" | sed 's/^/ /' + git checkout -- . >/dev/null 2>&1 + else + say " ok: the copy scripts change nothing" + fi +fi + +# -------------------------------------------------------------------------- +# 2. Preprocessor directives must balance in every port header. +# -------------------------------------------------------------------------- +say "" +say "== Preprocessor directives balance ==" + +unbalanced=0 +while IFS= read -r f; do + result="$(awk ' + /^[ \t]*#[ \t]*(if|ifdef|ifndef)/ { depth++ } + /^[ \t]*#[ \t]*endif/ { + depth-- + if (depth < 0 && first == 0) { first = NR } + } + END { printf "%d %d", depth, first } + ' "$f")" + depth="${result% *}" + first="${result#* }" + if [ "$depth" -ne 0 ]; then + if [ "$first" -ne 0 ]; then + fail "$f: #endif without a matching #if at line $first (final depth $depth)" + else + fail "$f: $depth unterminated #if (final depth $depth)" + fi + unbalanced=$((unbalanced + 1)) + fi +done < <(find ports ports_arch ports_module ports_smp -name "*.h" -type f \ + ! -path "*/example_build/*" 2>/dev/null | sort) + +[ "$unbalanced" -eq 0 ] && say " ok: every port header balances" + +# -------------------------------------------------------------------------- +# 3. No statements outside a function body in a port header. +# -------------------------------------------------------------------------- +# Tracks brace depth, ignoring preprocessor lines, comments and strings, and +# reports any statement that lands at depth zero. Declarations, typedefs, +# externs and macro definitions are expected there; assignments, calls and +# dereferences are not, and an orphaned function body shows up as exactly that. +say "" +say "== No code at file scope in port headers ==" + +orphans=0 +while IFS= read -r f; do + result="$(awk ' + # Skip preprocessor lines, including multi-line macro bodies, whose + # continuations are statements by design. + /^[ \t]*#/ { if (/\\[ \t]*$/) { in_macro = 1 }; next } + in_macro { if (!/\\[ \t]*$/) { in_macro = 0 }; next } + + # Skip comments, both kinds, including continuation lines. + in_comment { if (/\*\//) { in_comment = 0 }; next } + /^[ \t]*\/\// { next } + /\/\*/ { if (!/\*\//) { in_comment = 1 }; next } + + { + line = $0 + gsub(/"[^"]*"/, "", line) + gsub(/\/\/.*$/, "", line) + stripped = line + gsub(/^[ \t]+|[ \t]+$/, "", stripped) + + # A statement at file scope ends in a semicolon and either assigns, + # dereferences or opens a control structure. Declarations and + # prototypes also end in a semicolon but match none of these. + if (depth == 0 && stripped ~ /;[ \t]*$/ && + (stripped ~ /^\*\(/ || + stripped ~ /^[A-Za-z_][A-Za-z0-9_]*([ \t]*(\[[^]]*\]|->[ \t]*[A-Za-z_][A-Za-z0-9_]*|\.[A-Za-z_][A-Za-z0-9_]*))*[ \t]*=[^=]/ || + stripped ~ /^(return|if|while|for|switch|do)[ \t(]/)) { + print NR ": " stripped + found++ + } + + n = gsub(/{/, "{", line); m = gsub(/}/, "}", line) + depth += n - m + if (depth < 0) { depth = 0 } + } + END { exit (found > 0 ? 1 : 0) } + ' "$f")" + if [ -n "$result" ]; then + fail "$f: statement(s) outside any function body" + echo "$result" | sed 's/^/ /' + orphans=$((orphans + 1)) + fi +done < <(find ports ports_arch ports_module ports_smp -name "*.h" -type f \ + ! -path "*/example_build/*" 2>/dev/null | sort) + +[ "$orphans" -eq 0 ] && say " ok: no port header carries code at file scope" + +# -------------------------------------------------------------------------- +# 4. Report only: families with no copy script. +# -------------------------------------------------------------------------- +# These are maintained by hand, so a fix applied to one toolchain can silently +# miss the others. Nothing here fails the run; it is a prompt to look. +say "" +say "== Families with no copy script (report only) ==" + +if [ "$quiet" -eq 0 ]; then + for family in ports/cortex_m0 ports/cortex_m0+ ports/cortex_m23; do + [ -d "$family" ] || continue + for probe in "dsb 0xF" "isb 0xF"; do + have=""; missing="" + for header in "$family"/*/inc/tx_port.h; do + [ -f "$header" ] || continue + grep -q "0xE000ED04" "$header" || continue + tool="$(basename "$(dirname "$(dirname "$header")")")" + if grep -q "$probe" "$header"; then + have="$have $tool" + else + missing="$missing $tool" + fi + done + if [ -n "$have" ] && [ -n "$missing" ]; then + echo " $family: \"$probe\" present in$have but absent in$missing" + fi + done + done + say " (nothing above means the toolchains within each family agree)" +fi + +# -------------------------------------------------------------------------- +say "" +if [ "$failures" -eq 0 ]; then + say "All port consistency checks passed." + exit 0 +fi + +echo "$failures port consistency check(s) failed." +exit 1 diff --git a/scripts/prepare_release.sh b/scripts/prepare_release.sh index c33fa8f8d..c6b1d93c4 100755 --- a/scripts/prepare_release.sh +++ b/scripts/prepare_release.sh @@ -78,6 +78,20 @@ printf "\nThreadX release preparation\n" printf " Repository : %s\n" "${REPO_ROOT}" printf " Current version : %s\n" "${CURR_VER}" printf " Target version : %s\n\n" "${VERSION}" + +# -------------------------------------------------------------------------- +# Port consistency checks +# -------------------------------------------------------------------------- +# Run before anything is branched or rewritten, so a release is never cut on +# top of ports that have drifted from ports_arch or headers that cannot +# compile. Set SKIP_PORT_CHECKS=1 to proceed anyway. +if [ "${SKIP_PORT_CHECKS:-0}" = "1" ]; then + printf "Skipping the port consistency checks (SKIP_PORT_CHECKS=1).\n\n" +elif ! "${SCRIPT_DIR}/check_ports.sh"; then + printf "\nRelease preparation stopped: the port consistency checks failed.\n" + printf "Fix the problems above, or set SKIP_PORT_CHECKS=1 to proceed anyway.\n" + exit 1 +fi printf "Proceed with update? [y/N] " read -r CONFIRM case "${CONFIRM}" in