Skip to content

busybox-static: init #341780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alyssais wants to merge 1 commit into
base: master
Choose a base branch
from
Open

busybox-static: init #341780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkgs/applications/networking/gns3/server.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
{ lib
, python3Packages
, fetchFromGitHub
, pkgsStatic
, stdenv
, nixosTests
, testers
, busybox-static
, util-linux
, gns3-server
}:
Expand All @@ -27,7 +27,7 @@ python3Packages.buildPythonApplication {

# GNS3 2.3.26 requires a static BusyBox for the Docker integration
prePatch = ''
cp ${pkgsStatic.busybox}/bin/busybox gns3server/compute/docker/resources/bin/busybox
cp ${busybox-static}/bin/busybox gns3server/compute/docker/resources/bin/busybox
Copy link
Contributor

@wolfgangwalther wolfgangwalther Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

'';

propagatedBuildInputs = with python3Packages; [
Expand Down
8 changes: 2 additions & 6 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26041,12 +26041,8 @@ with pkgs;
overrideCC stdenv buildPackages.llvmPackages.clangNoLibcxx
else stdenv;
};
busybox-sandbox-shell = callPackage ../os-specific/linux/busybox/sandbox-shell.nix {
# musl roadmap has RISC-V support projected for 1.1.20
busybox = if !stdenv.hostPlatform.isRiscV && !stdenv.hostPlatform.isLoongArch64 && stdenv.hostPlatform.libc != "bionic"
then pkgsStatic.busybox
else busybox;
};
busybox-static = busybox.override { enableStatic = true; };
busybox-sandbox-shell = callPackage ../os-specific/linux/busybox/sandbox-shell.nix { };
Copy link
Member

@szlend szlend Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@wolfgangwalther wolfgangwalther Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@wolfgangwalther wolfgangwalther Sep 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@alyssais alyssais Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@szlend szlend Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@wolfgangwalther wolfgangwalther Sep 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The busybox-sandbox-shell changes seem to be in wrong place in a commit named busybox-static: init.


cachefilesd = callPackage ../os-specific/linux/cachefilesd { };

Expand Down