busybox: fix tests#534656
Conversation
nixpkgs-ci
Bot
added
10.rebuild-linux: 101-500
joaosreis
force-pushed
the
joaosreis/busybox-tests-fix
branch
3 times, most recently
from
12ef730 to
979ce5a
Compare
nixpkgs-ci
Bot
requested review from
a team,
TethysSvensson,
alyssais and
leona-ya
and removed request for
a team
| # fix issue introduced by the previous patch (CVE-2026-26157_CVE-2026-26158.patch) | ||
| (fetchpatch { | ||
| name = "CVE-2026-26157_CVE-2026-26158-2.patch"; | ||
| url = "https://github.com/vda-linux/busybox_mirror/commit/599f5dd8fac390c18b79cba4c14c334957605dae.patch"; |
There was a problem hiding this comment.
we probably should move this back to git.busybox.net when it's available again, but I validated this patch against the mailinglist.
There was a problem hiding this comment.
Yes, it's unfortunate that git.busybox.net is not working as expected. I included the mirror advertised on the project web page.
alyssais
left a comment
alyssais
left a comment
There was a problem hiding this comment.
Backporting the bug fix might be better as a separate PR so test review doesn't block it.
| EOF | ||
| chmod +x cpio.tests | ||
|
|
||
| # Use $bindir/busybox directly and 'which sh' instead of 'which ls' since /bin/sh exists in the sandbox but /bin/ls does not |
There was a problem hiding this comment.
It does by default, but it's configurable and not guaranteed.
There was a problem hiding this comment.
Then I guess there isn't a reproducible way to make this test work. Better to skip it.
|
|
||
| # /usr/bin is empty in the sandbox, so use /nix/store instead | ||
| sed -i "s|/usr/bin|/nix/store|" cpio.tests | ||
| sed -i "s/usr/nix/g" cpio.tests |
There was a problem hiding this comment.
What does this do? Will it try to copy /nix/store?
There was a problem hiding this comment.
It will just recreate the directory structure given an absolute path (/nix/store) inside a temp test dir (cpio.testdir/nix/store), and test this exact behavior. The actual contents of the nix store are never copied. In theory, any /<dir>/<subdir> pattern here would work.
|
|
||
| # wget tests rely on network access, use simple-http-server instead | ||
| simple-http-server --index & | ||
| sed -i 's|http://www.google.com|http://127.0.0.1:8000|' testsuite/wget/* |
There was a problem hiding this comment.
Diff is a bit annoying to read since this just moves around in the same commit other stuff is changed.
|
|
||
| # start-stop-daemon tests don't work with a symlinked /bin/false | ||
| # Use a custom script that exits with 1 instead | ||
| sed -i 's|/bin/false|${lib.getExe' falseAlt "false-alt"}|' start-stop-daemon.tests |
There was a problem hiding this comment.
Could use coreutils-prefixed's gfalse rather than having to make this script.
There was a problem hiding this comment.
The comment is at fault here. The issue is that the start-stop-daemon test with -a option does not work with multicall binaries (which usually define symlinks for each tool, such as busybox, coreutils, etc.), since it overrides argv[0].
|
|
||
| # Using yes seems to be ignoring the SIGPIPE signal, which causes the test to hang | ||
| # Use seq + sed hack to generate the same output instead | ||
| sed -i 's@text=`yes "$text" | head -c 9999`@text=`seq 9999 | sed -E "s/.+/$text/" | head -c 9999`@' md5sum.tests |
There was a problem hiding this comment.
This is changing the test quite a bit. Is it really worth the maintenance overhead rather than skipping? Might be nice to figure out why it doesn't work.
There was a problem hiding this comment.
It doesn't work because the sandbox has a SIGPIPE behavior that yes is not handling well.
But this got me thinking that maybe this could be something specific to busybox's implementation, so I tried to use coreutils' one and that one works. Replacing the usage of yes with the coreutils one seems like a less invasive and more maintainable choice.
|
Created a separate PR only with the tar fix backport so that it is not blocked by the tests review. |
joaosreis
force-pushed
the
joaosreis/busybox-tests-fix
branch
2 times, most recently
from
3120f48 to
b379677
Compare
nixpkgs-ci
Bot
added
10.rebuild-darwin: 0
joaosreis
force-pushed
the
joaosreis/busybox-tests-fix
branch
from
b379677 to
cc8706d
Compare
4 participants
The patch included by the commit 3aa07b8 introduced a bug (also present upstream). This PR adds yet another patch to fix this bug.EDIT: tracked in a different PR (#535282).This PR fixes and enables previously disabled tests (#529790).
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.