-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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/nixos-build-vms: use pkgs.qemu
for virtualisation
#101473
Conversation
When I test a change e.g. in the module system manually, I usually use `nixos-build-vms(8)` which also gives me a QEMU window where I can play around in the freshly built VM. It seems as this has changed recently when the default package for non-interactive VM tests using the same framework was switched to `pkgs.qemu_test` to reduce the closure size. While this is a reasonable decision for our CI tests, I think that you really want a QEMU window of the VM by default when using `nixos-build-vms(8)`. [1] bc2188b
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.
Looks reasonable. Sorry for breaking your workflow :)
I'm not too familiar with this tool. Is In any case, I think it would be better if |
It's intended to be a testing tool as it only starts a QEMU user session just like the NixOS testing framework.
Not sure if having another option doing (usually) the exact same thing is an actual benefit. Improving the option's description may be better. |
Actually, the change I did in #49403 adds the |
Ah, cool. So, shouldn't this be enough?
I'm not sure how I to test this... |
Oh yeah I see the issue now. I somehow did expect that the driver script was the only place that required the variable qemu parameter. I'll have to pass it down the module system eval as well. I'll get to work on fixing that right away. |
@andir I can take care of it as well here :) Not sure if I can get to it tonight though. |
I think you'll need the change proposed in this PR anyway. The entire test runner script has to be a bit refactored. Currently we are specifying the qemu package on waaaaay too many places. I'd like the module system to be the only place where it is being defined and then we just stick with that. |
I think I've a workable solution: https://github.com/andir/nixpkgs/tree/nixos-build-vms-qemu I didn't want to push it to this branch (just yet) as this change here can go in as is. I'd like to get some feedback on my additional proposed changes if things then work as expected with your use-case (and with -A |
@andir fair enough. Does that mean I should merge the PR for now? You can revert the change and switch to |
Yes, just merge this. This is good from my point of view. |
Done. Thanks a lot for your review and your work on this! :) |
Motivation for this change
When I test a change e.g. in the module system manually, I usually use
nixos-build-vms(8)
which also gives me a QEMU window where I can playaround in the freshly built VM.
It seems as this has changed recently when the default package for
non-interactive VM tests using the same framework was switched to
pkgs.qemu_test
to reduce the closure size. While this is a reasonabledecision for our CI tests, I think that you really want a QEMU window of
the VM by default when using
nixos-build-vms(8)
.[1] bc2188b
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)