feat: add llvm-cov, llvm-profdata, llvm-symbolizer, clang-scan-deps#115
feat: add llvm-cov, llvm-profdata, llvm-symbolizer, clang-scan-deps#115shenxianpeng wants to merge 6 commits into
Conversation
WalkthroughAdds ChangesLLVM tools addition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)CONTRIBUTING.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) README.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
release.py (1)
44-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPromote platform identifiers to a shared constant.
platformsis hardcoded here while the same contract is also defined inbuild.py. Sinceversions.jsonis downstream-facing metadata, this should come from one shared source to avoid silent drift.♻️ Proposed refactor
diff --git a/release.py b/release.py @@ - platforms = [ - "linux-amd64", - "linux-arm64", - "macos-amd64", - "macos-arm64", - "windows-amd64", - "windows-arm64", - ] + platforms = list(build.SUPPORTED_PLATFORMS)# build.py (shared contract suggestion) SUPPORTED_PLATFORMS = ( "linux-amd64", "linux-arm64", "macos-amd64", "macos-arm64", "windows-amd64", "windows-arm64", )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@release.py` around lines 44 - 51, The platforms list is hardcoded in release.py but duplicated in build.py, creating risk of silent drift between the two files. Extract the platforms list from release.py (currently containing linux-amd64, linux-arm64, macos-amd64, macos-arm64, windows-amd64, windows-arm64) into a shared constant named SUPPORTED_PLATFORMS that should be defined in build.py. Then import and use this constant in release.py to replace the hardcoded platforms list, ensuring both files reference the same single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.py`:
- Around line 280-293: The llvm-symbolizer input format in the subprocess.run
call is incorrect. The address and binary are being sent in reverse order on
separate lines, but llvm-symbolizer expects either just the address when the
object is supplied via the --obj command-line flag, or address and object
whitespace-separated. Fix this by adding the --obj flag with the test_bin path
to the subprocess command list and changing the input_str to contain only the
address so that llvm-symbolizer receives the inputs in the correct format and
can properly resolve the symbol to "test_func".
In `@scoop-examples/clang-apply-replacements.json`:
- Around line 16-20: The `bin` mapping is currently hard-coded to reference only
the amd64 executable filename (clang-apply-replacements-22_windows-amd64.exe),
but the manifest defines architecture-specific downloads for both amd64 and
arm64 with different filenames. Make the `bin` entry architecture-aware by using
conditional mappings or separate architecture-specific definitions (similar to
how the url entries are structured) so that ARM64 installs will reference the
correct clang-apply-replacements-22_windows-arm64.exe executable file.
In `@scoop-examples/clang-format.json`:
- Around line 16-21: The `bin` array currently uses the hardcoded AMD64 filename
`clang-format-22_windows-amd64.exe`, which fails for ARM64 installations that
require `clang-format-22_windows-arm64.exe`. Remove the `bin` array from the
global level (lines 16-21) and duplicate it within each architecture-specific
section, using the correct binary name for AMD64 in the `64bit` section and the
correct ARM64 binary name in the `arm64` section.
In `@scoop-examples/clang-include-cleaner.json`:
- Around line 16-20: The `bin` entry is currently at the root level hardcoding
the amd64 executable filename, which breaks ARM64 installations. Remove the
shared `bin` entry and instead add architecture-specific `bin` entries within
each architecture block (64bit and arm64 sections). In the 64bit architecture
block, map the amd64 executable filename to the clang-include-cleaner shim, and
in the arm64 architecture block, map the arm64 executable filename to the
clang-include-cleaner shim.
In `@scoop-examples/clang-query.json`:
- Around line 16-20: Remove the top-level `bin` array from the manifest and move
architecture-specific `bin` entries into each `architecture` block. In the
`64bit` architecture block, add a `bin` entry with the
clang-query-22_windows-amd64.exe filename mapped to clang-query. In the `arm64`
architecture block, add a separate `bin` entry with the
clang-query-22_windows-arm64.exe filename mapped to clang-query. This ensures
Scoop creates shims pointing to the correct executable for each architecture.
In `@scoop-examples/clang-scan-deps.json`:
- Around line 8-20: The manifest file has two issues that must be fixed: the
sha512 hash values in both the amd64 and arm64 blocks are placeholders
(UPDATE_ME) that need to be replaced with actual computed hashes for their
respective executables, and the bin array is incorrectly placed at the top level
hard-coding only the amd64 executable name. Move the bin array into each
architecture block so that the amd64 block references
clang-scan-deps-22_windows-amd64.exe and the arm64 block references
clang-scan-deps-22_windows-arm64.exe, ensuring ARM64 installations use the
correct executable. Compute the actual SHA512 hashes for each binary and replace
all UPDATE_ME placeholders.
In `@scoop-examples/llvm-cov.json`:
- Around line 8-20: The manifest file has two issues that prevent proper
installation across architectures. First, both hash values in the "64bit" and
"arm64" blocks contain placeholder text "sha512:UPDATE_ME" which must be
replaced with actual SHA512 hash values computed from the respective executable
files. Second, the global "bin" entry hard-codes the amd64 executable name
"llvm-cov-22_windows-amd64.exe", which will cause ARM64 installations to fail
since Scoop cannot find the correct executable. Move the "bin" entry into each
architecture-specific block—place a "bin" array within the "64bit" block mapping
the amd64 executable to "llvm-cov", and place a separate "bin" array within the
"arm64" block mapping the arm64 executable to "llvm-cov", then remove the global
"bin" entry.
In `@scoop-examples/llvm-profdata.json`:
- Around line 9-14: Replace the placeholder SHA512 hashes in the manifest with
actual hashes for download integrity verification. For the x64 architecture
entry, compute the SHA512 hash of the Windows binary from the provided URL and
replace the sha512:UPDATE_ME placeholder on line 9. Similarly, for the arm64
architecture entry, compute the SHA512 hash of the corresponding Windows-arm64
binary and replace the sha512:UPDATE_ME placeholder on line 13. Ensure both hash
fields contain complete, valid SHA512 digests to allow Scoop to verify downloads
and enable installation for both architectures.
---
Nitpick comments:
In `@release.py`:
- Around line 44-51: The platforms list is hardcoded in release.py but
duplicated in build.py, creating risk of silent drift between the two files.
Extract the platforms list from release.py (currently containing linux-amd64,
linux-arm64, macos-amd64, macos-arm64, windows-amd64, windows-arm64) into a
shared constant named SUPPORTED_PLATFORMS that should be defined in build.py.
Then import and use this constant in release.py to replace the hardcoded
platforms list, ensuring both files reference the same single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 696956cc-4950-40d7-89b3-3462670df915
📒 Files selected for processing (16)
.github/workflows/build.ymlCONTRIBUTING.mdREADME.mdbuild.pyrelease.pyscoop-examples/README.mdscoop-examples/clang-apply-replacements.jsonscoop-examples/clang-format.jsonscoop-examples/clang-format22.jsonscoop-examples/clang-include-cleaner.jsonscoop-examples/clang-query.jsonscoop-examples/clang-scan-deps.jsonscoop-examples/clang-tidy.jsonscoop-examples/llvm-cov.jsonscoop-examples/llvm-profdata.jsonscoop-examples/llvm-symbolizer.json
| if addr: | ||
| # Feed address + binary to llvm-symbolizer via stdin | ||
| input_str = f"0x{addr}\n{test_bin}\n" | ||
| result = subprocess.run( | ||
| [str(sym_exe)], | ||
| input=input_str, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| output = result.stdout + result.stderr | ||
| assert "test_func" in output, ( | ||
| f"Expected 'test_func' in symbolizer output, got:\n{output}" | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
What stdin input format does llvm-symbolizer expect (binary path and address order), and does it support the --obj option to specify the object file?
💡 Result:
llvm-symbolizer supports the --obj (or --exe, -e) option to specify the object file to be symbolized [1][2]. If this option is not provided, the tool expects the object file path to be specified as part of the input [3]. Regarding stdin input, llvm-symbolizer accepts a flexible format where object paths, addresses, and optional prefixes are provided as whitespace-separated values [1][4]. The expected order is as follows: 1. Optional Prefix: You may optionally prefix an input with CODE or DATA to specify how the address should be symbolized (CODE is the default) [1][4]. 2. Object File or Address: If no object file was specified via command-line arguments, the tool treats the first input value as an object file path [1]. Subsequent values (or all values if an object was provided via --obj) are interpreted as addresses [1][4]. 3. Format Flexibility: Object file paths can be explicitly prefixed with FILE: (the default) or BUILDID: (for a hex build ID) to distinguish them from standard addresses [1][2]. If you pass - to the --obj option, the tool will read the object file binary directly from standard input [1][5]. If you do not use the --obj option, you must provide the object file path for each input line or sequence [3].
Citations:
- 1: https://llvm.org/docs/CommandGuide/llvm-symbolizer.html
- 2: https://manpages.debian.org/trixie/llvm/llvm-symbolizer.1.en.html
- 3: https://llvm.googlesource.com/llvm-project.git/+/refs/heads/users/chapuni/yaml/blob/llvm/tools/llvm-symbolizer/Opts.td
- 4: https://releases.llvm.org/13.0.1/docs/CommandGuide/llvm-symbolizer.html
- 5: https://releases.llvm.org/18.1.4/docs/CommandGuide/llvm-symbolizer.html
Incorrect llvm-symbolizer stdin format will fail the smoke test on Linux/macOS.
llvm-symbolizer expects input in the format <object> <address> (whitespace-separated) when no object is specified via command-line, or just <address> when the object is supplied via --obj. Here the address and binary path are sent reversed and on separate lines (f"0x{addr}\n{test_bin}\n"), so llvm-symbolizer will treat the first line (0x{addr}) as the object file path and the second line (test_bin) as an address—the opposite of what's intended. The address then fails to resolve and "test_func" will not appear in the output, causing the subsequent assert to fail. Since nm is available on Linux/macOS runners, addr is set and this branch is exercised, breaking the smoke phase on these platforms.
🐛 Proposed fix: pass the object explicitly via --obj and send only the address
if addr:
- # Feed address + binary to llvm-symbolizer via stdin
- input_str = f"0x{addr}\n{test_bin}\n"
- result = subprocess.run(
- [str(sym_exe)],
+ # Feed the address via stdin; the object is supplied with --obj
+ input_str = f"0x{addr}\n"
+ result = subprocess.run(
+ [str(sym_exe), "--obj", str(test_bin)],
input=input_str,
capture_output=True,
text=True,
check=False,
)🧰 Tools
🪛 ast-grep (0.44.0)
[error] 282-288: Command coming from incoming request
Context: subprocess.run(
[str(sym_exe)],
input=input_str,
capture_output=True,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.18)
[error] 283-283: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.py` around lines 280 - 293, The llvm-symbolizer input format in the
subprocess.run call is incorrect. The address and binary are being sent in
reverse order on separate lines, but llvm-symbolizer expects either just the
address when the object is supplied via the --obj command-line flag, or address
and object whitespace-separated. Fix this by adding the --obj flag with the
test_bin path to the subprocess command list and changing the input_str to
contain only the address so that llvm-symbolizer receives the inputs in the
correct format and can properly resolve the symbol to "test_func".
Source: Linters/SAST tools
| "bin": [ | ||
| [ | ||
| "clang-apply-replacements-22_windows-amd64.exe", | ||
| "clang-apply-replacements" | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "clang-apply-replacements.json" -type fRepository: cpp-linter/clang-tools-static-binaries
Length of output: 223
🏁 Script executed:
# Once located, read the full file to understand the structure
cat -n scoop-examples/clang-apply-replacements.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1463
Make the bin mapping architecture-aware.
The manifest defines architecture-specific downloads at lines 8 and 12 with different executable filenames (clang-apply-replacements-22_windows-amd64.exe for 64bit and clang-apply-replacements-22_windows-arm64.exe for arm64), but the bin entry at lines 16-20 hard-codes only the amd64 filename. ARM64 installs will fail to create the shim because the referenced executable won't exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/clang-apply-replacements.json` around lines 16 - 20, The `bin`
mapping is currently hard-coded to reference only the amd64 executable filename
(clang-apply-replacements-22_windows-amd64.exe), but the manifest defines
architecture-specific downloads for both amd64 and arm64 with different
filenames. Make the `bin` entry architecture-aware by using conditional mappings
or separate architecture-specific definitions (similar to how the url entries
are structured) so that ARM64 installs will reference the correct
clang-apply-replacements-22_windows-arm64.exe executable file.
| "bin": [ | ||
| [ | ||
| "clang-format-22_windows-amd64.exe", | ||
| "clang-format", | ||
| "" | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify architecture-specific bin mismatches and placeholder hashes in scoop manifests.
set -euo pipefail
echo "== manifests with arm64 URL but amd64-only bin target =="
fd -e json . scoop-examples | while read -r f; do
jq -r '
. as $m
| ($m.architecture.arm64.url // "") as $arm
| ($m.bin // []) as $bin
| if ($arm|test("arm64")) and (($bin|tostring)|test("amd64")) then
input_filename
else empty end
' "$f"
done
echo
echo "== manifests containing placeholder hash tokens =="
rg -n "sha512:UPDATE_ME" scoop-examplesRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1240
🏁 Script executed:
cat -n scoop-examples/clang-format.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 2243
🏁 Script executed:
# Check if Scoop manifests support architecture-specific bin arrays
rg -A 5 -B 5 '"arm64"' scoop-examples/clang-format.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1375
🏁 Script executed:
# Check one of the other affected manifests to see if they have the same pattern
cat -n scoop-examples/clang-tidy.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1382
Fix ARM64 executable mapping in bin.
Lines 16-22 hardcode the AMD64 filename; ARM64 installs download ...arm64.exe, causing the shim target to be missing. Move the bin array into each architecture-specific section with the correct binary name.
Proposed fix
"architecture": {
"64bit": {
"url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-format-22_windows-amd64.exe",
- "hash": "sha512:d3d47b543446e0b698ea0a5ddc7e7e2857d4c7fa6dccbc94e82e07dda0b5be37b51c7b794957b0263ecaa7b9992ed5b5554a62001ea1a9fe829e72e3a7f251df"
+ "hash": "sha512:d3d47b543446e0b698ea0a5ddc7e7e2857d4c7fa6dccbc94e82e07dda0b5be37b51c7b794957b0263ecaa7b9992ed5b5554a62001ea1a9fe829e72e3a7f251df",
+ "bin": [
+ [
+ "clang-format-22_windows-amd64.exe",
+ "clang-format"
+ ]
+ ]
},
"arm64": {
"url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-format-22_windows-arm64.exe",
- "hash": "sha512:40473adaf02241269e372a1a8f9acb33288f55da177a1e53a66d20c3433488f60c253fb84d9e2df4e78e855ac551abde1b8fcb92854ef50f8f7973f186c6dbe2"
+ "hash": "sha512:40473adaf02241269e372a1a8f9acb33288f55da177a1e53a66d20c3433488f60c253fb84d9e2df4e78e855ac551abde1b8fcb92854ef50f8f7973f186c6dbe2",
+ "bin": [
+ [
+ "clang-format-22_windows-arm64.exe",
+ "clang-format"
+ ]
+ ]
}
},
- "bin": [
- [
- "clang-format-22_windows-amd64.exe",
- "clang-format",
- ""
- ]
- ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "bin": [ | |
| [ | |
| "clang-format-22_windows-amd64.exe", | |
| "clang-format", | |
| "" | |
| ] | |
| "architecture": { | |
| "64bit": { | |
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-format-22_windows-amd64.exe", | |
| "hash": "sha512:d3d47b543446e0b698ea0a5ddc7e7e2857d4c7fa6dccbc94e82e07dda0b5be37b51c7b794957b0263ecaa7b9992ed5b5554a62001ea1a9fe829e72e3a7f251df", | |
| "bin": [ | |
| [ | |
| "clang-format-22_windows-amd64.exe", | |
| "clang-format" | |
| ] | |
| ] | |
| }, | |
| "arm64": { | |
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-format-22_windows-arm64.exe", | |
| "hash": "sha512:40473adaf02241269e372a1a8f9acb33288f55da177a1e53a66d20c3433488f60c253fb84d9e2df4e78e855ac551abde1b8fcb92854ef50f8f7973f186c6dbe2", | |
| "bin": [ | |
| [ | |
| "clang-format-22_windows-arm64.exe", | |
| "clang-format" | |
| ] | |
| ] | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/clang-format.json` around lines 16 - 21, The `bin` array
currently uses the hardcoded AMD64 filename `clang-format-22_windows-amd64.exe`,
which fails for ARM64 installations that require
`clang-format-22_windows-arm64.exe`. Remove the `bin` array from the global
level (lines 16-21) and duplicate it within each architecture-specific section,
using the correct binary name for AMD64 in the `64bit` section and the correct
ARM64 binary name in the `arm64` section.
| "bin": [ | ||
| [ | ||
| "clang-include-cleaner-22_windows-amd64.exe", | ||
| "clang-include-cleaner" | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n scoop-examples/clang-include-cleaner.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1476
Move the bin entry into architecture-specific blocks.
The manifest downloads different executables per architecture (clang-include-cleaner-22_windows-amd64.exe for 64bit, clang-include-cleaner-22_windows-arm64.exe for arm64), but the shared bin entry hardcodes the amd64 filename. ARM64 installations will fail to locate the executable, breaking the shim.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/clang-include-cleaner.json` around lines 16 - 20, The `bin`
entry is currently at the root level hardcoding the amd64 executable filename,
which breaks ARM64 installations. Remove the shared `bin` entry and instead add
architecture-specific `bin` entries within each architecture block (64bit and
arm64 sections). In the 64bit architecture block, map the amd64 executable
filename to the clang-include-cleaner shim, and in the arm64 architecture block,
map the arm64 executable filename to the clang-include-cleaner shim.
| "bin": [ | ||
| [ | ||
| "clang-query-22_windows-amd64.exe", | ||
| "clang-query" | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat scoop-examples/clang-query.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1242
🌐 Web query:
Scoop manifest architecture-specific bin mapping
💡 Result:
In Scoop, architecture-specific bin mapping is achieved by defining the bin field within an architecture-specific object inside the application manifest [1][2]. To configure bin paths that depend on the user's processor architecture (typically 32bit, 64bit, or arm64), you should place the bin definition inside the corresponding architecture block in your JSON manifest [1][3][4]. Scoop's internal logic uses the arch_specific function to prioritize these values over any globally defined bin field [3]. Example of architecture-specific bin mapping: { "version": "1.0.0", "architecture": { "64bit": { "url": "https://example.com/app-x64.zip", "bin": [ [ "app-x64.exe", "app" ] ] }, "32bit": { "url": "https://example.com/app-x86.zip", "bin": [ [ "app-x86.exe", "app" ] ] } } } Key Implementation Details: 1. Architecture Blocks: The architecture object supports keys such as 32bit, 64bit, and arm64 [1][3][5]. You place all architecture-dependent properties (including bin, url, hash, and installer) inside these blocks [1][2]. 2. Resolution Priority: When installing, Scoop detects the system architecture and resolves the bin mapping by looking for the property inside the specific architecture block first [3]. If the property is not found within the architecture block, it falls back to the top-level definition in the manifest (if provided) [3]. 3. Bin Format: The bin field can be a simple string (the executable name) or an array of arrays (e.g., [["executable.exe", "alias"]]) to define shim aliases and arguments [1][2]. 4. Best Practices: While individual fields like bin can be defined per architecture, there is currently no native support for variable interpolation (like an archvar) to make these declarations "DRY" (Don't Repeat Yourself); each architecture block must explicitly define its own bin requirements [6]. This mechanism ensures that the correct executable is shimmed to the PATH based on the user's environment [5][7].
Citations:
- 1: https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
- 2: https://scoop.netlify.app/concepts/
- 3: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/lib/manifest.ps1
- 4: https://catalog.lintel.tools/schemas/schemastore/scoop-manifest/
- 5: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/lib/install.ps1
- 6: [Feature] add
archvarinarchitectureand use it inenv_add_path,bin,shortcutsetc rather than stuffing these hard-coded insidearchitectureScoopInstaller/Scoop#5961 - 7: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/libexec/scoop-info.ps1
Move bin mapping into architecture-specific blocks.
The manifest downloads different executables per architecture (amd64.exe for 64bit, arm64.exe for arm64), but the top-level bin entry hard-codes only the amd64 filename. Scoop will create a shim pointing to a non-existent file on ARM64 systems. Define separate bin entries inside each architecture block:
Example fix
"architecture": {
"64bit": {
"url": "...",
"hash": "...",
"bin": [["clang-query-22_windows-amd64.exe", "clang-query"]]
},
"arm64": {
"url": "...",
"hash": "...",
"bin": [["clang-query-22_windows-arm64.exe", "clang-query"]]
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/clang-query.json` around lines 16 - 20, Remove the top-level
`bin` array from the manifest and move architecture-specific `bin` entries into
each `architecture` block. In the `64bit` architecture block, add a `bin` entry
with the clang-query-22_windows-amd64.exe filename mapped to clang-query. In the
`arm64` architecture block, add a separate `bin` entry with the
clang-query-22_windows-arm64.exe filename mapped to clang-query. This ensures
Scoop creates shims pointing to the correct executable for each architecture.
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-scan-deps-22_windows-amd64.exe", | ||
| "hash": "sha512:UPDATE_ME" | ||
| }, | ||
| "arm64": { | ||
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/clang-scan-deps-22_windows-arm64.exe", | ||
| "hash": "sha512:UPDATE_ME" | ||
| } | ||
| }, | ||
| "bin": [ | ||
| [ | ||
| "clang-scan-deps-22_windows-amd64.exe", | ||
| "clang-scan-deps" | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat scoop-examples/clang-scan-deps.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1035
🌐 Web query:
Scoop manifest architecture-specific bin entry amd64 arm64 how to configure
💡 Result:
In Scoop, architecture-specific configurations for the 'bin' entry are defined within the 'architecture' object in the app manifest [1][2]. You can create separate sub-objects for '64bit', '32bit', or 'arm64' to specify unique 'bin' paths for each [1][3]. To configure architecture-specific 'bin' entries, nest the 'bin' property inside the corresponding architecture block [1][4]. Scoop will prioritize the architecture-specific configuration if it exists; otherwise, it falls back to the top-level (generic) 'bin' definition [5]. Example manifest structure for architecture-specific 'bin' entries: { "version": "1.0.0", "architecture": { "64bit": { "url": "https://example.com/app-x64.zip", "hash": "...", "bin": [ [ "bin\app_x64.exe", "app" ] ] }, "arm64": { "url": "https://example.com/app-arm64.zip", "hash": "...", "bin": [ [ "bin\app_arm64.exe", "app" ] ] } } } Key Implementation Details: - Nesting: Properties like 'bin', 'url', and 'hash' can be placed inside the architecture sub-objects to override global defaults [1][2]. - Format: The 'bin' property accepts a string or an array of strings [3][2]. When using aliases (renaming the executable), use an array format like [ "path\to\exe", "alias" ], and ensure this is wrapped in an outer array if defining a single bin entry [4][2]. - Fallback Behavior: Scoop's internal logic ('arch_specific' function) automatically checks the architecture-specific block first. If the property is missing there, it falls back to the global manifest property [5]. - ARM64 Considerations: Scoop supports 'arm64' as an architecture key [1][3]. Note that on some Windows versions, Scoop may map 'arm64' requests to '64bit' or '32bit' if a native 'arm64' manifest entry is not explicitly provided and the system supports emulation [5].
Citations:
- 1: https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
- 2: https://scoop.netlify.app/concepts/
- 3: https://catalog.lintel.tools/schemas/schemastore/scoop-manifest/
- 4: How to add shortcuts for app which has different bin for each architecture ScoopInstaller/Scoop#1507
- 5: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/lib/manifest.ps1
Complete the manifest: add missing hashes and configure architecture-specific bin entries.
Both hashes remain placeholders (sha512:UPDATE_ME) and cannot be published as-is. Additionally, the bin entry hard-codes the amd64 executable name at the top level. When installing on ARM64, Scoop will fall back to this top-level definition, causing the command shim to point to the non-existent amd64 binary, breaking ARM64 installations.
Move the bin entry into each architecture block so the amd64 variant references clang-scan-deps-22_windows-amd64.exe and the arm64 variant references clang-scan-deps-22_windows-arm64.exe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/clang-scan-deps.json` around lines 8 - 20, The manifest file
has two issues that must be fixed: the sha512 hash values in both the amd64 and
arm64 blocks are placeholders (UPDATE_ME) that need to be replaced with actual
computed hashes for their respective executables, and the bin array is
incorrectly placed at the top level hard-coding only the amd64 executable name.
Move the bin array into each architecture block so that the amd64 block
references clang-scan-deps-22_windows-amd64.exe and the arm64 block references
clang-scan-deps-22_windows-arm64.exe, ensuring ARM64 installations use the
correct executable. Compute the actual SHA512 hashes for each binary and replace
all UPDATE_ME placeholders.
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/llvm-cov-22_windows-amd64.exe", | ||
| "hash": "sha512:UPDATE_ME" | ||
| }, | ||
| "arm64": { | ||
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/llvm-cov-22_windows-arm64.exe", | ||
| "hash": "sha512:UPDATE_ME" | ||
| } | ||
| }, | ||
| "bin": [ | ||
| [ | ||
| "llvm-cov-22_windows-amd64.exe", | ||
| "llvm-cov" | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cat scoop-examples/llvm-cov.jsonRepository: cpp-linter/clang-tools-static-binaries
Length of output: 966
🏁 Script executed:
# Search for any existing patterns in other Scoop manifests in the repo
find . -name "*.json" -path "*/scoop-examples/*" -type f | head -5Repository: cpp-linter/clang-tools-static-binaries
Length of output: 373
🌐 Web query:
Scoop manifest bin entry architecture-specific handling amd64 arm64
💡 Result:
In Scoop, architecture-specific handling is managed primarily through the architecture manifest object [1]. When a package needs to provide different binaries or configurations for amd64 (64-bit), 32-bit (x86), or arm64, you define these settings inside the architecture block [1]. Architecture-Specific Binaries To handle architecture-specific bin entries, you nest the bin directive within the corresponding architecture key [1]. Scoop evaluates the user's system architecture at installation time and selects the appropriate block [2][3]. Example structure: { "architecture": { "64bit": { "bin": [ ["bin64/app.exe", "app"] ] }, "arm64": { "bin": [ ["bin-arm64/app.exe", "app"] ] }, "32bit": { "bin": [ ["bin32/app.exe", "app"] ] } } } Mechanism and Selection 1. Evaluation: During installation, Scoop's helper function arch_specific resolves the properties [2]. It first checks if the property (such as bin) exists under the specific architecture key; if not found, it falls back to the global manifest property [2]. 2. Arm64 Handling: Scoop includes logic to handle arm64 architecture, including fallbacks for older Windows builds that do not natively support arm64 apps [2]. 3. Overriding Architecture: Users can force a specific architecture during installation using the --arch command-line flag (e.g., scoop install --arch 32bit), provided the manifest supports that architecture [4]. 4. Shims: The create_shims function retrieves the bin definition based on the resolved architecture and creates the necessary shims to make the programs available on the PATH [3]. Current Limitations While you can define different bin paths per architecture, there is currently no native support for variable interpolation (like an $arch_suffix variable) across all fields within the architecture object [5]. This often leads to repetitive manifests where bin paths must be explicitly declared for each architecture block, even if the structure is similar [5].
Citations:
- 1: https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
- 2: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/lib/manifest.ps1
- 3: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/lib/install.ps1
- 4: https://github.com/ScoopInstaller/Scoop/blob/b588a06e/libexec/scoop-install.ps1
- 5: [Feature] add
archvarinarchitectureand use it inenv_add_path,bin,shortcutsetc rather than stuffing these hard-coded insidearchitectureScoopInstaller/Scoop#5961
Fix the manifest before publishing: add missing hashes and move bin entry to architecture-specific blocks.
Both hashes are placeholder values (sha512:UPDATE_ME) and cannot install. The global bin entry hard-codes the amd64 executable name, which will cause ARM64 installs to fail—Scoop falls back to the global definition when no architecture-specific bin is found. Move bin into each architecture block with the correct executable name per architecture, and fill in actual hash values for both 64bit and arm64.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/llvm-cov.json` around lines 8 - 20, The manifest file has two
issues that prevent proper installation across architectures. First, both hash
values in the "64bit" and "arm64" blocks contain placeholder text
"sha512:UPDATE_ME" which must be replaced with actual SHA512 hash values
computed from the respective executable files. Second, the global "bin" entry
hard-codes the amd64 executable name "llvm-cov-22_windows-amd64.exe", which will
cause ARM64 installations to fail since Scoop cannot find the correct
executable. Move the "bin" entry into each architecture-specific block—place a
"bin" array within the "64bit" block mapping the amd64 executable to "llvm-cov",
and place a separate "bin" array within the "arm64" block mapping the arm64
executable to "llvm-cov", then remove the global "bin" entry.
| "hash": "sha512:UPDATE_ME" | ||
| }, | ||
| "arm64": { | ||
| "url": "https://github.com/cpp-linter/clang-tools-static-binaries/releases/download/2026.06.15-a56c0263/llvm-profdata-22_windows-arm64.exe", | ||
| "hash": "sha512:UPDATE_ME" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
Replace placeholder SHA512 hashes before release.
Lines 9 and 13 contain sha512:UPDATE_ME placeholders. Scoop manifests require actual hashes for download integrity verification; these placeholders will block installation for both architectures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scoop-examples/llvm-profdata.json` around lines 9 - 14, Replace the
placeholder SHA512 hashes in the manifest with actual hashes for download
integrity verification. For the x64 architecture entry, compute the SHA512 hash
of the Windows binary from the provided URL and replace the sha512:UPDATE_ME
placeholder on line 9. Similarly, for the arm64 architecture entry, compute the
SHA512 hash of the corresponding Windows-arm64 binary and replace the
sha512:UPDATE_ME placeholder on line 13. Ensure both hash fields contain
complete, valid SHA512 digests to allow Scoop to verify downloads and enable
installation for both architectures.
Add four new tools to the static binary build pipeline: - llvm-cov: code coverage reporting - llvm-profdata: profile data manipulation (merge, show, etc.) - llvm-symbolizer: address-to-source-line symbolization - clang-scan-deps: C/C++ module and header dependency scanning Changes: - build.py: add tools to TOOLS list with version constraints (clang-scan-deps requires LLVM 12+, clang-include-cleaner requires 18+); add end-to-end smoke tests for each new tool - build.yml: update upload-artifact pattern to include llvm-* binaries - release.py: expand versions.json with tools list (and per-tool min_llvm_version constraints) and supported platforms — this is now the single source of truth for all downstream channels - README.md: update description, support matrix with 4 new tools, and versions.json documentation - CONTRIBUTING.md: update tools list and versions.json description - scoop-examples/: add manifest examples for the 4 new tools; update README with new tool table
shenxianpeng
force-pushed
the
feature/add-llvm-cov-profdata-symbolizer-scan-deps
branch
from
3ed82a4 to
7c3524d
Compare
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.py`:
- Around line 124-145: The temporary coverage test program in build.py is
returning a non-zero exit status because main() directly returns foo(42),
causing run([str(test_bin)]) to fail before .profraw is generated. Update the
generated test source in the coverage test setup so main() exits successfully
after calling foo(42) (or otherwise discards the result), and keep the change
consistent across the duplicated coverage test block referenced by the build
helper.
- Around line 723-747: Build and use a dedicated smoke-test clang instead of
assuming it already exists in bins: the smoke helpers are using clang_exe and
smoke_clang_scan_deps, but clang is not among the shipped tools and may be
missing or version-mismatched. Update the build flow to produce clang as a
non-shipped dependency for the smoke inputs, then pass that explicit clang path
into the profdata/cov/symbolizer compile steps and into smoke_clang_scan_deps
instead of relying on plain "clang" from PATH.
- Around line 373-381: The p1689 smoke test currently warns and continues when
json.loads fails in the p1689 validation block, which hides regressions. Update
the try/except around result_p1689.stdout so that the JSONDecodeError path fails
the smoke test instead of printing a warning, and keep the existing validation
logic in build.py’s p1689 check so invalid JSON from -format=p1689 causes the
test to stop immediately.
- Around line 196-207: The llvm-cov smoke test is asserting on a function name
even though llvm-cov report may only emit file totals by default. Update the
subprocess invocation in the coverage check to use function-level output, either
by adding the -show-functions flag to the llvm-cov report call or by switching
the command to llvm-cov show, then keep the existing assertion that looks for
the function name in result.stdout/result.stderr.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84f19b2f-bc4e-4426-a77a-12050c928ae8
📒 Files selected for processing (5)
.github/workflows/build.ymlCONTRIBUTING.mdREADME.mdbuild.pyrelease.py
✅ Files skipped from review due to trivial changes (2)
- CONTRIBUTING.md
- README.md
| "int foo(int x) { return x * x; }\nint main(void) { return foo(42); }\n", | ||
| ) | ||
|
|
||
| test_bin = tmpdir / ("profraw_test" + dot_exe) | ||
| profraw = tmpdir / "test.profraw" | ||
| profdata = tmpdir / "test.profdata" | ||
|
|
||
| # Compile with instrumentation | ||
| run( | ||
| [ | ||
| str(clang_exe), | ||
| "-fprofile-instr-generate", | ||
| "-fcoverage-mapping", | ||
| "-o", | ||
| str(test_bin), | ||
| str(src), | ||
| ] | ||
| ) | ||
|
|
||
| # Run to produce a .profraw | ||
| env = {**os.environ, "LLVM_PROFILE_FILE": str(profraw)} | ||
| run([str(test_bin)], env=env) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Make the generated coverage test binary exit successfully.
main() returns foo(42), so the temporary binary exits non-zero and run([str(test_bin)]) fails with check=True before producing usable coverage data.
🐛 Proposed fix
- "int foo(int x) { return x * x; }\nint main(void) { return foo(42); }\n",
+ "int foo(int x) { return x * x; }\nint main(void) { (void)foo(42); return 0; }\n",Also applies to: 177-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.py` around lines 124 - 145, The temporary coverage test program in
build.py is returning a non-zero exit status because main() directly returns
foo(42), causing run([str(test_bin)]) to fail before .profraw is generated.
Update the generated test source in the coverage test setup so main() exits
successfully after calling foo(42) (or otherwise discards the result), and keep
the change consistent across the duplicated coverage test block referenced by
the build helper.
| result = subprocess.run( | ||
| [str(cov_exe), "report", str(test_bin), "-instr-profile", str(profdata)], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if result.returncode != 0: | ||
| print(f" stderr: {result.stderr}") | ||
| raise RuntimeError(f"llvm-cov report failed (exit {result.returncode})") | ||
| # Verify the report contains our function name | ||
| assert "foo" in result.stdout or "foo" in result.stderr, ( | ||
| f"Expected 'foo' in llvm-cov report, got:\n{result.stdout}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate build.py and inspect the relevant section with line numbers.
git ls-files | rg '^build\.py$'
wc -l build.py
sed -n '170,230p' build.py | cat -nRepository: cpp-linter/clang-tools-static-binaries
Length of output: 2675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any llvm-cov usage in build.py and surrounding tests/docs.
rg -n "llvm-cov|show-functions|report" build.py README.md .github tests || trueRepository: cpp-linter/clang-tools-static-binaries
Length of output: 1498
🌐 Web query:
llvm-cov report -show-functions option documentation output function names file totals llvm-cov show
💡 Result:
The -show-functions option in the llvm-cov tool is used to display coverage summaries for each function rather than providing a summary only for entire files [1][2]. Key details regarding this option include: - Functionality: When enabled, it generates coverage reports at the function level for the specified source files [1][3]. - Requirement: The option defaults to false [1][4]. If used, source files must generally be specified, as the command requires them to generate function-specific summaries [5]. - Commands: It is primarily used with the llvm-cov report and llvm-cov show commands [1][4]. - Output: For the report command, it renders a table or list of function-level metrics [5]. If an output directory is specified via -output-dir, function reports may be written to specific files (e.g., PATH/functions.EXTENSION) [6]. For broader coverage reporting, the command provides a summary line for each file in the coverage data if no source files are provided [1][6]. When -show-functions is enabled, it shifts the focus to providing these detailed breakdowns for each function within the provided files [1][5].
Citations:
- 1: https://llvm.org/docs/CommandGuide/llvm-cov.html
- 2: https://releases.llvm.org/20.1.0/docs/CommandGuide/llvm-cov.html
- 3: https://github.com/llvm/llvm-project/blob/main/llvm/docs/CommandGuide/llvm-cov.rst
- 4: https://releases.llvm.org/22.1.0/docs/CommandGuide/llvm-cov.html
- 5: https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-cov/CodeCoverage.cpp
- 6: https://man.openbsd.org/llvm-cov
🌐 Web query:
llvm-cov report -show-functions option documentation output function names file totals llvm-cov show
💡 Result:
In the llvm-cov tool, the -show-functions option is used with the show and report subcommands to display coverage summaries for each function, rather than just providing a summary for the entire source file [1][2]. By default, the -show-functions option is set to false [1][3]. When enabled, it allows users to view detailed coverage information at the function level for the files specified in the command [1][4]. When using llvm-cov show without providing specific source files, the tool prints a summary line for every file present in the coverage data [1][5]. If source files are provided, you can use the -show-functions option to force the output to include summaries for each individual function within those specific files [1][3]. The -show-functions option is a standard feature in both the show and report subcommands across various versions of LLVM [1][2].
Citations:
- 1: https://llvm.org/docs/CommandGuide/llvm-cov.html
- 2: https://releases.llvm.org/16.0.0/docs/CommandGuide/llvm-cov.html
- 3: https://github.com/llvm/llvm-project/blob/main/llvm/docs/CommandGuide/llvm-cov.rst
- 4: https://releases.llvm.org/20.1.0/docs/CommandGuide/llvm-cov.html
- 5: https://man.openbsd.org/llvm-cov
Use function-level output before asserting on foo. llvm-cov report here can omit function names and only emit file totals, so the smoke test may fail even when coverage generation succeeds. Add -show-functions or switch to llvm-cov show before this assertion.
🧰 Tools
🪛 Ruff (0.15.18)
[error] 196-196: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.py` around lines 196 - 207, The llvm-cov smoke test is asserting on a
function name even though llvm-cov report may only emit file totals by default.
Update the subprocess invocation in the coverage check to use function-level
output, either by adding the -show-functions flag to the llvm-cov report call or
by switching the command to llvm-cov show, then keep the existing assertion that
looks for the function name in result.stdout/result.stderr.
| try: | ||
| data = json.loads(result_p1689.stdout) | ||
| assert "revision" in data or "rules" in data or "provides" in data, ( | ||
| f"p1689 output missing expected keys:\n{result_p1689.stdout}" | ||
| ) | ||
| print(" p1689 format validated") | ||
| except json.JSONDecodeError as exc: | ||
| print(f" [warn] p1689 output not valid JSON: {exc}") | ||
| else: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail when -format=p1689 returns invalid JSON.
If the command exits successfully but emits invalid JSON, the smoke test should fail; warning and continuing hides exactly the regression this branch is meant to catch.
🐛 Proposed fix
try:
data = json.loads(result_p1689.stdout)
assert "revision" in data or "rules" in data or "provides" in data, (
f"p1689 output missing expected keys:\n{result_p1689.stdout}"
)
print(" p1689 format validated")
except json.JSONDecodeError as exc:
- print(f" [warn] p1689 output not valid JSON: {exc}")
+ raise RuntimeError(f"p1689 output not valid JSON: {exc}") from exc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| data = json.loads(result_p1689.stdout) | |
| assert "revision" in data or "rules" in data or "provides" in data, ( | |
| f"p1689 output missing expected keys:\n{result_p1689.stdout}" | |
| ) | |
| print(" p1689 format validated") | |
| except json.JSONDecodeError as exc: | |
| print(f" [warn] p1689 output not valid JSON: {exc}") | |
| else: | |
| try: | |
| data = json.loads(result_p1689.stdout) | |
| assert "revision" in data or "rules" in data or "provides" in data, ( | |
| f"p1689 output missing expected keys:\n{result_p1689.stdout}" | |
| ) | |
| print(" p1689 format validated") | |
| except json.JSONDecodeError as exc: | |
| raise RuntimeError(f"p1689 output not valid JSON: {exc}") from exc | |
| else: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.py` around lines 373 - 381, The p1689 smoke test currently warns and
continues when json.loads fails in the p1689 validation block, which hides
regressions. Update the try/except around result_p1689.stdout so that the
JSONDecodeError path fails the smoke test instead of printing a warning, and
keep the existing validation logic in build.py’s p1689 check so invalid JSON
from -format=p1689 causes the test to stop immediately.
| clang_exe = bins / f"clang{dot_exe}" | ||
|
|
||
| # All tools get the basic --version smoke test | ||
| for tool in tools: | ||
| exe = bins / f"{tool}{dot_exe}" | ||
| print(f"\nSmoke-testing {exe} ...") | ||
| run([str(exe), "--version"]) | ||
|
|
||
| # Tool-specific smoke tests that exercise real functionality | ||
| import tempfile | ||
|
|
||
| with tempfile.TemporaryDirectory(prefix="smoke_") as tmpdir_str: | ||
| smokes = Path(tmpdir_str) | ||
|
|
||
| if "llvm-profdata" in tools: | ||
| smoke_llvm_profdata(bins, dot_exe, smokes, clang_exe) | ||
|
|
||
| if "llvm-cov" in tools: | ||
| smoke_llvm_cov(bins, dot_exe, smokes, clang_exe) | ||
|
|
||
| if "llvm-symbolizer" in tools: | ||
| smoke_llvm_symbolizer(bins, dot_exe, smokes, clang_exe) | ||
|
|
||
| if "clang-scan-deps" in tools: | ||
| smoke_clang_scan_deps(bins, dot_exe, smokes, version) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Build and use a matching clang for the smoke inputs.
Line 723 assumes bins/clang exists, but the build targets come from tools, which does not include clang. The new profdata/cov/symbolizer smoke tests will fail before artifact renaming if the clang executable is not built. clang-scan-deps also uses plain "clang" from PATH, which can be absent or version-mismatched. Build clang as a non-shipped smoke dependency and use that path in the compile database.
🐛 Proposed fix
- tools = active_tools(version)
+ tools = active_tools(version)
+ build_tools = list(tools)
+ if any(
+ tool in tools
+ for tool in ("llvm-profdata", "llvm-cov", "llvm-symbolizer", "clang-scan-deps")
+ ):
+ build_tools.append("clang")
build_cmd = (
[
"cmake",
"--build",
str(build_dir),
]
+ build_args_by_os(is_windows)
+ ["--target"]
- + tools
+ + build_tools
) def smoke_clang_scan_deps(
bins: Path,
dot_exe: str,
tmpdir: Path,
version: str,
+ clang_exe: Path,
) -> None:- "clang",
+ str(clang_exe),- smoke_clang_scan_deps(bins, dot_exe, smokes, version)
+ smoke_clang_scan_deps(bins, dot_exe, smokes, version, clang_exe)Also applies to: 324-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.py` around lines 723 - 747, Build and use a dedicated smoke-test clang
instead of assuming it already exists in bins: the smoke helpers are using
clang_exe and smoke_clang_scan_deps, but clang is not among the shipped tools
and may be missing or version-mismatched. Update the build flow to produce clang
as a non-shipped dependency for the smoke inputs, then pass that explicit clang
path into the profdata/cov/symbolizer compile steps and into
smoke_clang_scan_deps instead of relying on plain "clang" from PATH.
1 participant
Summary
Adds 4 new tools to the clang-tools-static-binaries build pipeline: llvm-cov, llvm-profdata, llvm-symbolizer, and clang-scan-deps.
All four are standard LLVM/Clang build targets — no extra build system changes needed. They build on all 6 supported platforms (Linux x64/arm64, macOS x64/arm64, Windows x64/arm64).
What changed
build.pyTOOLSlistCLANG_SCAN_DEPS_MIN_VERSION = 12(clang-scan-deps was introduced in LLVM 12; llvm-cov/profdata/symbolizer are available in all supported versions)-fprofile-instr-generate -fcoverage-mapping, run it to produce a.profraw, thenllvm-profdata merge+llvm-profdata show.profdatato runllvm-cov reportand verify the function name appears-g, resolvetest_func's address vianm/dumpbin, feed tollvm-symbolizerand verify the output contains the symbol namecompile_commands.json, run the dependency scanner, verify it parses the source file; additionally test-format=p1689on LLVM 16+.github/workflows/build.ymlupload-artifactpath pattern to include bothclang-*andllvm-*binariesrelease.pyversions.jsonto include:tools: list of all 9 shipped tools, each with itsmin_llvm_versionconstraint (if any)platforms: the 6 supported platform stringsREADME.mdversions.jsondocumentationCONTRIBUTING.md,scoop-examples/Testing
--versionsmoke tests (same as existing tools)test-releasejob loops over all uploaded binaries and runs--version, so it will automatically cover the new toolsversions.json format (expanded)
{ "built_at": "...", "release_tag": "...", "llvm_versions": { "22": "llvm-project-22.1.0.src", ... }, "tools": { "clang-format": {}, "clang-include-cleaner": { "min_llvm_version": 18 }, "clang-scan-deps": { "min_llvm_version": 12 }, "llvm-cov": {}, ... }, "platforms": ["linux-amd64", ...] }Closes #...
Summary by CodeRabbit