Skip to content

libepoxy: propagate buildInputs#403479

Merged
leona-ya merged 1 commit intoNixOS:staging-nextfrom
SigmaSquadron:push-swnwqsmylnwn
May 8, 2025
Merged

libepoxy: propagate buildInputs#403479
leona-ya merged 1 commit intoNixOS:staging-nextfrom
SigmaSquadron:push-swnwqsmylnwn

Conversation

@SigmaSquadron
Copy link
Contributor

@SigmaSquadron SigmaSquadron commented May 1, 2025

Some packages that depend on libepoxy fail to build due to libGL and libX11 not being present during the build. One example is qemu_xen (or really any QEMU built with GL support), as it's built with libepoxy, which links against libGL, but since libGL isn't present, the build fails.

Going to staging due to the NixOS tests (QEMU), as well as literally everything that depends on GTK and Qt.

ZHF: #403336

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Some packages that depend on libepoxy fail to build due to libGL and
libX11 not being present during the build. One example is `qemu_xen`
(or really any QEMU built with GL support), as it's built with libepoxy,
which links against libGL, but since libGL isn't present, the build fails.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
@SigmaSquadron SigmaSquadron added the 0.kind: ZHF Fixes Fixes during the Zero Hydra Failures (ZHF) campaign label May 1, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels May 1, 2025
@LordGrimmauld
Copy link
Contributor

Ah, i suspect this is another one of the SDL fallouts - qemu previously had libGL/libX11 as it was (incorrectly) being propagated from SDL2, and since that propagation was removed the libepoxy stuff came to light. I'll take a look tomorrow.

@wolfgangwalther wolfgangwalther mentioned this pull request May 3, 2025
4 tasks
Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Command for checking: rg '#include' $(nix build nixpkgs#libepoxy.dev --print-out-paths)/include/epoxy

$ rg '#include' result-dev/include/epoxy
result-dev/include/epoxy/gl.h
35:#include "epoxy/common.h"
89:#include "epoxy/gl_generated.h"

result-dev/include/epoxy/egl_generated.h
6:#include <inttypes.h>
7:#include <stddef.h>
9:#include "epoxy/common.h"
10:#include "epoxy/gl.h"
11:#include "EGL/eglplatform.h"

result-dev/include/epoxy/glx.h
35:#include <epoxy/gl.h>
36:#include <X11/Xlib.h>
37:#include <X11/Xutil.h>
49:#include "epoxy/glx_generated.h"

result-dev/include/epoxy/egl.h
35:#include "epoxy/common.h"
46:#include "epoxy/egl_generated.h"

result-dev/include/epoxy/glx_generated.h
21:#include <inttypes.h>
22:#include <stddef.h>
24:#include "epoxy/common.h"
25:#include "epoxy/gl.h"
26:#include <X11/Xlib.h>
27:#include <X11/Xutil.h>

result-dev/include/epoxy/gl_generated.h
21:#include <inttypes.h>
22:#include <stddef.h>
24:#include "epoxy/common.h"

Looking at this, both libGL (for EGL includes) and libX11 (for X11 includes) seems to be required.
(no, libGL does not propagate libX11)

buildInputs =
propagatedBuildInputs =
lib.optionals (x11Support && !stdenv.hostPlatform.isDarwin) [
libGL

This comment was marked as resolved.

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Sorry, i am blind! I didn't catch libX11 was guarded behind the non-darwin check. Github hid that part, i hate the UI! Sorry, LGTM!

@LordGrimmauld
Copy link
Contributor

(these headers are on linux - i did not check on darwin, i do not have builder access there)

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented May 6, 2025

$ rg '#include' $(nix build nixpkgs#libepoxy.dev --print-out-paths --system x86_64-darwin )/include/epoxy
/nix/store/avf0jw5srky3vl3zkwlgb4dj8cikhzyc-libepoxy-1.5.10-dev/include/epoxy/gl.h
35:#include "epoxy/common.h"
89:#include "epoxy/gl_generated.h"

/nix/store/avf0jw5srky3vl3zkwlgb4dj8cikhzyc-libepoxy-1.5.10-dev/include/epoxy/gl_generated.h
21:#include <inttypes.h>
22:#include <stddef.h>
24:#include "epoxy/common.h"

libGL propagation might not even be necessary on darwin. It might even just be a stub anyways. But not sure there, not a darwin expert.

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented May 6, 2025

$ nix build nixpkgs#libGL --print-out-paths --system x86_64-darwin
error: expected flake output attribute 'legacyPackages.x86_64-darwin.libGL' to be a derivation or path but found null: null

Okay so yeah, libGL is not a thing on darwin - so non-darwin on that is fine!

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Alright, now having checked the darwin logic: This looks fine! I did not check this is actually a complete fix for the linked issue, but it definitely fixes a packaging bug so i believe this should be merged.

@LordGrimmauld
Copy link
Contributor

Okay, i just confirmed qemu_xen does build with this

@leona-ya leona-ya marked this pull request as draft May 7, 2025 07:11
@leona-ya leona-ya changed the base branch from staging to staging-next May 7, 2025 07:11
@github-actions github-actions bot added the 4.workflow: backport This targets a stable branch label May 7, 2025
@leona-ya leona-ya marked this pull request as ready for review May 7, 2025 07:11
@alyssais
Copy link
Member

alyssais commented May 7, 2025

Are these dependencies always required, or only if certain optional headers are included? If the latter, they shouldn't be propagated.

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented May 7, 2025

Are these dependencies always required, or only if certain optional headers are included? If the latter, they shouldn't be propagated.

Only if epoxy/egl.h is included (libGL) or epoxy/glx.h (libX11). But it makes no sense to propagate libX11 and not libGL, as we had it before? Nvm i am blind again!

The reason this exploded on qemu_xen was the fact sdl2-compat no longer propagates libGL. Previously that propagation obscured the need for libGL for the epoxy/egl.h-includes. Looking at the darwin support, glx/egl seems very much optional if they are straight missing on darwin.

We have ~220 direct users of libepoxy, some of them mass rebuilds. Checking all those for epoxy/egl.h will be tedious . Your point is valid though!

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented May 7, 2025

rg libepoxy -l | grep -v libGL drops the number down to 109 packages that need checking
(though it does miss some...)

@alyssais
Copy link
Member

alyssais commented May 7, 2025

Okay, then I guess this is fine — from what I can tell you'd basically always include the header for EGL, GLX, or WGL (Windows) to do anything useful with the library.

@LordGrimmauld
Copy link
Contributor

There exists the epoxy/gl.h header, which is the only one available on darwin anyways. But i am not sure how many projects use that without any of the other headers.

@LordGrimmauld
Copy link
Contributor

Sidenote: the correct fix to #403483 would have been to just add libGL to the openGLSupport section:

diff --git a/pkgs/applications/virtualization/qemu/default.nix b/pkgs/applications/virtualization/qemu/default.nix
index 841415a4b6e2..5d9e7dafbb50 100644
--- a/pkgs/applications/virtualization/qemu/default.nix
+++ b/pkgs/applications/virtualization/qemu/default.nix
@@ -29,6 +29,7 @@
   attr,
   libcap,
   libcap_ng,
+  libGL,
   socat,
   libslirp,
   apple-sdk_13,
@@ -239,6 +240,7 @@ stdenv.mkDerivation (finalAttrs: {
     ++ lib.optionals openGLSupport [
       libgbm
       libepoxy
+      libGL
       libdrm
     ]
     ++ lib.optionals rutabagaSupport [ rutabaga_gfx ]

But eh, propagation would catch that.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 7, 2025
@wolfgangwalther wolfgangwalther removed the 4.workflow: backport This targets a stable branch label May 7, 2025
@SigmaSquadron SigmaSquadron mentioned this pull request May 7, 2025
3 tasks
@leona-ya
Copy link
Member

leona-ya commented May 7, 2025

What do you think about what to do? Merge this one or another fix? This is not clear to me.

@SigmaSquadron
Copy link
Contributor Author

This is the best solution as far as I can tell. Fixing each individual package to depend on libGL when we can just propagate it from a package that already needs it.

@LordGrimmauld
Copy link
Contributor

My vote is on merge, even though libGL/libX11 is technically optional. Because on linux, almost all packages will use one (or both) of the two.

Copy link
Member

@leona-ya leona-ya left a comment

Choose a reason for hiding this comment

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

For me this is okay. I'd like to wait a few hours for another judgement from @alyssais. Otherwise I'll merge and maybe we'll move to another situation later.

@leona-ya leona-ya merged commit b941a67 into NixOS:staging-next May 8, 2025
74 checks passed
@SeqBra
Copy link
Contributor

SeqBra commented May 8, 2025

Thanks for merging and your work🙇

wolfgangwalther added a commit to wolfgangwalther/nixpkgs that referenced this pull request May 9, 2025
The underlying missing libX11 dependency was solved by NixOS#403479.
@SigmaSquadron SigmaSquadron deleted the push-swnwqsmylnwn branch July 3, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: ZHF Fixes Fixes during the Zero Hydra Failures (ZHF) campaign 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants