Make install.sh repair partial installs and surface required reboots#65
Open
cafedomingo wants to merge 8 commits into
Open
Make install.sh repair partial installs and surface required reboots#65cafedomingo wants to merge 8 commits into
cafedomingo wants to merge 8 commits into
Conversation
Three fixes: 1. Version check now gates only the binary download. Everything else (boot config, unit file) is cheap and idempotent to reapply, so a re-run converges on a correct install no matter where a previous run died. Previously the early exit skipped the unit-file write and never surfaced that a reboot was required. 2. Atomic binary install via place_binary(): stage to a .new file and rename into place, so a truncated download can never leave the service crash-looping on a half-written executable. 3. Unit-file change detection: sha256sum the unit before and after writing, and restart the service if it changed (or if the binary was updated, or if the service isn't running).
Persist a reboot-pending marker on tmpfs (/run) so a re-run that finds the boot config already written but not yet live still holds off on starting the service. Previously needs_reboot only tracked changes made in the current run, so re-running after a partial install (config written, then failed or never rebooted) would restart the service against an un-rebooted kernel where i2c-dev is not loaded and the I2C baudrate/overlays are not applied. The marker is cleared by the reboot itself. Stage the binary into a single mktemp file and swap it in with an atomic rename, consuming the temp file in the rename instead of copying it through a .new intermediate and deleting it separately. No stray display.XXXXXX is left in the install directory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The script's header promises "Idempotent — safe to re-run after partial failures", but the up-to-date early exit broke that promise three ways:
configure_bootandexit 0'd, so a run that existed specifically to repair the boot config fixed it and then never told the user a reboot was required. The fix silently never took effect.install_service— the re-run that was supposed to repair the damage skipped past the missing piece every time.What changed:
place_binary()downloads to a temp file and renames into place atomically, so a failed or interrupted download never overwrites the live executable with a partial file. It does not protect against a successful but corrupt download; that is checksum territory deferred to Verify release checksums and run the display service unprivileged #61.Note on unit-file changes: this PR does not detect whether the unit file changed and does not restart the service for unit-only updates. A re-run will rewrite and reload the unit, but a running service only picks up those changes on its next restart (binary update, manual restart, or reboot). Wiring unit-change restarts to the install script is left for #61, which is the only PR that changes the unit content.
This is a standalone extract of the fixes from #62, without the privilege-separation work from #61.