fix: support distributions that report no OS version (Arch, rolling releases) - #525
fix: support distributions that report no OS version (Arch, rolling releases)#525moha-bekh wants to merge 3 commits into
Conversation
`SupportedOs::from_os` read `sysinfo::System::os_version()` before looking at the OS at all and bailed with "Failed to get OS version" when it was absent. That value comes from `VERSION_ID` in `/etc/os-release`, which rolling releases do not ship: on Arch (`ID=archarm`, `BUILD_ID=rolling`) every command aborted before it could determine anything about the host. The version only matters for the distributions we publish packages for, and those all expose one — the rest are already handled by `is_supported()`. So on Linux fall back to `"unknown"` with a `debug!` rather than failing, and keep the hard error on macOS, where the version is always available and is what we report to the API. Refs COD-3072 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VZZqxhWtwHhurnV2hsJ48H
`install_valgrind` went straight to `apt::install_cached`, which resolves the `valgrind-codspeed` deb for the host through `get_codspeed_valgrind_target`. That mapping only covers the Debian/Ubuntu versions we publish packages for, so on anything else — Arch and other rolling releases, non-apt distributions — it bailed with a bare "Unsupported system" and the run stopped at setup. Yet `ValgrindExecutor::support_level` already reports `RequiresManualInstallation` for those hosts, so the executor was advertising a path that setup refused to take. Mirror that support level in the setup: when no package exists for the host, return early if a valgrind installation is already present, and otherwise fail with an error that says CodSpeed publishes nothing for this distribution and points at valgrind-codspeed for a manual install. The libc debug symbol check is already skipped on non-apt systems, so an existing build is enough. Refs COD-3072 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR allows Linux distributions without a reported OS version to initialize using an
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking diagnostic gap for manual installations on unsupported Ubuntu or Debian versions. The changed setup branch correctly avoids automatic package installation where no package exists, but its error message cannot guide users whose compatible Valgrind installation fails only because libc debug symbols are missing. Files Needing Attention: src/executor/valgrind/setup.rs
|
| Filename | Overview |
|---|---|
| src/system/os.rs | Linux now falls back to an unknown version while preserving strict macOS detection; no concrete compatibility defect was identified. |
| src/executor/valgrind/setup.rs | Manual installations are now accepted outside the package matrix, but the failure guidance does not mention the libc debug symbols required on apt-compatible hosts. |
Prompt To Fix All With AI
### Issue 1
src/executor/valgrind/setup.rs:260-264
**Manual requirement omitted**
On unsupported Ubuntu or Debian versions, `is_valgrind_installed` also requires resolvable libc debug symbols, but this error only instructs users to install `valgrind-codspeed`. A user who follows that instruction can receive the same error again without learning that the debug-symbol package is the remaining requirement.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(valgrind): accept a manual installat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| bail!( | ||
| "CodSpeed does not publish a valgrind package for {}, so it cannot be installed automatically. \ | ||
| Install valgrind-codspeed {} or higher manually, see https://github.com/CodSpeedHQ/valgrind-codspeed", | ||
| system_info.os, | ||
| VALGRIND_CODSPEED_VERSION_STRING.as_str() |
There was a problem hiding this comment.
On unsupported Ubuntu or Debian versions, is_valgrind_installed also requires resolvable libc debug symbols, but this error only instructs users to install valgrind-codspeed. A user who follows that instruction can receive the same error again without learning that the debug-symbol package is the remaining requirement.
Knowledge Base Used: Valgrind measurement
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/executor/valgrind/setup.rs
Line: 260-264
Comment:
**Manual requirement omitted**
On unsupported Ubuntu or Debian versions, `is_valgrind_installed` also requires resolvable libc debug symbols, but this error only instructs users to install `valgrind-codspeed`. A user who follows that instruction can receive the same error again without learning that the debug-symbol package is the remaining requirement.
**Knowledge Base Used:** [Valgrind measurement](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/valgrind-measurement.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Merging this PR will not alter performance
|
On the systems we publish no valgrind package for (rolling releases, non-apt distributions, ...), the setup used to give up immediately and ask for a manual installation. Try a best-effort source build instead: check the build toolchain, clone the sources, compile them and install system-wide, only falling back to the manual instructions when any of those steps fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Refs COD-3072
The CLI aborted on Arch Linux before it could do anything:
SupportedOs::from_osreadsysinfo::System::os_version()up front and failed with "Failed to get OS version" when it was absent. That value comes fromVERSION_IDin/etc/os-release, which rolling releases do not ship.Fixing that surfaced a second failure right after, at setup:
install_valgrindwent straight to thevalgrind-codspeeddeb, which only exists for the Debian/Ubuntu versions we publish — so it bailed with a bare "Unsupported system", even thoughValgrindExecutor::support_levelalready advertisesRequiresManualInstallationfor those hosts.Changes
src/system/os.rs— on Linux, fall back to"unknown"with adebug!instead of failing. The version only matters for the distributions we ship packages for, and those all expose one; the rest are handled byis_supported(). macOS keeps the hard error, since the version is always available there et is what we report to the API.src/executor/valgrind/setup.rs— mirror the executor's support level.