Skip to content

tools/checkpatch.sh: add Make vs CMake build-system parity check - #21012

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
Swatantra-66:feature/checkpatch-cmake-parity
Oct 27, 2026
Merged

xiaoxiang781216 merged 1 commit into
apache:masterfrom
Swatantra-66:feature/checkpatch-cmake-parity

Conversation

@Swatantra-66

Copy link
Copy Markdown
Contributor

Summary

This adds an automated check in tools/checkpatch.sh to enforce build-system parity between GNU Make and CMake, as suggested by @acassis on apache/nuttx-apps#3920.

Whenever a PR creates or modifies a Makefile, Make.defs, or *.mk file without also creating or updating the corresponding CMakeLists.txt, checkpatch.sh fails with an actionable error message during the fast check workflow, preventing build divergence before the heavy CI matrix spins up.

Key Features:

  1. Scope: Automatically triggered when inspecting commits (-g) or patches (-p). Excludes root Makefile, tools/, and Documentation/.
  2. Flexible CMake Resolution: Resolves CMakeLists.txt co-located in the same directory or parent component (including repository root).
  3. Paired Build-File Deletions: Allows deleting both Makefile and CMakeLists.txt together without false-positive errors.
  4. Reliable Path Extraction: In patch mode (-p), parses touched paths directly from patch metadata via git apply --numstat to catch newly added untracked files.
  5. Exact Literal Matching: Uses grep -Fqx -- "$cmake_file" to avoid regex false positives.
  6. Commit Tree Resolution: Evaluates file existence in the commit's snapshot via git cat-file -e rather than relying solely on the current working tree.
  7. Bypass Mechanism:
    Commit message tags: [skip-cmake-parity], [skip-cmake-check], or ci:make-only (isolated per-commit).
    CLI flags: -k (force enable) and -K (disable).

Impact

Protects both apache/nuttx and apache/nuttx-apps PRs from Make/CMake divergence.
No impact on existing build configurations or non-build file modifications.

Testing

Verified syntax clean with bash -n tools/checkpatch.sh (0 errors, 0 warnings).
Validated command-line options -h, -k, -K, -g, and -p.
Tested failure cases on unsynced Makefiles and pass cases on paired modifications.
Verified commit message bypass isolation across multiple commits.

This adds an automated check in checkpatch.sh to detect when a
Makefile, Make.defs, or *.mk file is created or modified without
the corresponding CMakeLists.txt also being created or updated:
- Checks git commit ranges (-g) and patch files (-p)
- Resolves CMakeLists.txt co-located in the same directory or
  parent component
- Allows paired build-file deletions without false positives
- Diffs commits against their parent via git diff-tree to ensure
  reliable single-revision inspection on clean trees
- Supports per-commit bypass via '[skip-cmake-parity]',
  '[skip-cmake-check]', or 'ci:make-only' commit message tags
- Provides -k to force enable and -K to disable the check

Suggested-by: Alan Carvalho de Assis <acassis@apache.org>
Signed-off-by: Swatantra Yadav <maverickswatantra@gmail.com>
@github-actions github-actions Bot added Area: Build system Size: M The size of the change in this PR is medium labels Oct 26, 2026
@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

No memory changes detected for:


Catch memory regressions in CI with MemBrowse. Free for open source.

@xiaoxiang781216
xiaoxiang781216 merged commit a31c154 into apache:master Oct 27, 2026
51 of 53 checks passed

@jerpelea jerpelea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this is an AI assisted patch and was merged without following contribution guidelines

please clarify!

@Swatantra-66

Copy link
Copy Markdown
Contributor Author

I apologize for omitting the Assisted-by disclosure line. I have taken note of this requirement and will ensure to include in accordance with the community guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Build system Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants