Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Versions follow semver beta: `1.0.0-beta.N`, bumped on each dev->master promotio
### Fixed
- Installer: agent-container runtime install now prefers the `incus-base` package over the full `incus` metapackage, whose extras have unsatisfiable dependencies on Debian Bookworm ARM64 (held broken packages); falls back to `incus` where incus-base is not a candidate (#1555).
- Models: a download that finishes the transfer but wrote no data (or the wrong number of bytes) is no longer marked complete. Both the torrent and HTTP paths now validate the file exists, is non-empty, matches the expected size, and passes its checksum before the model is reported installed (#1548).
- Installer: the RK3588 NPU install no longer aborts on the first clean run on boards that have binutils. The librknnrt version check piped `strings` (a 7MB binary) into an `awk` that exited on the first match; the early exit SIGPIPEd `strings`, which under `set -o pipefail` + `set -e` killed the whole install. It succeeded on a second run only because the pin was already applied and the block was skipped. The check now reads to EOF and is guarded (#1560, #1543). Reported by @mandresve.

## [1.0.0-beta.17] - 2026-07-02

Expand Down
32 changes: 30 additions & 2 deletions scripts/install-rknpu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ die() { printf '\033[1;31m[rknpu]\033[0m %s\n' "$*" >&2; exit 1; }
# never drift if upstream rewords the version string.
librknnrt_current_version() {
if [[ -f "$LIBRKNNRT_DEST" ]] && command -v strings >/dev/null 2>&1; then
strings "$LIBRKNNRT_DEST" | awk '/librknnrt version: / { print $3; exit }'
# No early awk exit: it SIGPIPEs strings and trips pipefail (see the
# note at the version-verify site). Read to EOF and print the first
# match only. Do not swallow strings' stderr: if it fails on the
# installed library (vendor perms, corrupted ELF) the real error should
# surface in the log rather than the banner silently dropping the
# runtime line. Both callers guard the call with `|| true`, so a probe
# failure cannot abort the install.
strings "$LIBRKNNRT_DEST" | awk '/librknnrt version: / && !seen { print $3; seen=1 }'
fi
}

Expand Down Expand Up @@ -337,7 +344,28 @@ pin_librknnrt() {
# when the tool is unavailable; only die on a genuine version mismatch.
local verified=""
if command -v strings >/dev/null 2>&1; then
verified="$(strings "$tmp" | awk '/librknnrt version: / { print $3; exit }')"
# awk must NOT exit early here: an early `exit` closes the pipe and
# SIGPIPEs `strings`, which under `set -o pipefail` fails the whole
# command substitution, and with `set -e` that aborted the ENTIRE
# install on the first clean run wherever binutils was present (#1560,
# #1543). The second run only worked because librknnrt was already at
# 2.3.0 and this whole pin block was skipped. Read to EOF (no early
# exit) and guard the assignment so a strings/awk hiccup is never fatal.
# Capture the substitution status so a genuine strings/awk failure is
# not silently indistinguishable from "no version string" (which used
# to mean only "binutils missing"); warn so a future first-run failure
# stays diagnosable.
# Do NOT swallow strings' stderr: if the probe fails (truncated
# download, bad perms, corrupted ELF) its real error is the diagnostic,
# so let it through to the log alongside the warn below. On a valid
# runtime strings is quiet on stderr, so this adds no normal-path noise.
if verified="$(strings "$tmp" | awk '/librknnrt version: / && !seen { print $3; seen=1 }')"; then
:
else
# A failed substitution already leaves $verified empty, so no reset
# is needed; the fallback below re-reads the installed path.
warn "strings/awk version probe failed on the downloaded runtime; falling back to the installed path"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: The warn here is a step up from silent failure, but it still loses the root cause: strings "$tmp" 2>/dev/null discards strings's own stderr, so when the probe genuinely fails (truncated download, perms, corrupted ELF, etc.) the operator only sees "strings/awk version probe failed on the downloaded runtime" with no clue why. Either drop the 2>/dev/null on strings so its real error surfaces, or capture and include it in the warn, e.g. if ! err="$(strings "$tmp" 2>&1 >/dev/null)"; then warn "strings failed: $err"; fi paired with a separate stdout-only capture. Also, verified="" on the next line is redundant — when the if-condition fails, verified is already empty from the failed substitution.

Suggested change
warn "strings/awk version probe failed on the downloaded runtime; falling back to the installed path"
if verified="$(strings "$tmp" 2>/dev/null | awk '/librknnrt version: / && !seen { print $3; seen=1 }')"; then
:
else
warn "strings/awk version probe failed on the downloaded runtime; falling back to the installed path"
fi

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

fi
if [[ -z "$verified" ]]; then
# Fall back to re-reading the installed path (legacy behaviour).
verified="$(librknnrt_current_version || true)"
Expand Down
Loading