diff --git a/.gitea/workflows/shellcheck.yml b/.gitea/workflows/shellcheck.yml index c537e55..9c8d51b 100644 --- a/.gitea/workflows/shellcheck.yml +++ b/.gitea/workflows/shellcheck.yml @@ -1,4 +1,4 @@ -# Check every bash script with shellcheck (settings in .shellcheckrc) +# Check every bash script with shellcheck: make lint (settings in .shellcheckrc) name: shellcheck on: [push, pull_request] @@ -12,11 +12,6 @@ jobs: run: | SUDO=$(command -v sudo || true) $SUDO apt-get update -q - $SUDO apt-get install -y -q shellcheck + $SUDO apt-get install -y -q shellcheck make - name: Run shellcheck - run: | - git ls-files -z | while IFS= read -r -d '' f; do - [ -L "$f" ] && continue - case "$f" in *.sh|*.conf) echo "$f"; continue ;; esac - head -n 1 "$f" | grep -q '^#!.*bash' && echo "$f" - done | xargs -d '\n' shellcheck -S warning + run: make lint diff --git a/.shellcheckrc b/.shellcheckrc index b9cbf16..eef8cdc 100644 --- a/.shellcheckrc +++ b/.shellcheckrc @@ -1,10 +1,7 @@ -# Settings for shellcheck, run on every push by .gitea/workflows/shellcheck.yml -shell=bash -# Scripts source files by paths built at run time, like $HOME/.local/share/coredesktop/utils.sh -disable=SC1090,SC1091 -# configure.d items set TITLE and DESCRIPTION for configure, which sources them -disable=SC2034 -# Messages that print ~ for the user to read -disable=SC2088 -# mkdir -p -m 700 on a directory whose parents already exist +# Hooks, configure items and config files are sourced from paths only known at +# run time +disable=SC1090 + +# mkdir -p -m only sets the mode of the last directory; every use here makes +# one directory in a parent that already exists disable=SC2174 diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..2728c15 --- /dev/null +++ b/Makefile @@ -0,0 +1,14 @@ +# Every tracked file (not symlink) that starts with a bash or sh shebang (\043 is "#", which +# make would otherwise read as the start of a comment) +SCRIPTS := $(shell git ls-files -s | awk '$$1 != 120000 { print $$4 }' | xargs awk 'FNR == 1 && /^#!.*[\/ ](ba)?sh/ { print FILENAME }') + +.PHONY: check lint test + +check: lint test + +# Errors and warnings only, for now; drop --severity to see style notes too +lint: + shellcheck --severity=warning $(SCRIPTS) + +test: + bats tests diff --git a/README.md b/README.md index 30e6b72..8a4d5b7 100644 --- a/README.md +++ b/README.md @@ -326,7 +326,7 @@ This whole "project" is licensed under the MIT License. Want to help make CoreDe Found a bug or have an idea? [Open an issue](https://gitea.federated.computer/Federated_Computer/coredesktop/issues). -Every push is checked with [shellcheck](https://www.shellcheck.net) (settings in .shellcheckrc). Run `shellcheck` on the scripts you change before sending a PR. +Every push is checked with [shellcheck](https://www.shellcheck.net) (settings in .shellcheckrc). Before sending a PR, run `make check`: it runs shellcheck (`make lint`) and the [bats](https://github.com/bats-core/bats-core) tests in tests/ (`make test`). Changes to install.sh only reach new installs. If existing machines need the change too, also add a migration: a script named `migrations/$(date +%s).sh` that `update` runs once on each machine. It may also run on fresh installs, so make it safe to repeat. diff --git a/claude.sh b/claude.sh index eb368ea..c98af90 100755 --- a/claude.sh +++ b/claude.sh @@ -15,10 +15,10 @@ fi # The native installer places the binary in ~/.local/bin LINE='export PATH="$HOME/.local/bin:$PATH"' if grep -qF "$LINE" ~/.bashrc; then - echo "~/.local/bin already on PATH in .bashrc" + echo "\~/.local/bin already on PATH in .bashrc" else echo "$LINE" >> ~/.bashrc - echo "~/.local/bin added to PATH in .bashrc" + echo "\~/.local/bin added to PATH in .bashrc" fi # Desktop notifications via notify-send (libnotify) diff --git a/configure.d/10-core.sh b/configure.d/10-core.sh index a25081c..1a3b442 100755 --- a/configure.d/10-core.sh +++ b/configure.d/10-core.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="Federated Computer Core" +# shellcheck disable=SC2034 DESCRIPTION="Superpanel, Nextcloud, email, files and Element" # Each Core is in cores/.conf, with what you entered last time, so @@ -309,7 +312,7 @@ step() { # can't be reached. ask_password() { local tries - for tries in 1 2 3; do + for _ in 1 2 3; do read -rsp "Your Core password: " PASSWORD echo if [ -n "$1" ]; then nextcloud_app_password "$1"; else nextcloud_sign_in; fi @@ -381,7 +384,7 @@ change_password() { local tries reached=1 load_core "$1" STATUS=0 - for tries in 1 2 3; do + for _ in 1 2 3; do read -rsp "Your new Core password: " PASSWORD echo nextcloud_sign_in diff --git a/configure.d/15-tailscale.sh b/configure.d/15-tailscale.sh index 9272534..a16e8de 100644 --- a/configure.d/15-tailscale.sh +++ b/configure.d/15-tailscale.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="Tailscale" +# shellcheck disable=SC2034 DESCRIPTION="Join your Core's Tailscale network" CORE_CONF="$HOME/.config/coredesktop/core.conf" diff --git a/configure.d/20-backup.sh b/configure.d/20-backup.sh index 4bbc5f4..d8ea07f 100644 --- a/configure.d/20-backup.sh +++ b/configure.d/20-backup.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="Backup" +# shellcheck disable=SC2034 DESCRIPTION="Back up your home folder daily to your Nextcloud files" CORE_CONF="$HOME/.config/coredesktop/core.conf" @@ -33,7 +36,7 @@ nextcloud_login() { local tries [ -n "$(backup_secret nextcloud "$WEBDAV")" ] \ && rclone lsf --max-depth 1 ":webdav:" > /dev/null 2>&1 && return 0 - for tries in 1 2 3; do + for _ in 1 2 3; do read -rsp "Your Core password: " PASSWORD echo nextcloud_app_password "CoreDesktop backup on $HOSTNAME" diff --git a/configure.d/30-claude.sh b/configure.d/30-claude.sh index 9d6c952..745b173 100644 --- a/configure.d/30-claude.sh +++ b/configure.d/30-claude.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="Claude Code" +# shellcheck disable=SC2034 DESCRIPTION="Sign in to Anthropic" is_installed() { diff --git a/configure.d/31-opencode.sh b/configure.d/31-opencode.sh index 789bb45..e89d125 100644 --- a/configure.d/31-opencode.sh +++ b/configure.d/31-opencode.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="opencode" +# shellcheck disable=SC2034 DESCRIPTION="Sign in to AI providers" is_installed() { diff --git a/configure.d/32-hermes.sh b/configure.d/32-hermes.sh index ba8a4a2..cbe5690 100644 --- a/configure.d/32-hermes.sh +++ b/configure.d/32-hermes.sh @@ -1,6 +1,9 @@ #!/bin/bash +# configure reads TITLE and DESCRIPTION +# shellcheck disable=SC2034 TITLE="Hermes Agent" +# shellcheck disable=SC2034 DESCRIPTION="Choose a model provider and tools" is_installed() { diff --git a/system/backup b/system/backup index abe82ae..c1d5c5a 100755 --- a/system/backup +++ b/system/backup @@ -27,7 +27,7 @@ source "$COREDESKTOP/system/backup-env.sh" || exit 1 [ -n "$RESTIC_PASSWORD_COMMAND" ] || fail "The backup's passphrase isn't in your keyring. Run: configure backup" # Run at login, perhaps before the network is up; give it a few minutes -for try in {1..10}; do +for _ in {1..10}; do curl -sf --max-time 10 -o /dev/null "${BACKUP_WEBDAV%%/remote.php/*}/status.php" && break sleep 30 done diff --git a/tests/test_helper.bash b/tests/test_helper.bash new file mode 100644 index 0000000..a48f521 --- /dev/null +++ b/tests/test_helper.bash @@ -0,0 +1,51 @@ +#!/bin/bash + +# Shared setup for the bats tests. Each test gets its own HOME and a bin dir at +# the front of PATH where it can put fake versions of pacman, yay, sudo, etc. + +# Used by the .bats files that load this one +# shellcheck disable=SC2034 +REPO="$(cd "$BATS_TEST_DIRNAME/.." && pwd)" + +common_setup() { + export HOME="$BATS_TEST_TMPDIR/home" + export STUB_BIN="$BATS_TEST_TMPDIR/bin" + export STUB_LOG="$BATS_TEST_TMPDIR/stub.log" + mkdir -p "$HOME" "$STUB_BIN" + : > "$STUB_LOG" + export PATH="$STUB_BIN:$PATH" + unset COREDESKTOP +} + +# A test file that needs more setup defines its own setup that calls this first +setup() { + common_setup +} + +# stub NAME [BODY]: put a fake NAME on PATH that appends "NAME args..." to +# $STUB_LOG and then runs BODY, a bash snippet that sees the args as "$@" +stub() { + local name="$1" body="${2:-}" + cat > "$STUB_BIN/$name" << EOF +#!/bin/bash +echo "$name \$*" >> "\$STUB_LOG" +$body +EOF + chmod +x "$STUB_BIN/$name" +} + +# called NAME ARGS...: true if the stub log has exactly the line "NAME ARGS..." +called() { + grep -qxF "$*" "$STUB_LOG" +} + +# not_called NAME: true if the stub NAME never ran +not_called() { + ! grep -q "^$1 " "$STUB_LOG" +} + +# uncalled NAME ARGS...: the opposite of called. (A bare "! called ..." would +# not fail the test: bats, like set -e, ignores the status of a ! command.) +uncalled() { + ! called "$@" +} diff --git a/tests/update.bats b/tests/update.bats new file mode 100644 index 0000000..8c024d6 --- /dev/null +++ b/tests/update.bats @@ -0,0 +1,143 @@ +#!/usr/bin/env bats + +load test_helper + +# Each test runs the real update script from a fake CoreDesktop checkout in the +# test's HOME, with fake hooks in updates.d/ that log to $RAN when they run. + +setup() { + # A checkout with nothing but the update script and the files it installs + common_setup + CD="$HOME/.local/share/coredesktop" + mkdir -p "$CD/updates.d" "$HOME/.local/bin" + cp "$REPO/update" "$CD/update" + for script in refresh sibling version configure audit; do + echo '#!/bin/bash' > "$CD/$script" + done + + export RAN="$BATS_TEST_TMPDIR/ran" + unset UPDATE_REEXECED UPDATE_PULL_FAILED UPDATE_FORCE + + stub clear + stub git 'exit "${GIT_STATUS:-0}"' + stub sudo 'exit "${SUDO_STATUS:-0}"' + stub df 'printf "Avail\n%s\n" "${DF_AVAIL:-104857600}"' + # Let go of bats' output pipes so the sudo keepalive's leftover sleep + # doesn't keep the test waiting + stub sleep 'exec > /dev/null 2>&1 3>&-; exec /usr/bin/sleep "$@"' +} + +# hook NAME BODY: add updates.d/NAME.sh with BODY as its run_update +hook() { + printf '#!/bin/bash\nrun_update() {\n%s\n}\n' "$2" > "$CD/updates.d/$1.sh" +} + +ran() { + [ "$(cat "$RAN" 2> /dev/null)" = "$(printf '%s\n' "$@")" ] +} + +@test "update pulls, installs its scripts, and runs every hook in order" { + hook 20-b 'echo b >> "$RAN"' + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 0 ] + [[ "$output" == *"Done."* ]] + called git -C "$CD" pull --ff-only + ran a b + for script in update refresh sibling version configure audit; do + [ -x "$HOME/.local/bin/$script" ] + done +} + +@test "update pulls only once, before re-running its new copy" { + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 0 ] + [ "$(grep -c '^git ' "$STUB_LOG")" -eq 1 ] + ran a +} + +@test "a failing hook doesn't stop the hooks after it" { + hook 10-a 'echo a >> "$RAN"' + hook 20-b 'echo b >> "$RAN"; return 1' + hook 30-c 'echo c >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 1 ] + [[ "$output" == *"Done, with errors in: 20-b.sh"* ]] + ran a b c +} + +@test "a hook that exits doesn't end the update" { + hook 10-a 'echo a >> "$RAN"; exit 0' + hook 20-b 'echo b >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 0 ] + ran a b +} + +@test "a hook using set -e fails at its first failing command" { + hook 10-a 'set -e; echo a >> "$RAN"; false; echo after >> "$RAN"' + hook 20-b 'echo b >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 1 ] + [[ "$output" == *"errors in: 10-a.sh"* ]] + ran a b +} + +@test "a hook's is_installed and functions don't leak into the next hook" { + printf '%s\n' '#!/bin/bash' 'is_installed() { false; }' \ + 'run_update() { echo a >> "$RAN"; }' > "$CD/updates.d/10-a.sh" + # 10-a's is_installed must not stop 20-b, and its run_update must not run + # again for 30-c, which has none + printf '%s\n' '#!/bin/bash' 'run_update() { echo b >> "$RAN"; }' \ + > "$CD/updates.d/20-b.sh" + printf '%s\n' '#!/bin/bash' 'true' > "$CD/updates.d/30-c.sh" + + run "$CD/update" + [ "$status" -eq 0 ] + ran b +} + +@test "update runs the hooks on disk when git pull fails, and reports it" { + export GIT_STATUS=1 + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 1 ] + [[ "$output" == *"could not pull"* ]] + [[ "$output" == *"errors in: git pull"* ]] + ran a +} + +@test "update stops before any hook when / is low on space" { + export DF_AVAIL=1048576 + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 1 ] + [[ "$output" == *"needs at least 10 GiB"* ]] + [ ! -e "$RAN" ] +} + +@test "UPDATE_FORCE skips the free space check" { + export DF_AVAIL=1048576 UPDATE_FORCE=1 + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 0 ] + ran a +} + +@test "update stops when sudo fails" { + export SUDO_STATUS=1 + hook 10-a 'echo a >> "$RAN"' + + run "$CD/update" + [ "$status" -eq 1 ] + [ ! -e "$RAN" ] +} diff --git a/tests/utils.bats b/tests/utils.bats new file mode 100644 index 0000000..0fc029e --- /dev/null +++ b/tests/utils.bats @@ -0,0 +1,221 @@ +#!/usr/bin/env bats + +load test_helper + +# --- run_migrations --------------------------------------------------------- + +# migration NAME [EXIT]: add migrations/NAME.sh that logs its name and exits EXIT +migration() { + mkdir -p "$COREDESKTOP/migrations" + printf '#!/bin/bash\necho %s >> "%s"\nexit %s\n' "$1" "$RAN" "${2:-0}" \ + > "$COREDESKTOP/migrations/$1.sh" +} + +setup_migrations() { + export COREDESKTOP="$BATS_TEST_TMPDIR/coredesktop" + RAN="$BATS_TEST_TMPDIR/ran" + STATE="$HOME/.local/state/coredesktop/migrations" + source "$REPO/utils.sh" +} + +@test "run_migrations runs each migration once, oldest first" { + setup_migrations + migration 1792592710 + migration 1792592700 + migration 1792592720 + + run run_migrations + [ "$status" -eq 0 ] + [ "$(cat "$RAN")" = "$(printf '1792592700\n1792592710\n1792592720')" ] + [ -f "$STATE/1792592700.sh" ] + [ -f "$STATE/1792592720.sh" ] + + run run_migrations + [ "$status" -eq 0 ] + [ "$(wc -l < "$RAN")" -eq 3 ] +} + +@test "run_migrations stops at a failure and retries it next time" { + setup_migrations + migration 1792592700 + migration 1792592710 1 + migration 1792592720 + + run run_migrations + [ "$status" -eq 1 ] + [ "$(cat "$RAN")" = "$(printf '1792592700\n1792592710')" ] + [ -f "$STATE/1792592700.sh" ] + [ ! -f "$STATE/1792592710.sh" ] + [ ! -f "$STATE/1792592720.sh" ] + + migration 1792592710 + run run_migrations + [ "$status" -eq 0 ] + [ "$(tail -n 2 "$RAN")" = "$(printf '1792592710\n1792592720')" ] +} + +@test "run_migrations does nothing without a migrations dir" { + setup_migrations + run run_migrations + [ "$status" -eq 0 ] + [ ! -e "$RAN" ] +} + +@test "migrations are named by timestamp so they sort in the order written" { + for file in "$REPO"/migrations/*; do + [[ "$(basename "$file")" =~ ^[0-9]{10}\.sh$ ]] || { + echo "bad migration name: $file" + return 1 + } + done +} + +# --- install_packages / remove_packages ------------------------------------- + +# A fake pacman that knows about the packages in $INSTALLED, of which those in +# $AS_DEPS were installed as dependencies +stub_pacman() { + stub pacman ' +case "$1" in + -Qi) [[ " $INSTALLED " == *" $2 "* ]] ;; + -Qg) exit 1 ;; + -Qdq) shift; for p; do [[ " $AS_DEPS " == *" $p "* ]] && echo "$p"; done; exit 0 ;; +esac' + stub sudo + stub yay +} + +@test "install_packages installs only the missing packages" { + export INSTALLED="git vim" AS_DEPS="" + stub_pacman + source "$REPO/utils.sh" + + run install_packages git tmux vim neovim + [ "$status" -eq 0 ] + called yay -S --noconfirm --answerdiff=None --answeredit=None --answerclean=None tmux neovim + not_called sudo +} + +@test "install_packages does nothing when everything is installed" { + export INSTALLED="git vim" AS_DEPS="" + stub_pacman + source "$REPO/utils.sh" + + run install_packages git vim + [ "$status" -eq 0 ] + not_called yay +} + +@test "install_packages marks packages installed as dependencies explicit" { + export INSTALLED="git python" AS_DEPS="python" + stub_pacman + source "$REPO/utils.sh" + + run install_packages git python + [ "$status" -eq 0 ] + called sudo pacman -D --asexplicit python + not_called yay +} + +@test "remove_packages removes only exact matches, not providers" { + # herdr-bin provides herdr, so pacman -Qq herdr prints herdr-bin + stub pacman ' +[ "$1" = -Qq ] || exit 1 +case "$2" in + herdr) echo herdr-bin ;; + vim) echo vim ;; + *) exit 1 ;; +esac' + stub sudo + source "$REPO/utils.sh" + + run remove_packages herdr vim missing + [ "$status" -eq 0 ] + called sudo pacman -Rns --noconfirm vim +} + +@test "remove_packages does nothing when nothing is installed" { + stub pacman 'exit 1' + stub sudo + source "$REPO/utils.sh" + + run remove_packages vim + [ "$status" -eq 0 ] + not_called sudo +} + +# --- enable_services -------------------------------------------------------- + +@test "enable_services enables disabled services and skips the rest" { + stub systemctl '[ "$1" = is-enabled ] && [ "$2" = sshd ]' + stub sudo + mkdir -p "$HOME/.config/coredesktop" + echo "system cups" > "$HOME/.config/coredesktop/services-off" + source "$REPO/utils.sh" + + run enable_services --now sshd cups bluetooth + [ "$status" -eq 0 ] + called sudo systemctl daemon-reload + called sudo systemctl enable --now bluetooth + uncalled sudo systemctl enable --now sshd + uncalled sudo systemctl enable --now cups + [[ "$output" == *"cups is turned off"* ]] +} + +@test "enable_services without --now only enables" { + stub systemctl 'exit 1' + stub sudo + source "$REPO/utils.sh" + + run enable_services bluetooth + [ "$status" -eq 0 ] + called sudo systemctl enable bluetooth + uncalled sudo systemctl daemon-reload +} + +# --- initramfs_build_failed ------------------------------------------------- + +pacman_log() { + mkdir -p "$HOME/.local/state/coredesktop" + printf '%s\n' "$@" > "$HOME/.local/state/coredesktop/pacman-update.log" + source "$REPO/utils.sh" +} + +@test "initramfs_build_failed is false without a log" { + source "$REPO/utils.sh" + run initramfs_build_failed + [ "$status" -ne 0 ] +} + +@test "initramfs_build_failed is false when no initramfs was built" { + pacman_log "upgrading vim..." + run initramfs_build_failed + [ "$status" -ne 0 ] +} + +@test "initramfs_build_failed is false when every image built" { + pacman_log "(3/4) Updating linux initcpios..." \ + "==> Building image from preset: /etc/mkinitcpio.d/linux.preset: 'default'" \ + "==> Image generation successful" \ + "==> Building image from preset: /etc/mkinitcpio.d/linux.preset: 'fallback'" \ + "==> Initcpio image generation successful" + run initramfs_build_failed + [ "$status" -ne 0 ] +} + +@test "initramfs_build_failed is true when mkinitcpio reports an error" { + pacman_log "(3/4) Updating linux initcpios..." \ + "==> ERROR: module not found" \ + "==> Image generation successful" + run initramfs_build_failed + [ "$status" -eq 0 ] +} + +@test "initramfs_build_failed is true when an image didn't build" { + pacman_log "(3/4) Updating linux initcpios..." \ + "==> Building image from preset: /etc/mkinitcpio.d/linux.preset: 'default'" \ + "==> Image generation successful" \ + "==> Building image from preset: /etc/mkinitcpio.d/linux.preset: 'fallback'" + run initramfs_build_failed + [ "$status" -eq 0 ] +} diff --git a/updates.d/20-aur.sh b/updates.d/20-aur.sh index 2181d5b..e0a31be 100644 --- a/updates.d/20-aur.sh +++ b/updates.d/20-aur.sh @@ -12,9 +12,11 @@ if ! yay --version &>/dev/null; then sudo pacman -R --noconfirm yay cd /tmp || exit 1 rm -rf yay - git clone https://aur.archlinux.org/yay.git && cd yay || exit 1 + git clone https://aur.archlinux.org/yay.git + cd yay || exit 1 makepkg -si --noconfirm - cd .. && rm -rf yay + cd .. + rm -rf yay cd || exit 1 fi diff --git a/updates.d/80-gnome-extensions.sh b/updates.d/80-gnome-extensions.sh index df5042b..9abec3d 100755 --- a/updates.d/80-gnome-extensions.sh +++ b/updates.d/80-gnome-extensions.sh @@ -50,7 +50,7 @@ reinstall_incompatible_extensions() { rm -rf "$backup" else echo "Reinstall of $uuid failed; restoring the previous version." - rm -rf "${ext_root:?}/$uuid" + rm -rf "${ext_root:?}/${uuid:?}" mv "$backup/$uuid" "$ext_root/$uuid" rmdir "$backup" fi