Skip to content

feat: Switch clang tools from static binaries to wheels#368

Merged
shenxianpeng merged 15 commits intomainfrom
fix-364
Apr 10, 2026
Merged

feat: Switch clang tools from static binaries to wheels#368
shenxianpeng merged 15 commits intomainfrom
fix-364

Conversation

@shenxianpeng
Copy link
Copy Markdown
Member

@shenxianpeng shenxianpeng commented Nov 3, 2025

closes #364

Summary by CodeRabbit

  • Documentation

    • Clarified installation instructions per OS with prioritized fallback sources (Homebrew/PyPI/static binaries where applicable), added direct wheel references for tools, and updated license wording to "MIT License".
  • Chores

    • Installer now better detects platform, validates/normalizes version inputs, installs selected tools sequentially with clearer per-tool logging, and handles empty or invalid version inputs gracefully.

@shenxianpeng shenxianpeng requested a review from a team as a code owner November 3, 2025 20:44
@shenxianpeng shenxianpeng requested review from 2bndy5 and removed request for a team November 3, 2025 20:44
@shenxianpeng shenxianpeng added the enhancement New feature or request label Nov 3, 2025
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 3, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 3, 2025

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

README install instructions now prefer wheels with OS-specific fallback order and updated links/licensing. action.yml adds version validation/normalization, switches Windows detection to build_os, and installs clang tools per-tool (clang-tidy/clang-format) with conditional installer selection.

Changes

Cohort / File(s) Summary
Documentation
README.md
Reworked installation instructions to prefer PyPI wheels with ordered fallbacks per OS (Linux, macOS, Windows); removed clang-tools-static-binaries link; added explicit PyPI links for clang-tidy/clang-format; adjusted license text to “MIT License”.
Action manifest / install logic
action.yml
Replaced OS detection with `version

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • 2bndy5
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately reflects the main objective of switching clang tools installation from static binaries to wheels, as implemented in both README.md and action.yml.
Linked Issues check ✅ Passed The PR implements the core objective from issue #364 by updating both documentation and code to adopt wheels-based installation via clang-tools-wheel CLI.
Out of Scope Changes check ✅ Passed All changes directly support the wheels migration objective: README updates document the new installation approaches, and action.yml implements the wheels-based tool selection logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-364

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 3, 2025

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v16.0.6) reports: 2 file(s) not formatted
  • docs/examples/demo/demo.cpp
  • docs/examples/demo/demo.hpp
clang-tidy (v16.0.6) reports: 7 concern(s)

Have any feedback or feature suggestions? Share it here.

2bndy5

This comment was marked as outdated.

@2bndy5
Copy link
Copy Markdown
Collaborator

2bndy5 commented Nov 3, 2025

I think there's a bug in clang-tools-pip. It does not seem to match v9.0.0. Probably a logical error (>= vs >).

coderabbitai[bot]

This comment was marked as resolved.

@2bndy5

This comment was marked as resolved.

@2bndy5 2bndy5 force-pushed the fix-364 branch 3 times, most recently from 29a60ef to d3e381b Compare November 4, 2025 19:17
coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

I have pushed any changes that I requested.

This is now the least breaking implementation that I can muster.

Also, the README now states the origin of clang tools in order of install attempts.

Caution

This should be tested in test repo before merging.
We have made some significant changes to how version input is used before it is passed to cpp-linter.

coderabbitai[bot]

This comment was marked as resolved.

@2bndy5
Copy link
Copy Markdown
Collaborator

2bndy5 commented Mar 28, 2026

Seems to work as expected when version is an integer. just need to check when version is a

  • path
  • empty string

@2bndy5
Copy link
Copy Markdown
Collaborator

2bndy5 commented Mar 28, 2026

ok, so the nushell script handles a path as the version input:

Version is not an integer
Using custom clang tools installation at path: D:\a_temp/llvm

This works if the path points to a folder where clang-tidy and/or clang-format binaries exist.
But if the path is wrong (above example should be D:\a\_temp/llvm/bin), then cpp-linter uses the version installed in the venv:

INFO:CPP Linter:D:\a\_actions\cpp-linter\cpp-linter-action\fix-364\.venv\Scripts\clang-format.EXE --version: 21.1.2
INFO:CPP Linter:D:\a\_actions\cpp-linter\cpp-linter-action\fix-364\.venv\Scripts\clang-tidy.EXE --version: 21.1.1

This is because cpp-linter-hooks installs a version of clang tools as dependencies:

https://github.com/cpp-linter/cpp-linter-hooks/blob/4a6686cd87dae7b5c1d8beba7563f6e0dd26f6b1/pyproject.toml#L33-L38

@shenxianpeng Can we remove the unnecessary dependencies in cpp-linter-hooks?

This isn't a deal breaker. It just causes confusion when the given path is wrong.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 10, 2026
@shenxianpeng shenxianpeng removed the documentation Improvements or additions to documentation label Apr 10, 2026
@shenxianpeng
Copy link
Copy Markdown
Member Author

Let's merge this PR and publish a minor release.

@shenxianpeng shenxianpeng merged commit 77c390c into main Apr 10, 2026
44 checks passed
@shenxianpeng shenxianpeng deleted the fix-364 branch April 10, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request minor A minor version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switch clang tools from static binaries to wheels?

3 participants