Skip to content
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: fix qemu_test being used in normal VMs #101246

Merged
merged 1 commit into from Oct 22, 2020
Merged

nixos: fix qemu_test being used in normal VMs #101246

merged 1 commit into from Oct 22, 2020

Conversation

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Oct 21, 2020

Motivation for this change

It looks like since PR #49403 the VMs built by nixos-rebuild built-vm are using the stripped-down qemu package, which lacks gtk and spice support.

Things done
  • Tested that nixosTests uses qemu_test package
  • Tested that build-vm uses the normal qemu package
  • Test I have not broken more things by removing config.system.build.qemu
Copy link
Member

@andir andir left a comment

I just wrote a very similar patch before I found this.

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Otherwise it looks fine. I haven't executed this code yet tho..

@rnhmjoj
Copy link
Contributor Author

@rnhmjoj rnhmjoj commented Oct 21, 2020

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Wait, isn't this exactly what I'm doing in this PR?

@andir
Copy link
Member

@andir andir commented Oct 21, 2020

We should probably add a virtualisation.qemu.package = pkgs.qemu_test; line to nixos/modules/testing/test-instrumentation.nix to ensure the tests are still using that.

Wait, isn't this exactly what I'm doing in this PR?

Yes, sorry. The line breaking on the (small) GitHub Window didn't really make that obvious. It looked more like you did just reflow some of the code but indeed you are doing exactly that. :)

andir
andir approved these changes Oct 21, 2020
@andir
Copy link
Member

@andir andir commented Oct 21, 2020

* [ ]  Test I have not broken more things by removing `config.system.build.qemu`

There is one case of system.build.qemu in nixos/lib/build-vms.nix.

@rnhmjoj
Copy link
Contributor Author

@rnhmjoj rnhmjoj commented Oct 21, 2020

Thank you, I'll take a look at build-vms now.

@rnhmjoj
Copy link
Contributor Author

@rnhmjoj rnhmjoj commented Oct 21, 2020

It looks like that line can simply be removed, since the test package is already being configured by test-instrumentation.

@andir
Copy link
Member

@andir andir commented Oct 22, 2020

I am building all the tests over night and will merge this in the morning when everything looks fine.

@andir andir merged commit 8935152 into NixOS:master Oct 22, 2020
16 checks passed
@xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Oct 25, 2020

Ah so I think this pr reintroduced the eval error which was supposed to be fixed by rnhmjoj@2578557#diff-ec9255f81e919804497f4a2ae13abd965816f4c519b08d037be851055ac53b4e ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants