-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
qemu: add option to build with gtk support #38930
Conversation
Success on aarch64-linux (full log) Attempted: qemu Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: qemu Partial log (click to expand)
|
@@ -55,6 +56,7 @@ stdenv.mkDerivation rec { | |||
++ optionals numaSupport [ numactl ] | |||
++ optionals pulseSupport [ libpulseaudio ] | |||
++ optionals sdlSupport [ SDL2 ] | |||
++ optionals gtkSupport [ gtk3 gettext gnome3.vte-ng ] |
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.
Is not standard vte
enough? vte-ng
is a fork that exposes some internal APIs.
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.
Thanks. Changed to standard vte
, doesn't seem to make a difference.
Success on x86_64-linux (full log) Attempted: qemu Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: qemu Partial log (click to expand)
|
@GrahamcOfBorg build qemu_test_gtk |
Success on x86_64-linux (full log) Attempted: qemu_test_gtk Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: qemu_test_gtk Partial log (click to expand)
|
pkgs/top-level/all-packages.nix
Outdated
@@ -13870,6 +13870,7 @@ with pkgs; | |||
qemu_xen_4_10-light = lowPrio (qemu.override { hostCpuOnly = true; xenSupport = true; xen = xen_4_10-light; }); | |||
|
|||
qemu_test = lowPrio (qemu.override { hostCpuOnly = true; nixosTestRunner = true; }); | |||
qemu_test_gtk = lowPrio (qemu.override { hostCpuOnly = true; nixosTestRunner = true; gtkSupport = true; }); |
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.
Why do you change qemu_test =
instead of qemu
?
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.
qemu_test
is the variant used by nixos tests and nixos-build-vms
, so it is convenient to have a prebuilt version of this with gtk support. Of course one could add qemu_gtk
and qemu_kvm_gtk
but I didn't want to create too many variants in all_packages.nix
.
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.
Instead of creating more variants, we could also consider enabling gtk support by default. Probably just a question of closure size...
Edit: building with gtk support increases qemu_test
closure size from 240MB to 349MB.
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.
So the only thing, which qemu_test, but not qemu
is a patch for 9p
. If that would be added there as well, one could just use qemu
in place for system.build.qemu
. If then gtk3
support is enabled as well in qemu
, there would be no need for a separate qemu_gtk
.
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.
That patch overrides some 9p
permission checks and should not be used in production for security reasons, so we do need a separate qemu_test
. Anyway, why don't we keep it simple and enable gtk3
support by default (except for qemu-xen
which never needs graphics and should be kept small)? We can add variants later as needed.
Updated to enable |
Failure on aarch64-linux (full log) Attempted: qemu Partial log (click to expand)
|
@GrahamcOfBorg build qemu_kvm |
Success on aarch64-linux (full log) Attempted: qemu_kvm Partial log (click to expand)
|
Don't understand why ofborg didn't build this on x86_64-linux, so try again. |
Success on aarch64-linux (full log) Attempted: qemu_kvm Partial log (click to expand)
|
@grahamc looks like ofborg stopped building PRs on x86_64-linux about a day ago. |
Success on x86_64-linux (full log) Attempted: qemu_kvm Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: qemu_kvm Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: qemu Partial log (click to expand)
|
Failures of full |
ping.
LGTM. Let's merge.
|
??? this was merged 5 days ago... |
Ah, sorry. Github or email bugged out, I guess. I didn't get the merge notification and my local patch tracker (which parses github mail) still marks this as unmerged. I wonder how many more "open" issues are actually closed... =/ |
Motivation for this change
Fix #30889 . Default SDL rendering of
qemu
window is ugly and often not readable, in particular when using a tiling window manager.Add a build option
gtkSupport
to buildqemu
with gtk3 support.Edit: modified to enable gtk support by default, except for
qemu_xen
(doesn't need graphics at all).Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)