From dd863a34027dafd601fc9a81e21069975b200276 Mon Sep 17 00:00:00 2001 From: Rick Peters Date: Sat, 3 Oct 2026 12:52:59 +0200 Subject: [PATCH] refactor: best_effort log for quiet steps, registered temp dirs, one exit handler --- AGENTS.md | 10 ++++++++++ CHANGELOG.md | 1 + TROUBLESHOOTING.md | 7 +++++++ lib/bios.sh | 2 +- lib/boot-session.sh | 2 +- lib/cec.sh | 36 ++++++++++++++++++------------------ lib/common.sh | 37 +++++++++++++++++++++++++++++++++++-- lib/fremont-poweroff.sh | 6 +++--- lib/login-manager.sh | 2 +- lib/menu.sh | 1 - lib/packages.sh | 2 +- lib/single-user.sh | 2 +- lib/state.sh | 2 +- lib/steam-desktop.sh | 2 +- lib/steam-machine.sh | 20 ++++++++++---------- lib/steamos-extras.sh | 2 +- steamify.sh | 8 ++++++++ 17 files changed, 100 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 52ccaab..cf5a15d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,16 @@ gamescope and the Plasma desktop. Primary target: the Valve Steam Machine - Bash, `set -uo pipefail` (no `-e`): check exit codes of steps that matter explicitly (`|| { err ...; exit 1; }` or `|| return 1`). +- **Failure and quiet steps.** A function returns 0 when what it was asked + to do is in place and 1 when it isn't (after `err`); a step that is only an + extra (an icon cache, a service that may not exist) warns or stays silent and + doesn't change the return value. Never `cmd 2>/dev/null` on a step that + changes the system: use `best_effort cmd...` (same exit status, its errors go + to `$STATE_DIR/steamify.log`). `2>/dev/null` stays for probes (`pacman -Q`, + `grep -q`, `is-active`) whose failure is the answer. +- **Temp dirs.** `make_tmpdir ` (not `$(mktemp -d)`, whose + subshell can't register it): the entry point's `on_exit` removes them, also + when a run stops half way. That handler is the only `EXIT` trap. - Use the helpers from `lib/common.sh` (`info`, `ok`, `warn`, `err`, `ask_yn`, `backup_file`) for all output and prompts. - Runs as the normal user; use `sudo` per command, never require root. diff --git a/CHANGELOG.md b/CHANGELOG.md index 65e8235..34d61b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ one per merged pull request. - **refactor: `is_wanted` and `is_current` instead of repeated `[[ "${WANTED[x]}" == 1 ]]` checks in the menu, the app's backend and the entry point** - **refactor: the start of every item's JSON (id, label, hint, kind, parent) is built in one place (`json_item_head`) for the app and the installer page, and `json_bool` replaces the `&& echo true || echo false` copies** - **ci: a Lint workflow runs shellcheck on the modules and helper scripts, ruff on the app and `patches/*.py` (`ruff.toml`) and qmllint on the QML (not blocking until it has run once on the runner); `make lint` runs the same locally** +- **refactor: `best_effort` for the steps whose failure doesn't matter (stopping or disabling units, removing packages, `modprobe -r`, ...): same behaviour, but their errors go to `~/.local/state/steamify/steamify.log` instead of `/dev/null`; temp dirs are registered with `make_tmpdir` and removed by the one exit handler (also when a run is interrupted), which replaces the menu's own `EXIT` trap** - **test: the menu rules, the plan and the app's and installer's JSON are checked against a golden file (`tests/menu-test.sh` in steamify-cachyos-dev); `make check`, `.editorconfig`, `.shellcheckrc`** - **docs: README lists the sources and projects Steamify builds on, with licences and thanks** diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index 050ea3c..2650301 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -2,6 +2,13 @@ See [TECHNICAL.md](TECHNICAL.md) for how each part works. +## Where are the errors of the quiet steps? + +Steps that may fail without it mattering (stopping a service that isn't +running, removing a package that isn't installed) don't show their errors in the +wizard. They are kept in `~/.local/state/steamify/steamify.log` (the last +512 KB), with the command and its exit status: attach it when you ask for help. + ## Gaming mode shows a black screen or keeps restarting Press **Ctrl+Alt+F3**, log in with your username and password, and run: diff --git a/lib/bios.sh b/lib/bios.sh index e9090f3..a97756f 100644 --- a/lib/bios.sh +++ b/lib/bios.sh @@ -32,7 +32,7 @@ bios_lookup_newest() { local tmp desc BIOS_REPO="$(valve_newest_repo "$BIOS_REPO_PREFIX" 10)" [[ -n "$BIOS_REPO" ]] || return 1 - tmp="$(mktemp -d)" + make_tmpdir tmp if curl -fsL --max-time 30 "$(valve_repo_url "$BIOS_REPO")/$BIOS_REPO.files" -o "$tmp/files" && curl -fsL --max-time 30 "$(valve_repo_url "$BIOS_REPO")/$BIOS_REPO.db" -o "$tmp/db"; then desc="$(tar -tf "$tmp/files" 2>/dev/null | grep -E '^fremont-hw-support-[0-9][^/]*/files$' | head -n 1)" diff --git a/lib/boot-session.sh b/lib/boot-session.sh index 3c86836..64d56aa 100644 --- a/lib/boot-session.sh +++ b/lib/boot-session.sh @@ -31,7 +31,7 @@ boot_enable() { boot_disable() { info "Setting the PC to boot into gaming mode again..." - sudo systemctl disable "$BOOT_UNIT_NAME" 2>/dev/null + best_effort sudo systemctl disable "$BOOT_UNIT_NAME" sudo rm -f "$BOOT_UNIT" sudo systemctl daemon-reload # Next boot into gamescope, as the conversion sets it up. diff --git a/lib/cec.sh b/lib/cec.sh index 41ecd26..b74b48d 100644 --- a/lib/cec.sh +++ b/lib/cec.sh @@ -39,10 +39,10 @@ cec_link_steamos_manager() { # this item ran. pacman -Q steamos-manager >/dev/null 2>&1 || return 0 user_systemctl daemon-reload - user_systemctl enable steamos-manager-configure-cecd.service 2>/dev/null - user_systemctl start steamos-manager-configure-cecd.service 2>/dev/null - user_systemctl restart cecd.service 2>/dev/null - user_systemctl restart steamos-manager.service 2>/dev/null + best_effort user_systemctl enable steamos-manager-configure-cecd.service + best_effort user_systemctl start steamos-manager-configure-cecd.service + best_effort user_systemctl restart cecd.service + best_effort user_systemctl restart steamos-manager.service return 0 } @@ -55,10 +55,10 @@ cec_status() { cec_installed && [[ -f "$CEC_STEAM_DROPIN" ]] && cec_driver_ok; } cec_reload_driver() { # cecd keeps /dev/cec0 open, so the old module can't be unloaded under it. - user_systemctl stop cecd.service 2>/dev/null - sudo modprobe -r cros_ec_cec 2>/dev/null - sudo modprobe cros_ec_cec 2>/dev/null - user_systemctl start cecd.service 2>/dev/null + best_effort user_systemctl stop cecd.service + best_effort sudo modprobe -r cros_ec_cec + best_effort sudo modprobe cros_ec_cec + best_effort user_systemctl start cecd.service } cec_driver_enable() { @@ -68,7 +68,7 @@ cec_driver_enable() { sudo pacman -S --needed --noconfirm dkms patch || { err "Installing dkms failed."; return 1; } fi install_kernel_headers || return 1 - tmp="$(mktemp -d)" + make_tmpdir tmp if [[ -f "$CEC_DRIVER_CACHE" ]] && echo "$CEC_DRIVER_SHA256 $CEC_DRIVER_CACHE" | sha256sum -c --quiet - 2>/dev/null; then cp "$CEC_DRIVER_CACHE" "$tmp/cros-ec-cec.c" else @@ -80,7 +80,7 @@ cec_driver_enable() { return 1 fi # The name carries the checksum: a newer pin gets its own file, the old one goes. - sudo find "${CEC_DRIVER_CACHE%/*}" -maxdepth 1 -name 'cros-ec-cec-*.c' ! -name "${CEC_DRIVER_CACHE##*/}" -delete 2>/dev/null + best_effort sudo find "${CEC_DRIVER_CACHE%/*}" -maxdepth 1 -name 'cros-ec-cec-*.c' ! -name "${CEC_DRIVER_CACHE##*/}" -delete sudo install -Dm644 "$tmp/cros-ec-cec.c" "$CEC_DRIVER_CACHE" fi # So it finds amdgpu's HDMI port (see the patch for why). @@ -124,7 +124,7 @@ cec_enable() { sudo pacman -S --needed --noconfirm linuxconsole || { err "Installing linuxconsole failed."; return 1; } state_set cec installed_linuxconsole 1 fi - tmp="$(mktemp -d)" + make_tmpdir tmp for p in "${CEC_PKGS[@]}"; do f="$(fetch_holo_pkg "$tmp" "$p")" || { rm -rf "$tmp"; return 1; } files+=("$f") @@ -152,10 +152,10 @@ cec_enable() { user_systemctl daemon-reload # The TV remote's volume keys: SteamOS enables this socket through its # preset, which pacman doesn't apply, so nothing else does on CachyOS. - user_systemctl enable --now cec-audio-control.socket 2>/dev/null + best_effort user_systemctl enable --now cec-audio-control.socket cec_link_steamos_manager if compgen -G "/dev/cec*" >/dev/null; then - user_systemctl restart cecd.service 2>/dev/null + best_effort user_systemctl restart cecd.service ok "HDMI-CEC on ($(cd /dev && echo cec*)); Steam shows its settings from the next gaming mode start." info "Turn on CEC on your TV too (e.g. Sony: BRAVIA Sync, Samsung: Anynet+, LG: SimpLink)." else @@ -166,15 +166,15 @@ cec_enable() { cec_disable() { info "Removing HDMI-CEC..." - user_systemctl disable --now cecd.service cec-audio-control.socket cec-audio-control.service 2>/dev/null - user_systemctl disable steamos-manager-configure-cecd.service 2>/dev/null - sudo pacman -Rns --noconfirm "${CEC_PKGS[@]}" 2>/dev/null + best_effort user_systemctl disable --now cecd.service cec-audio-control.socket cec-audio-control.service + best_effort user_systemctl disable steamos-manager-configure-cecd.service + best_effort sudo pacman -Rns --noconfirm "${CEC_PKGS[@]}" cec_driver_disable sudo rm -f "$CEC_ORDER_DROPIN" "$CEC_STEAM_DROPIN" "$CEC_STEAM_ENV" - sudo rmdir "$(dirname "$CEC_ORDER_DROPIN")" "$(dirname "$CEC_STEAM_DROPIN")" "$(dirname "$CEC_STEAM_ENV")" 2>/dev/null + best_effort sudo rmdir "$(dirname "$CEC_ORDER_DROPIN")" "$(dirname "$CEC_STEAM_DROPIN")" "$(dirname "$CEC_STEAM_ENV")" user_systemctl daemon-reload if [[ -n "$(state_get cec installed_linuxconsole)" ]]; then - sudo pacman -Rns --noconfirm linuxconsole 2>/dev/null + best_effort sudo pacman -Rns --noconfirm linuxconsole state_clear cec fi sudo udevadm control --reload diff --git a/lib/common.sh b/lib/common.sh index ac12b53..691eb9c 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -39,6 +39,39 @@ ask_yn() { [[ "$reply" =~ ^[Yy]$ ]] } +best_effort() { + # best_effort ...: for a step whose failure is fine (stopping a + # unit that isn't running, removing what isn't installed). Same exit status + # and output as the command, but its errors go to the log + # (~/.local/state/steamify/steamify.log) instead of vanishing, so a problem + # can still be found afterwards. + local log="${STEAMIFY_LOG:-${STATE_DIR:-$HOME/.local/state/steamify}/steamify.log}" e rc + e="$(mktemp)" || { "$@" 2>/dev/null; return; } + "$@" 2>"$e"; rc=$? + if [[ -s "$e" ]]; then + mkdir -p "${log%/*}" 2>/dev/null + # Kept small: a run that fails the same way every time shouldn't fill the disk. + [[ "$(stat -c %s "$log" 2>/dev/null || echo 0)" -gt 524288 ]] && : > "$log" + { echo "$(date -Is) exit $rc: $*"; sed 's/^/ /' "$e"; } >> "$log" 2>/dev/null + fi + rm -f "$e" + return $rc +} + +# Temp dirs of this run, removed at exit (on_exit in steamify.sh), also when it +# stops half way: make_tmpdir . +TMP_DIRS=() +make_tmpdir() { + local d + d="$(mktemp -d)" || return 1 + TMP_DIRS+=("$d") + printf -v "$1" '%s' "$d" +} + +cleanup_tmpdirs() { + [[ ${#TMP_DIRS[@]} -eq 0 ]] || rm -rf "${TMP_DIRS[@]}" +} + require_root_helper() { # Re-exec any privileged step through sudo rather than requiring the # whole script to run as root, so $HOME/whoami stay correct for the @@ -102,7 +135,7 @@ stop_plasmashell_for_edit() { PLASMASHELL_WAS_RUNNING=false if pgrep -u "$USER" -x plasmashell >/dev/null; then PLASMASHELL_WAS_RUNNING=true - systemctl --user stop plasma-plasmashell.service 2>/dev/null + best_effort systemctl --user stop plasma-plasmashell.service pkill -u "$USER" -x plasmashell && sleep 2 fi } @@ -110,7 +143,7 @@ stop_plasmashell_for_edit() { restart_plasmashell_if_stopped() { if [[ "${PLASMASHELL_WAS_RUNNING:-false}" == true ]]; then rm -rf ~/.cache/plasmashell* ~/.cache/org.kde.dirmodel-qml.kcache - systemctl --user start plasma-plasmashell.service 2>/dev/null || + best_effort systemctl --user start plasma-plasmashell.service || { setsid plasmashell >/dev/null 2>&1 & } fi } diff --git a/lib/fremont-poweroff.sh b/lib/fremont-poweroff.sh index 2fb1d6c..b82f297 100644 --- a/lib/fremont-poweroff.sh +++ b/lib/fremont-poweroff.sh @@ -25,7 +25,7 @@ poweroff_fix_enable() { sudo pacman -S --needed --noconfirm dkms || { err "Installing dkms failed."; return 1; } fi install_kernel_headers || return 1 - tmp="$(mktemp -d)" + make_tmpdir tmp patch_file "$POWEROFF_DKMS_NAME.c" > "$tmp/$POWEROFF_DKMS_NAME.c" echo "obj-m += $POWEROFF_DKMS_NAME.o" >"$tmp/Makefile" patch_file "$POWEROFF_DKMS_NAME.dkms.conf" | fill NAME="$POWEROFF_DKMS_NAME" VERSION="$POWEROFF_DKMS_VER" >"$tmp/dkms.conf" @@ -43,13 +43,13 @@ poweroff_fix_enable() { warn "Building the power-off fix for $k failed; with that kernel it may start again after shutting down." done echo "$POWEROFF_DKMS_NAME" | sudo tee "$POWEROFF_MODULES_LOAD" >/dev/null - sudo modprobe "$POWEROFF_DKMS_NAME" 2>/dev/null + best_effort sudo modprobe "$POWEROFF_DKMS_NAME" return 0 } poweroff_fix_disable() { sudo rm -f "$POWEROFF_MODULES_LOAD" - sudo modprobe -r "$POWEROFF_DKMS_NAME" 2>/dev/null + best_effort sudo modprobe -r "$POWEROFF_DKMS_NAME" [[ -d "$POWEROFF_DKMS_SRC" ]] || return 0 sudo dkms remove "$POWEROFF_DKMS_NAME/$POWEROFF_DKMS_VER" --all >/dev/null 2>&1 sudo rm -rf "$POWEROFF_DKMS_SRC" diff --git a/lib/login-manager.sh b/lib/login-manager.sh index 6e8cc5b..4411509 100644 --- a/lib/login-manager.sh +++ b/lib/login-manager.sh @@ -210,7 +210,7 @@ remove_session_sync() { # The plasmalogin sync bridge from an earlier run isn't needed on SDDM. if [[ -f /etc/systemd/system/sync-steamos-session.path ]]; then info "Removing the plasma-login-manager sync bridge (not needed with SDDM)..." - sudo systemctl disable --now sync-steamos-session.path 2>/dev/null + best_effort sudo systemctl disable --now sync-steamos-session.path sudo rm -f /etc/systemd/system/sync-steamos-session.path \ /etc/systemd/system/sync-steamos-session.service \ /usr/local/bin/sync-steamos-session.sh diff --git a/lib/menu.sh b/lib/menu.sh index 1fb8301..12b8923 100644 --- a/lib/menu.sh +++ b/lib/menu.sh @@ -446,7 +446,6 @@ draw_menu_tui() { run_menu_tui() { local cursor=0 key rest count tput civis 2>/dev/null - trap 'tput cnorm 2>/dev/null' EXIT while true; do draw_menu_tui "$cursor" count=${#MENU_ITEMS[@]} diff --git a/lib/packages.sh b/lib/packages.sh index 14164d0..b7b6d67 100644 --- a/lib/packages.sh +++ b/lib/packages.sh @@ -33,7 +33,7 @@ bootstrap_yay() { sudo pacman -S --needed --noconfirm base-devel git || return 1 local tmp_dir - tmp_dir="$(mktemp -d)" + make_tmpdir tmp_dir info "Cloning yay-bin from the AUR..." if ! git clone --depth 1 https://aur.archlinux.org/yay-bin.git "$tmp_dir/yay-bin"; then err "Failed to clone yay-bin from the AUR." diff --git a/lib/single-user.sh b/lib/single-user.sh index 2adc95c..d3baded 100644 --- a/lib/single-user.sh +++ b/lib/single-user.sh @@ -42,7 +42,7 @@ single_wallet_enable() { ok "Single user mode's wallet (no password) is back; your own is kept as kdewallet.kwl$WALLET_BACKUP." return 0 fi - tmp="$(mktemp -d)" + make_tmpdir tmp if ! fetch_valve_presets "$tmp"; then rm -rf "$tmp" warn "Couldn't get Valve's empty wallet; apps may still ask for the wallet password." diff --git a/lib/state.sh b/lib/state.sh index 818ee09..fe579fb 100644 --- a/lib/state.sh +++ b/lib/state.sh @@ -123,7 +123,7 @@ migrate_layout() { "$(wizard_desktop_dir)/$WIZARD_DESKTOP_NAME"; do [[ -f "$f" ]] && sed -i "s|$old_bin/|$STEAMIFY_BIN/|g" "$f" done - user_systemctl daemon-reload 2>/dev/null + best_effort user_systemctl daemon-reload fi if [[ -f "$old_icon" ]]; then if [[ -e "$WIZARD_ICON" ]]; then rm -f "$old_icon"; else mv "$old_icon" "$WIZARD_ICON"; fi diff --git a/lib/steam-desktop.sh b/lib/steam-desktop.sh index 3c04561..bfdc511 100644 --- a/lib/steam-desktop.sh +++ b/lib/steam-desktop.sh @@ -33,7 +33,7 @@ silent_old_unit_remove() { # Before 2.11.0 the option was a systemd user unit: it would start Steam a second time. local unit="$HOME/.config/systemd/user/steam-desktop-autostart.service" [[ -f "$unit" ]] || return 0 - user_systemctl disable --now steam-desktop-autostart.service 2>/dev/null + best_effort user_systemctl disable --now steam-desktop-autostart.service rm -f "$unit" user_systemctl daemon-reload } diff --git a/lib/steam-machine.sh b/lib/steam-machine.sh index 77833a8..07272bd 100644 --- a/lib/steam-machine.sh +++ b/lib/steam-machine.sh @@ -410,14 +410,14 @@ serial_enable() { 'z /sys/class/dmi/id/product_serial 0444 - - -' | sudo tee "$SERIAL_TMPFILES" > /dev/null || { err "Making the serial number readable for Steam failed."; return 1; } # The file applies at every boot; now too, unless /sys is read-only (the ISO's installer runs in a chroot). - sudo systemd-tmpfiles --create "$SERIAL_TMPFILES" 2>/dev/null || + best_effort sudo systemd-tmpfiles --create "$SERIAL_TMPFILES" || info "The serial number becomes readable for Steam at the next boot." return 0 } serial_disable() { sudo rm -f "$SERIAL_TMPFILES" - sudo chmod 0400 /sys/class/dmi/id/product_serial 2>/dev/null + best_effort sudo chmod 0400 /sys/class/dmi/id/product_serial return 0 } @@ -450,7 +450,7 @@ EOF wifi_backend_enable || return 1 sudo systemctl enable --now inputplumber.service sudo systemctl enable --now steamos-manager.service - user_systemctl enable steamos-manager.service 2>/dev/null + best_effort user_systemctl enable steamos-manager.service # HDMI-CEC ran before this and couldn't link cecd to steamos-manager yet. cec_status && cec_link_steamos_manager @@ -467,19 +467,19 @@ EOF machine_disable() { info "Removing Steam Machine support..." - user_systemctl disable --now steamos-manager.service 2>/dev/null - sudo systemctl disable --now steamos-manager.service 2>/dev/null - sudo systemctl disable --now inputplumber.service 2>/dev/null - sudo pacman -Rns --noconfirm steamos-manager inputplumber 2>/dev/null + best_effort user_systemctl disable --now steamos-manager.service + best_effort sudo systemctl disable --now steamos-manager.service + best_effort sudo systemctl disable --now inputplumber.service + best_effort sudo pacman -Rns --noconfirm steamos-manager inputplumber serial_disable wifi_backend_disable sudo rm -f "$LED_UDEV_RULE" /etc/modules-load.d/leds-valve.conf sudo udevadm control --reload - sudo modprobe -r leds-valve 2>/dev/null - sudo pacman -Rns --noconfirm leds-valve-dkms-git 2>/dev/null + best_effort sudo modprobe -r leds-valve + best_effort sudo pacman -Rns --noconfirm leds-valve-dkms-git krevert machine reload_powerdevil - sudo systemctl disable ensure-kernel-headers.service 2>/dev/null + best_effort sudo systemctl disable ensure-kernel-headers.service led_dkms_override_remove sudo rm -f "$HEADERS_UNIT" "$HEADERS_SCRIPT" sudo rm -rf "$(dirname "$HEADERS_SCRIPT")" "$(dirname "$HEADERS_SCRIPT_OLD")" diff --git a/lib/steamos-extras.sh b/lib/steamos-extras.sh index c7bba9b..4dee758 100644 --- a/lib/steamos-extras.sh +++ b/lib/steamos-extras.sh @@ -40,7 +40,7 @@ extras_enable() { done local tmp_dir src - tmp_dir="$(mktemp -d)" + make_tmpdir tmp_dir if ! fetch_valve_presets "$tmp_dir"; then rm -rf "$tmp_dir" return 1 diff --git a/steamify.sh b/steamify.sh index 638917c..00a961e 100755 --- a/steamify.sh +++ b/steamify.sh @@ -64,6 +64,14 @@ TARGET_USER="$(id -un)" migrate_layout notify_seen +# One exit handler (a second trap would replace it): the cursor the menu hid +# and the run's temp dirs, also when the run stops half way. +on_exit() { + [[ -t 1 ]] && tput cnorm 2>/dev/null + cleanup_tmpdirs +} +trap on_exit EXIT + restart_needed() { [[ -n "${BIOS_NEEDS_RESTART:-}" || "$RESTART_FOR_LOGIN" == true ]]; } restart_now() {