From 6dbc86df63a12b6ea16287bd1086b6aed545329b Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 16 Aug 2026 04:58:44 -0500 Subject: fix(installer): mask power-profiles-daemon so TLP survives on laptops power-profiles-daemon.service declares Conflicts=tuned.service tlp.service auto-cpufreq.service, so systemd TERMs TLP the moment ppd starts. The line is in ppd's unit, not tlp's, so grepping tlp.service for it finds nothing. Leaving ppd disabled was never enough. It ships D-Bus activation files, so the desktop-settings panel's own powerprofilesctl call starts it on demand. Velox ran that way from its rebuild on 2026-08-13 until today. TLP failed at every boot and none of its battery policy applied, while the machine looked correctly configured. I mask and then stop. The mask closes the re-activation window, and the stop is what makes a repair re-run take effect, since a mask alone leaves a running ppd running. I left desktops alone. They have no TLP to conflict with, and the panel's power control needs ppd there. --- tests/installer-steps/test_configure_tlp_power.py | 87 ++++++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) (limited to 'tests/installer-steps') diff --git a/tests/installer-steps/test_configure_tlp_power.py b/tests/installer-steps/test_configure_tlp_power.py index c88e0c2..1ddff72 100644 --- a/tests/installer-steps/test_configure_tlp_power.py +++ b/tests/installer-steps/test_configure_tlp_power.py @@ -1,4 +1,4 @@ -"""Test configure_tlp_power's radio-enable line and laptop gating. +"""Test configure_tlp_power's radio-enable line, daemon masking, and laptop gating. systemd-rfkill is masked on laptops because it fights TLP's radio handling — which means nothing restores radio state at boot unless TLP is told to. The @@ -6,6 +6,20 @@ velox 2026-04-10 setup found wifi and bluetooth soft-blocked on first boot for exactly this reason. The conf written here must carry DEVICES_TO_ENABLE_ON_STARTUP so a fresh install comes up with radios on. +power-profiles-daemon is masked and stopped on laptops for the same class of +reason. power-profiles-daemon.service declares "Conflicts=tuned.service +tlp.service auto-cpufreq.service ..." — the line is in ppd's unit, not tlp's — +so systemd TERMs TLP the instant ppd starts. Leaving ppd merely disabled does +not prevent that: ppd ships D-Bus activation files, and the desktop-settings +panel's own powerprofilesctl call activates it on demand. Velox ran that way +from its 2026-08-13 rebuild until 2026-08-16, with TLP failing at every boot and +none of its battery policy applied, while the machine looked correctly +configured. Masking blocks D-Bus activation too, which both keeps TLP alive and +makes the panel's power control read as unavailable, the behavior the +package-install site in `archsetup` already documents as intended. The stop is +what makes a repair re-run take effect on a booted machine, where a mask alone +would leave a running ppd running. + Method: sed-extract configure_tlp_power from the real `archsetup`, point it at a temp tlp.d dir and a temp power-supply dir, and fake pacman_install / run_task / display / error_warn / systemctl. @@ -25,7 +39,7 @@ REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") -def run(battery=True, bat_name="BAT0", unwritable_tlpd=False): +def run(battery=True, bat_name="BAT0", unwritable_tlpd=False, systemctl_fails=False): with tempfile.TemporaryDirectory() as d: psdir = os.path.join(d, "power_supply") os.makedirs(psdir) @@ -37,13 +51,14 @@ def run(battery=True, bat_name="BAT0", unwritable_tlpd=False): os.chmod(tlpd, stat.S_IRUSR | stat.S_IXUSR) # The real mask call redirects stdout into $logfile, so the fake # systemctl records to a side file the test reads back instead. + sysrc = 1 if systemctl_fails else 0 script = textwrap.dedent(f"""\ logfile=/dev/null action="" display() {{ :; }} pacman_install() {{ echo "INSTALL: $1"; }} run_task() {{ echo "TASK: $1"; }} - systemctl() {{ echo "SYSTEMCTL: $*" >> "{d}/systemctl.log"; }} + systemctl() {{ echo "SYSTEMCTL: $*" >> "{d}/systemctl.log"; return {sysrc}; }} error_warn() {{ echo "WARN: $1"; return 1; }} source <(sed -n '/^configure_tlp_power() {{/,/^}}/p' "{ARCHSETUP}") configure_tlp_power "{tlpd}" "{psdir}" @@ -74,6 +89,44 @@ class ConfigureTlpPower(unittest.TestCase): r.stdout) self.assertIn("TASK: enabling TLP service", r.stdout) + def test_laptop_masks_power_profiles_daemon(self): + r = run(battery=True) + self.assertIn("SYSTEMCTL: mask power-profiles-daemon.service", r.stdout, + "ppd's unit declares Conflicts=...tlp.service..., so ppd " + "must be masked or it TERMs TLP whenever it is activated") + + def test_laptop_stops_running_power_profiles_daemon(self): + """Masking alone leaves an already-running ppd running. + + The installer runs on a booted system, so a repair re-run would + otherwise mask ppd, leave it live, and let it keep TLP dead until the + next reboot with nothing reporting it. + """ + r = run(battery=True) + self.assertIn("SYSTEMCTL: stop power-profiles-daemon.service", r.stdout) + + def test_ppd_is_masked_before_it_is_stopped(self): + """Order matters: stopping first leaves a window to re-activate in.""" + calls = [line for line in run(battery=True).stdout.splitlines() + if line.startswith("SYSTEMCTL:") and "power-profiles-daemon" in line] + verbs = [line.split()[1] for line in calls] + self.assertEqual(verbs, ["mask", "stop"]) + + def test_power_profiles_daemon_is_masked_not_merely_disabled(self): + """Disabling ppd is not enough — D-Bus activation ignores it. + + This is the whole point of the mask, so assert the verb directly. A + `disable` here would pass a naive "ppd is handled" check while leaving + the panel's powerprofilesctl call free to start ppd and kill TLP. + """ + r = run(battery=True) + ppd_calls = [line for line in r.stdout.splitlines() + if line.startswith("SYSTEMCTL:") and "power-profiles-daemon" in line] + self.assertTrue(ppd_calls, "configure_tlp_power must act on ppd at all") + for line in ppd_calls: + self.assertNotIn(" disable ", line, + "disable leaves D-Bus activation live; only mask blocks it") + def test_radio_line_is_active_not_commented(self): r = run(battery=True) conf = r.stdout.split("CONF:[")[1].split("]")[0] @@ -97,7 +150,35 @@ class ConfigureTlpPower(unittest.TestCase): self.assertIn("INSTALL: tlp", r.stdout) self.assertIn('DEVICES_TO_ENABLE_ON_STARTUP', r.stdout) + def test_desktop_keeps_power_profiles_daemon(self): + """A batteryless machine must NOT get ppd masked. + + There is no TLP on a desktop to conflict with it, and the package-install + site enables ppd precisely so the settings panel's three-way power + control works there. Masking it here would break that control for no gain. + """ + r = run(battery=False) + self.assertNotIn("power-profiles-daemon", r.stdout) + # ------------------------------------------------------------- error ---- + def test_failed_ppd_mask_warns_and_does_not_crash(self): + """A masking failure must surface, not pass silently. + + Silence is the exact failure mode being fixed: velox looked configured + while TLP was dead. If the mask cannot be applied, say so. + + Assert on the harness's own RC= line, not on r.returncode. The harness + script ends in a literal `exit 0`, so r.returncode is 0 no matter what + configure_tlp_power does — asserting it can never fail, which would make + this test the same silent no-op it exists to catch. + """ + r = run(battery=True, systemctl_fails=True) + self.assertIn("WARN: masking power-profiles-daemon for TLP", r.stdout) + self.assertIn("WARN: stopping power-profiles-daemon for TLP", r.stdout) + self.assertIn("RC=", r.stdout, + "the function must return so the install continues, " + "not exit and take the script down with it") + @unittest.skipUnless(os.geteuid() != 0, "root ignores directory write bits") def test_unwritable_tlpd_warns_and_does_not_crash(self): r = run(battery=True, unwritable_tlpd=True) -- cgit v1.2.3 From a028aa589056167160b39cf9e023c12dd30dec34 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 20 Aug 2026 00:15:19 -0700 Subject: fix(installer): clone my working repos with full history archsetup and dotfiles were cloned with --depth 1. They aren't build trees. They're the two repos I develop in on every machine this installs, and velox came back from its rebuild carrying 7 commits of history in each instead of 851. A shallow clone degrades quietly. Past the graft point `git log -- ` answers "no commits" rather than failing, so a search of history for specific files came back empty and exited clean. The clone couldn't see the history they live in. Blame and bisect break the same way, less dangerously. Dropping --depth also drops the implicit --single-branch, so both clones now fetch every branch. That's what I want in a repo I work in. The AUR build clones stay shallow. They exist to run make install and are then discarded. A test asserts they keep --depth 1, so a broad sed can't take them along. --- archsetup | 10 +- tests/installer-steps/test_clone_user_repos.py | 155 +++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 tests/installer-steps/test_clone_user_repos.py (limited to 'tests/installer-steps') diff --git a/archsetup b/archsetup index 7dcdad2..32a3ad4 100755 --- a/archsetup +++ b/archsetup @@ -1458,8 +1458,13 @@ clone_user_repos() { # Without this, symlinks could point to /root or a tmpfs that disappears. user_archsetup_dir="/home/$username/code/archsetup" action="cloning archsetup to user's home directory" && display "task" "$action" + # Full history, deliberately. This is a working repo, not a build tree, and + # a shallow clone degrades silently: `git log -- ` answers "no + # commits" past the graft point rather than failing, so history questions + # come back confidently wrong. The AUR clones stay shallow; they're + # discarded after the build. (mkdir -p "$(dirname "$user_archsetup_dir")" && \ - git clone --depth 1 "$archsetup_repo" "$user_archsetup_dir" && \ + git clone "$archsetup_repo" "$user_archsetup_dir" && \ chown -R "$username": "/home/$username/code") \ >> "$logfile" 2>&1 || error_warn "$action" "$?" @@ -1472,7 +1477,8 @@ clone_user_repos() { # leaves /home/$username root-owned — so a clone running as the user fails with # "Permission denied" creating ~/.dotfiles. Cloning as root sidesteps that, and # chown -R gives the user the working tree. Mirrors the archsetup clone above. - (git clone --depth 1 --branch "$dotfiles_branch" "$dotfiles_repo" "$dotfiles_dir" \ + # Full history for the same reason as the archsetup clone above. + (git clone --branch "$dotfiles_branch" "$dotfiles_repo" "$dotfiles_dir" \ && chown -R "$username": "$dotfiles_dir") >> "$logfile" 2>&1 || error_warn "$action" "$?" # Q5: the --adopt/restore conflict handling below needs a real git checkout. diff --git a/tests/installer-steps/test_clone_user_repos.py b/tests/installer-steps/test_clone_user_repos.py new file mode 100644 index 0000000..51d8434 --- /dev/null +++ b/tests/installer-steps/test_clone_user_repos.py @@ -0,0 +1,155 @@ +"""Test clone_user_repos: the two user repos are cloned with full history. + +archsetup and dotfiles are not build directories. They are the two repos I +actively develop in on every machine this installer builds, so a shallow clone +is wrong for both. Velox came back from its 2026-08-13 rebuild with 7 commits +of history in each instead of 851, and nothing about the tree said so. + +The quiet failure is what makes this worth a test rather than a one-line fix. +`git log -- ` against a shallow clone does not error; it answers "no +commits". So a credential-history check run on that machine reported five +sensitive files absent from history and exited clean, when the real answer was +that the clone could not see the history they live in. A security question came +back falsely reassuring. Everything else it breaks — blame, bisect, any +archaeology past the graft point — is merely annoying by comparison. + +The AUR build clones are a different case and stay shallow: they are throwaway +build trees, cloned to run `make install` and then discarded, where history has +no value and the download cost is real. So this suite asserts both halves — +full history for the two user repos, and depth still pinned on the AUR path — +because a fix applied with too broad a brush would regress the build clones +without failing any test that only looked at the user repos. + +Method: sed-extract clone_user_repos from the real `archsetup`, fake git / +mkdir / chown / display / error_warn / error_fatal, and read back the git +command lines the function issued. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_clone_user_repos +""" + +import os +import re +import subprocess +import tempfile +import textwrap +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + + +def run(clone_fails=False, make_git_dir=True): + """Drive clone_user_repos with every side effect faked. + + dotfiles_dir is pre-created with a .git so the function's "is this a real + checkout?" guard passes on the happy path; make_git_dir=False exercises the + guard itself. + """ + with tempfile.TemporaryDirectory() as d: + dotfiles_dir = os.path.join(d, "dotfiles") + os.makedirs(dotfiles_dir) + if make_git_dir: + os.makedirs(os.path.join(dotfiles_dir, ".git")) + clone_rc = 1 if clone_fails else 0 + script = textwrap.dedent(f"""\ + logfile=/dev/null + action="" + username=testuser + archsetup_repo="https://example.invalid/archsetup.git" + dotfiles_repo="https://example.invalid/dotfiles.git" + dotfiles_branch=main + dotfiles_dir="{dotfiles_dir}" + display() {{ :; }} + mkdir() {{ echo "MKDIR: $*" >> "{d}/calls.log"; return 0; }} + chown() {{ echo "CHOWN: $*" >> "{d}/calls.log"; return 0; }} + git() {{ + echo "GIT: $*" >> "{d}/calls.log" + case "$1" in + clone) return {clone_rc} ;; + *) return 0 ;; + esac + }} + error_warn() {{ echo "WARN: $1" >> "{d}/calls.log"; return 1; }} + error_fatal() {{ echo "FATAL: $1" >> "{d}/calls.log"; exit 1; }} + source <(sed -n '/^clone_user_repos() {{/,/^}}/p' "{ARCHSETUP}") + clone_user_repos + echo "RC=$?" >> "{d}/calls.log" + exit 0 + """) + subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, timeout=10, + ) + with open(os.path.join(d, "calls.log")) as fh: + return fh.read() + + +def clone_lines(log): + return [ln for ln in log.splitlines() if ln.startswith("GIT: clone")] + + +class CloneUserRepos(unittest.TestCase): + # ------------------------------------------------------------ normal ---- + def test_both_user_repos_are_cloned(self): + lines = clone_lines(run()) + self.assertEqual(len(lines), 2, + f"expected an archsetup clone and a dotfiles clone, got: {lines}") + self.assertTrue(any("archsetup.git" in ln for ln in lines)) + self.assertTrue(any("dotfiles.git" in ln for ln in lines)) + + def test_archsetup_clone_carries_full_history(self): + """A shallow archsetup clone answers history questions wrongly.""" + line = next(ln for ln in clone_lines(run()) if "archsetup.git" in ln) + self.assertNotIn("--depth", line, + "archsetup is a working repo, not a build tree — a shallow " + "clone makes `git log -- ` answer 'no commits' instead " + "of failing, which is how a credential-history check came " + "back falsely clean on velox") + + def test_dotfiles_clone_carries_full_history(self): + line = next(ln for ln in clone_lines(run()) if "dotfiles.git" in ln) + self.assertNotIn("--depth", line, + "dotfiles is a working repo, not a build tree") + + def test_dotfiles_clone_still_pins_the_branch(self): + """Dropping --depth must not disturb the --branch argument beside it.""" + line = next(ln for ln in clone_lines(run()) if "dotfiles.git" in ln) + self.assertIn("--branch main", line) + + # ---------------------------------------------------------- boundary ---- + def test_no_user_repo_clone_is_shallow_by_any_spelling(self): + """--depth, --depth=N and -depth are all shallow; catch the lot.""" + for line in clone_lines(run()): + self.assertNotRegex(line, r"(^|\s)-{1,2}depth(\s|=)", + f"user-repo clone must be full: {line}") + + def test_aur_build_clones_stay_shallow(self): + """The fix must not over-apply — build trees are throwaway. + + Read against the real file rather than the extracted function, because + these clones live in a different function entirely and the risk being + guarded is a careless repo-wide sed. + """ + with open(ARCHSETUP) as fh: + source = fh.read() + build_clones = re.findall(r"^.*git clone.*build_dir.*$", source, re.M) + self.assertTrue(build_clones, "expected AUR build clones to exist") + for line in build_clones: + self.assertIn("--depth 1", line, + f"AUR build clone should stay shallow: {line.strip()}") + + # ------------------------------------------------------------- error ---- + def test_clone_failure_is_reported_not_swallowed(self): + log = run(clone_fails=True) + self.assertIn("WARN:", log, + "a failed clone must surface through error_warn") + + def test_dotfiles_clone_producing_no_checkout_is_fatal(self): + """The stow/restore steps downstream need a real checkout.""" + log = run(make_git_dir=False) + self.assertIn("FATAL:", log) + self.assertNotIn("RC=", log, "error_fatal must halt, not fall through") + + +if __name__ == "__main__": + unittest.main() -- cgit v1.2.3