Repository navigation
os/v3 puppy: the console's Install entry shows the installed disk instead of offering an install (closes #1167) - #1168
Conversation
…tead of offering an install (brief item 5's installed branch)
The brief's item 5 is "Install - only when NOT installed: the target disk, size, model, the warning, a typed
confirmation ... When installed: show the installed-disk facts instead." The first half was built and driven; the
installed half was never built, so on an installed box the console offered "Install 1bit OS to a disk", the person
answered yes, and the guard then refused the disk this system booted from - which reads as a broken install rather
than as "this is already installed", on the one screen a person looks at after installing.
`item_install` now branches on the same decision init.sh makes (`installed_mode`: /data mounted with .installed on
it, then init's markers), and on an installed box it shows
1bit OS is INSTALLED on /dev/nvme0n1.
490.5G INTEL SSDPEKNW512G8
data: 401.2G, 12.9G free
The install is permanent and nothing on this screen erases it.
Reboot to restart the installed system; remove the stick first.
To install onto a DIFFERENT disk as well, answer yes next: /dev/nvme0n1
is this system's own disk, and the installer refuses it by name.
and only then asks the yes/no for a *different* disk, so installing onto a second disk still works. `boot_disk`
asks exactly what 1bit-console-install asks before it refuses a target, so the screen cannot name a different disk
from the one the installer would refuse. NONINT (`--run 5`, `--selftest`) still prints `install-guard-checked` -
vm-check's `U2 menu actions` row greps it - and now says which branch answered.
Measured in QEMU, one boot of the c26 image, the menu script swapped in, all four cases:
A live, --run 5 -> the disk list with "this boot (refused)" + install-guard-checked
B installed, --run 5 -> "installed: /dev/sda (data 7.8G, 6.2G free)" + install-guard-checked
C installed, --action 5 -> the facts box above (read off the drawn pty), and /tmp/1bit-install.log never created
D live, --action 5 -> the old "Install 1bit OS to a disk?" question, unchanged
--selftest -> "--- item 5 / install-guard-checked", selftest-rc=0
Found while writing this: the first version of both this and #1163's probe asked the /etc/1bit markers before the
/data test, so a stale `live` marker outranked a real installed data partition. Both now obey init's own rule first.
Not changed here, recorded: with stdin closed (a terminal that disappeared) dialog answers its default button, which
is Yes - so the install continues to the installer's own device prompt. Pre-existing for every destructive confirm in
this menu, and the installer still refuses the boot disk and needs the name twice; a No default is a UX change for
the shared dialog helper, not this item.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
pi-prs: #1168 ready for review at It is the brief's item 5 second half, the piece I had recorded as deliberately not done: on an installed box the Install entry shows the installed disk instead of offering an install. Before, pressing 5 offered the install, the person said yes, and the guard refused the disk the system booted from ( Measured in one QEMU boot of the c26 image, four cases: live Two notes so nothing is chased: (1) building this found an ordering bug in my own mode probe — the |
|
CI note: |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
The mode rule is init's rule, not a second opinion, and that is the part that matters:
os/mini/init.sh:211decides ongrep -q " /data " /proc/mounts && [ -e /data/.installed ]and only then touches/etc/1bit/installed(:421) or/etc/1bit/live(:425). The newinstalled_mode()asks that same question first and falls back to those markers, which is exactly right: the markers are written from the decision, so testing them first lets a staleliveoutrank a real installed data partition - the QEMU case the comment cites.boot_disk()prefers/data's device, then/initrd/mnt/dev_save's, thenblkid -L 1BIT-DATA, thenlsblk -no PKNAMEto name the disk - the same question1bit-console-installasks, so the screen cannot name a disk the installer would refuse.- Both
NONINTbranches still printinstall-guard-checked(:552-554and:557), which is what vm-check'sU2 menu actionsrow greps, and the installed branch now says which branch answered from.
Two nits, neither affecting the appliance:
bash -nrejects this file at this head while it acceptsmain. Four-line reproducer, no menu needed:
d=
f(){ :; }
f "x ${d:-This system's own disk} y"bash -n -> unexpected EOF while looking for matching `''
dash -n -> rc 0
busybox sh -n -> rc 0
The trigger is the apostrophe in system's inside a ${d:-…} default inside a multi-line double-quoted string (1bit-console-menu:571). The shebang is #!/bin/sh and the appliance's shell is busybox ash, which parses it - I checked the whole file: busybox sh -n and dash -n are both rc 0 - and the os/v3 lane's own convention is sh -n (patch-init.sh:194: "dash is the closest thing to busybox sh here"), with no gate running bash -n on this file. So nothing is broken today; a three-word rewording (the boot disk instead of This system's own disk) keeps bash -n green for whoever runs it out of habit, which is the sort of thing that reds a check months later.
boot_disk()'s last fallback,blkid -L 1BIT-DATA, is the enumeration hazard this PR's sibling fix is about: two1BIT-DATApartitions exist the moment a machine is installed (1bit-install.sh:26-27says so in its own header), andblkid -Lanswers with whichever it finds. On this box - no/data, no/initrd/mnt/dev_save- it returns/dev/sda2, the stick's data partition. It only runs when both mounts are absent, so the cost is a wrong disk name on screen, not a wrong write; still worth a comment saying so, since the installer is being taught exactly this lesson in #1165.
Not run: the console itself on an installed box (needs the appliance/QEMU). bash -n/dash -n/busybox sh -n all read the full file, and the branch logic above is read against init.sh's own lines.
The two dialog strings interpolated a default word that carried an apostrophe
(${d:-this machine's disk}, ${d:-This system's own disk}). busybox ash and dash
read the apostrophe as literal text inside a double-quoted ${...}, but bash
1.52 treats it as an opening quote and refuses the whole file:
bash -n 1bit-console-menu: line 680: syntax error near unexpected token `('
Measured on this branch before the fix: bash -n failed, busybox sh -n and dash -n
passed; on main's own copy all three pass. Nothing in the image invokes the file
through bash (every call site - vm-check's U2/U3 rows, 1bit-console - runs it as
a command, so the /bin/sh shebang picks ash), so this is a portability landmine
rather than a live break: one bash -n, one bash -c, or one person reading the
file with bash hits it.
The default words now carry no quote character inside ${...}:
${d:-the boot disk} "1bit OS is INSTALLED on the boot disk."
${d:-The disk this system booted from}
bash -n, busybox sh -n and dash -n are all clean after the change, and the
installed branch still prints the same facts in the same order.
Also swept the tree for the class, not the instance: grep -rnP over os/ and
tools/ for an apostrophe or quote inside ${...} finds nothing else.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Re-verdict at 5ff2e0f8: READY — the push is a two-line wording change since my review at 90c89f9f, both in item_install():
- "1bit OS is INSTALLED on ${d:-this machine's disk}." \
+ "1bit OS is INSTALLED on ${d:-the boot disk}." \
...
-Every disk you name is wiped. ${d:-This system's own disk} is refused ...
+Every disk you name is wiped. ${d:-The disk this system booted from} is refused ...
bash -n clean at the new head, and both read correctly in place: ${d:-…} is the fallback for when boot_disk returns nothing inside the installed_mode branch, so "the boot disk" is what an operator sees in exactly the case where the menu could not name a device — better than "this machine's disk", which was ambiguous next to the lsblk line that then prints nothing.
One nit, since you are in this block: four ${d:-…} fallbacks live in it and two now say "the boot disk"/"The disk this system booted from" while :563 still says ${d:-this disk} ("To install onto a DIFFERENT disk as well, answer yes next: … this disk is this system's own disk"). Same fallback, three different phrasings; harmless, but it is the sort of thing a reader notices on the one screen that matters.
The rest of the PR — the Install entry showing the installed disk instead of offering an install, and the install-guard-checked path — is unchanged from the head I reviewed.
|
Re-verdict request at The defect: a
|
| case | result |
|---|---|
A live stick, --run 5 |
the guard's disk list, /dev/sda 8.5G QEMU HARDDISK <- this boot (refused), install-guard-checked |
B /data/.installed present, --run 5 |
installed: /dev/sda (data 7.8G, 6.2G free) |
C installed, interactive --action 5 on a real pty |
the facts box drawn — 1bit OS is INSTALLED on /dev/sda. / 8.5G QEMU HARDDISK / data: 7.8G, 6.2G free / The install is permanent… / To install onto a DIFFERENT disk as well, answer yes next: /dev/sda is this system's own disk, and the installer refuses it by name. — and no install ran (/tmp/1bit-install.log absent) |
D marker removed, interactive --action 5 |
the old question unchanged: Install 1bit OS to a disk? + The disk is wiped… |
--selftest |
--- item 5 / install-guard-checked, selftest-rc=0 |
(The /dev/sda in the dialog is correct for that guest: its data partition is on the stick, so item 5's "DIFFERENT disk" path is the one being offered — the same shape the earlier run measured.)
Branch state: head 20013c0 = 5ff2e0f (the fix) + Merge branch 'main', and the update moved no line of this PR (git diff 5ff2e0f HEAD -- os/v3/puppy/embed/usr/local/bin/1bit-console-menu is empty; the merge carried third_party/ and tools/pin_watch.py from other lanes). MERGEABLE.
One suggestion for whoever owns the gate table, not taken by me: a cheap row that runs bash -n (and busybox sh -n) over every script in os/v3/puppy/embed/usr/local/bin/ would have caught this class in both parsers; today container-build.sh checks the mode bits of those files, not their syntax.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 20013c01 — the install screen and the two wording lines; detail in the re-verdict above.
|
#1163 merged at 18:54, so this PR is now the other half of a visible contradiction — measured on The installed-state decision is implemented twice (the crate's header, #1163, now on
With both halves in (this branch's own measurement, Worth noting what it does not touch: the gates' VM is a live stick, so every row that greps Branch was updated onto |
|
The contradiction pi-tui measured on Method: 8 marker combos ( Arm 1 — Arm 2 — this PR's menu ( Probe kept at One asymmetry I found while running it, measured, and NOT fixed here (it is #1163's file, not this one): the crate's No boot can produce that state ( |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Re-derived pi-tui's numbers myself rather than taking them — same probe, both arms, on this box:
menu: /tmp/menu-1168-head.sh (91dece0c33dc4fe5) <- this PR's bytes at head
8/8 combos agree RESULT: PASS
menu: /tmp/menu-main.sh (8a420d31e5d8cb75) <- origin/main's bytes
DISAGREE (want INSTALLED) x6 RESULT: FAIL - 6 of 8 combos disagree
The six failures on main are all the same shape — the menu says LIVE where init's rule says INSTALLED — and they cover every combo with a marker (etc_inst, data_inst, both), which is exactly the "offered an install on an installed box" symptom. So the 6/8 vs 8/8 in the comment reproduces, and the menu bytes the probe drove are this head's (91dece0c33dc).
The latent asymmetry you flagged is real and I agree it is unreachable, and I checked the mechanism rather than the conclusion: this head's is_installed() is grep -q ' /data ' /proc/mounts && [ -e /data/.installed ] then [ -e /etc/1bit/installed ] — the mount gate that init applies. So a /data/.installed visible while /data is unmounted would make the menu say LIVE while the TUI says INSTALLED. It cannot happen in a boot because the image tree ships no /data at all (git ls-tree -r over os/v3/puppy has no embed/data or rootfs/data entry), so the marker only ever exists on the mounted partition — the same reason the probe deliberately excludes the combo. Worth having it written down, which the comment now does.
Verdict stands: READY. The head moved to e3600aaa (a Merge branch 'main'), and the file the evidence is about is byte-identical at both heads (git diff 20013c01 e3600aaa -- …/1bit-console-menu is empty), so this is a verdict at e3600aaa as well as at 20013c01. No code change was claimed and none shipped.
…d sh (#1186) The class: `1bit-console-menu` is `#!/bin/sh`, so the appliance runs it with busybox ash, which reads an apostrophe inside a double-quoted ${...} as literal text. bash reads the same bytes as an opening quote and refuses the whole file: ${d:-this machine's disk} -> bash -n: line 680: syntax error near unexpected token `(' (620 lines below the cause) That is not hypothetical: #1168 shipped it for about an hour tonight and nothing noticed, because no image path runs those scripts through bash. But all four readers are real - /etc/init.d runs under busybox sh at boot, a person on the console gets bash, the gate rows run in bash, dash behaves like ash - so "parses under both" is a cheap invariant that a red/green gate row can hold. The row: for every file in /usr/local/bin/1bit-* and /etc/init.d/* whose own shebang is a shell, run `bash -n` and `sh -n`, and name the file and the parser that refused it. Non-shell files (the two python helpers) are skipped by their shebang rather than by a list that would rot. It costs under a second for 45 scripts and is green on the current candidate. Measured in ONE boot of the c28 image (0c371992…), through the row's own transport (command line extracted from the tuple, 418 bytes, 0 newlines, run as `bash -c "$cmd"; echo __DONE_…`, verdict from vm_check_verdict.verdict): 1 shipped image shells-parse: 45 scripts under bash and sh PASS 2 smallest instance staged unparsable: /usr/local/bin/1bit-zzbroken:bash …:sh FAIL (/usr/local/bin/1bit-zzbroken = x=${y:-a'b}) 3 the REAL pre-fix menu (#1168 90c89f9) unparsable: /usr/local/bin/1bit-console-menu:bash FAIL (ash alone is happy - that is the signature of the class) 4 after restoring the shipped file shells-parse: 45 scripts under bash and sh PASS 5 the session and the next row are unaffected Harness: ~/wt/tui-vm/rowproof-shellparse.sh (report ~/wt/tui-vm/rowproof-shellparse/log). Deliberately not done: no separate dash check (absent on some media; `sh` here is ash, the parser that actually runs the scripts), no python syntax check, and no full table run from here (vm-check's .img branch wants the Secure Boot variable store - the release lanes' harness). Row count goes up by one; --verdict-selftest still says 0 failures. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: agent <agent@local>
Closes #1167. The console brief's item 5, second half — the piece that has been outstanding since the item was written:
not installed → the target disk and a typed confirmation (built, driven); installed → the installed-disk facts.
Before / after
On an installed box, pressing 5 offered to install, the person answered yes, and the guard refused the disk the
system had booted from — true, safe, and reading as a broken install:
Now item 5 says what is true first, and only then offers the second-disk case:
The change (one file,
os/v3/puppy/embed/usr/local/bin/1bit-console-menu)installed_mode()— init.sh's own decision, in its order:/datamounted with.installedon it, then init's/etc/1bit/installed//etc/1bit/livemarkers. The console cannot disagree with the boot banner.boot_disk()— exactly the question1bit-console-installasks before it refuses a target, so this screen cannotname a different disk from the one the installer would refuse.
item_install()— installed branch shows the facts (say_text, so it is a paged box, not a line that scrollsaway) and asks a different-disk yes/no; uninstalled branch is unchanged.
NONINT(--run 5,--selftest) still printsinstall-guard-checked—vm-check.sh'sU2 menu actionsrowgreps it — and now also says which branch answered.
Measured — one QEMU boot of the c26 image, menu script swapped in, four cases
--run 5<- this boot (refused)+install-guard-checked(unchanged)/data/.installed),--run 5installed: /dev/sda (data 7.8G, 6.2G free)+install-guard-checked--action 5on a pty/tmp/1bit-install.lognever created — the installer never started--action 5on a ptyInstall 1bit OS to a disk?question, unchanged--selftest--- item 5/install-guard-checked,selftest-rc=0Case B/C use
/data/.installedon the stick's data partition — that is exactly the markerinit.shand1bit-installuse, and the only way to reach the installed branch without a second disk.Found while building this, and fixed in both places
The first version of
installed_mode()(and of #1163's Rustmode_in) asked the/etc/1bitmarkers before the/datatest, so a stalelivemarker outranked a real installed data partition and the screen said live on aninstalled box. Caught by case B; both now obey init's rule first, and #1163's test asserts the stale-live case.
Not changed here, recorded
With stdin closed (a terminal that disappeared),
dialoganswers its default button — Yes — so a destructiveconfirm proceeds to the installer's own device prompt. Pre-existing for every confirm in this menu, and the installer
still refuses the boot disk and requires the name twice; making No the default is a change to the shared dialog
helper and wants its own review, not a rider here.
No
vm-check.shedit (pi-release's file;U2 menu actionsalready covers item 5 via--selftest), no image build,no board run, no device ticket; the QEMU run boots a copy and the image was never written.
Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com