diff --git a/CHANGELOG.md b/CHANGELOG.md index 478e503..80c60e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. - Binary-probe fallback in `guide.sh` when the post-install snapshot refresh is stale. ### Fixed +- `make upgrade` hid every pinned tool, whatever the pin. A release skipped with `s` ("ask again when newer patch available") hid the tool for good. A pin now hides a tool only while it is `never`, equals the target release (`s`), equals the installed version (`p`), or equals the cycle. +- `make upgrade` auto-update no longer reports an upgrade as "Updated" just because the install script exited 0. The version is compared after the re-audit, the same check the interactive `Y`/`a` answers use; an unchanged version counts as "Failed" with the old and target version. A package manager without a newer package (`bwrap` on apt) and a binary identical to the target release with a stale version string (`sd` 1.1.0 reports 1.0.0) count as "Skipped". "Held back" requires the install command to succeed and, for apt, the installed package to be the candidate and to own the binary found on PATH; otherwise the unchanged version counts as "Failed". Without an upstream version the result is reported as unverified ("Skipped"). - difftastic 0.71.0 puts the version into its release file names (`difft-0.71.0-x86_64-unknown-linux-gnu.tar.gz`); the catalog download URL now includes it. byobu is tagged `trustmux-v7.19` since the trustmux rename, and those tags fill the first page of the tags API, so the installer found no stable tag; it now accepts both tag forms. - `cmd_update_local` in MERGE mode now refreshes multi-version cycle entries (`python@3.14`, …) instead of only the base-tool entry. Resolved false-negative "Upgrade did not succeed" messages after successful uv installs. diff --git a/scripts/guide.sh b/scripts/guide.sh index 0208725..a1147c4 100755 --- a/scripts/guide.sh +++ b/scripts/guide.sh @@ -2,6 +2,7 @@ set -euo pipefail trap '' PIPE # Graceful interrupt handling +# shellcheck disable=SC2034 # written by the INT trap INTERRUPTED=0 trap 'INTERRUPTED=1; echo; echo "⚠️ Interrupted. Partial summary:"; print_summary; exit 130' INT @@ -23,6 +24,9 @@ LAST_MULTI_COUNT=0 # summary hint); de-duplicated when counted. GUIDE_DUP_LIST="" +# Installers leave .already-current / .held-back here +MARKER_DIR="${CLI_AUDIT_MARKER_DIR:-/tmp/.cli-audit}" + # Summary counters SUMMARY_UPDATED=0 SUMMARY_SKIPPED=0 @@ -87,8 +91,10 @@ CACHE_MAX_AGE_HOURS="${CACHE_MAX_AGE_HOURS:-24}" check_cache_age() { [ ! -f "$SNAP_FILE" ] && { echo "⚠️ Warning: Snapshot cache missing" >&2; return 1; } - local now=$(date +%s) - local snap_time=$(stat -c %Y "$SNAP_FILE" 2>/dev/null || stat -f %m "$SNAP_FILE" 2>/dev/null || echo 0) + local now + now=$(date +%s) || true + local snap_time + snap_time=$(stat -c %Y "$SNAP_FILE" 2>/dev/null || stat -f %m "$SNAP_FILE" 2>/dev/null || echo 0) || true local age_hours=$(( (now - snap_time) / 3600 )) if [ $age_hours -gt $CACHE_MAX_AGE_HOURS ]; then echo "⚠️ Warning: Snapshot cache is ${age_hours}h old (threshold: ${CACHE_MAX_AGE_HOURS}h)" >&2 @@ -183,6 +189,26 @@ osc8() { [ -n "$url" ] && printf '\e]8;;%s\e\\%s\e]8;;\e\\' "$url" "$text" || printf '%s' "$text" } +# PATH without virtualenv/conda bin dirs: the dirs the Python audit treats as +# environments (pyvenv.cfg next to bin/, venv/conda name patterns) +installation_path() { + local dir parent out="" + local -a dirs=() + IFS=: read -ra dirs <<<"$PATH" + for dir in "${dirs[@]}"; do + [ -n "$dir" ] || continue + # PEP 405 venvs carry pyvenv.cfg next to bin/ (no dirname: PATH may lack it) + parent="${dir%/}" + parent="${parent%/*}" + [ -f "$parent/pyvenv.cfg" ] && continue + case "${dir%/}/" in + */venv/bin/ | */.venv/bin/ | */env/bin/ | */venvs/* | */.venvs/* | */virtualenvs/* | */.virtualenvs/* | */envs/* | */conda/* | */miniconda* | */anaconda*) continue ;; + esac + out="${out:+$out:}$dir" + done + printf '%s' "$out" +} + # Probe the installed version directly from the binary — bypasses the # snapshot round-trip. Used as a fallback in upgrade-success checks so a # stale snapshot (e.g. after a transient endoflife failure) doesn't mask a @@ -208,7 +234,8 @@ probe_installed_version() { [ -x "$binary" ] || return 1 bin_path="$binary" else - bin_path="$(command -v "$binary" 2>/dev/null)" || return 1 + # An activated venv's copy is no installation + bin_path="$(PATH="$(installation_path)" command -v "$binary" 2>/dev/null)" || return 1 fi # Try --version first, then -v, capture both stdout and stderr. Extract @@ -217,6 +244,110 @@ probe_installed_version() { printf '%s\n' "$ver" | grep -oE '[0-9]+\.[0-9]+(\.[0-9]+)?' | head -n1 } +# True while a pin still hides the tool: "never", the release the user chose +# to skip (s: pin == latest), the version the user chose to hold (p: pin == +# installed), or for a cycle the cycle itself. A skipped release no longer +# hides the tool once a newer one is out. +# Args: pin latest installed [cycle] +pin_applies() { + local pin="$1" latest="$2" installed="$3" cycle="${4:-}" + [ -n "$pin" ] || return 1 + [ "$pin" = "never" ] && return 0 + [ -n "$cycle" ] && [ "$pin" = "$cycle" ] && return 0 + [ "$pin" = "$latest" ] || [ "$pin" = "$installed" ] +} + +# Remove installer markers before an install, so a marker left by an earlier +# run (make upgrade-, an interrupted guide, another cycle of the same +# tool) cannot decide this run's verdict. +clear_upgrade_markers() { + rm -f "$MARKER_DIR/${1}.already-current" "$MARKER_DIR/${1}.held-back" +} + +# Classify the outcome of an install/upgrade run. Call after the re-audit. +# An install script that exits 0 has not necessarily changed anything: a +# shadowed binary, an unchanged package or a stale version string all exit 0. +# Args: script_ok catalog_tool tool installed latest [version_cycle] +# Echoes: updated | failed | unchanged | unverified | already-current | held-back +# unverified: version unchanged, but no upstream version to compare with +# already-current: installer found the binary identical to the target release +# held-back: package manager has no newer version than the installed one +upgrade_verdict() { + local script_ok="$1" catalog_tool="$2" tool="$3" installed="$4" latest="$5" + local version_cycle="${6:-}" marker="" new_installed="" probed="" + local marker_dir="$MARKER_DIR" + [ -f "$marker_dir/${catalog_tool}.already-current" ] && marker="already-current" + [ -f "$marker_dir/${catalog_tool}.held-back" ] && marker="held-back" + rm -f "$marker_dir/${catalog_tool}.already-current" "$marker_dir/${catalog_tool}.held-back" + + if [ "$script_ok" != "1" ]; then + echo "failed" + return 0 + fi + + new_installed="$(json_field "$tool" installed)" + # If the snapshot still reports the pre-install version, the refresh + # may have hit a transient failure (endoflife timeout, flaky audit). + # Probe the binary directly as a tiebreaker. + if [ -z "$new_installed" ] || [ "$new_installed" = "$installed" ]; then + probed="$(probe_installed_version "$catalog_tool" "$version_cycle" 2>/dev/null || true)" + if [ -n "$probed" ] && [ "$probed" != "$installed" ]; then + new_installed="$probed" + fi + fi + + if [ -n "$new_installed" ] && [ "$new_installed" != "$installed" ]; then + echo "updated" + elif [ -n "$marker" ]; then + echo "$marker" + elif [ -z "$new_installed" ]; then + # Nothing detectable after an install: it did not happen + echo "unchanged" + elif [ -z "$latest" ]; then + echo "unverified" + elif [ -n "$new_installed" ] && { [[ "$latest" == "$new_installed".* ]] || [[ "$new_installed" == "$latest".* ]]; }; then + # Short version form (3.13 vs 3.13.11): detection truncates, upgrade worked. + # The dot boundary keeps 1.1 from matching 1.12.0. + echo "updated" + else + echo "unchanged" + fi +} + +# Print the verdict of an upgrade and update the summary counters. +# Args: verdict tool installed latest +report_upgrade_verdict() { + local verdict="$1" tool="$2" installed="$3" latest="$4" + case "$verdict" in + updated) + SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) + ;; + failed) + printf " ⚠️ Upgrade failed (install script error)\n" + SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) + ;; + unchanged) + printf " ⚠️ Upgrade did not take effect: still %s, target %s\n" "${installed:-}" "${latest:-}" + SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) + ;; + already-current) + # Upstream version string is stale (sd 1.1.0 reports 1.0.0). No pin: + # the guide hides every pinned tool, which would also hide the next + # real release. + printf " ✓ Binary already matches release %s (its version string is stale)\n" "$latest" + SUMMARY_SKIPPED=$((SUMMARY_SKIPPED + 1)) + ;; + unverified) + printf " ⚠️ No upstream version known; cannot tell whether %s changed\n" "${installed:-the install}" + SUMMARY_SKIPPED=$((SUMMARY_SKIPPED + 1)) + ;; + held-back) + printf " ⏸ Package manager has no newer version than %s (upstream: %s)\n" "${installed:-}" "${latest:-}" + SUMMARY_SKIPPED=$((SUMMARY_SKIPPED + 1)) + ;; + esac +} + # Print installed status line (reusable for auto-update and interactive prompts) print_installed_status() { local installed="$1" @@ -308,7 +439,8 @@ process_tool() { local is_multi_version="" local version_cycle="" if [[ "$tool" == *"@"* ]]; then - local base_tool="$(json_field "$tool" base_tool)" + local base_tool + base_tool="$(json_field "$tool" base_tool)" || true version_cycle="$(json_field "$tool" version_cycle)" if [ -n "$base_tool" ]; then catalog_tool="$base_tool" @@ -321,22 +453,32 @@ process_tool() { fi # Get tool data from audit JSON (use full tool name for JSON queries) - local icon="$(json_field "$tool" state_icon)" - local installed="$(json_field "$tool" installed)" - local latest="$(json_field "$tool" latest_upstream)" - local url="$(json_field "$tool" latest_url)" - local method="$(json_field "$tool" installed_method)" - local is_up_to_date="$(json_bool "$tool" is_up_to_date)" + local icon + icon="$(json_field "$tool" state_icon)" || true + local installed + installed="$(json_field "$tool" installed)" || true + local latest + latest="$(json_field "$tool" latest_upstream)" || true + local url + url="$(json_field "$tool" latest_url)" || true + local method + method="$(json_field "$tool" installed_method)" || true + local is_up_to_date + is_up_to_date="$(json_bool "$tool" is_up_to_date)" || true # Get metadata from catalog (use base tool name for catalog queries) - local display="$(catalog_get_guide_property "$catalog_tool" display_name "$catalog_tool")" + local display + display="$(catalog_get_guide_property "$catalog_tool" display_name "$catalog_tool")" || true # For multi-version tools, append version cycle to display name if [ -n "$is_multi_version" ] && [ -n "$version_cycle" ]; then display="$display $version_cycle" fi - local install_action="$(catalog_get_guide_property "$catalog_tool" install_action "")" - local description="$(catalog_get_property "$catalog_tool" description)" - local homepage="$(catalog_get_property "$catalog_tool" homepage)" + local install_action + install_action="$(catalog_get_guide_property "$catalog_tool" install_action "")" || true + local description + description="$(catalog_get_property "$catalog_tool" description)" || true + local homepage + homepage="$(catalog_get_property "$catalog_tool" homepage)" || true # Multi-version tools (python@3.13, php@8.3, etc.) store auto-update per cycle, # so 'a' on one cycle doesn't silently apply to other cycles. Non-multi-version # tools use the bare catalog name. @@ -344,7 +486,8 @@ process_tool() { if [ -n "$is_multi_version" ]; then auto_update_key="$tool" fi - local auto_update="$(config_get_auto_update "$auto_update_key")" + local auto_update + auto_update="$(config_get_auto_update "$auto_update_key")" || true # Check if runtime requirements are satisfied (e.g., npm requires node) local missing_req @@ -426,7 +569,8 @@ process_tool() { print_installed_status "$installed" "$method" # Show target; for self-managed tools (skip_upstream) show "self-managed" instead of local target_display="${latest:-}" - local skip_upstream="$(catalog_get_property "$catalog_tool" skip_upstream)" + local skip_upstream + skip_upstream="$(catalog_get_property "$catalog_tool" skip_upstream)" || true if [ "$target_display" = "" ] && [ "$skip_upstream" = "true" ]; then target_display="self-managed" fi @@ -444,6 +588,7 @@ process_tool() { # Execute the install with version-specific environment variables local auto_update_success=0 + clear_upgrade_markers "$catalog_tool" if [ "$catalog_tool" = "python" ] || [ -n "$is_multi_version" ] && [ "$catalog_tool" = "python" ]; then UV_PYTHON_SPEC="$latest" "$ROOT"/scripts/$install_cmd && auto_update_success=1 || true elif [ "$catalog_tool" = "ruby" ]; then @@ -461,13 +606,9 @@ process_tool() { # Re-audit with fresh collection for this specific tool CLI_AUDIT_JSON=1 CLI_AUDIT_COLLECT=1 CLI_AUDIT_MERGE=1 "$CLI" audit.py "$tool" >/dev/null 2>&1 || true reload_audit_json - # Clean up any already-current marker left by installer - rm -f "/tmp/.cli-audit/${catalog_tool}.already-current" - if [ "$auto_update_success" = "0" ]; then - SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) - else - SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) - fi + report_upgrade_verdict \ + "$(upgrade_verdict "$auto_update_success" "$catalog_tool" "$tool" "$installed" "$latest" "$version_cycle")" \ + "$tool" "$installed" "$latest" return 0 fi @@ -481,7 +622,8 @@ process_tool() { # Show target; for self-managed tools (skip_upstream) show "self-managed" instead of local target_display_p="${latest:-}" - local skip_upstream_p="$(catalog_get_property "$catalog_tool" skip_upstream)" + local skip_upstream_p + skip_upstream_p="$(catalog_get_property "$catalog_tool" skip_upstream)" || true if [ "$target_display_p" = "" ] && [ "$skip_upstream_p" = "true" ]; then target_display_p="self-managed" fi @@ -570,6 +712,7 @@ process_tool() { [Yy]) # Handle tool-specific version environment variables local upgrade_success=0 + clear_upgrade_markers "$catalog_tool" if [ "$catalog_tool" = "python" ]; then UV_PYTHON_SPEC="$latest" "$ROOT"/scripts/$install_cmd && upgrade_success=1 || true elif [ "$catalog_tool" = "ruby" ]; then @@ -590,51 +733,22 @@ process_tool() { # Reload full audit JSON from updated snapshot (needed for subsequent tools) reload_audit_json - # Check if upgrade succeeded by comparing versions - local new_installed="$(json_field "$tool" installed)" - # If the snapshot still reports the pre-install version, the refresh - # may have hit a transient failure (endoflife timeout, flaky audit). - # Probe the binary directly as a tiebreaker — it's the ground truth. - if [ -z "$new_installed" ] || [ "$new_installed" = "$installed" ]; then - local probed_y - probed_y="$(probe_installed_version "$catalog_tool" "$version_cycle" 2>/dev/null || true)" - if [ -n "$probed_y" ] && [ "$probed_y" != "$installed" ]; then - new_installed="$probed_y" - fi - fi - # Check if installer flagged binary as already at target (hash match) - local already_current_marker="/tmp/.cli-audit/${catalog_tool}.already-current" - local binary_already_current="" - if [ -f "$already_current_marker" ]; then - binary_already_current="true" - rm -f "$already_current_marker" - fi - if [ "$upgrade_success" = "0" ]; then - # Install script failed - printf "\n ⚠️ Upgrade failed (install script error)\n" - SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) - prompt_pin_version "$tool" "$installed" - elif [ -n "$binary_already_current" ]; then - # Binary hash matches target release - upgrade succeeded despite version string - printf "\n ✓ Binary already matches target release (upstream version string may be stale)\n" - elif [ "$new_installed" = "$installed" ] && [ "$new_installed" != "$latest" ]; then - # Version didn't change and not at target - # BUT: if installed is a prefix of latest (e.g., 3.13 vs 3.13.11), consider it success - # This happens when version detection returns short form but upgrade actually worked - if [[ "$latest" == "$new_installed"* ]] || [[ "$new_installed" == "$latest"* ]]; then - : # Prefix match - upgrade likely succeeded, don't warn - else - printf "\n ⚠️ Upgrade did not succeed (version unchanged)\n" + local verdict + verdict="$(upgrade_verdict "$upgrade_success" "$catalog_tool" "$tool" "$installed" "$latest" "$version_cycle")" + report_upgrade_verdict "$verdict" "$tool" "$installed" "$latest" + case "$verdict" in + failed|unchanged|held-back) prompt_pin_version "$tool" "$installed" - fi - else - # Upgrade succeeded - remove any existing pin to avoid stale pins - SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) - local existing_pin="$(pins_get "$tool")" - if [ -n "$existing_pin" ] && [ "$existing_pin" != "never" ]; then - "$ROOT"/scripts/unpin_version.sh "$tool" || true - fi - fi + ;; + updated) + # Remove any existing pin to avoid stale pins + local existing_pin + existing_pin="$(pins_get "$tool")" + if [ -n "$existing_pin" ] && [ "$existing_pin" != "never" ]; then + "$ROOT"/scripts/unpin_version.sh "$tool" || true + fi + ;; + esac ;; [Aa]) # Install/upgrade AND enable auto-update for future. Use the cycle-qualified @@ -644,6 +758,7 @@ process_tool() { # Handle tool-specific version environment variables local upgrade_success_a=0 + clear_upgrade_markers "$catalog_tool" if [ "$catalog_tool" = "python" ]; then UV_PYTHON_SPEC="$latest" "$ROOT"/scripts/$install_cmd && upgrade_success_a=1 || true elif [ "$catalog_tool" = "ruby" ]; then @@ -662,45 +777,21 @@ process_tool() { CLI_AUDIT_JSON=1 CLI_AUDIT_COLLECT=1 CLI_AUDIT_MERGE=1 "$CLI" audit.py "$tool" >/dev/null 2>&1 || true reload_audit_json - # Check if upgrade succeeded - local new_installed_a="$(json_field "$tool" installed)" - # Binary-probe fallback (see [Yy] branch for rationale). - if [ -z "$new_installed_a" ] || [ "$new_installed_a" = "$installed" ]; then - local probed_a - probed_a="$(probe_installed_version "$catalog_tool" "$version_cycle" 2>/dev/null || true)" - if [ -n "$probed_a" ] && [ "$probed_a" != "$installed" ]; then - new_installed_a="$probed_a" - fi - fi - # Check if installer flagged binary as already at target (hash match) - local already_current_marker_a="/tmp/.cli-audit/${catalog_tool}.already-current" - local binary_already_current_a="" - if [ -f "$already_current_marker_a" ]; then - binary_already_current_a="true" - rm -f "$already_current_marker_a" - fi - if [ "$upgrade_success_a" = "0" ]; then - printf "\n ⚠️ Upgrade failed (install script error)\n" - printf " Auto-update is still enabled - will try again next time.\n" - SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) - elif [ -n "$binary_already_current_a" ]; then - printf " ✓ Auto-update enabled. Binary already matches target release.\n" - SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) - elif [ "$new_installed_a" = "$installed" ] && [ "$new_installed_a" != "$latest" ]; then - # Version didn't change - but check for prefix match (e.g., 3.13 vs 3.13.11) - if [[ "$latest" == "$new_installed_a"* ]] || [[ "$new_installed_a" == "$latest"* ]]; then - printf " ✓ Auto-update enabled. This tool will update automatically in future.\n" - SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) - else - printf "\n ⚠️ Upgrade did not succeed (version unchanged)\n" + local verdict_a + verdict_a="$(upgrade_verdict "$upgrade_success_a" "$catalog_tool" "$tool" "$installed" "$latest" "$version_cycle")" + report_upgrade_verdict "$verdict_a" "$tool" "$installed" "$latest" + case "$verdict_a" in + failed|unchanged) printf " Auto-update is still enabled - will try again next time.\n" - SUMMARY_FAILED=$((SUMMARY_FAILED + 1)) - fi - else - printf " ✓ Auto-update enabled. This tool will update automatically in future.\n" - SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1)) + ;; + *) + printf " ✓ Auto-update enabled. This tool will update automatically in future.\n" + ;; + esac + if [ "$verdict_a" = "updated" ]; then # Remove any existing pin - local existing_pin_a="$(pins_get "$tool")" + local existing_pin_a + existing_pin_a="$(pins_get "$tool")" if [ -n "$existing_pin_a" ]; then "$ROOT"/scripts/unpin_version.sh "$tool" || true fi @@ -757,13 +848,15 @@ process_tool() { reload_audit_json # Check if removal succeeded - local still_installed="$(json_field "$tool" installed)" + local still_installed + still_installed="$(json_field "$tool" installed)" || true if [ -z "$still_installed" ]; then printf " ✓ %s has been removed\n" "$tool" SUMMARY_REMOVED=$((SUMMARY_REMOVED + 1)) else # Check if remaining installation is a system/apt binary that we can't remove - local remaining_method="$(json_field "$tool" installed_method)" + local remaining_method + remaining_method="$(json_field "$tool" installed_method)" || true if [ "$remaining_method" = "apt" ] || [ "$remaining_method" = "system" ]; then printf " ✓ User-managed %s removed (system %s still present at %s — managed by OS)\n" \ "$tool" "$still_installed" "$remaining_method" @@ -826,7 +919,8 @@ prompt_pin_version() { local tool="$1" local current_version="$2" - [ -z "$current_version" ] && current_version="" + # Nothing installed: there is no version to pin + [ -z "$current_version" ] && return 0 printf " Pin to version %s to stop upgrade prompts? [y/N] " "$current_version" @@ -853,11 +947,16 @@ process_deprecated_tool() { fi # Get tool data - local installed="$(json_field "$tool" installed)" - local method="$(json_field "$tool" installed_method)" - local description="$(catalog_get_property "$catalog_tool" description)" - local superseded_by="$(catalog_get_superseded_by "$catalog_tool")" - local deprecation_msg="$(catalog_get_deprecation_message "$catalog_tool")" + local installed + installed="$(json_field "$tool" installed)" || true + local method + method="$(json_field "$tool" installed_method)" || true + local description + description="$(catalog_get_property "$catalog_tool" description)" || true + local superseded_by + superseded_by="$(catalog_get_superseded_by "$catalog_tool")" || true + local deprecation_msg + deprecation_msg="$(catalog_get_deprecation_message "$catalog_tool")" || true # Get replacement tool info local replacement_desc="" @@ -904,12 +1003,11 @@ process_deprecated_tool() { printf " Migrating to %s...\n" "$superseded_by" # Check if replacement is already installed - local replacement_installed="$(json_field "$superseded_by" installed)" + local replacement_installed + replacement_installed="$(json_field "$superseded_by" installed)" || true - local already_installed="" if [ -n "$replacement_installed" ]; then printf " ✓ %s %s already installed (skipping install)\n" "$superseded_by" "$replacement_installed" - already_installed="true" else # Install the replacement "$ROOT"/scripts/install_tool.sh "$superseded_by" || true @@ -942,7 +1040,8 @@ process_deprecated_tool() { CLI_AUDIT_JSON=1 CLI_AUDIT_COLLECT=1 CLI_AUDIT_MERGE=1 "$CLI" audit.py "$tool" >/dev/null 2>&1 || true reload_audit_json - local still_installed="$(json_field "$tool" installed)" + local still_installed + still_installed="$(json_field "$tool" installed)" || true if [ -z "$still_installed" ]; then printf " ✓ Migration complete: %s → %s\n" "$tool" "$superseded_by" else @@ -967,7 +1066,8 @@ process_deprecated_tool() { CLI_AUDIT_JSON=1 CLI_AUDIT_COLLECT=1 CLI_AUDIT_MERGE=1 "$CLI" audit.py "$tool" >/dev/null 2>&1 || true reload_audit_json - local still_there="$(json_field "$tool" installed)" + local still_there + still_there="$(json_field "$tool" installed)" || true if [ -z "$still_there" ]; then printf " ✓ %s has been removed\n" "$tool" else @@ -1035,8 +1135,9 @@ while read -r line; do if [ "$multi_pin" = "never" ]; then continue fi - # Skip if this specific version cycle is pinned to a version - if [ -n "$multi_pin" ]; then + # Skip while the cycle pin still applies + if [ -n "$multi_pin" ] && pin_applies "$multi_pin" "$(json_field "$tool_name" latest_upstream)" \ + "$(json_field "$tool_name" installed)" "$version_cycle"; then continue fi else @@ -1045,8 +1146,9 @@ while read -r line; do continue fi - # Skip if pinned to any specific version (don't prompt for upgrades) - if [ -n "$pinned_version" ]; then + # Skip while the pin still applies (don't prompt for that release) + if [ -n "$pinned_version" ] && pin_applies "$pinned_version" "$(json_field "$tool_name" latest_upstream)" \ + "$(json_field "$tool_name" installed)"; then continue fi fi diff --git a/scripts/installers/github_release_binary.sh b/scripts/installers/github_release_binary.sh index e897dc7..8d1915c 100755 --- a/scripts/installers/github_release_binary.sh +++ b/scripts/installers/github_release_binary.sh @@ -283,7 +283,7 @@ if [ -n "$PRESERVE_DIR" ] && [ -n "$EXTRACT_DIR" ]; then mkdir -p "$LIB_DIR" # Remove old installation - rm -rf "$LIB_DIR/$PRESERVE_DIR" + rm -rf "${LIB_DIR:?}/${PRESERVE_DIR:?}" # Move entire directory to ~/.local/lib mv "$EXTRACT_DIR/$PRESERVE_DIR" "$LIB_DIR/" @@ -314,8 +314,9 @@ if [ -n "$path" ]; then printf "[%s] path: %s\n" "$TOOL" "$path"; fi if [ "$BINARY_ALREADY_CURRENT" = "true" ]; then printf "[%s] note: binary already matches target release %s (upstream version string may be stale)\n" "$TOOL" "$LATEST" # Signal already-current status to callers (e.g., guide.sh) - mkdir -p /tmp/.cli-audit - echo "$LATEST" > "/tmp/.cli-audit/${TOOL}.already-current" + marker_dir="${CLI_AUDIT_MARKER_DIR:-/tmp/.cli-audit}" + mkdir -p "$marker_dir" + echo "$LATEST" > "$marker_dir/${TOOL}.already-current" fi # Refresh snapshot after successful installation diff --git a/scripts/installers/package_manager.sh b/scripts/installers/package_manager.sh index 26ec4a5..6ff264f 100755 --- a/scripts/installers/package_manager.sh +++ b/scripts/installers/package_manager.sh @@ -42,6 +42,21 @@ if [ -n "${GO_VERSION:-}" ] && [ "$TOOL" = "go" ]; then VERSIONED_BINARY="go${GO_VERSION}" fi +# Print the packages owning the first of the given files that dpkg knows, +# one per line. Parses `dpkg -S`: skips "diversion by X from/to:" and +# "local diversion" lines, splits "a, b: /path", drops ":arch" suffixes. +dpkg_owners() { + local file line + for file in "$@"; do + [ -n "$file" ] || continue + line="$(LC_ALL=C dpkg -S "$file" 2>/dev/null | grep -vE '^(local )?diversion ' | head -1 || true)" + [ -n "$line" ] || continue + line="${line%%: /*}" + printf '%s\n' "$line" | tr ',' '\n' | sed 's/^ *//; s/:.*$//' + return 0 + done +} + # Get current version (use versioned binary if specified) get_version() { local bin="$1" @@ -73,11 +88,15 @@ fi # Install via appropriate package manager installed=false +# true only when the package manager itself ran without error; a failed +# install (dpkg lock, refused sudo, no network) must not read as "no newer +# version available" +pm_ok=false if have brew; then pkg="$(echo "$PACKAGES" | jq -r '.brew // empty')" if [ "$pkg" != "null" ] && [ -n "$pkg" ]; then - brew install "$pkg" || brew upgrade "$pkg" || true + if brew install "$pkg" || brew upgrade "$pkg"; then pm_ok=true; fi installed=true fi fi @@ -107,7 +126,38 @@ if ! $installed && have apt-get; then if ! $ppa_added; then sudo apt-get update || true fi - sudo apt-get install -y $pkg || true + if sudo apt-get install -y $pkg; then + # Installed version must equal the candidate, or the unchanged version + # means something else (e.g. another copy earlier on PATH) + first_pkg="${pkg%% *}" + pkg_installed="$(dpkg-query -W -f='${Version}' "$first_pkg" 2>/dev/null || true)" + # apt-cache translates "Candidate:" (German: "Installationskandidat:") + pkg_candidate="$(LC_ALL=C apt-cache policy "$first_pkg" 2>/dev/null | awk '/Candidate:/ { print $2; exit }' || true)" + if [ -n "$pkg_installed" ] && [ "$pkg_installed" = "$pkg_candidate" ]; then + pm_ok=true + fi + # The binary on PATH must belong to one of these packages; otherwise + # another copy shadows the package and "unchanged" says nothing about apt. + # Versions cannot decide this: universal-ctags 5.9.20210829.0 prints 5.9.0. + bin_path="$(command -v "$VERSIONED_BINARY" 2>/dev/null || true)" + bin_real="$(readlink -f "$bin_path" 2>/dev/null || true)" + owned=false + if [ -n "$bin_path" ]; then + # Owners of the first of these paths that dpkg knows at all (a known + # path with a foreign owner does not fall through to the next one): + # the resolved path (alternatives: ctags -> ctags-universal), the PATH + # entry, then /bin/ (merged /usr: some packages still record + # /bin/x while readlink gives /usr/bin/x) + for owner in $(dpkg_owners "$bin_real" "$bin_path" "/bin/${bin_real##*/}"); do + if [[ " $pkg " == *" $owner "* ]]; then + owned=true + fi + done + fi + if ! $owned; then + pm_ok=false + fi + fi installed=true fi fi @@ -115,7 +165,7 @@ fi if ! $installed && have dnf; then pkg="$(echo "$PACKAGES" | jq -r '.dnf // .rpm // empty')" if [ "$pkg" != "null" ] && [ -n "$pkg" ]; then - sudo dnf install -y "$pkg" || true + if sudo dnf install -y "$pkg"; then pm_ok=true; fi installed=true fi fi @@ -123,7 +173,7 @@ fi if ! $installed && have pacman; then pkg="$(echo "$PACKAGES" | jq -r '.pacman // .arch // empty')" if [ "$pkg" != "null" ] && [ -n "$pkg" ]; then - sudo pacman -S --noconfirm "$pkg" || true + if sudo pacman -S --noconfirm "$pkg"; then pm_ok=true; fi installed=true fi fi @@ -151,8 +201,13 @@ printf "[%s] after: %s\n" "$DISPLAY_NAME" "${after:-}" if [ -n "$path" ]; then printf "[%s] path: %s\n" "$DISPLAY_NAME" "$path"; fi # Warn if version didn't change (package manager can't provide newer version) -if [ -n "$before" ] && [ -n "$after" ] && [ "$before" = "$after" ]; then +if $pm_ok && [ -n "$before" ] && [ -n "$after" ] && [ "$before" = "$after" ]; then printf "[%s] Note: Package manager has no newer version available\n" "$DISPLAY_NAME" >&2 + # Signal held-back status to callers (guide.sh), so the run is not counted + # as an upgrade + marker_dir="${CLI_AUDIT_MARKER_DIR:-/tmp/.cli-audit}" + mkdir -p "$marker_dir" + echo "$after" > "$marker_dir/${TOOL}.held-back" fi # Refresh snapshot after successful installation diff --git a/tests/test_guide_upgrade_verdict.py b/tests/test_guide_upgrade_verdict.py new file mode 100644 index 0000000..717e2a9 --- /dev/null +++ b/tests/test_guide_upgrade_verdict.py @@ -0,0 +1,293 @@ +"""Tests for the upgrade verdict in scripts/guide.sh. + +The auto-update branch counted an upgrade as "Updated" whenever the install +script exited 0. A run where black, isort, python@3.14, codex, sd and bwrap all +stayed at their old version reported "Updated: 6". The verdict now compares the +version after the re-audit, for auto-update and interactive upgrades alike. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).parent.parent +GUIDE = PROJECT_ROOT / "scripts" / "guide.sh" + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="Shell script tests require POSIX shell") + + +def _function(name: str) -> str: + """Return the source of one top-level function of guide.sh.""" + match = re.search(rf"^{name}\(\) \{{\n.*?^\}}\n", GUIDE.read_text(), re.S | re.M) + assert match, f"{name} not found in guide.sh" + return match.group(0) + + +def _run( + tmp_path: Path, + *, + script_ok: str, + installed: str, + latest: str, + audited: str, + probed: str = "", + marker: str = "", + clear_first: bool = False, +) -> tuple[str, str]: + """Run upgrade_verdict + report_upgrade_verdict; return (stdout, counters).""" + marker_dir = tmp_path / "markers" + marker_dir.mkdir() + tool = "verdicttest" + if marker: + (marker_dir / f"{tool}.{marker}").write_text(latest) + root = tmp_path / "root" + (root / "scripts").mkdir(parents=True) + pin_log = tmp_path / "pins.log" + (root / "scripts" / "pin_version.sh").write_text(f'#!/bin/sh\necho "$@" >> "{pin_log}"\n') + (root / "scripts" / "pin_version.sh").chmod(0o755) + script = "\n".join( + [ + "set -euo pipefail", + f'ROOT="{root}"', + f'MARKER_DIR="{marker_dir}"', + "SUMMARY_UPDATED=0 SUMMARY_SKIPPED=0 SUMMARY_FAILED=0", + f'json_field() {{ echo "{audited}"; }}', + f'probe_installed_version() {{ echo "{probed}"; }}', + _function("upgrade_verdict"), + _function("report_upgrade_verdict"), + _function("clear_upgrade_markers"), + f'clear_upgrade_markers "{tool}"' if clear_first else ":", + f'report_upgrade_verdict "$(upgrade_verdict "{script_ok}" "{tool}" "{tool}" "{installed}" "{latest}")" ' + f'"{tool}" "{installed}" "{latest}"', + 'echo "COUNTERS updated=$SUMMARY_UPDATED skipped=$SUMMARY_SKIPPED failed=$SUMMARY_FAILED"', + ] + ) + proc = subprocess.run(["bash", "-c", script], capture_output=True, text=True, check=True) + for suffix in ("already-current", "held-back"): + assert not (marker_dir / f"{tool}.{suffix}").exists(), "marker must be consumed" + counters = proc.stdout.strip().splitlines()[-1] + return proc.stdout, counters + + +def test_exit_zero_without_version_change_is_a_failure(tmp_path): + # The black run: uv upgraded a shadowed copy, the audited version stayed 25.11.0 + out, counters = _run(tmp_path, script_ok="1", installed="25.11.0", latest="26.5.1", audited="25.11.0", probed="25.11.0") + assert counters == "COUNTERS updated=0 skipped=0 failed=1" + assert "did not take effect: still 25.11.0, target 26.5.1" in out + + +def test_version_change_is_an_update(tmp_path): + _out, counters = _run(tmp_path, script_ok="1", installed="0.70.0", latest="0.71.0", audited="0.71.0") + assert counters == "COUNTERS updated=1 skipped=0 failed=0" + + +def test_probe_rescues_a_stale_snapshot(tmp_path): + _out, counters = _run(tmp_path, script_ok="1", installed="0.70.0", latest="0.71.0", audited="0.70.0", probed="0.71.0") + assert counters == "COUNTERS updated=1 skipped=0 failed=0" + + +def test_script_error_is_a_failure(tmp_path): + out, counters = _run(tmp_path, script_ok="0", installed="0.70.0", latest="0.71.0", audited="0.70.0") + assert counters == "COUNTERS updated=0 skipped=0 failed=1" + assert "install script error" in out + + +def test_held_back_package_is_skipped(tmp_path): + out, counters = _run(tmp_path, script_ok="1", installed="0.9.0", latest="0.12.0", audited="0.9.0", marker="held-back") + assert counters == "COUNTERS updated=0 skipped=1 failed=0" + assert "no newer version than 0.9.0" in out + + +def test_already_current_binary_is_skipped_without_a_pin(tmp_path): + # A pin would hide the tool from every later run, the next real release included + out, counters = _run(tmp_path, script_ok="1", installed="1.0.0", latest="1.1.0", audited="1.0.0", marker="already-current") + assert counters == "COUNTERS updated=0 skipped=1 failed=0" + assert not (tmp_path / "pins.log").exists() + + +def test_marker_from_an_earlier_run_is_cleared_before_install(tmp_path): + out, counters = _run( + tmp_path, script_ok="1", installed="0.9.0", latest="0.12.0", audited="0.9.0", marker="held-back", clear_first=True + ) + assert counters == "COUNTERS updated=0 skipped=0 failed=1" + + +def test_short_version_needs_a_dot_boundary(tmp_path): + # 1.1 is not a short form of 1.12.0 + _out, counters = _run(tmp_path, script_ok="1", installed="1.1", latest="1.12.0", audited="1.1") + assert counters == "COUNTERS updated=0 skipped=0 failed=1" + + +def test_short_version_form_counts_as_update(tmp_path): + _out, counters = _run(tmp_path, script_ok="1", installed="3.13", latest="3.13.11", audited="3.13") + assert counters == "COUNTERS updated=1 skipped=0 failed=0" + + +def test_every_upgrade_branch_uses_the_verdict(): + # auto-update, [Yy] and [Aa] must all judge the outcome the same way + source = GUIDE.read_text() + assert source.count('$(upgrade_verdict "') == 3 + assert source.count(' clear_upgrade_markers "$catalog_tool"') == 3 + assert "pin_version.sh" not in _function("report_upgrade_verdict") + assert "SUMMARY_UPDATED=$((SUMMARY_UPDATED + 1))" not in source.replace(_function("report_upgrade_verdict"), "") + + +SCRIPTS = PROJECT_ROOT / "scripts" + + +def _run_package_manager( + tmp_path: Path, + *, + install_rc: int, + candidate: str, + detected: str = "0.9.0", + lang: str = "C", + owner: str = "bubblewrap", + dpkg_stub: str = "", +) -> bool: + """Run package_manager.sh bwrap against stub apt tools; return whether held-back was marked.""" + if not shutil.which("jq"): + pytest.skip("jq not installed") + fake = tmp_path / "fakebin" + fake.mkdir() + stubs = { + "sudo": 'exec "$@"', + "apt-get": f'[ "$1" = install ] && exit {install_rc}; exit 0', + "dpkg-query": "echo 0.9.0-1ubuntu0.3", + # dpkg -S : which package owns the binary found on PATH + "dpkg": dpkg_stub or f'[ "$1" = -S ] && [ -n "{owner}" ] && echo "{owner}: $2" && exit 0; exit 1', + # apt-cache translates its labels unless LC_ALL=C + "apt-cache": ( + 'label="Candidate:"; [ "${LC_ALL:-}" != C ] && [ "${LANG:-C}" != C ] && label="Installationskandidat:"\n' + f'echo " Installed: 0.9.0-1ubuntu0.3"; echo " $label {candidate}"' + ), + "bwrap": f"echo 'bubblewrap {detected}'", + "python3": "exit 0", # refresh_snapshot must not touch the real snapshot + } + for name, body in stubs.items(): + (fake / name).write_text(f"#!/bin/bash\n{body}\n") + (fake / name).chmod(0o755) + (fake / "jq").symlink_to(shutil.which("jq")) + marker = tmp_path / "markers" / "bwrap.held-back" + env = {**os.environ, "PATH": f"{fake}:/usr/bin:/bin", "CLI_AUDIT_MARKER_DIR": str(tmp_path / "markers"), "LANG": lang} + env.pop("LC_ALL", None) + subprocess.run( + ["bash", str(SCRIPTS / "installers" / "package_manager.sh"), "bwrap"], + env=env, + capture_output=True, + text=True, + check=True, + ) + return marker.exists() + + +def test_package_manager_marks_held_back_when_candidate_is_installed(tmp_path): + assert _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3") + + +def test_failed_install_is_not_held_back(tmp_path): + # dpkg lock, refused sudo, no network: the version is unchanged for another reason + assert not _run_package_manager(tmp_path, install_rc=100, candidate="0.9.0-1ubuntu0.3") + + +def test_newer_candidate_is_not_held_back(tmp_path): + # apt has a newer package, yet the detected version did not move: something shadows it + assert not _run_package_manager(tmp_path, install_rc=0, candidate="0.12.0-1") + + +def _pin_applies(*args: str) -> bool: + script = "\n".join(["set -euo pipefail", _function("pin_applies"), "pin_applies " + " ".join(f'"{a}"' for a in args)]) + return subprocess.run(["bash", "-c", script]).returncode == 0 + + +def test_skipped_release_hides_the_tool_until_a_newer_one_is_out(): + # s = "Skip only 1.1.0 (ask again when newer patch available)" + assert _pin_applies("1.1.0", "1.1.0", "1.0.0") + assert not _pin_applies("1.1.0", "1.2.0", "1.0.0") + + +def test_held_version_keeps_hiding_the_tool(): + # p = "Pin to 1.0.0 (don't ask for upgrades)" + assert _pin_applies("1.0.0", "1.2.0", "1.0.0") + + +def test_never_and_cycle_pins_apply(): + assert _pin_applies("never", "1.2.0", "") + assert _pin_applies("3.13", "3.13.11", "3.13.4", "3.13") + assert not _pin_applies("", "1.2.0", "1.0.0") + + +def test_guide_loop_uses_pin_applies_for_both_pin_kinds(): + assert GUIDE.read_text().count('&& pin_applies "$') == 2 + + +def test_held_back_detection_is_locale_independent(tmp_path): + assert _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", lang="de_DE.UTF-8") + + +def test_shadowed_package_is_not_held_back(tmp_path): + # apt installed its newest 0.9.0, but a copy no package owns answers on PATH + assert not _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", detected="0.8.0", owner="") + + +def test_package_version_longer_than_binary_version_is_held_back(tmp_path): + # universal-ctags 5.9.20210829.0-1 prints 5.9.0; ownership decides, not versions + assert _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", detected="0.9") + + +def test_binary_owned_by_another_package_is_not_held_back(tmp_path): + assert not _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", owner="otherpkg") + + +def test_install_that_left_nothing_is_a_failure(tmp_path): + out, counters = _run(tmp_path, script_ok="1", installed="", latest="", audited="") + assert counters == "COUNTERS updated=0 skipped=0 failed=1" + + +def test_unknown_upstream_is_not_a_failure(tmp_path): + out, counters = _run(tmp_path, script_ok="1", installed="2.0.0", latest="", audited="2.0.0") + assert counters == "COUNTERS updated=0 skipped=1 failed=0" + assert "No upstream version known" in out + + +def test_probe_path_skips_venv_dirs(tmp_path): + venv = tmp_path / "env-with-any-name" # recognised by pyvenv.cfg alone + (venv / "bin").mkdir(parents=True) + (venv / "pyvenv.cfg").write_text("home = /usr/bin\n") + named = tmp_path / "proj" / "venv" / "bin" + named.mkdir(parents=True) + keep = tmp_path / ".local" / "bin" + keep.mkdir(parents=True) + path = ":".join([str(venv / "bin") + "/", str(named), str(keep)]) + script = "\n".join(["set -euo pipefail", _function("installation_path"), f'PATH="{path}"', "installation_path"]) + out = subprocess.run(["/bin/bash", "-c", script], capture_output=True, text=True, check=True).stdout + assert out == str(keep) + + +# Real `dpkg -S` output shapes (Ubuntu 24.04): diversions, several owners, +# multiarch suffixes, and merged-/usr packages that still record /bin/x +DPKG_DIVERTED = 'echo "diversion by other from: $2"; echo "diversion by other to: $2.other"; echo "bubblewrap, other: $2"' +DPKG_MULTIARCH = 'echo "bubblewrap:amd64: $2"' +# A comma inside the path must not become a second "owner" before the colon split +DPKG_COMMA_PATH = 'echo "other: /opt/x, bubblewrap"' +DPKG_BIN_ONLY = 'case "$2" in /bin/bwrap) echo "bubblewrap: /bin/bwrap" ;; *) echo "no path found" >&2; exit 1 ;; esac' + + +@pytest.mark.parametrize( + "dpkg_stub", + [DPKG_DIVERTED, DPKG_MULTIARCH, DPKG_BIN_ONLY], + ids=["diverted", "multiarch", "bin-only"], +) +def test_owner_is_found_in_real_dpkg_output_shapes(tmp_path, dpkg_stub): + assert _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", dpkg_stub=dpkg_stub) + + +def test_comma_in_the_path_is_not_an_owner(tmp_path): + assert not _run_package_manager(tmp_path, install_rc=0, candidate="0.9.0-1ubuntu0.3", dpkg_stub=DPKG_COMMA_PATH)