Skip to content

fix: do not fail on check for tool presence (via package managers)#345

Merged
2bndy5 merged 2 commits into
mainfrom
debug-apt
Jun 10, 2026
Merged

fix: do not fail on check for tool presence (via package managers)#345
2bndy5 merged 2 commits into
mainfrom
debug-apt

Conversation

@2bndy5

@2bndy5 2bndy5 commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

I added some debug logs and ran the clang-tools binary in a docker container. I found that the apt package manager support was failing because the check for any existing clang tool failed when the tool was not installed.

I converted the check for a clang tool's presence into a soft check. Meaning, if it somehow fails then just fall back to installing the clang tool as if doing a fresh/normal install.

Note

The container I used seemed to be too new; the LLVM install script does not yet support Ubuntu "resolute". But at least the clang-tools binary was able to fall back to static binaries. Plus, the debug and error logs are explanatory enough.

Summary by CodeRabbit

  • Bug Fixes

    • Improved package installation reliability by gracefully handling detection failures and proceeding with installation attempts when needed.
  • Chores

    • Enhanced diagnostic logging for package installation operations to aid troubleshooting.

2bndy5 added 2 commits June 9, 2026 15:13
package managers may not have any version of the desired tool installed
@2bndy5 2bndy5 added the bug Something isn't working label Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR refines the native package installation flow in clang-tools-manager to handle version-check failures gracefully and improve diagnostic logging. When version verification fails (executable path unavailable or version capture unsuccessful), the installation now proceeds rather than erroring out immediately. Enhanced logging provides visibility into executed commands and apt installation failures.

Changes

Package installation flow improvements

Layer / File(s) Summary
Fast-path version check error handling
clang-tools-manager/src/downloader/native_packages/mod.rs
The "already installed" check in try_install_package now treats failures in get_exe_path() and capture_version() as non-matches using let Ok(...) guards instead of ? propagation, allowing the function to proceed to the installation attempt branch instead of returning an error.
Installation command logging and error diagnostics
clang-tools-manager/src/downloader/native_packages/unix.rs
Debug logging is added before executing the package manager command, showing the full command with sudo prefix when applicable. Error-level logging is also added for failed apt installs with a specified version, capturing the package ID and decoded stderr for better troubleshooting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • cpp-linter/cpp-linter-rs#279: Introduces the on-demand native package installation flow that this PR refines with improved error handling and logging in the version-matching and apt-install paths.

Suggested labels

bug, rust

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: converting the tool presence check from a hard error to a soft fallback mechanism, allowing installation to proceed when the check fails.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch debug-apt

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.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.88%. Comparing base (b213c8a) to head (e35dca3).

Files with missing lines Patch % Lines
...ols-manager/src/downloader/native_packages/unix.rs 62.50% 3 Missing ⚠️
...ools-manager/src/downloader/native_packages/mod.rs 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   91.14%   92.88%   +1.74%     
==========================================
  Files          22       22              
  Lines        3354     3361       +7     
==========================================
+ Hits         3057     3122      +65     
+ Misses        297      239      -58     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@2bndy5 2bndy5 merged commit 89d9a7e into main Jun 10, 2026
45 checks passed
@2bndy5 2bndy5 deleted the debug-apt branch June 10, 2026 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant