#1885 fix: run every pending migration in version order and record it

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 - Hyprland: dropped the legacy hyprlang dot, the files it deployed no longer exist
 
 ### Fixed
+- Core: every migration that has not been applied yet now runs in version order and is recorded, instead of only the newest one running and retiring the rest unseen
 - Core: app launchers no longer show a false error when an unrelated `DEBUG` variable contains a non-boolean value such as `release`
 - Desktop: the generated battery notification startup command now launches `batterynotify.lua` instead of the removed shell implementation
 - Hyprland: Lua keybinds again match the documented shortcuts for window management, screenshots, wallpapers, Waybar, selectors, workspaces and the scratchpad

Scripts/global_fn.sh

@@ -180,3 +180,38 @@ print_log() {
         cat
     fi
 }
+
+# Runs each migration in "$1" missing from the record in "$2", in version order,
+# and records the ones that exit zero. Migrations must therefore be safe to run
+# on a machine that has no record yet, which replays all of them once.
+run_pending_migrations() {
+    local migrationDir="$1"
+    local stateFile="$2"
+    local migrationFile
+    local applied=0
+    local pending=0
+
+    [ -d "${migrationDir}" ] || return 0
+    find "${migrationDir}" -maxdepth 1 -type f | grep -q . || return 0
+
+    mkdir -p "$(dirname "${stateFile}")"
+    [ -f "${stateFile}" ] || : >"${stateFile}"
+
+    while read -r migrationFile; do
+        [ -n "${migrationFile}" ] || continue
+        grep -qxF "${migrationFile}" "${stateFile}" && continue
+        pending=$((pending + 1))
+        echo "Found migration file: ${migrationFile}"
+        # stdin is closed for the migration: inheriting the loop's stdin let one
+        # that reads input swallow the names of every migration after it.
+        if sh "${migrationDir}/${migrationFile}" </dev/null; then
+            printf '%s\n' "${migrationFile}" >>"${stateFile}"
+            applied=$((applied + 1))
+        else
+            print_log -warn "Migration" "Failed to execute ${migrationFile}"
+        fi
+    done < <(find "${migrationDir}" -maxdepth 1 -type f -printf '%f\n' | sort -V)
+
+    [ "${pending}" -gt 0 ] || echo "No outstanding migrations in ${migrationDir}."
+    return 0
+}

Scripts/install.sh

@@ -453,16 +453,9 @@ if has_operation "restore"; then
 
 	echo "Running migrations from: ${migrationDir}"
 
-	if [ -d "${migrationDir}" ] && find "${migrationDir}" -type f | grep -q .; then
-		migrationFile=$(find "${migrationDir}" -maxdepth 1 -type f -printf '%f\n' | sort -r | head -n 1)
+	migrationStateFile="${XDG_STATE_HOME:-${HOME}/.local/state}/hyde/migration/applied"
 
-		if [[ -n "${migrationFile}" && -f "${migrationDir}/${migrationFile}" ]]; then
-			echo "Found migration file: ${migrationFile}"
-			sh "${migrationDir}/${migrationFile}" || { true && print_log -warn "Migration" "Failed to execute ${migrationFile}"; }
-		else
-			echo "No migration file found in ${migrationDir}. Skipping migrations."
-		fi
-	fi
+	run_pending_migrations "${migrationDir}" "${migrationStateFile}"
 
 fi
 

tests/README.md

@@ -22,7 +22,7 @@ is adding a file.
 | `test_app_wrapper.sh` | Launching through either `app.sh` execution path does not leak an unrelated non-boolean `DEBUG` value into app2unit or xdg-terminal-exec |
 | `test_binds.sh` | Loads the Lua keybinds against a stubbed Hyprland API: no two binds share a combination once modifiers are folded to what Hyprland matches on, no keysym sits in a modifier position, no bind uses the `code:NN` form the Lua parser rejects, every bind has a description, every `hyde-shell` command it runs exists. Touchpad gestures are checked in the same pass: a valid finger count, a direction and action Hyprland accepts, and no two gestures on the same finger count and direction |
 | `test_git.sh` | The tree holds no gitlink without a matching `.gitmodules` entry, which would break `git submodule` and anything walking submodules |
-| `test_dots.sh` | Every installer metafile under `Scripts/dots` parses, declares the keys the installer needs, uses a known action, and points at a source directory and source paths that exist. It also keeps Grimblast on the fixed official source |
+| `test_dots.sh` | Every installer metafile under `Scripts/dots` parses, declares the keys the installer needs, uses a known action, and declares a usable `source` where it has one. A local entry's `source_root` and its non-glob paths have to exist inside the checkout; an entry backed by a remote `source` is exempt from those checks, since its files come from an archive the installer downloads, but its path types are still checked. It also keeps Grimblast on the fixed official source |
 | `test_lua_syntax.sh` | Every shipped Lua file parses |
 | `test_schema.sh` | Generated schema artifacts use the current Lua battery notification daemon rather than the removed shell implementation |
 | `test_screenshot_wrapper.sh` | Satty receives a compatible default GTK renderer while preserving explicit renderer overrides |

tests/python/check_dots.py

@@ -20,32 +20,71 @@
 
 
 def declared_paths(table: dict) -> list:
-    """`paths` is a single string or a list of them."""
+    """`paths` is a single string or a list of them.
+
+    Anything else is wrapped rather than iterated, so a malformed value is
+    reported as a wrong type instead of crashing the checker.
+    """
     paths = table.get("paths", [])
-    return [paths] if isinstance(paths, str) else list(paths)
+    if isinstance(paths, str):
+        return [paths]
+    if isinstance(paths, list):
+        return list(paths)
+    return [paths]
 
 
 def has_glob(relative: str) -> bool:
     """A pattern is resolved at deploy time, so it cannot be checked here."""
     return any(character in relative for character in "*?[")
 
 
-def inside_repo(relative: str) -> bool:
-    """A source path has to stay in the checkout, absolute or `..` included."""
-    root = REPO_ROOT.resolve()
+def inside(root: pathlib.Path, relative: str) -> bool:
+    """A declared path has to stay under `root`, absolute or `..` included."""
     try:
         return (root / relative).resolve().is_relative_to(root)
-    except (OSError, ValueError):
+    except (OSError, RuntimeError, ValueError):
+        # Older Python raises RuntimeError rather than OSError on a symlink loop.
         return False
 
 
-def entries(document: dict) -> list[tuple[str, dict]]:
-    """Yields (component, table) for every files table in a metafile."""
+def inside_repo(relative: str) -> bool:
+    """A source root has to stay in the checkout."""
+    return inside(REPO_ROOT.resolve(), relative)
+
+
+def declared_sources(document: dict) -> list[tuple[str, object]]:
+    """Yields (where, source) for every source declared in a metafile."""
     found = []
     for component, body in document.items():
         if isinstance(body, dict):
+            if "source" in body:
+                found.append((component, body["source"]))
+            for index, table in enumerate(body.get("files", []) or []):
+                if isinstance(table, dict) and "source" in table:
+                    found.append((f"{component}.files[{index}]", table["source"]))
+    return found
+
+
+def is_remote_source(source: object) -> bool:
+    """Only a non-empty string names something the installer can fetch."""
+    return isinstance(source, str) and bool(source.strip())
+
+
+def entries(document: dict) -> list[tuple[str, dict, bool]]:
+    """Yields (component, table, remote) for every files table in a metafile.
+
+    `remote` is true when the component or the entry declares a usable `source`.
+    Those files come from an archive the installer downloads, so their paths
+    resolve against the extracted tree rather than this checkout and cannot be
+    validated here. A malformed `source` does not earn that exemption.
+    """
+    found = []
+    for component, body in document.items():
+        if isinstance(body, dict):
+            component_remote = is_remote_source(body.get("source"))
             for table in body.get("files", []) or []:
-                found.append((component, table))
+                remote = component_remote or is_remote_source(table.get("source"))
+                found.append((component, table, remote))
     return found
 
 
@@ -80,7 +119,11 @@ def fail(message: str) -> None:
             fail(f"{name} is not valid TOML: {error}")
             continue
 
-        for component, table in entries(document):
+        for where_source, source in declared_sources(document):
+            if not is_remote_source(source):
+                fail(f"{name} [{where_source}] declares an unusable source {source!r}")
+
+        for component, table, remote in entries(document):
             where = f"{name} [{component}.files]"
 
             if "paths" not in table:
@@ -92,19 +135,26 @@ def fail(message: str) -> None:
             if action is not None and action not in ACTIONS:
                 fail(f"{where} has an unknown action {action!r}")
 
+            for relative in declared_paths(table):
+                if not isinstance(relative, str):
+                    fail(f"{where} declares a path as {type(relative).__name__}, expected a string")
+
             source_root = table.get("source_root")
-            if source_root is not None:
-                if not isinstance(source_root, str):
-                    fail(f"{where} declares source_root as {type(source_root).__name__}, expected a string")
-                elif not inside_repo(source_root):
+            if source_root is not None and not isinstance(source_root, str):
+                fail(f"{where} declares source_root as {type(source_root).__name__}, expected a string")
+            elif isinstance(source_root, str) and not remote:
+                if not inside_repo(source_root):
                     fail(f"{where} points outside the repository with source_root {source_root!r}")
                 elif not (REPO_ROOT / source_root).is_dir():
                     fail(f"{where} points at a missing source_root {source_root!r}")
                 else:
+                    root = (REPO_ROOT / source_root).resolve()
                     for relative in declared_paths(table):
                         if not isinstance(relative, str):
-                            fail(f"{where} declares a path as {type(relative).__name__}, expected a string")
-                        elif not has_glob(relative) and not (REPO_ROOT / source_root / relative).exists():
+                            continue
+                        if not inside(root, relative):
+                            fail(f"{where} points outside its source_root with path {relative!r}")
+                        elif not has_glob(relative) and not (root / relative).exists():
                             fail(f"{where} points at a missing path {relative!r}")
 
         for component, table in dependencies(document):

tests/test_migrations.sh

@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+# Migrations have to run in version order, once each, and record what succeeded.
+
+. "$(dirname -- "$0")/lib/common.sh"
+
+# shellcheck source=/dev/null
+if ! . "$REPO_ROOT/Scripts/global_fn.sh" 2>/dev/null; then
+    fail "unable to source global_fn.sh"
+    finish
+fi
+
+if ! command -v run_pending_migrations >/dev/null 2>&1 && ! type run_pending_migrations >/dev/null 2>&1; then
+    fail "run_pending_migrations is not defined in global_fn.sh"
+    finish
+fi
+
+work_dir=$(mktemp -d)
+trap 'rm -rf "$work_dir"' EXIT
+
+migration_dir="$work_dir/migrations"
+state_file="$work_dir/state/applied"
+order_log="$work_dir/order.log"
+mkdir -p "$migration_dir"
+
+for version in v25.9.1 v26.10.1; do
+    printf '#!/usr/bin/env sh\nprintf "%%s\\n" "%s" >>"%s"\n' "$version" "$order_log" \
+        >"$migration_dir/$version.sh"
+done
+# Reads stdin: inheriting the runner's stdin would let it swallow the names of
+# the migrations queued after it.
+printf '#!/usr/bin/env sh\ncat >/dev/null\nprintf "%%s\\n" "v26.4.3" >>"%s"\n' "$order_log" \
+    >"$migration_dir/v26.4.3.sh"
+printf '#!/usr/bin/env sh\nexit 1\n' >"$migration_dir/v26.11.0.sh"
+chmod +x "$migration_dir"/*.sh
+
+run_pending_migrations "$migration_dir" "$state_file" >/dev/null 2>&1
+
+expected_order='v25.9.1
+v26.4.3
+v26.10.1'
+actual_order=$(cat "$order_log" 2>/dev/null || true)
+[ "$actual_order" = "$expected_order" ] ||
+    fail "migrations ran out of version order: $(echo "$actual_order" | tr '\n' ' ')"
+
+for version in v25.9.1 v26.4.3 v26.10.1; do
+    grep -qxF "$version.sh" "$state_file" ||
+        fail "$version.sh succeeded but was not recorded as applied"
+done
+
+grep -qxF 'v26.11.0.sh' "$state_file" &&
+    fail "a migration that exited non-zero was recorded as applied"
+
+: >"$order_log"
+run_pending_migrations "$migration_dir" "$state_file" >/dev/null 2>&1
+
+[ -s "$order_log" ] &&
+    fail "already applied migrations ran a second time: $(tr '\n' ' ' <"$order_log")"
+
+second_pass=$(run_pending_migrations "$migration_dir" "$state_file" 2>&1 || true)
+case $second_pass in
+*"No outstanding migrations"*)
+    fail "a migration was still pending but the run reported none outstanding"
+    ;;
+esac
+
+grep -q 'run_pending_migrations' "$REPO_ROOT/Scripts/install.sh" ||
+    fail "install.sh does not call run_pending_migrations"
+
+missing_dir_output=$(run_pending_migrations "$work_dir/absent" "$state_file" 2>&1)
+[ -z "$missing_dir_output" ] ||
+    fail "a missing migration directory produced output: $missing_dir_output"
+
+finish