feat(busybox): run before 80base so applets replace host binaries - #2451
feat(busybox): run before 80base so applets replace host binaries#2451nadzyah wants to merge 2 commits into
Conversation
3bdbbef to
f7663c2
Compare
|
I would prefer a solution that would work without adding the I looked at Commit 4e78a87 ("fix(busybox): install busybox symlinks later in the generation process"). After a search I found the corresponding #536 PR which links to #534. This bug is fixed by commit 95ba032 but the renumbering is unrelated to it. Commit 3975e26 ("feat(busybox): use busybox --install to install itself") says:
So how about moving busybox from What do you think? @devkontrol @Conan-Kudo |
YES. THIS.
On Alpine when both coreutils and busybox are installed (as Alpine packages), I think this is not always the case today. Let's understand when exactly the "expected" initrd generation happening already. So there might be a way to trigger similar kind of code path with Ubuntu as well. I am not against making additional fixes in dracut to make this scenario more accessible, but adding a new global flag is probably not the best option here. |
f7663c2 to
06787f2
Compare
bdrung
left a comment
There was a problem hiding this comment.
The change looks good to me. I only have some minor remarks.
How long will this test take? Maybe it could be folded into an existing test in case the setup takes some time.
We should prefer busybox over dash in shell-interpreter (separate PR).
What kind of test have you done? Did you compare the initrd size on Ubuntu with this change?
06787f2 to
4deac43
Compare
bd3bba0 to
d5b2b4a
Compare
d5b2b4a to
3c4ea59
Compare
|
Fedora failure for test 83 might be related to #2454, please double-check (see also #2455 (comment)). |
Yes, I think so. @devkontrol Looking at TEST-83 in PR #2451, it checks cp/ls/mv/rm/mkdir/sleep/tr under Is it necessary to normalize all applet paths to |
40c21ba to
55bc281
Compare
One way to proceed is to exclude test 83 for Fedora in the GitHub Action and deal with it as a follow-up PR. |
55bc281 to
67e919e
Compare
0e29c00 to
1b891dd
Compare
|
"host bloat" is not accurate for the issue. The issue is that you have two separate implementations of UNIX core utilities installed and you want to use different ones for initramfs and the real system. Whether your choices are correct or not are separate issues, but regardless, it isn't "bloat". Please don't misuse words here, because it will confuse people reviewing it later. |
The wording in question was not introduced by me. The title and commit message were changed during review, and I kept that wording when amending the commit to address subsequent review comments. I’m happy to change the terminology if “host bloat” is not considered accurate. However, the comment assumes I chose that wording intentionally, which wasn’t the case. A simple suggestion for alternative wording would have been sufficient. |
|
The term "bloat" carries a negative connotation. Let's try to avoid it.
The PR no longer uses this flag, lets remove it from the PR description. |
1b891dd to
60b9562
Compare
| # pre-stage host binaries (e.g. Fedora's 01-dist.conf, see #2454). | ||
| # Modules that need the host binary must drop the symlink and reinstall | ||
| [ -L "${_dstdir}/$_path" ] && continue | ||
| [ -e "${_dstdir}/$_path" ] && rm -f "${_dstdir}/$_path" |
There was a problem hiding this comment.
We should install the additional items from install_optional_items after we ran the modules. Then we would not need to do this workaround here. But let's do this change separately.
There was a problem hiding this comment.
#2511 is controversial. So replacing existing binaries by symlinks would be controversial as well. So can we not do it in this PR?
There was a problem hiding this comment.
This means that the test needs to ignore cat, rm, etc that dracut.conf.d/fedora/01-dist.conf includes via install_optional_items on Fedora.
60b9562 to
2d5ead8
Compare
| # busybox is not installed in the azurelinux image | ||
| - container: azurelinux:3.0 | ||
| test: "83" |
There was a problem hiding this comment.
This exclusion can be dropped. We have busybox in azurelinux now: 874c545
|
Note that GNU coreutils has supported building as a single binary since 8.23, released 2014-07-18. If your only concern is size, I am not sure why you would need busybox. See: $ { ./configure --enable-single-binary CFLAGS="$(rpm --eval '%{build_cflags}') -fno-strict-aliasing -fpic"; make -j 16; strip src/coreutils; } > /dev/null 2>&1
$ du --summarize --apparent-size src/coreutils /bin/busybox
1450 src/coreutils
1408 /bin/busybox |
Distros that ship large coreutils binaries (e.g. Ubuntu 25.10 with rust-coreutils) inflate the initrd because 80base copies the host's binaries like cp, ls, mv in full. Rename modules.d/81busybox to modules.d/10busybox so the busybox module runs first. It lays down /usr/bin/busybox and applet symlinks before 80base's inst_multiple calls. The host's util-linux switch_root is still preferred. Bug-Ubuntu: https://bugs.launchpad.net/bugs/2150657
2d5ead8 to
9d0ea3d
Compare
|
Discussion from #2512: we currently cannot use blkid from busybox. The output is different and we use several arguments that are not supported by busybox. |
It would be beneficial to have a busybox test case, independently from resolving https://bugs.launchpad.net/bugs/2150657. Perhaps test case should be factored out into its own PR and for now the test case would check for One of my concern is that distro's package |
Distros that ship large coreutils binaries (e.g. Ubuntu 25.10 with rust-coreutils) inflate the initrd because 80base copies the host's binaries like cp, ls, mv in full.
When the busybox module is included, 80base now skips any name busybox provides as an applet. 81busybox then symlinks those names to /usr/bin/busybox, restoring the smaller initrd footprint.
Bug-Ubuntu: https://bugs.launchpad.net/bugs/2150657
Checklist