-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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.
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
nixos/binfmt: Add option to use static emulators when available #334859
Conversation
d0777e3
to
d80ca52
Compare
I was hoping to get away without having to modify qemu again, but alas, Result of 5 packages marked as broken and skipped:
1 package blacklisted:
8 packages failed to build:
86 packages built:
|
buildInputs = [ glib zlib ] | ||
++ lib.optionals (!minimal) [ dtc pixman vde2 lzo snappy libtasn1 gnutls nettle libslirp ] | ||
++ lib.optionals (!userOnly) [ curl ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not really sure how to verify this but disabling features we don't need is probably a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I did:
- Verify manually that for
!minimal && !userOnly
(i.e. all the normal qemus like qemu, qemu-full, qemu_kvm…), only the order of inputs changes (which is hopefully irrelevant). - qemu-user working as intended is covered by a test added here
- For qemu-utils:
nix build github:NixOS/nixpkgs/{c46d43c625f161d3bd22052a5f8f9df703d81be8,f69411bcfb0dd3565a19667fdf509bbadb38f008}#qemu-utils
- The sizes of the output binaries are all the same:
ls -l result*/bin/* | cut -d\ -f5- | sort
, so it's unlikely that some larger functionality went missing - Looking e.g. through
nix run nixpkgs#diffoscope result*/bin/qemu-storage-daemon
shows some probably irrelevant differences in the length of some dummy strings (Probably due toerror: derivation '…-qemu-utils-9.0.2.drv' may not be deterministic: output '…-qemu-utils-9.0.2' differs
-.-)
8d1bf60
to
f69411b
Compare
lib and pkgs changes look fine to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise LGTM and I would say we merge this
The fixBinary flag will be enabled if a static emulator is in use.
@SuperSandro2000 Can I do something to make this easier to merge? Maybe split off the changes to pkgs/ into a different PR? (They're not strictly necessary, but should help to avoid future build failures.) Or is this simply ready to hit the button? Rebased due to merge conflicts. nemu and quickgui fail as before. Result of 8 packages marked as broken and skipped:
1 package blacklisted:
4 packages failed to build:
157 packages built:
|
Looks all good to me. Just give me a ping when you fixed the test and as long as the changes done there are looking good, I would just merge this. |
The test failure on linux seems to be flaky, I couldn't reproduce it in 5 tries, a no-change force push made it go away. (darwin times out as always.) I'd guess it's unrelated to this PR. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/docker-ignoring-platform-when-run-in-nixos/21120/18 |
4658a06 has a downside: the closure size of QEMU is now bigger, since the package now contains 34 emulators instead of just one. To measure this, I added I'm not sure how to do it better though. If we go back to overriding |
Motivation
This PR makes running binaries via
boot.binfmt.emulatedSystems
in chroots "just work", at the cost of adding/setting a single option.The goal is to change
into
The other prominent use-case is to
nixos-enter
/chroot
into a broken linux install mounted to/mnt
of an other machine for debugging, where that linux install is for a different platform (e.g. a raspi).Description of changes
boot.binfmt.preferStaticEmulators
, which changesboot.binfmt.registrations.*.interpreter
to an executable frompkgsStatic
where possible.Things I'd like advice on
Note how the error message in the motivational examples is abysmal?
/bin/uname
exists in both examples. What's not being found is/run/binfmt/armv7l-linux
.Instead of making this an option, would it be conceivable to make this always-on?
Notes
This is the third attempt to land this in nixpkgs. Related PRs/issues:
boot.binfmt.emulatedSystems
#160300(abandoned)
(abandoned)
(only made
pkgsStatic.qemu.override { userOnly = true; }
work)Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.