-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Switch clang tools from static binaries to wheels #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated README to prefer clang wheels with ordered fallback sequences per OS and adjusted links/licensing wording. Reworked Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (5)📚 Learning: 2025-08-17T12:34:25.463ZApplied to files:
📚 Learning: 2025-08-17T12:50:57.038ZApplied to files:
📚 Learning: 2025-08-17T12:31:25.319ZApplied to files:
📚 Learning: 2025-08-17T13:20:51.048ZApplied to files:
📚 Learning: 2025-08-17T12:34:25.463ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (3)
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. Comment |
Cpp-Linter Report
|
2bndy5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree with deprecated any version of clang-tidy before v13.
|
I think there's a bug in clang-tools-pip. It does not seem to match v9.0.0. Probably a logical error ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: In the cpp-linter-action project, technical improvements can be deferred to separate PRs when they're not critical to the current changes, especially when the current implementation is known to work correctly.
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: README.md:206-211
Timestamp: 2025-08-17T13:20:51.048Z
Learning: For GitHub Actions documentation references in cpp-linter-action, prefer stable workflow syntax documentation links over "canonical" specialized pages that may redirect, as the comprehensive workflow syntax page provides better context and examples for container configuration.
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 306
File: .readthedocs.yaml:9-16
Timestamp: 2025-08-13T09:33:35.208Z
Learning: Read the Docs v2 config supports `tools.python: latest` as a valid value, not just explicit version strings like "3.12". This was confirmed by a successful build in cpp-linter-action project.
📚 Learning: 2025-08-17T12:34:25.463Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: The cpp-linter/cpp-linter-action repository manages the downstream parsing of command-line arguments through their own cpp-linter/cpp-linter project, so passing empty string values for flags like --extra-arg="" is acceptable and known to work in their implementation.
Applied to files:
action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (windows-latest, 21)
- GitHub Check: test (windows-latest, 13)
- GitHub Check: test (windows-latest, 10)
- GitHub Check: test (macos-latest, 19)
- GitHub Check: test (macos-latest, 15)
- GitHub Check: test (macos-latest, 9)
- GitHub Check: test (ubuntu-latest, 15)
🔇 Additional comments (2)
action.yml (2)
43-43: Documentation update is clear and helpful.The added constraint note aligns with the fallback logic implemented in the installation step and sets proper user expectations.
388-396: Tool selection logic is clean and efficient.Only clang-tidy and clang-format are installed when actually needed based on inputs, avoiding unnecessary installation overhead.
Apparently, there are only wheels for linux from clang-format v9 (or older) PyPI package: JSON API response snippet
"6.0.1": [
{
"comment_text": "",
"digests": {
"blake2b_256": "e371db01cf5d188d0669af9cfd9f5cf8cfb47898df6ad5c86bc008ab3b73d327",
"md5": "fe41668bfa155409944fc6e7dc060b42",
"sha256": "1b71b62346e281f96aa9d45a8892427ce1eae7783c24ac0e1efbab1d5340748d"
},
"downloads": -1,
"filename": "clang_format-6.0.1-py2-none-manylinux1_x86_64.whl",
"has_sig": false,
"md5_digest": "fe41668bfa155409944fc6e7dc060b42",
"packagetype": "bdist_wheel",
"python_version": "py2",
"requires_python": null,
"size": 1285495,
"upload_time": "2019-11-04T14:55:48",
"upload_time_iso_8601": "2019-11-04T14:55:48.150563Z",
"url": "https://files.pythonhosted.org/packages/e3/71/db01cf5d188d0669af9cfd9f5cf8cfb47898df6ad5c86bc008ab3b73d327/clang_format-6.0.1-py2-none-manylinux1_x86_64.whl",
"yanked": false,
"yanked_reason": null
},
{
"comment_text": "",
"digests": {
"blake2b_256": "166065c642879f290c4233ac75d7f91038509750347271cd135a62cf0b86019c",
"md5": "cd777571cebf4190bc04ed4638d68bfd",
"sha256": "8028c358d0838e83ea85254bedba846f2106348406c9cdc4f2724779704eedca"
},
"downloads": -1,
"filename": "clang_format-6.0.1-py2.py3-none-manylinux1_x86_64.whl",
"has_sig": false,
"md5_digest": "cd777571cebf4190bc04ed4638d68bfd",
"packagetype": "bdist_wheel",
"python_version": "py2.py3",
"requires_python": null,
"size": 1295165,
"upload_time": "2019-11-09T15:05:57",
"upload_time_iso_8601": "2019-11-09T15:05:57.500683Z",
"url": "https://files.pythonhosted.org/packages/16/60/65c642879f290c4233ac75d7f91038509750347271cd135a62cf0b86019c/clang_format-6.0.1-py2.py3-none-manylinux1_x86_64.whl",
"yanked": false,
"yanked_reason": null
}
],
"7.1.0": [
{
"comment_text": "",
"digests": {
"blake2b_256": "1728feeb87d11c17d4734763db4c0a1a6939448f0c11a1713fac142e8dd2f107",
"md5": "ca817583469776c6b76aee6e8d058e8d",
"sha256": "32629996956b3e34494a926a76835e3371f10c2823c3078f5fbef08bfebbaae7"
},
"downloads": -1,
"filename": "clang_format-7.1.0-py2.py3-none-manylinux1_x86_64.whl",
"has_sig": false,
"md5_digest": "ca817583469776c6b76aee6e8d058e8d",
"packagetype": "bdist_wheel",
"python_version": "py2.py3",
"requires_python": null,
"size": 1330841,
"upload_time": "2019-11-09T16:17:01",
"upload_time_iso_8601": "2019-11-09T16:17:01.797915Z",
"url": "https://files.pythonhosted.org/packages/17/28/feeb87d11c17d4734763db4c0a1a6939448f0c11a1713fac142e8dd2f107/clang_format-7.1.0-py2.py3-none-manylinux1_x86_64.whl",
"yanked": false,
"yanked_reason": null
}
],
"8.0.1": [
{
"comment_text": "",
"digests": {
"blake2b_256": "376675e3c14f22c8a7a02c7e337547e29d42bed516ba8067fdef87d3d9346e52",
"md5": "2053ad2ea07c26dd5ad62cb7999722d7",
"sha256": "d3d0d62281ace883bc6e4188f949308b8f2e4f6de8fac463e5abd1ae02c03db6"
},
"downloads": -1,
"filename": "clang_format-8.0.1-py2.py3-none-manylinux1_x86_64.whl",
"has_sig": false,
"md5_digest": "2053ad2ea07c26dd5ad62cb7999722d7",
"packagetype": "bdist_wheel",
"python_version": "py2.py3",
"requires_python": null,
"size": 1377569,
"upload_time": "2019-11-20T18:37:41",
"upload_time_iso_8601": "2019-11-20T18:37:41.956369Z",
"url": "https://files.pythonhosted.org/packages/37/66/75e3c14f22c8a7a02c7e337547e29d42bed516ba8067fdef87d3d9346e52/clang_format-8.0.1-py2.py3-none-manylinux1_x86_64.whl",
"yanked": false,
"yanked_reason": null
}
],
"9.0.0": [
{
"comment_text": "",
"digests": {
"blake2b_256": "25f2d989afaf8a91385f18fd06e6202644be0bc3a1d14548c18a8ece4911e005",
"md5": "7b0f1d7a2785a5c2c4e9d695dfdf4583",
"sha256": "085342f9e238c9b03a019e20ec23f242f8795a3cca9296ab2427b1dea45e7014"
},
"downloads": -1,
"filename": "clang_format-9.0.0-py2.py3-none-manylinux1_x86_64.whl",
"has_sig": false,
"md5_digest": "7b0f1d7a2785a5c2c4e9d695dfdf4583",
"packagetype": "bdist_wheel",
"python_version": "py2.py3",
"requires_python": null,
"size": 1375189,
"upload_time": "2019-11-20T19:37:14",
"upload_time_iso_8601": "2019-11-20T19:37:14.542044Z",
"url": "https://files.pythonhosted.org/packages/25/f2/d989afaf8a91385f18fd06e6202644be0bc3a1d14548c18a8ece4911e005/clang_format-9.0.0-py2.py3-none-manylinux1_x86_64.whl",
"yanked": false,
"yanked_reason": null
}
] |
cc0b06f to
29a60ef
Compare
Co-authored-by: Brendan <2bndy5@gmail.com>
Co-authored-by: Brendan <2bndy5@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
action.yml (1)
402-417: Version-aware installation logic is correct; minor performance optimization opportunity.The fallback logic correctly implements the version-aware installation strategy:
- Clang-tidy < v13: uses static binaries (
clang-toolsCLI)- Clang-format ≤ v9 on non-Linux: uses static binaries (wheels unavailable for these versions on Windows/macOS)
- Otherwise: uses wheels (
clang-tools-wheel)This aligns with prior feedback and PR objectives. One minor optimization: the platform detection at line 409 is called once per tool in the loop. Consider computing it once before the loop to avoid redundant system calls:
+ let is_linux = (sys host | get 'long_os_version') | str starts-with "Linux" for tool in $tools { print $"Installing ($tool) ($version)" let cmd = if ( (($version < 13) and ($tool | str ends-with "tidy")) or ( ($version <= 9) and ($tool | str ends-with "format") - and not ((sys host | get 'long_os_version') | str starts-with "Linux") + and (not $is_linux) ) ) { [clang-tools --tool $tool --install $version] } else { [clang-tools-wheel --tool $tool --version $version] } ^$'($env.UV_INSTALL_DIR)/uv' ...$uv_args ...$cmd }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)action.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-17T12:34:25.463Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: In the cpp-linter-action project, technical improvements can be deferred to separate PRs when they're not critical to the current changes, especially when the current implementation is known to work correctly.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:50:57.038Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:273-321
Timestamp: 2025-08-17T12:50:57.038Z
Learning: In Nu shell, `$in` is shorthand for a closure that takes 1 argument, so `(...) | complete | $in.exit_code == 0` is equivalent to `(...) | complete | {|in| $in.exit_code == 0}`.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:31:25.319Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:273-321
Timestamp: 2025-08-17T12:31:25.319Z
Learning: In Nu shell, the pattern `(...) | complete | $in.exit_code == 0` is valid syntax for checking command exit codes, where `complete` captures the command result and `$in` refers to the piped input.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:34:25.463Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: The cpp-linter/cpp-linter-action repository manages the downstream parsing of command-line arguments through their own cpp-linter/cpp-linter project, so passing empty string values for flags like --extra-arg="" is acceptable and known to work in their implementation.
Applied to files:
action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test (macos-latest, 21)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (macos-latest, 14)
- GitHub Check: test (macos-latest, 15)
- GitHub Check: test (macos-latest, 13)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (ubuntu-latest, 21)
🔇 Additional comments (1)
action.yml (1)
391-401: Tool selection logic is sound; edge case with neither tool in use.The code correctly identifies which tools to install (clang-tidy if enabled in tidy-checks, clang-format if style is not empty). However, there is an edge case: if a user disables both tools (e.g.,
tidy-checks: '-*'andstyle: ''), the tools array will be empty, and the print message will read awkwardly ("Ensuring 18 are present").This is unlikely in practice and the empty loop is harmless, so no action required unless you want to add a guard to skip the message/loop entirely if no tools are selected.
2bndy5
left a comment
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-17T12:34:25.463Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: In the cpp-linter-action project, technical improvements can be deferred to separate PRs when they're not critical to the current changes, especially when the current implementation is known to work correctly.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:50:57.038Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:273-321
Timestamp: 2025-08-17T12:50:57.038Z
Learning: In Nu shell, `$in` is shorthand for a closure that takes 1 argument, so `(...) | complete | $in.exit_code == 0` is equivalent to `(...) | complete | {|in| $in.exit_code == 0}`.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:31:25.319Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:273-321
Timestamp: 2025-08-17T12:31:25.319Z
Learning: In Nu shell, the pattern `(...) | complete | $in.exit_code == 0` is valid syntax for checking command exit codes, where `complete` captures the command result and `$in` refers to the piped input.
Applied to files:
action.yml
📚 Learning: 2025-08-17T12:34:25.463Z
Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-action PR: 314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: The cpp-linter/cpp-linter-action repository manages the downstream parsing of command-line arguments through their own cpp-linter/cpp-linter project, so passing empty string values for flags like --extra-arg="" is acceptable and known to work in their implementation.
Applied to files:
action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test (windows-latest, 15)
- GitHub Check: test (windows-latest, 18)
- GitHub Check: test (windows-latest, 19)
- GitHub Check: test (windows-latest, 13)
- GitHub Check: test (macos-latest, 14)
- GitHub Check: test (macos-latest, 19)
- GitHub Check: test (ubuntu-latest, 19)
- GitHub Check: test (macos-latest, 18)
- GitHub Check: test (macos-latest, 15)
- GitHub Check: test (macos-latest, 9)
🔇 Additional comments (1)
action.yml (1)
379-395: Version input validation handles all documented cases correctly.The try-catch block properly handles three cases: empty string (exits early), valid integer (proceeds to installation), and path input (validates existence and exits gracefully). This aligns well with the documented API where version can be empty, an integer, or a path.
| mut tools = [] | ||
| if ("${{ inputs.tidy-checks }}" != "-*") { | ||
| # clang-tidy is being used | ||
| $tools = $tools | append "clang-tidy" | ||
| } | ||
| if ("${{ inputs.style }}" | is-not-empty) { | ||
| # clang-format is being used | ||
| $tools = $tools | append "clang-format" | ||
| } | ||
| print $"\n(ansi purple)Ensuring ($tools | str join " and ") ${{ inputs.version }} are present(ansi reset)" | ||
| for tool in $tools { | ||
| print $"Installing ($tool) ($version)" | ||
| let cmd = if ( | ||
| (($version < 13) and ($tool | str ends-with "tidy")) | ||
| or ( | ||
| ($version <= 9) | ||
| and ($tool | str ends-with "format") | ||
| and not ((sys host | get 'long_os_version') | str starts-with "Linux") | ||
| ) | ||
| ) { | ||
| [clang-tools --tool $tool --install $version] | ||
| } else { | ||
| [clang-tools-wheel --tool $tool --version $version] | ||
| } | ||
| ^$'($env.UV_INSTALL_DIR)/uv' ...$uv_args ...$cmd | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Per-tool conditional installation with platform-aware fallback logic is well-structured.
The logic correctly implements the PR objective to switch to wheels while maintaining backward compatibility:
- Clang-tidy < v13 falls back to
clang-tools(static binary) as required - Clang-format ≤ v9 on non-Linux platforms falls back to static binaries (addressing the wheel availability issue noted in issue Switch clang tools from static binaries to wheels? #364)
- All other cases use
clang-tools-wheel - Tools are only installed if they're actually being used (conditional tool list)
However, verify that the platform detection at line 420 is robust across all runners. The code uses sys host | get 'long_os_version' | str starts-with "Linux" to detect Linux, whereas other steps use GitHub Actions' runner.os variable. Please confirm this approach works consistently on Linux, macOS, and Windows runners:
🏁 Script executed:
#!/bin/bash
# Search for other platform detection patterns in action.yml
rg -n "runner\.os|sys host" action.ymlLength of output: 365
The web search didn't provide specific field details. Let me search more specifically for sys host output examples.Excellent! The GitHub issue shows that users can switch on (sys).host.name as a builtin OS detector that works on Windows, confirming that the name field exists and is used for platform detection. However, I still need to determine if using long_os_version with a string prefix check is equally robust. Let me search more specifically for the consistency of these fields.
The web search confirms that (sys).host.name is a builtin OS detector used for cross-platform detection. However, I need to verify whether the code at line 420 should use the same name field for consistency, or if long_os_version is intentionally different. Let me check the context within the action.yml file more directly to understand the code structure.
Let me search for any tests or documentation in the repository itself:
Actually, let me take a step back and look at the actual action.yml file around those lines to understand the logic better:
🏁 Script executed:
#!/bin/bash
# Get more context around line 351 and line 420 to understand their contexts
rg -n -B5 -A5 "sys host" action.yml | head -50Length of output: 978
🏁 Script executed:
#!/bin/bash
# Check if there are any test files or discussions about this
rg -rn "long_os_version|sys.*host.*name" . --type yaml --type md | head -20Length of output: 139
🏁 Script executed:
#!/bin/bash
# Extract the relevant sections to understand context
sed -n '345,360p' action.yml
sed -n '415,430p' action.ymlLength of output: 1258
Refactor platform detection to use consistent name field for robustness.
The verification found an inconsistency in platform detection approaches within the same file:
- Line 351 uses
(sys host | get 'name') == 'Windows'for direct OS comparison - Line 420 uses
not ((sys host | get 'long_os_version') | str starts-with "Linux")with string prefix matching on version data
The long_os_version field approach at line 420 is fragile—if the version string format changes, the Linux detection breaks. For consistency and robustness across all runners, refactor line 420 to use the established pattern:
and not ((sys host | get 'name') == 'Linux')This matches the direct field comparison approach already used in line 351 and is more reliable for cross-platform detection.
🤖 Prompt for AI Agents
In action.yml around lines 402 to 428, replace the fragile platform check that
uses long_os_version prefix matching with the consistent direct name check:
change the condition that reads not ((sys host | get 'long_os_version') | str
starts-with "Linux") to use and not ((sys host | get 'name') == 'Linux') so
platform detection matches the existing pattern (e.g., the check at line 351)
and is robust across runners; update only that boolean expression so the overall
if/else logic and command selection remain the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
long_os_versionfield approach at line 420 is fragile
This is invalid because name does not identify Linux distribution in a generic way. Rather, it returns the flavor of the Linux distribution, which makes it harder to identify Linux in particular.
> sys host | get name
UbuntuPerhaps we can use version | get build_os instead. This should return the compilation target of the nu binary.
> version | get build_os
linux-x86_64There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for nushell documentation or code examples showing what sys host name returns on different platforms
rg -n "sys.*host.*name|build_os" action.yml -B2 -A2
Length of output: 278
🌐 Web query:
nushell version build_os command
💡 Result:
Use the built-in version command and select the build_os field. Examples:
- Show full version record:
version - Show only build_os:
version | get build_os
(You can also inspect other fields like build_target, rust_version, build_time.) [1][2]
🌐 Web query:
nushell version build_os output format examples
💡 Result:
Nushell’s version output is a record with keys like build_os (string), build_target, rust_version, build_time, etc. Example outputs showing common build_os values:
-
Windows (example):
build_os | windows-x86_64. [1] -
macOS (example):
build_os | macos-aarch64. [2] -
Linux / release artifacts (examples):
build_os entries appear alongside platform tarballs such as x86_64-unknown-linux-gnu or aarch64-unknown-linux-gnu (packaging names; build_os strings in version records are e.g. linux-x86_64 / aarch64). [3]
Full example record (typical output from running version):
version | 0.105.1
major | 0
minor | 105
patch | 1
build_os | windows-x86_64
build_target | x86_64-pc-windows-msvc
rust_version | rustc 1.88.0 ...
build_time | 2025-07-07 09:19:56 +02:00. [1]
References:
[1] nushell issue showing version record (build_os: windows-x86_64).
[2] example showing build_os: macos-aarch64.
[3] release/package listings showing Linux build names.
@2bndy5 Thank you for the correction! You're absolutely right—I misunderstood how sys host | get 'name' behaves on Linux systems. Using version | get build_os is a much more robust approach.
Based on the nushell version command, build_os returns consistent values like "windows-x86_64", "macos-aarch64", and "linux-x86_64", which makes platform detection straightforward across all runners.
For consistency, here's the corrected implementation for line 420 (and optionally line 351 for uniformity):
Line 420 (Linux detection in the clang-tools fallback logic):
and not ((version | get build_os) | str starts-with "linux")Line 351 (optional: update Windows detection for consistency):
let is_windows = (version | get build_os) | str starts-with "windows"This approach ensures robust platform detection across all GitHub Actions runners and avoids fragile string matching on version strings. Would you like me to update both locations for consistency, or just line 420?
closes #364
Summary by CodeRabbit
Documentation
Chores