mirror of
https://github.com/rsyslog/rsyslog.git
synced 2026-06-15 23:22:50 +02:00
devtools: add read-only C format check
Why: Agents often skip the mutating format step because it is not part of the regular deterministic local validation path. That lets formatting issues escape to review even when the local check helper is otherwise used. Impact: Changed C and header files are now checked by the local validation helper without rewriting files. Missing clang-format remains a warning, not a local workflow blocker. Before/After: Before, the local helper called the mutating formatter. After, it runs a clang-format dry-run check and keeps explicit formatting as a separate action. Technical Overview: Add --check and --check-if-available modes to devtools/format-code.sh. Use clang-format's dry-run and Werror mode for read-only validation. Extend the changed C/H file helper to include committed branch changes, staged and unstaged edits, and untracked C/H files. Pass the validation planner base ref into the format helper so both scripts classify the same delta. Document the workflow in AGENTS.md and the local validation/commit skills. Validation: - shellcheck devtools/format-code.sh devtools/list-git-changed-c-h-files.sh devtools/local-validation-plan.sh - checkbashisms -p devtools/local-validation-plan.sh - bash -n devtools/format-code.sh devtools/list-git-changed-c-h-files.sh - sh -n devtools/local-validation-plan.sh - devtools/local-validation-plan.sh --run - devtools/format-code.sh --git-changed --check --check-if-available - untracked C probe with devtools/list-git-changed-c-h-files.sh - untracked C probe with devtools/format-code.sh --git-changed --check --check-if-available - cubic review --print-logs --base origin/main With the help of AI-Agents: OpenAI Codex
This commit is contained in:
parent
8feb4dd635
commit
1e3bef2f5b
@ -9,14 +9,23 @@ This skill standardizes the final step of the development workflow: committing a
|
||||
|
||||
## Quick Start
|
||||
|
||||
1. **Format First**: Run `devtools/format-code.sh`.
|
||||
1. **Format First**: For C/H changes, agents MUST run
|
||||
`devtools/format-code.sh --git-changed` to rewrite formatting when needed.
|
||||
2. **Commit Message**: Follow the 62/72 rule and the mandatory "Why" structure.
|
||||
3. **Attribution**: Include the AI-Agent footer.
|
||||
|
||||
## Detailed Instructions
|
||||
|
||||
### 1. Pre-Commit Checklist
|
||||
- **Code Style**: Run `bash devtools/format-code.sh` IF any `.c` or `.h` files were modified. This is mandatory for C source changes.
|
||||
- **Code Style**: Agents MUST run `devtools/format-code.sh --git-changed` if
|
||||
any `.c` or `.h` files were modified and formatting may need to be applied.
|
||||
This is mandatory for C source changes before commit. For a read-only local
|
||||
validation gate, agents MUST run
|
||||
`devtools/format-code.sh --git-changed --check --check-if-available`; this
|
||||
skips with a warning if the exact configured `clang-format` executable is not
|
||||
installed. CI will not pass with improperly formatted C/H code; a missing
|
||||
local formatter is only a local tooling limitation, not permission to skip
|
||||
formatting.
|
||||
- **Python Style**: If Python files changed and `pycodestyle` is installed, run `devtools/format-python.sh <changed-python-files>`. Use `devtools/format-python.sh --fix <changed-python-files>` only when you intentionally want `autopep8` rewrites. If the tools are missing, suggest installing them (`sudo apt-get install -y pycodestyle python3-autopep8` on Debian/Ubuntu) but do not block unrelated build or test validation. The shared 120-column style configuration lives in `setup.cfg`; review `autopep8` output carefully for legacy Python-2-style scripts.
|
||||
- **Local Preflight Linters**: Before heavier validation or opening/updating a
|
||||
PR, run useful diff-scoped linters when installed: `shellcheck` for changed
|
||||
@ -46,7 +55,7 @@ This skill standardizes the final step of the development workflow: committing a
|
||||
local container validation unless that skill explicitly permits the reduced
|
||||
lane.
|
||||
- **Mock Smoke Check**: If you added or renamed test files, run `make distcheck TEST_RUN_TYPE=MOCK-OK -j$(nproc)` as a final distribution check.
|
||||
- **Note**: If you already successfully built and tested your changes immediately *before* formatting, you do NOT need to re-run the build/test cycle. Formatting is a normalization step and does not affect functionality.
|
||||
- **Note**: If you already successfully built and tested your changes immediately *before* formatting, you do NOT need to re-run the build/test cycle. Formatting is a normalization step and does not affect functionality. A read-only formatting check is already part of the deterministic local validation helper for changed C/H files.
|
||||
|
||||
### 2. Commit Message Structure
|
||||
Rsyslog requires rich, structured commit messages (plain ASCII).
|
||||
|
||||
@ -70,7 +70,13 @@ tool-presence guarded:
|
||||
- changed infrastructure/config files: `trivy config` on the changed paths or
|
||||
smallest relevant directory
|
||||
- larger source/test PRs: `jscpd` as an advisory duplication check
|
||||
- changed C sources or headers: `devtools/format-code.sh`
|
||||
- changed C sources or headers MUST run the exact read-only check:
|
||||
`devtools/format-code.sh --git-changed --check --check-if-available`. This
|
||||
is a read-only `clang-format-18` dry-run gate for deterministic validation;
|
||||
if the exact formatter is unavailable, it warns and leaves hosted CI or a
|
||||
fuller local environment to cover the gap. CI will not pass with improperly
|
||||
formatted C/H code. Use `devtools/format-code.sh --git-changed` separately
|
||||
when you intentionally want to rewrite local files.
|
||||
|
||||
Do not run `cppcheck` routinely unless a maintainer explicitly asks for it; it
|
||||
is too noisy for routine rsyslog PR validation.
|
||||
|
||||
18
AGENTS.md
18
AGENTS.md
@ -192,6 +192,24 @@ Each major subtree contains a specialized `AGENTS.md` that points to area-specif
|
||||
changes that touch print statements, exception syntax, imports, or line
|
||||
continuations.
|
||||
|
||||
## C Formatting Validation
|
||||
|
||||
- C style is governed by `.clang-format` and the exact formatter configured in
|
||||
`devtools/format-code.sh` (`clang-format-18` at the time of writing).
|
||||
- For C or header edits, agents MUST run
|
||||
`devtools/format-code.sh --git-changed` before committing when local files
|
||||
may need formatting.
|
||||
- For deterministic local validation, agents MUST use the read-only gate:
|
||||
`devtools/format-code.sh --git-changed --check --check-if-available`.
|
||||
This runs `clang-format` in dry-run mode and fails if changed C/H files would
|
||||
be rewritten. If the exact formatter is missing, it warns and leaves hosted CI
|
||||
or a fuller local environment to cover the gap.
|
||||
- CI will not pass with improperly formatted C/H code. A missing local
|
||||
formatter is only a local tooling limitation, not permission to skip
|
||||
formatting.
|
||||
- `devtools/local-validation-plan.sh --run` executes this read-only C format
|
||||
check automatically for changed C/H files before heavier validation.
|
||||
|
||||
## Local Preflight Linters
|
||||
|
||||
CodeFactor and CI provide centralized lint feedback, but agents SHOULD run
|
||||
|
||||
@ -7,11 +7,14 @@
|
||||
# It's intended to enforce the canonical code format for the repository.
|
||||
#
|
||||
# Usage:
|
||||
# ./devtools/format-code.sh [-h] [--git-changed]
|
||||
# ./devtools/format-code.sh [-h] [--git-changed] [--check]
|
||||
# [--check-if-available]
|
||||
#
|
||||
# Options:
|
||||
# -h, --help Display this help message and exit.
|
||||
# --git-changed Format only changed tracked .c/.h files known to Git.
|
||||
# -h, --help Display this help message and exit.
|
||||
# --git-changed Format only changed .c/.h files known to Git.
|
||||
# --check Check formatting without modifying files.
|
||||
# --check-if-available If clang-format is missing, warn and exit 0.
|
||||
#
|
||||
# Description:
|
||||
# This script performs an in-place reformatting of all C source (.c) and
|
||||
@ -31,8 +34,14 @@
|
||||
# parentheses `\( ... \)` to ensure that `clang-format` is executed for
|
||||
# both `.c` and `.h` files as intended.
|
||||
#
|
||||
# With '--git-changed', the script limits formatting to changed tracked
|
||||
# .c/.h files reported by Git. If no such files exist, it exits 0.
|
||||
# With '--git-changed', the script limits formatting to changed .c/.h files
|
||||
# reported by Git. This includes committed branch changes relative to
|
||||
# RSYSLOG_LOCAL_VALIDATION_BASE or origin/main, staged and unstaged tracked
|
||||
# changes, and untracked files. If no such files exist, it exits 0.
|
||||
#
|
||||
# With '--check', the script runs clang-format in dry-run mode and fails if
|
||||
# any target file would be reformatted. This mode is intended for
|
||||
# deterministic local validation gates.
|
||||
#
|
||||
# Before running, ensure 'clang-format-18' is installed on your system.
|
||||
# It is highly recommended to commit your current changes or create a backup
|
||||
@ -51,6 +60,8 @@ set -euo pipefail # Exit on error, unset variables, and pipefail
|
||||
# --- Configuration ---
|
||||
readonly CLANG_FORMAT="clang-format-18" # Specify the clang-format version to use
|
||||
git_changed_only=0
|
||||
check_only=0
|
||||
check_if_available=0
|
||||
|
||||
# --- Functions ---
|
||||
|
||||
@ -69,6 +80,12 @@ while [[ $# -gt 0 ]]; do
|
||||
--git-changed)
|
||||
git_changed_only=1
|
||||
;;
|
||||
--check)
|
||||
check_only=1
|
||||
;;
|
||||
--check-if-available)
|
||||
check_if_available=1
|
||||
;;
|
||||
*)
|
||||
echo "Error: Unknown option '$1'" >&2
|
||||
show_help
|
||||
@ -81,6 +98,11 @@ done
|
||||
|
||||
# Check if clang-format is installed and executable
|
||||
if ! command -v "$CLANG_FORMAT" &> /dev/null; then
|
||||
if [[ "$check_if_available" -eq 1 ]]; then
|
||||
echo "Warning: '$CLANG_FORMAT' command not found; skipping C format check." >&2
|
||||
echo "Install it on Ubuntu with: sudo apt install $CLANG_FORMAT" >&2
|
||||
exit 0
|
||||
fi
|
||||
echo "Error: '$CLANG_FORMAT' command not found." >&2
|
||||
echo "Please install it. On Ubuntu, you can run: sudo apt install $CLANG_FORMAT" >&2
|
||||
exit 1
|
||||
@ -94,10 +116,17 @@ if ! find . -maxdepth 2 -name ".clang-format" -print -quit | grep -q .; then
|
||||
echo ""
|
||||
fi
|
||||
|
||||
echo "Starting code formatting for .c and .h files using 'find -exec ... +'..."
|
||||
echo "Using $CLANG_FORMAT -i -style=file"
|
||||
echo "This may take a moment. Any clang-format errors for individual files will be printed directly."
|
||||
echo "Note: '$CLANG_FORMAT' only modifies files that deviate from the specified style."
|
||||
if [[ "$check_only" -eq 1 ]]; then
|
||||
CLANG_FORMAT_ARGS=(--dry-run --Werror -style=file)
|
||||
echo "Checking code formatting for .c and .h files."
|
||||
echo "Using $CLANG_FORMAT --dry-run --Werror -style=file"
|
||||
else
|
||||
CLANG_FORMAT_ARGS=(-i -style=file)
|
||||
echo "Starting code formatting for .c and .h files using 'find -exec ... +'..."
|
||||
echo "Using $CLANG_FORMAT -i -style=file"
|
||||
echo "This may take a moment. Any clang-format errors for individual files will be printed directly."
|
||||
echo "Note: '$CLANG_FORMAT' only modifies files that deviate from the specified style."
|
||||
fi
|
||||
echo ""
|
||||
|
||||
# --- Formatting Logic ---
|
||||
@ -114,7 +143,7 @@ if [[ "$git_changed_only" -eq 1 ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if ! "$CLANG_FORMAT" -i -style=file "${target_files[@]}"; then
|
||||
if ! "$CLANG_FORMAT" "${CLANG_FORMAT_ARGS[@]}" "${target_files[@]}"; then
|
||||
echo "Error: The overall code formatting process failed." >&2
|
||||
echo "Please review the output above for any specific clang-format errors." >&2
|
||||
exit 2
|
||||
@ -126,7 +155,7 @@ else
|
||||
# The '{} +' syntax passes multiple filenames to a single clang-format invocation,
|
||||
# which is more efficient.
|
||||
# The parentheses '\( ... \)' are crucial for correctly grouping the -name conditions.
|
||||
if ! find . \( -name "*.c" -o -name "*.h" \) -exec "$CLANG_FORMAT" -i -style=file {} +; then
|
||||
if ! find . \( -name "*.c" -o -name "*.h" \) -exec "$CLANG_FORMAT" "${CLANG_FORMAT_ARGS[@]}" {} +; then
|
||||
echo "Error: The overall code formatting process failed." >&2
|
||||
echo "Please review the output above for any specific clang-format errors." >&2
|
||||
exit 2
|
||||
@ -140,8 +169,12 @@ fi
|
||||
echo ""
|
||||
echo "--- Formatting Summary ---"
|
||||
echo "Total .c/.h files processed: $TOTAL_PROCESSED_FILES"
|
||||
echo "Code formatting completed successfully."
|
||||
echo "The number of files actually changed depends on their adherence to the style."
|
||||
echo "Please review changes using 'git diff' if in a Git repository."
|
||||
if [[ "$check_only" -eq 1 ]]; then
|
||||
echo "Code formatting check completed successfully."
|
||||
else
|
||||
echo "Code formatting completed successfully."
|
||||
echo "The number of files actually changed depends on their adherence to the style."
|
||||
echo "Please review changes using 'git diff' if in a Git repository."
|
||||
fi
|
||||
|
||||
# --- Script End ---
|
||||
|
||||
@ -6,7 +6,12 @@ if ! git rev-parse --show-toplevel >/dev/null 2>&1; then
|
||||
exit 2
|
||||
fi
|
||||
|
||||
base_ref="${RSYSLOG_LOCAL_VALIDATION_BASE:-origin/main}"
|
||||
|
||||
{
|
||||
git diff --diff-filter=d --name-only -- '*.c' '*.h'
|
||||
git diff --cached --diff-filter=d --name-only -- '*.c' '*.h'
|
||||
if git rev-parse --verify "$base_ref" >/dev/null 2>&1; then
|
||||
git diff --diff-filter=d --name-only "$base_ref"...HEAD -- '*.c' '*.h'
|
||||
fi
|
||||
git diff --diff-filter=d --name-only HEAD -- '*.c' '*.h'
|
||||
git ls-files --others --exclude-standard -- '*.c' '*.h'
|
||||
} | awk 'NF && !seen[$0]++'
|
||||
|
||||
@ -267,7 +267,7 @@ run_format_code_check() {
|
||||
return 0
|
||||
fi
|
||||
if [ -x devtools/format-code.sh ]; then
|
||||
devtools/format-code.sh --git-changed
|
||||
RSYSLOG_LOCAL_VALIDATION_BASE="$base_ref" devtools/format-code.sh --git-changed --check --check-if-available
|
||||
else
|
||||
echo "warning: devtools/format-code.sh missing or not executable; skipping C format check" >&2
|
||||
fi
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user