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.

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

Jump to bottom

qemu: fix build when desktop file does not exist #110721

Merged
1 commit merged into from Jan 25, 2021
Merged

qemu: fix build when desktop file does not exist #110721

1 commit merged into from Jan 25, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jan 24, 2021 •
edited by ghost

The qemu-user variants as used by binfmt emulation through (lib.systems.elaborate lib.systems.examples.aarch64-multiplatform).emulator pkgs does not install a .desktop file since qemu 5.2.0. This change allows the build to continue if deletion of the desktop file fails.

Motivation for this change

My systems that use the binfmt module could not be built:

builder for '/nix/store/rfhpc57x4qlf5scnnkg4iq6xkjvaby5z-qemu-5.2.0.drv' failed with exit code 1; 
  rm: cannot remove '/nix/store/60nv5372byadkkwkwvdimmzn4jm9r3d4-qemu-5.2.0/share/applications/qemu.desktop': No such file or directory
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from edolstra January 24, 2021 21:47
@ghost ghost changed the title qemu: remove lines to delete nonexistant desktop file qemu: remove lines to delete nonexistent desktop file Jan 24, 2021
@ghost ghost marked this pull request as draft January 24, 2021 22:06
@ghost ghost changed the title qemu: remove lines to delete nonexistent desktop file qemu: fix build when desktop file does not exist Jan 25, 2021
@ghost ghost marked this pull request as ready for review January 25, 2021 02:45
@ghost ghost requested review from andir and peterhoeg January 25, 2021 02:46
peterhoeg
peterhoeg reviewed Jan 25, 2021
View reviewed changes
pkgs/applications/virtualization/qemu/default.nix Outdated
@@ -143,7 +143,7 @@ stdenv.mkDerivation rec {

postFixup = ''
# the .desktop is both invalid and pointless
rm $out/share/applications/qemu.desktop
rm $out/share/applications/qemu.desktop || true
Copy link
Member

@peterhoeg peterhoeg Jan 25, 2021 •
edited

Choose a reason for hiding this comment

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

rm -f ?

Copy link
Member

Choose a reason for hiding this comment

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

rm -f means we won't notice when it is removed. Same with || true. If the file is really removed there is no need for this.

Suggested change
rm $out/share/applications/qemu.desktop || true
rm $out/share/applications/qemu.desktop

Copy link
Member

Choose a reason for hiding this comment

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

We will have to either use || true, -f or make the nix code emit different code here. That is that I understand the reasoning right. Some of our builds are not producing .desktop files and thus we must choose one of those options. The suggestion from @SuperSandro2000 would make this diff a no-op.

Copy link
Member

Choose a reason for hiding this comment

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

Some of our builds are not producing .desktop files

Did not know that. Maybe add a comment about this.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 110721 run on x86_64-darwin 1

11 packages marked as broken and skipped:
  • alpine-make-vm-image
  • cloud-init
  • cloud-utils
  • multibootusb
  • python37Packages.guestfs
  • python38Packages.guestfs
  • python39Packages.guestfs
  • qemu_xen
  • qemu_xen-light
  • qemu_xen_4_10
  • qemu_xen_4_10-light
8 packages failed to build:
  • cot (python38Packages.cot)
  • out-of-tree
  • python37Packages.cot
  • python39Packages.cot
  • qemu
  • qemu-utils
  • qemu_kvm
  • qemu_test

qemu: fix build when desktop file does not exist
d98d76c 
The qemu-user variants as used by binfmt emulation through
`(lib.systems.elaborate lib.systems.examples.aarch64-multiplatform).emulator pkgs`
does not install a .desktop file since qemu 5.2.0. This change allows
the build to continue if deletion of the desktop file fails.
mohe2015
mohe2015 approved these changes Jan 25, 2021
View reviewed changes
Copy link
Contributor

@mohe2015 mohe2015 left a comment

Choose a reason for hiding this comment

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

It builds and runs :)

@lovesegfault lovesegfault mentioned this pull request Jan 25, 2021
Closed
10 tasks
@ghost ghost merged commit b7871c3 into NixOS:master Jan 25, 2021
@indocomsoft indocomsoft mentioned this pull request Jan 26, 2021
Closed
@Xe
Copy link
Contributor

Xe commented Jan 27, 2021

When will this percolate out to nixos-unstable? This is preventing me from upgrading my systems.

@ghost
Copy link
Author

ghost commented Jan 27, 2021

When will this percolate out to nixos-unstable? This is preventing me from upgrading my systems.

Soon. To be more accurate, when the first column has all checkmarks: https://hydra.nixos.org/job/nixos/trunk-combined/tested#tabs-constituents

@Mic92
Copy link
Member

Mic92 commented Jan 29, 2021

When will this percolate out to nixos-unstable? This is preventing me from upgrading my systems.

maybe try nixos-unstable-small in the mean time.

risicle pushed a commit to risicle/nixpkgs that referenced this pull request Apr 27, 2021
@risicle
qemu: fix build when desktop file does not exist (NixOS#110721)
9957e6d 
The qemu-user variants as used by binfmt emulation through
`(lib.systems.elaborate lib.systems.examples.aarch64-multiplatform).emulator pkgs`
does not install a .desktop file since qemu 5.2.0. This change allows
the build to continue if deletion of the desktop file fails.
(cherry picked from commit b7871c3)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra Awaiting requested review from edolstra

@andir andir Awaiting requested review from andir

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SuperSandro2000 @Xe @Mic92 @andir @peterhoeg @mohe2015