de10nano: /bin/sh and root's login shell are bash, as on stock - #145
Merged
Conversation
Stock ships /bin/sh -> bash and root:x:0:0:root:/root:/bin/bash. We shipped Buildroot's defaults -- BusyBox ash as /bin/sh, root on /bin/sh -- because no fragment ever set BR2_SYSTEM_BIN_SH, and nothing recorded the difference. Bash was already in the image for wifi.sh. Consequences of the gap: a user script in /media/fat/Scripts with a #!/bin/sh shebang and bash syntax runs on stock and may not here, and an interactive root session gets ash instead of bash (WinSCP's error text in #142 literally says "BASH is recommended"). One symbol, BR2_SYSTEM_BIN_SH_BASH=y, gives stock's exact layout: Buildroot's SKELETON_INIT_COMMON_SET_BIN_SH finalize hook both re-links /bin/sh and rewrites root's passwd shell. No overlay or post-build edit needed. It depends on BUSYBOX_SHOW_OTHERS (already on); the fragment carries a WARNING because kconfig would drop it silently otherwise. - de10nano golden.sha256 line regenerated (the only stack that changes) - docs/buildroot-config.md §5.19 rationale; docs/init-parity.md new row - ci-tests.sh asserts ./usr/bin/sh -> bash and root's passwd shell from rootfs.tar, and runs the dhcpcd timezone hook under the target's own bash --posix (what /bin/sh now is on the box) as well as BusyBox ash Verified: make all -> output/target/bin/sh -> bash, root:/bin/bash, both in rootfs.tar; ci-tests new checks PASS; check-linux-img.sh all assertions passed; check-config-fragments OK. Fixes #144 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vq3yLoYntjiJ1wUaAdruLU
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The functional changes are straightforward and well-documented with CI coverage; remaining feedback is limited to minor maintainability nits in comments/test robustness.
Pull request overview
This PR aligns the DE10-Nano image with stock MiSTer behavior by making /bin/sh resolve to bash and setting root’s login shell to /bin/bash, and then documents and asserts that parity in CI.
Changes:
- Enable
BR2_SYSTEM_BIN_SH_BASH=yfor the DE10-Nano image fragment to make/bin/sh -> bashand root’s passwd shell/bin/bash. - Update documentation to record the parity decision and rationale.
- Update CI assertions to verify
/bin/shand root’s login shell, and execute the timezone hook tests under the target’s bash (POSIX mode) in addition to BusyBox ash.
File summaries
| File | Description |
|---|---|
configs/fragments/de10nano-image.fragment |
Enables BR2_SYSTEM_BIN_SH_BASH and adds an explanatory warning comment. |
configs/fragments/golden.sha256 |
Updates the DE10-Nano golden hash to match the new fragment output. |
docs/buildroot-config.md |
Documents the rationale/behavior of BR2_SYSTEM_BIN_SH_BASH in this project. |
docs/init-parity.md |
Records /bin/sh + root shell parity with stock. |
scripts/ci-tests.sh |
Adds CI assertions for /bin/sh and root’s shell, and expands timezone-hook testing under target bash --posix. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ot grep Copilot review nits: spell out BR2_SYSTEM_BIN_SH_BASH / BR2_PACKAGE_BUSYBOX_SHOW_OTHERS in the fragment WARNING and the ci-tests comment so they grep; take only the first root: line from passwd so a malformed file fails the check legibly instead of confusing the case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vq3yLoYntjiJ1wUaAdruLU
Follow-up to BR2_SYSTEM_BIN_SH_BASH (#144): an audit of every #!/bin/sh file, udev RUN+= line, inittab entry, sourced dhcpcd hook and profile.d snippet on the target found nothing that behaves differently under bash in POSIX mode (all parse under `bash --posix -n`; the one bashism in the tree, usr/bin/timidity's `function`, is a stock file written for bash). It did find five statements that still call BusyBox ash the shell that runs things on the box: - docs/stock-reconciliation.md timidity row: the ash BASH_COMPAT note was the accommodation, not the design - README.md, scripts/test-timezone.sh header/comments, ADR 0025 (dated addendum): the timezone hook test now runs three times -- host shell, target bash --posix (what /bin/sh is on the box), target BusyBox ash Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vq3yLoYntjiJ1wUaAdruLU
…fragment survived The three exFAT cases (exfat, fsck-request, symlink) have been failing locally with "mount: mounting /dev/vdb1 on /mnt/fat failed: No such device": the QEMU test kernel had `# CONFIG_EXFAT_FS is not set` even though scripts/test-initramfs/qemu-test-kernel.config says =y. Cause: the kernel's merge_config.sh ends with a BARE `make ... alldefconfig` in the current directory (no -C). Invoked from this repo's root, that make hits the wrapper Makefile, which forwards `alldefconfig` to Buildroot, which dies with "Can't read seed configuration", and merge_config.sh exits before writing the merged config. Symbols multi_v7_defconfig already had looked merged; the two it lacked (CONFIG_EXFAT_FS, CONFIG_FAT_DEFAULT_UTF8) were silently dropped. Reproduced with a fresh defconfig + merge from the repo root (dropped) and from inside the kernel tree (kept). Fix: cd into $KERNEL_SRC for the merge, pass ARCH/CROSS_COMPILE, and die if it fails. Backstop: after olddefconfig, assert every `CONFIG_X=y` in the fragment is in the resolved .config, with a message that names the stale-cache remedy (rm -rf work/test-initramfs-kbuild) -- a cache built by a failed merge keeps its broken .config forever because the merge only runs on bootstrap. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vq3yLoYntjiJ1wUaAdruLU
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
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.
Fixes #144. Companion to #143 (WinSCP
resizefix) but independent: separate root cause, separate branch off master.What
BR2_SYSTEM_BIN_SH_BASH=yinconfigs/fragments/de10nano-image.fragment, plus the de10nano golden hash, docs, and CI assertions.Why
release_20250402)/etc/passwd/bin/bash/bin/sh/bin/bash/bin/sh-> bash-> bashNo fragment ever set
BR2_SYSTEM_BIN_SH, so Buildroot's BusyBox default applied and nothing indocs/init-parity.mdrecorded it. User scripts with#!/bin/sh+ bash syntax work on stock and could fail here; interactive root got ash.One symbol does both halves: Buildroot's
SKELETON_INIT_COMMON_SET_BIN_SHfinalize hook runsln -sf bash /bin/shandsed '/^root:/s,[^/]*$,bash,' /etc/passwd. No overlay or post-build change. BusyBoxashstays built and in/etc/shells, as on stock.Changes
de10nano-image.fragment: the symbol, with a WARNING that it silently depends onBUSYBOX_SHOW_OTHERS.golden.sha256: de10nano line regenerated viacheck-config-fragments.sh --update-golden(only stack affected; de10nano-kernel/de25nano/rt unchanged).docs/buildroot-config.md§5.19: rationale.docs/init-parity.md: new row for/etc/passwd(root's shell) +/bin/sh.scripts/ci-tests.sh: asserts./usr/bin/sh -> bashand root's passwd shell fromrootfs.tar(P3.4 section), and runs the dhcpcd timezone hook under the target's ownbash --posix(what/bin/shis on the box now) in addition to BusyBox ash.Verification
make de10nano-defconfig: resolved.confighasBR2_SYSTEM_BIN_SH_BASH=y,BR2_SYSTEM_BIN_SH="bash".check-config-fragments.sh: OK, 4 stacks regenerate cleanly.make all(incremental):output/target/bin/sh -> bash,root:x:0:0:root:/root:/bin/bash, same inrootfs.tar;check-linux-img.shall assertions passed.scripts/ci-tests.sh: the four new/changed checks PASS. The only failures are the three known exFAT cases of the local QEMU initramfs harness (harness kernel issue, pre-existing, unrelated).Not tested on hardware yet; no runtime behaviour changes beyond which interpreter
shresolves to.Follow-ups folded in (per review)
grep -m1for root's passwd line.#!/bin/shfile, udevRUN+=line, inittab entry, sourced dhcpcd hook andprofile.dsnippet on the target was checked. All parse underbash --posix -n; none uses a construct that differs between BusyBox ash (BASH_COMPAT) and bash in POSIX mode. Eight of them are stock files vendored byte-identical, for which bash is the original interpreter. The initramfs and installer images are separate defconfigs and stay on ash. Five doc/comment statements that still called ash "the shell on the box" were updated (stock-reconciliation timidity row, README, test-timezone.sh header, ADR 0025 dated addendum).scripts/test-initramfs.shinvoked the kernel'smerge_config.shfrom the repo root, and that script ends with a baremake alldefconfigin the cwd, which hit our wrapper Makefile and failed, soCONFIG_EXFAT_FS/CONFIG_FAT_DEFAULT_UTF8were silently dropped from the test kernel and the three exFAT cases failed with "No such device". Now runs from the kernel tree, dies on failure, and asserts every fragment=ysurvived (with the stale-cache remedy in the message). Unrelated to the shell change but it is what made "the only failures are the known exFAT cases" true.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vq3yLoYntjiJ1wUaAdruLU