busybox-static: init #341780
busybox-static: init #341780
Conversation
The main reason to use pkgsStatic inside pkgs seems to just be for busybox. Using pkgsStatic inside the package set is a layering violation — the idea behind pkgsStatic was to give _users_ a way to build _all_ packages statically — and bloats build closures with a whole separate toolchain for no good reason. Since there's a legitimate need for a static busybox, let's just add a normal package for it. As part of this, I've changed busybox-sandbox-shell to just use the normal Busybox package, since it already overrides the passed Busybox to be static.
aec6ea7 to
e1fe3dc
Compare
| else busybox; | ||
| }; | ||
| busybox-static = busybox.override { enableStatic = true; }; | ||
| busybox-sandbox-shell = callPackage ../os-specific/linux/busybox/sandbox-shell.nix { }; |
There was a problem hiding this comment.
Busybox is used by nix inside its sandbox for /bin/sh, which needs to be self-standing. So I'm pretty sure this needs to be built with useMusl otherwise it leaves references to other /nix/store paths.
There was a problem hiding this comment.
If that was true, how would this have worked in the else branch above, before? i.e. for RISC-V or LoongArch64 or bionic? Those didn't use musl - did those cases not have those references for a different reason?
There was a problem hiding this comment.
Also, @szlend, this kind of contradicts the comment you added in https://github.com/NixOS/nixpkgs/pull/314845/files#diff-baf2a52d7f904b2863c773307b5c8fd5969e0da50b97b696ead87818f001c710R9-R11. Essentially this PR just takes it a step further and follows through on that comment.
There was a problem hiding this comment.
I don't think this is true. Just because libc can access some paths, it doesn't mean that the shell will want to, and in fact I'd say it's extremely unlikely.
There was a problem hiding this comment.
You might be right, it's possible failing to find those references (e.g. glibc locales) is not fatal, but it still feels wrong to have broken /nix/store references in the nix sandbox. I'd leave it to nix maintainers to decide.
| cp ${pkgsStatic.busybox}/bin/busybox gns3server/compute/docker/resources/bin/busybox | ||
| cp ${busybox-static}/bin/busybox gns3server/compute/docker/resources/bin/busybox |
There was a problem hiding this comment.
I confirmed that removing the whole prePatch makes the build for gns3-server fail, but using busybox-static keeps it working, so this change should be good!
| else busybox; | ||
| }; | ||
| busybox-static = busybox.override { enableStatic = true; }; | ||
| busybox-sandbox-shell = callPackage ../os-specific/linux/busybox/sandbox-shell.nix { }; |
There was a problem hiding this comment.
nit: The busybox-sandbox-shell changes seem to be in wrong place in a commit named busybox-static: init.
Description of changes
Most of the uses of pkgsStatic inside pkgs are just for busybox. Using pkgsStatic inside the package set is a layering violation — the idea behind pkgsStatic was to give users a way to build all packages statically — and bloats build closures with a whole separate toolchain for no good reason. Since there's a legitimate need for a static busybox, let's just add a normal package for it.
As part of this, I've changed busybox-sandbox-shell to just use the normal Busybox package, since it already overrides the passed Busybox to be static.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.