Support installing multi-user on OpenRC and with BusyBox cp#14756
Support installing multi-user on OpenRC and with BusyBox cp#14756holysoles wants to merge 3 commits intoNixOS:masterfrom
Conversation
scripts/install-multi-user.sh
Outdated
| } | ||
|
|
||
| is_cp_busybox() { | ||
| if [ "$(readlink $(which cp))" = "/bin/busybox" ]; then |
There was a problem hiding this comment.
Wouldn't it be more robust to check the output of cp --help. It outputs something like if it's busybox:
cp --help
BusyBox v1.36.1 () multi-call binary.
There was a problem hiding this comment.
We could test a condition like this
"$(cp --help 2>&1 | awk '{print $1; exit}')" = "BusyBox"However, I believe my implementation is more robust. While I agree calling other two programs introduces room for error, I don't think the help text of a program should be scripted against. Assuming readlink and which succeed, the current approach will always work, whereas BusyBox could change their help text and break the install script.
I am open to changing this though if needed
There was a problem hiding this comment.
In that case maybe check the basename and not the absolute path? Hardcoding /bin doesn't seem ideal
There was a problem hiding this comment.
You make an excellent point, though the command is less readable now..
if you think your suggestion is more readable/maintainable I'm happy to swap approaches
There was a problem hiding this comment.
the current approach will always work
Not if you’re somewhere that uses busybox compiled to individual binaries instead of a multi-call binary
No idea what the best approach would be here tho
|
Would also be nice if we could get installer tests for alpine in hydra so this kind of stuff doesn't happen again. The tests are here https://github.com/NixOS/nix/blob/master/tests/installer/default.nix. Just needs a proper alpine image to test against |
786c86b to
9aaa32d
Compare
I'll give this a shot |
|
I was able to get an installer test for Alpine working. Unfortunately to do things right there was some scope creep. The first challenge was a couple packages aren't pre-installed ( I confirmed that the installer test fails before the fix commit, and passes after, and does not appear to cause regressions. I was also able to fix the existing issue of shell profile files not being loaded by ssh. I ended up implementing openrc installation support in the installer, testing the daemon installer without it was clunky. I have that workaround in the commit history though if we don't want to add openrc support right now. |
|
Awesome! Tysm |
|
Sorry I didn't run the formatting check before my earlier push, |
|
could someone please retry the failed CI jobs? it looks like they all failed with a 502 error when pulling artifacts |
Motivation
When trying to install nix via the install scripts on a system that uses BusyBox for common utilities, the installation fails due to invalid
cpusage.Context
#13480 details this issue, but to recap, the current install scripts do not work on (base) Alpine Linux:
The implementation strategy follows how testing for FreeBSD/Darwin based installs are handled: a function that returns true/false in the multi-user install script, and inline logic during the single-user install script.
To address this, I've added tests for if
cpis being provided by BusyBox (checks ifcpis a symlink to/bin/busybox). If it is, we use thecpcommand that uses-RPp.Closes #13480
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.