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

[20.09] libguestfs: unmark broken #102077

Merged
merged 2 commits into from Oct 29, 2020

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Oct 29, 2020

Motivation for this change

wrong variant was marked broken

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.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/upgrade-20-03-to-20-09-failes-due-to-brocken-libguestfs-1-40-2/9721/3

@jonringer jonringer changed the title libguestfs: unmark broken [20.09] libguestfs: unmark broken Oct 29, 2020
@mweinelt
Copy link
Member

wrong variant was marked broken

This should probably be part of the commit message for libguestfs, as it explains why this change happened.

@cole-h
Copy link
Member

cole-h commented Oct 29, 2020

FWIW, I was able to build libguestfs-with-appliance on the latest commit of release-20.09, da973f9, just now.

vin@scadrial ~/w/v/n/release-20.09 (release-20.09)> NIXPKGS_ALLOW_BROKEN=1 nix-build -A libguestfs-with-appliance
/nix/store/mb45hrmrzprm07lm1abg5bvmcaba7477-libguestfs-1.40.2
vin@scadrial ~/w/v/n/release-20.09 (release-20.09)> nix path-info -Sh /nix/store/mb45hrmrzprm07lm1abg5bvmcaba7477-libguestfs-1.40.2
/nix/store/mb45hrmrzprm07lm1abg5bvmcaba7477-libguestfs-1.40.2      5.4G

and libguestfs:

vin@scadrial ~/w/v/n/release-20.09 (release-20.09)> NIXPKGS_ALLOW_BROKEN=1 nix-build -A libguestfs
/nix/store/s9s65lm2gsc8plw1y03gml6qnwx860yv-libguestfs-1.40.2
vin@scadrial ~/w/v/n/release-20.09 (release-20.09)> nix path-info -Sh /nix/store/s9s65lm2gsc8plw1y03gml6qnwx860yv-libguestfs-1.40.2
/nix/store/s9s65lm2gsc8plw1y03gml6qnwx860yv-libguestfs-1.40.2      1.4G

@jonringer
Copy link
Contributor Author

@cole-h that's valid.

However, it fails on hydra because the output size is too big. The expression seems to correctly state hydraPlatforms = [];, but it's not being respected

@jonringer
Copy link
Contributor Author

jonringer commented Oct 29, 2020

I think I figured it out:

  libguestfs-with-appliance = libguestfs.override { appliance = libguestfs-appliance; };

it overrides the base libguestfs derivation. So the meta isn't respected from libguestfs-appliance.

This also explains why we don't see libguestfs-appliance in hydra outputs.

@jonringer
Copy link
Contributor Author

I updated the base libguestfs to inherit hydraPlatforms from an appliance.

Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Diff LGTM, thanks! I think this should be forward-ported, right? (At least, the hydraPlatforms change -- I don't think libguestfs was marked broken on master.)

@jonringer
Copy link
Contributor Author

jonringer commented Oct 29, 2020

Diff LGTM, thanks! I think this should be forward-ported, right? (At least, the hydraPlatforms change -- I don't think libguestfs was marked broken on master.)

agreed

Also, why I'm keeping it as two commits :)

@jonringer
Copy link
Contributor Author

hmm, we need a "needs port to unstable" label

@jonringer
Copy link
Contributor Author

waiting on ofborg

@jonringer
Copy link
Contributor Author

looks like referencing the meta of an override causes infinite recursion:

while evaluating 'g' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-3/lib/attrsets.nix:276:19, called from undefined position:
while evaluating anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-3/pkgs/top-level/release-lib.nix:144:38, called from undefined position:
while evaluating the attribute 'meta.hydraPlatforms' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-3/pkgs/development/libraries/libguestfs/default.nix:93:5:
infinite recursion encountered, at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/packet-spot-eval-3/pkgs/development/libraries/libguestfs/default.nix:91:17

@jonringer
Copy link
Contributor Author

nevermind, it was the combination of rec + platforms

[12:15:28] jon@jon-desktop /home/jon/projects/nixpkgs (correct-libguestfs)
$ nix eval -f default.nix libguestfs.meta.hydraPlatforms
[ "aarch64-linux" "armv5tel-linux" "armv6l-linux" "armv7a-linux" "armv7l-linux" "mipsel-linux" "i686-linux" "x86_64-linux" "powerpc64le-linux" "riscv32-linux" "riscv64-linux" ]
[12:15:41] jon@jon-desktop /home/jon/projects/nixpkgs (correct-libguestfs)
$ nix eval -f default.nix libguestfs-with-appliance.meta.hydraPlatforms
[ ]

@jonringer
Copy link
Contributor Author

waiting on ofborg

@cole-h
Copy link
Member

cole-h commented Oct 29, 2020

Darwin isn't going to run since the builder is dead, aarch isn't supported because xen isn't, and the rest is green. I think this is good to go.

@jonringer jonringer merged commit 265f2f5 into NixOS:release-20.09 Oct 29, 2020
@jonringer jonringer deleted the correct-libguestfs branch October 29, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants